| Message ID | 20b7f194-066f-c3bf-a830-deb1cde8f1be@adacore.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] AVR watchdog | expand |
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
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 >
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
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.
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); }
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: In the case the guest wants to reset the board through the watchdog, would that make sense to swap to that? Best Regards, Fred