Watchdog speichert Reset-Adresse -- für atmega328p

[Uhu U. (uhu)]

Wer hat nicht schon einmal versucht herauszubekommen, wo das Programm 
vom Watchdog zur Strecke gebracht wurde?

Der Sucherei soll dieses kleine Paket ein schnelles Ende bereiten:

 - Wenn ein Watchdog-Timeout auftritt, speichert die Watchdog-Interrupt-
   Routine die Rückkehradresse in einem Speicherbereich, der beim
   Neustart nach einem Reset nicht überschrieben wird - man kann also
   die Adresse nach dem Absturz auslesen.
 - Zusätzlich unterstützt das Paket die sleep-Modi:
   statt einem einfachen sleep ruft man idle() auf. Tritt während der
   Ruhe der CPU ein Watchdog-Overflow auf, fängt die Watchdog-ISR
   den Überlauf ab und beendet den idle-Aufruf wie nach einem normalen
   Interrupt.

Initialisiert wird das Paket durch

     void initWatchdog(uint8_t timeout);

timeout ist eine der in avr/wdt.h definierten WDTO_* - Konstanten.

initWatchdog muss möglichst früh in main() aufgerufen werden.
Wenn das System durch einen Power-On-Reset neu gestartet wurde, wird die 
Variable WdtTrapAddress auf 0 gesetzt.
Dann wird das MCUSR-Register wird auf 0 gesetzt.
Anschließend wird festgestellt, wie groß der Offset zwischen SP und der 
Watchdog-Interrupt-Return-Adresse ist, damit die ISR im Fall eines 
Watchdog-Resets, der nicht in idle auftrat, die Rückkehr-Adresse 
abspeichern kann.
Zum Schluss wird der Watchdog mit dem angegebenen Timeout aktiviert.

Das Interrupt-Flag bleibt unverändert.

Durch die Interrupt-Service-Routine verdoppelt sich die Zeit zwischen 
dem Watchdog-Overflow zum Watchdog-Reset. Die Zeit zwischen erstem und 
zweitem Watchdog-Overflow wird aktiv wartend in der ISR verbracht. Dort 
können auch Routinen aufgerufen werden, die den Controller in einen 
sicheren Zustand bringen. (Man könnte die Sache aber auch abkürzen, 
indem man den Watchdog in der ISR auf die kürzeste Zeit einstellt.)


Wenn ein Watchdog-Reset aufgetreten ist, findet das Hauptprogramm die 
Code-Adresse, an der der Reset auftrat, in der Variablen

      uint16_t WdtTrapAddress;


Falls ein Bootlader auf dem µC läuft, muss geprüft werden, ob das 
MCUSR-Register vom Bootlader schon auf 0 gesetzt wurde.

Der auf Arduinos laufende Optiboot setzt MCUSR auf 0 und gibt dessen 
Inhalt in r2 an die Anwendung weiter. R2 muss in diesem Fall in der 
.init0-Section in eine Variable kopiert werden und initWatchdog darf 
nicht auf MCUSR zugreifen, sondern auf die Kopie.

[Andreas H. (Gast)]

Vielen Dank für den interessanten Codeauszug.

Zwei Anmerkungen/Fragen:
1)
in der Zeile

// scan wdt isr for return address offset

 wird noch #5 addiert. Woraus resultiert dieses "5"?

2)
Bei der Ermittlung der Stacktiefe

while (0x920fu == (code & 0xfe0fu))

hätte ich kein gutes Gefühl. Offenbar lebt die Ermittlung davon, dass 
die "Push"-Befehle (auf diese wird ja gescannt) alle zu Beginn und dicht 
aufeinander folgen. Das ist möglicherweise (evtl. von Codeoptimierung 
und verwendetem Code innerhalb der Watchdog-ISR abhängig) nicht immer 
der Fall. Mein Listfile enthält einige ISRs und da ist das durchaus 
nicht der Fall.
Sicherer ist eventuell die Methode, für jede Codeänderung den 
Assemblercode zu sichten und ein #define für die akuell verwendete 
Stacktiefe anzulegen - etwas Besseres fällt mir momentan auch nicht ein.

