[RFC] AVR watchdog

[RFC] AVR watchdog

[Fred Konrad]

Hi,

I fall on a segfault while running the wdr instruction on AVR:

(gdb) bt
      #0  0x00005555add0b23a in gdb_get_cpu_pid (cpu=0x5555af5a4af0) at
        ../gdbstub.c:718
      #1  0x00005555add0b2dd in gdb_get_cpu_process (cpu=0x5555af5a4af0) at
        ../gdbstub.c:743
      #2  0x00005555add0e477 in gdb_set_stop_cpu (cpu=0x5555af5a4af0) at
        ../gdbstub.c:2742
      #3  0x00005555adc99b96 in cpu_handle_guest_debug (cpu=0x5555af5a4af0) at
        ../softmmu/cpus.c:306
      #4  0x00005555adcc66ab in rr_cpu_thread_fn (arg=0x5555af5a4af0) at
        ../accel/tcg/tcg-accel-ops-rr.c:224
      #5  0x00005555adefaf12 in qemu_thread_start (args=0x5555af5d9870) at
        ../util/qemu-thread-posix.c:521
      #6  0x00007f692d940ea5 in start_thread () from /lib64/libpthread.so.0
      #7  0x00007f692d6699fd in clone () from /lib64/libc.so.6

Wondering if there are some plan/on-going work to implement this watchdog?

---

Also meanwhile I though about a workaround like that:

diff --git a/target/avr/helper.c b/target/avr/helper.c
index 35e1019594..7944ed21f4 100644
--- a/target/avr/helper.c
+++ b/target/avr/helper.c
@@ -24,6 +24,7 @@
  #include "exec/exec-all.h"
  #include "exec/address-spaces.h"
  #include "exec/helper-proto.h"
+#include "sysemu/runstate.h"

  bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
  {
@@ -191,7 +192,7 @@ void helper_wdr(CPUAVRState *env)
      CPUState *cs = env_cpu(env);

      /* WD is not implemented yet, placeholder */
-    cs->exception_index = EXCP_DEBUG;
+    qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
      cpu_loop_exit(cs);
  }

In the case the guest wants to reset the board through the watchdog, would that
make sense to swap to that?

Best Regards,
Fred

Re: [RFC] AVR watchdog

[no-reply]

Patchew URL: https://patchew.org/QEMU/20b7f194-066f-c3bf-a830-deb1cde8f1be@adacore.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20b7f194-066f-c3bf-a830-deb1cde8f1be@adacore.com
Subject: [RFC] AVR watchdog

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20b7f194-066f-c3bf-a830-deb1cde8f1be@adacore.com -> patchew/20b7f194-066f-c3bf-a830-deb1cde8f1be@adacore.com
Switched to a new branch 'test'
9d267c4 AVR watchdog

=== OUTPUT BEGIN ===
ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 15 lines checked

Commit 9d267c4ce773 (AVR watchdog) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20b7f194-066f-c3bf-a830-deb1cde8f1be@adacore.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [RFC] AVR watchdog

[Michael Rolnik]

