| Message ID | 20230907112640.292104-5-chigot@adacore.com |
|---|---|
| State | New |
| Headers | show |
| Series | Risc-V/gdb: replace exit calls with proper shutdown | expand |
On Thu, Sep 7, 2023 at 9:26 PM Clément Chigot <chigot@adacore.com> wrote: > > This replaces the exit calls by shutdown requests, ensuring a proper > cleanup of Qemu. Otherwise, some connections like gdb could be broken > before its final packet ("Wxx") is being sent. This part, being done > inside qemu_cleanup function, can be reached only when the main loop > exits after a shutdown request. > > Signed-off-by: Clément Chigot <chigot@adacore.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > hw/char/riscv_htif.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c > index 37d3ccc76b..7e9b6fcc98 100644 > --- a/hw/char/riscv_htif.c > +++ b/hw/char/riscv_htif.c > @@ -31,6 +31,7 @@ > #include "qemu/error-report.h" > #include "exec/address-spaces.h" > #include "sysemu/dma.h" > +#include "sysemu/runstate.h" > > #define RISCV_DEBUG_HTIF 0 > #define HTIF_DEBUG(fmt, ...) \ > @@ -205,7 +206,9 @@ static void htif_handle_tohost_write(HTIFState *s, uint64_t val_written) > g_free(sig_data); > } > > - exit(exit_code); > + qemu_system_shutdown_request_with_code( > + SHUTDOWN_CAUSE_GUEST_SHUTDOWN, exit_code); > + return; > } else { > uint64_t syscall[8]; > cpu_physical_memory_read(payload, syscall, sizeof(syscall)); > -- > 2.25.1 >
On Thu, Sep 7, 2023 at 9:26 PM Clément Chigot <chigot@adacore.com> wrote: > > This replaces the exit calls by shutdown requests, ensuring a proper > cleanup of Qemu. Otherwise, some connections like gdb could be broken > before its final packet ("Wxx") is being sent. This part, being done > inside qemu_cleanup function, can be reached only when the main loop > exits after a shutdown request. > > Signed-off-by: Clément Chigot <chigot@adacore.com> Do you mind rebasing this on: https://github.com/alistair23/qemu/tree/riscv-to-apply.next Alistair > --- > hw/char/riscv_htif.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c > index 37d3ccc76b..7e9b6fcc98 100644 > --- a/hw/char/riscv_htif.c > +++ b/hw/char/riscv_htif.c > @@ -31,6 +31,7 @@ > #include "qemu/error-report.h" > #include "exec/address-spaces.h" > #include "sysemu/dma.h" > +#include "sysemu/runstate.h" > > #define RISCV_DEBUG_HTIF 0 > #define HTIF_DEBUG(fmt, ...) \ > @@ -205,7 +206,9 @@ static void htif_handle_tohost_write(HTIFState *s, uint64_t val_written) > g_free(sig_data); > } > > - exit(exit_code); > + qemu_system_shutdown_request_with_code( > + SHUTDOWN_CAUSE_GUEST_SHUTDOWN, exit_code); > + return; > } else { > uint64_t syscall[8]; > cpu_physical_memory_read(payload, syscall, sizeof(syscall)); > -- > 2.25.1 >
On Fri, Sep 22, 2023 at 7:20 AM Alistair Francis <alistair23@gmail.com> wrote: > > On Thu, Sep 7, 2023 at 9:26 PM Clément Chigot <chigot@adacore.com> wrote: > > > > This replaces the exit calls by shutdown requests, ensuring a proper > > cleanup of Qemu. Otherwise, some connections like gdb could be broken > > before its final packet ("Wxx") is being sent. This part, being done > > inside qemu_cleanup function, can be reached only when the main loop > > exits after a shutdown request. > > > > Signed-off-by: Clément Chigot <chigot@adacore.com> > > Do you mind rebasing this on: > https://github.com/alistair23/qemu/tree/riscv-to-apply.next Thanks for the review. Just a quick question on the procedure side, is there any special tag or something to say in the cover letter to state that it has been rebased on riscv-to-apply instead of the usual master? Clément > Alistair > > > --- > > hw/char/riscv_htif.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c > > index 37d3ccc76b..7e9b6fcc98 100644 > > --- a/hw/char/riscv_htif.c > > +++ b/hw/char/riscv_htif.c > > @@ -31,6 +31,7 @@ > > #include "qemu/error-report.h" > > #include "exec/address-spaces.h" > > #include "sysemu/dma.h" > > +#include "sysemu/runstate.h" > > > > #define RISCV_DEBUG_HTIF 0 > > #define HTIF_DEBUG(fmt, ...) \ > > @@ -205,7 +206,9 @@ static void htif_handle_tohost_write(HTIFState *s, uint64_t val_written) > > g_free(sig_data); > > } > > > > - exit(exit_code); > > + qemu_system_shutdown_request_with_code( > > + SHUTDOWN_CAUSE_GUEST_SHUTDOWN, exit_code); > > + return; > > } else { > > uint64_t syscall[8]; > > cpu_physical_memory_read(payload, syscall, sizeof(syscall)); > > -- > > 2.25.1 > >
On Mon, Oct 2, 2023 at 7:32 PM Clément Chigot <chigot@adacore.com> wrote: > > On Fri, Sep 22, 2023 at 7:20 AM Alistair Francis <alistair23@gmail.com> wrote: > > > > On Thu, Sep 7, 2023 at 9:26 PM Clément Chigot <chigot@adacore.com> wrote: > > > > > > This replaces the exit calls by shutdown requests, ensuring a proper > > > cleanup of Qemu. Otherwise, some connections like gdb could be broken > > > before its final packet ("Wxx") is being sent. This part, being done > > > inside qemu_cleanup function, can be reached only when the main loop > > > exits after a shutdown request. > > > > > > Signed-off-by: Clément Chigot <chigot@adacore.com> > > > > Do you mind rebasing this on: > > https://github.com/alistair23/qemu/tree/riscv-to-apply.next > > Thanks for the review. > Just a quick question on the procedure side, is there any special tag > or something to say in the cover letter to state that it has been > rebased on riscv-to-apply instead of the usual master? You can use the "Based-on" tag. There is some more details here: https://www.qemu.org/docs/master/devel/submitting-a-patch.html#id33 Alistair > > Clément > > > Alistair > > > > > --- > > > hw/char/riscv_htif.c | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c > > > index 37d3ccc76b..7e9b6fcc98 100644 > > > --- a/hw/char/riscv_htif.c > > > +++ b/hw/char/riscv_htif.c > > > @@ -31,6 +31,7 @@ > > > #include "qemu/error-report.h" > > > #include "exec/address-spaces.h" > > > #include "sysemu/dma.h" > > > +#include "sysemu/runstate.h" > > > > > > #define RISCV_DEBUG_HTIF 0 > > > #define HTIF_DEBUG(fmt, ...) \ > > > @@ -205,7 +206,9 @@ static void htif_handle_tohost_write(HTIFState *s, uint64_t val_written) > > > g_free(sig_data); > > > } > > > > > > - exit(exit_code); > > > + qemu_system_shutdown_request_with_code( > > > + SHUTDOWN_CAUSE_GUEST_SHUTDOWN, exit_code); > > > + return; > > > } else { > > > uint64_t syscall[8]; > > > cpu_physical_memory_read(payload, syscall, sizeof(syscall)); > > > -- > > > 2.25.1 > > >
diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c index 37d3ccc76b..7e9b6fcc98 100644 --- a/hw/char/riscv_htif.c +++ b/hw/char/riscv_htif.c @@ -31,6 +31,7 @@ #include "qemu/error-report.h" #include "exec/address-spaces.h" #include "sysemu/dma.h" +#include "sysemu/runstate.h" #define RISCV_DEBUG_HTIF 0 #define HTIF_DEBUG(fmt, ...) \ @@ -205,7 +206,9 @@ static void htif_handle_tohost_write(HTIFState *s, uint64_t val_written) g_free(sig_data); } - exit(exit_code); + qemu_system_shutdown_request_with_code( + SHUTDOWN_CAUSE_GUEST_SHUTDOWN, exit_code); + return; } else { uint64_t syscall[8]; cpu_physical_memory_read(payload, syscall, sizeof(syscall));
This replaces the exit calls by shutdown requests, ensuring a proper cleanup of Qemu. Otherwise, some connections like gdb could be broken before its final packet ("Wxx") is being sent. This part, being done inside qemu_cleanup function, can be reached only when the main loop exits after a shutdown request. Signed-off-by: Clément Chigot <chigot@adacore.com> --- hw/char/riscv_htif.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)