[Uhu U. (uhu)]

Andreas H. schrieb:
> wird noch #5 addiert. Woraus resultiert dieses "5"?

Das sind die 5 Befehle, die der Compiler zu Beginn jeder ISR absetzt:

  1f 92         push  r1
  0f 92         push  r0
  0f b6         in  r0, 0x3f  ; 63
  0f 92         push  r0
  11 24         eor  r1, r1

Die folgende Anzahl pushs hängt davon ab, wieviele Register in der ISR 
tatsächlich gebraucht werden. Nur die werden auch gerettet.

> Offenbar lebt die Ermittlung davon, dass die "Push"-Befehle (auf diese
> wird ja gescannt) alle zu Beginn und dicht aufeinander folgen.

Richtig.

> Sicherer ist eventuell die Methode, für jede Codeänderung den
> Assemblercode zu sichten und ein #define für die akuell verwendete
> Stacktiefe anzulegen - etwas Besseres fällt mir momentan auch nicht ein.

Kann man machen, aber das ist fehleranfällig und nicht gerade 
pflegeleicht. Zudem sind die Folgen einer Falschberechnung im 
vorliegenden Fall nicht sicherheitsrelevant - es reicht, nach Änderungen 
an der ISR einen wdt-Reset zu provozieren und nachzusehen, ob die 
gespeicherte Adresse korrekt ist.

Wahrscheinlich wird sich selbst nach größeren Änderungen an der ISR 
nichts wesentliches ändern, so lange man den Programmzweig, der die 
Interrupt-Rückkehradresse speichert, in Lage und Aufbau unverändert 
lässt.

Das Programm ist für gcc geschrieben.

[Andreas H. (Gast)]

Vielen Dank, das hilft mir weiter.
Nachdem ich mein Assemblerlisting noch einmal bezüglich Deiner Antwort 
angeschaut habe, ist mir aufgefallen, dass diese 5 Befehle Standard 
ISR-Beginn sind.
Nun wird mir alles klar, eben auch

isrStackOffset = 4;

Genau so etwas habe ich gesucht - nicht von schlechten Eltern Deine 
Idee! Werde es für mein Projekt übernehmen.
Könntest noch die ISR-Lage in Abhängigkeit vom verwendeten Prozessor 
(0x1A) so gut es einstellbar gestalten:

const uint16_t *pCode = (const uint16_t *) (*((const __flash uint16_t *) 0x1a) << 1) + 5;

 - das kann man vielleicht aus dem jeweiligen Prozessor-Headerfile 
herausholen?

[Uhu U. (uhu)]

Andreas H. schrieb:
> - das kann man vielleicht aus dem jeweiligen Prozessor-Headerfile
> herausholen?

Die Konstante dafür scheint WDT_vect_num zu sein, folglich sollte die 
Initialisierung so Prozessor-typunabhängig sein, zumindest bis 128 kb 
Flash:

const uint16_t *pCode = (const uint16_t *) (*((const __flash uint16_t *) (2 + 4 * WDT_vect_num)) << 1) + 5;

Das 2 + … überspringt den OP-Code des jmp-Befehls. Der Ausdruck

  (2 + 4 * WDT_vect_num)

ergibt 26 == 0x1a für WDT_vect_num == 6 beim ATMEGA 328p

Ich habs nicht getestet…

[Andreas H. (Gast)]

Du hast es richtig dargelegt. Ich habe diese Codezeile in mein Projekt 
übernommen und für den Atmega324 (Projekt Rollladensteuerung) generiert 
und getestet - alles ok.

Danke und Gruß
Andreas

[Bauform B. (bauformb)]

Uhu U. schrieb:
> Die folgende Anzahl pushs hängt davon ab, wieviele Register in der ISR
> tatsächlich gebraucht werden. Nur die werden auch gerettet.
>
>> Offenbar lebt die Ermittlung davon, dass die "Push"-Befehle (auf diese
>> wird ja gescannt) alle zu Beginn und dicht aufeinander folgen.
>
> Richtig.
>
>> Sicherer ist eventuell die Methode, für jede Codeänderung den
>> Assemblercode zu sichten und ein #define für die akuell verwendete
>> Stacktiefe anzulegen - etwas Besseres fällt mir momentan auch nicht ein.