[-- Attachment #1: Type: text/plain, Size: 2028 bytes --]

Hi Fred.

How can I reproduce it?
Thank you.
Michael Rolnik

Sent from my cell phone, please ignore typos

On Wed, Apr 28, 2021, 5:17 PM Fred Konrad <konrad@adacore.com> wrote:

> Hi,
>
> I fall on a segfault while running the wdr instruction on AVR:
>
> (gdb) bt
>       #0  0x00005555add0b23a in gdb_get_cpu_pid (cpu=0x5555af5a4af0) at
>         ../gdbstub.c:718
>       #1  0x00005555add0b2dd in gdb_get_cpu_process (cpu=0x5555af5a4af0) at
>         ../gdbstub.c:743
>       #2  0x00005555add0e477 in gdb_set_stop_cpu (cpu=0x5555af5a4af0) at
>         ../gdbstub.c:2742
>       #3  0x00005555adc99b96 in cpu_handle_guest_debug
> (cpu=0x5555af5a4af0) at
>         ../softmmu/cpus.c:306
>       #4  0x00005555adcc66ab in rr_cpu_thread_fn (arg=0x5555af5a4af0) at
>         ../accel/tcg/tcg-accel-ops-rr.c:224
>       #5  0x00005555adefaf12 in qemu_thread_start (args=0x5555af5d9870) at
>         ../util/qemu-thread-posix.c:521
>       #6  0x00007f692d940ea5 in start_thread () from /lib64/libpthread.so.0
>       #7  0x00007f692d6699fd in clone () from /lib64/libc.so.6
>
> Wondering if there are some plan/on-going work to implement this watchdog?
>
> ---
>
> Also meanwhile I though about a workaround like that:
>
> diff --git a/target/avr/helper.c b/target/avr/helper.c
> index 35e1019594..7944ed21f4 100644
> --- a/target/avr/helper.c
> +++ b/target/avr/helper.c
> @@ -24,6 +24,7 @@
>   #include "exec/exec-all.h"
>   #include "exec/address-spaces.h"
>   #include "exec/helper-proto.h"
> +#include "sysemu/runstate.h"
>
>   bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>   {
> @@ -191,7 +192,7 @@ void helper_wdr(CPUAVRState *env)
>       CPUState *cs = env_cpu(env);
>
>       /* WD is not implemented yet, placeholder */
> -    cs->exception_index = EXCP_DEBUG;
> +    qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
>       cpu_loop_exit(cs);
>   }
>
> In the case the guest wants to reset the board through the watchdog, would
> that
> make sense to swap to that?
>
> Best Regards,
> Fred
>

[-- Attachment #2: Type: text/html, Size: 2722 bytes --]

Re: [RFC] AVR watchdog

[Fred Konrad]

Le 4/28/21 à 8:17 PM, Michael Rolnik a écrit :
> Hi Fred.
> 
> How can I reproduce it?
> Thank you.
> Michael Rolnik
> 

Hi Michael,

First sorry for the patchew noise, I didn't meant to sent a patch just an
inlined diff.

For the reproducer, that's pretty straight-forward with v6.0.0-rc5:

$ cat > foo.S << EOF
 > __start:
 >     wdr
 > EOF

$ avr-gcc -nostdlib -nostartfiles -mmcu=avr6 foo.S -o foo.elf
$ xxx/qemu-system-avr -serial mon:stdio -nographic -no-reboot -M mega \
   -bios foo.elf -d in_asm --singlestep
IN:
0x00000000:  WDR

Segmentation fault (core dumped)

Note that I put "--singlestep" here to avoid translating NOPs after the WDR,
it breaks without as well.

Fred

Re: [RFC] AVR watchdog

[Philippe Mathieu-Daudé]

On 4/28/21 4:17 PM, Fred Konrad wrote:
> Hi,
> 
> I fall on a segfault while running the wdr instruction on AVR:
> 
> (gdb) bt
>      #0  0x00005555add0b23a in gdb_get_cpu_pid (cpu=0x5555af5a4af0) at
>        ../gdbstub.c:718
>      #1  0x00005555add0b2dd in gdb_get_cpu_process (cpu=0x5555af5a4af0) at
>        ../gdbstub.c:743
>      #2  0x00005555add0e477 in gdb_set_stop_cpu (cpu=0x5555af5a4af0) at
>        ../gdbstub.c:2742
>      #3  0x00005555adc99b96 in cpu_handle_guest_debug
> (cpu=0x5555af5a4af0) at
>        ../softmmu/cpus.c:306
>      #4  0x00005555adcc66ab in rr_cpu_thread_fn (arg=0x5555af5a4af0) at
>        ../accel/tcg/tcg-accel-ops-rr.c:224
>      #5  0x00005555adefaf12 in qemu_thread_start (args=0x5555af5d9870) at
>        ../util/qemu-thread-posix.c:521
>      #6  0x00007f692d940ea5 in start_thread () from /lib64/libpthread.so.0
>      #7  0x00007f692d6699fd in clone () from /lib64/libc.so.6
> 
> Wondering if there are some plan/on-going work to implement this watchdog?
> 
> ---
> 
> Also meanwhile I though about a workaround like that:
> 
> diff --git a/target/avr/helper.c b/target/avr/helper.c
> index 35e1019594..7944ed21f4 100644
> --- a/target/avr/helper.c
> +++ b/target/avr/helper.c
> @@ -24,6 +24,7 @@
>  #include "exec/exec-all.h"
>  #include "exec/address-spaces.h"
>  #include "exec/helper-proto.h"
> +#include "sysemu/runstate.h"
> 
>  bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>  {
> @@ -191,7 +192,7 @@ void helper_wdr(CPUAVRState *env)
>      CPUState *cs = env_cpu(env);
> 
>      /* WD is not implemented yet, placeholder */
> -    cs->exception_index = EXCP_DEBUG;
> +    qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);

Eh, this is the opposite... This opcode kicks the watchdog,
it does not trigger it.

>      cpu_loop_exit(cs);
>  }
> 
> In the case the guest wants to reset the board through the watchdog,
> would that
> make sense to swap to that?

Why not simply log the opcode and keep going?

-- >8 --
diff --git a/target/avr/helper.c b/target/avr/helper.c
index 35e10195940..981c29da453 100644
--- a/target/avr/helper.c
+++ b/target/avr/helper.c
@@ -190,7 +190,3 @@ void helper_wdr(CPUAVRState *env)
 {
-    CPUState *cs = env_cpu(env);
-
-    /* WD is not implemented yet, placeholder */
-    cs->exception_index = EXCP_DEBUG;
-    cpu_loop_exit(cs);
+    qemu_log_mask(LOG_UNIMP, "Watchdog Timer Reset\n");
 }
---

Regards,

Phil.