Forum: Projekte & Code Watchdog speichert Reset-Adresse -- für atmega328p


von Uhu U. (uhu)


Angehängte Dateien:

Lesenswert?

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.

: Bearbeitet durch User
von Andreas H. (Gast)


Lesenswert?

Vielen Dank für den interessanten Codeauszug.

Zwei Anmerkungen/Fragen:
1)
in der Zeile
1
// scan wdt isr for return address offset
 wird noch #5 addiert. Woraus resultiert dieses "5"?

2)
Bei der Ermittlung der Stacktiefe
1
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.

von Uhu U. (uhu)


Lesenswert?

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

Das sind die 5 Befehle, die der Compiler zu Beginn jeder ISR absetzt:
1
  1f 92         push  r1
2
  0f 92         push  r0
3
  0f b6         in  r0, 0x3f  ; 63
4
  0f 92         push  r0
5
  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.

: Bearbeitet durch User
von Andreas H. (Gast)


Lesenswert?

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
1
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:
1
const uint16_t *pCode = (const uint16_t *) (*((const __flash uint16_t *) 0x1a) << 1) + 5;
 - das kann man vielleicht aus dem jeweiligen Prozessor-Headerfile 
herausholen?

von Uhu U. (uhu)


Lesenswert?

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:
1
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…

: Bearbeitet durch User
von Andreas H. (Gast)


Lesenswert?

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

von Bauform B. (bauformb)


Lesenswert?

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.
1
-fstack-usage
2
3
Makes the compiler output stack usage information for the program,
4
on a per-function basis. The filename for the dump is made by
5
appending .su to the auxname. auxname is generated from the name
6
of the output file, if explicitly specified and it is not an
7
executable, otherwise it is the basename of the source file.
8
An entry is made up of three fields:
9
        The name of the function.
10
        A number of bytes.
11
        One or more qualifiers: static, dynamic, bounded. 
12
The qualifier static means that the function manipulates the stack
13
statically: a fixed number of bytes are allocated for the frame
14
on function entry and released on function exit; no stack
15
adjustments are otherwise made in the function. The second field
16
is this fixed number of bytes.
17
18
The qualifier dynamic means that the function manipulates the stack
19
dynamically: in addition to the static allocation described above,
20
stack adjustments are made in the body of the function, for example
21
to push/pop arguments around function calls. If the qualifier
22
bounded is also present, the amount of these adjustments is bounded
23
at compile time and the second field is an upper bound of the total
24
amount of stack used by the function. If it is not present, the
25
amount of these adjustments is not bounded at compile time and the
26
second field only represents the bounded part.

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

von Oliver S. (oliverso)


Lesenswert?

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

Oliver

von Uhu U. (uhu)


Lesenswert?

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:
1
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:
1
   push r20
2
   push r21
3
   ret

: Bearbeitet durch User
von Andreas H. (Gast)


Lesenswert?

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.

von Oliver S. (oliverso)


Lesenswert?

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

von Uhu U. (uhu)


Lesenswert?

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.

: Bearbeitet durch User
von Oliver S. (oliverso)


Lesenswert?

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):
1
000001e6 <__vector_12>:
2
 1e6:  1f 92         push  r1
3
 1e8:  0f 92         push  r0
4
 1ea:  0f b6         in  r0, 0x3f  ; 63
5
 1ec:  0f 92         push  r0
6
 1ee:  11 24         eor  r1, r1
7
 1f0:  0b b6         in  r0, 0x3b  ; 59
8
 1f2:  0f 92         push  r0
9
...

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

von Uhu U. (uhu)


Lesenswert?

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…

: Bearbeitet durch User
von Oliver S. (oliverso)


Lesenswert?

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

: Bearbeitet durch User
von Uhu U. (uhu)


Lesenswert?

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

Lies den Thread-Titel…

von Andreas H. (Gast)


Lesenswert?

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.

von Uhu U. (uhu)


Angehängte Dateien:

Lesenswert?

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

: Bearbeitet durch User
Bitte melde dich an um einen Beitrag zu schreiben. Anmeldung ist kostenlos und dauert nur eine Minute.
Bestehender Account
Schon ein Account bei Google/GoogleMail? Keine Anmeldung erforderlich!
Mit Google-Account einloggen
Noch kein Account? Hier anmelden.