könnte man nicht den gcc fragen? Der kann angeblich ein Textfile mit den 
genauen Zahlen erzeugen. Mit ein wenig Makefile-Magie müsste sich das 
auch automatisieren lassen.

-fstack-usage

Makes the compiler output stack usage information for the program,
on a per-function basis. The filename for the dump is made by
appending .su to the auxname. auxname is generated from the name
of the output file, if explicitly specified and it is not an
executable, otherwise it is the basename of the source file.
An entry is made up of three fields:
        The name of the function.
        A number of bytes.
        One or more qualifiers: static, dynamic, bounded. 
The qualifier static means that the function manipulates the stack
statically: a fixed number of bytes are allocated for the frame
on function entry and released on function exit; no stack
adjustments are otherwise made in the function. The second field
is this fixed number of bytes.

The qualifier dynamic means that the function manipulates the stack
dynamically: in addition to the static allocation described above,
stack adjustments are made in the body of the function, for example
to push/pop arguments around function calls. If the qualifier
bounded is also present, the amount of these adjustments is bounded
at compile time and the second field is an upper bound of the total
amount of stack used by the function. If it is not present, the
amount of these adjustments is not bounded at compile time and the
second field only represents the bounded part.

https://gcc.gnu.org/onlinedocs/gcc-7.4.0/gcc/Developer-Options.html#index-fstack-usage

[Oliver S. (oliverso)]

Am einfachsten wäre es, die ISR in Assembler zu schreiben. Für solche 
Anwendungen ist das sinnvoll.

Oliver

[Uhu U. (uhu)]

Bauform B. schrieb:
> -fstack-usage

Bei mir erzeugt er keinen .su-File. Dafür bekomme ich eine Warnung an 
einem Stück inline-asm-Code, das mir das MCUSR-Register kopiert, das vom 
Bootlader in r2 übergeben wird:

warning: stack usage computation not supported for this target [enabled by default]|

Was  wenn die ISR unterwegs in einem anderen Programmzweig noch ein paar 
Bytes ¹) auf dem Stack allokiert? Die erscheinen in stack usage, sind 
aber an der Stelle, wo die ISR die Rückkehradresse holen will, nicht 
reserviert - dann langt das Reckkehradresse-Kopieren ganz bös daneben.

Oliver S. schrieb:
> Am einfachsten wäre es, die ISR in Assembler zu schreiben. Für solche
> Anwendungen ist das sinnvoll.

Es gibt deutlich bessere Gründe, eine ISR in asm zu schreiben, z.B. 
Speicherplatzoptimierung und Tuning. Das Abspeichern der Rückkehradresse 
in der Watchdog-ISR gehört sicher nicht dazu.

---

Nachtrag: die .su-Files liegen im obj-Verzeichnis, nicht in bin, wo ich 
nachgesehen hatte. Die Warnung ist egal.

¹) Z.B. für einen berechneten call, den man so ähnlich realisieren kann:

   push r20
   push r21
   ret

[Andreas H. (Gast)]

Man kann den ISR-Rahmen auch ganz weglassen, sofern das Ende der ISR ein 
Reset ist, das dürfte, wie hier angedacht, bei der WDT-ISR der Fall 
sein.
Nested Interrupts unterstützt der ISR ja nicht.
Damit sollte das Ganze vereinfacht werden können.

[Oliver S. (oliverso)]

Uhu U. schrieb:
> Es gibt deutlich bessere Gründe, eine ISR in asm zu schreiben, z.B.
> Speicherplatzoptimierung und Tuning. Das Abspeichern der Rückkehradresse
> in der Watchdog-ISR gehört sicher nicht dazu.

Doch, genau das gehört dazu, da nur das sich nicht auf irgendein nicht 
garantiertes Verhalten des Compilers verlässt. Zudem sind süß doch nur 
ein paar Befehle. Register retten, Sp laden und korrigieren, 
Rücksprungadresse wegspeichern, Register wieder restoren, fertig.

Oliver

[Uhu U. (uhu)]

Andreas H. schrieb:
> Man kann den ISR-Rahmen auch ganz weglassen, sofern das Ende der ISR ein
> Reset ist, das dürfte, wie hier angedacht, bei der WDT-ISR der Fall
> sein.

Aber nicht, wenn man den wdt auch als normalen Timer verwenden will - 
das wollte ich mir nicht verbauen.

[Oliver S. (oliverso)]

Uhu U. schrieb:
> Das sind die 5 Befehle, die der Compiler zu Beginn jeder ISR absetzt:
> 1f 92         push  r1
>   0f 92         push  r0
>   0f b6         in  r0, 0x3f  ; 63
>   0f 92         push  r0
>   11 24         eor  r1, r1
>
> Die folgende Anzahl pushs hängt davon ab, wieviele Register in der ISR
> tatsächlich gebraucht werden. Nur die werden auch gerettet.

Ich habe hier einen avr-gcc 7.2.0, der erzeugt das folgende (für einen 
Mega2560):

000001e6 <__vector_12>:
 1e6:  1f 92         push  r1
 1e8:  0f 92         push  r0
 1ea:  0f b6         in  r0, 0x3f  ; 63
 1ec:  0f 92         push  r0
 1ee:  11 24         eor  r1, r1
 1f0:  0b b6         in  r0, 0x3b  ; 59
 1f2:  0f 92         push  r0
...

Da wird auch noch das RAMPZ-Register 0x3b auf den Stack gepusht (was die 
kleineren Megas nicht haben).
Ich bin nach wie vor für Assembler.

Oliver

[Uhu U. (uhu)]

Das kannst du gerne so machen - ich machs aus den genannten Gründen 
nicht.

Ich habe lange genug Assembler programmiert, um zu wissen, dass das 1. 
sehr zeitaufwendig ist und 2. alles andere als pflegeleicht - vor allem, 
wenn das Projekt weiterentwickelt wird. Compiler haben durchaus ihre 
Berechtigung…

[Oliver S. (oliverso)]

Je nun, kannst du machen wie du willst. Deine Lösung funktioniert so auf 
den größeren Megas nicht. Das kann man jetzt auch noch abfangen, aber ob 
du dann alle Möglichkeiten abgedeckt hast, und ob das auch immer so 
bleibt, kannst du nie wissen.
Dann doch lieber eine handvoll Assemblerbefehle für die ISR, von denen 
man genau weiß, was die tun. Der ganze Rest kann und soll in C bleiben.

Oliver

[Uhu U. (uhu)]

Oliver S. schrieb:
> Deine Lösung funktioniert so auf den größeren Megas nicht.

Lies den Thread-Titel…

[Andreas H. (Gast)]

Entscheidend ist die eingangsseitige Idee, wobei die Umsetzung jeder für 
sich nachnutzen oder adaptieren kann. Und wenn man meint, auf 
Assemblerniveau besser unterwegs zu sein: bitte.

[Uhu U. (uhu)]

Hier eine verbesserte Version des Watchdog-Paketchens.

An Schnittstelle und Verhalten hat sich nichts geändert.

Geändert wurde die Bestimmung der ISR-Returnadresse, falls der Watchdog 
außerhalb eines Idle-Aufrufs überläuft: Statt die push-Befehle nach der 
Präambel der ISR zu zählen, wird auf die gcc-Erweiterung

  __builtin_return_address(0)

zurückgegriffen. Damit kann die Return-Adresse direkt aus dem Stack 
ausgelesen werden. Auf dem AVR muss dieser Wert noch mit 2 multipliziert 
werden, um die Byte-Adresse im Code zu erhalten.


Für Details:
6.51 Getting the Return or Frame Address of a Function
https://gcc.gnu.org/onlinedocs/gcc/Return-Address.html