| Message ID | 20250215021654.1786679-4-keithp@keithp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Renesas RX target fixes | expand |
+Peter/Igor On 15/2/25 03:16, Keith Packard via wrote: > This ensure that the CPU gets reset every time QEMU resets. > > Signed-off-by: Keith Packard <keithp@keithp.com> > --- > target/rx/cpu.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/target/rx/cpu.c b/target/rx/cpu.c > index 37a6fdd569..04dd34b310 100644 > --- a/target/rx/cpu.c > +++ b/target/rx/cpu.c > @@ -27,6 +27,7 @@ > #include "hw/loader.h" > #include "fpu/softfloat.h" > #include "tcg/debug-assert.h" > +#include "system/reset.h" > > static void rx_cpu_set_pc(CPUState *cs, vaddr value) > { > @@ -129,6 +130,13 @@ static ObjectClass *rx_cpu_class_by_name(const char *cpu_model) > return oc; > } > > +static void rx_cpu_reset(void *opaque) > +{ > + RXCPU *cpu = opaque; > + > + cpu_reset(CPU(cpu)); > +} > + > static void rx_cpu_realize(DeviceState *dev, Error **errp) > { > CPUState *cs = CPU(dev); > @@ -142,9 +150,10 @@ static void rx_cpu_realize(DeviceState *dev, Error **errp) > } > > qemu_init_vcpu(cs); > - cpu_reset(cs); > > rcc->parent_realize(dev, errp); > + > + qemu_register_reset(rx_cpu_reset, RX_CPU(cs)); Yeah. I guess this is part of some generic effort to generalize CPU reset, previously started here https://lore.kernel.org/qemu-devel/20230918160257.30127-1-philmd@linaro.org/ which led to another issue Igor is trying to solves: https://lore.kernel.org/qemu-devel/20250207162048.1890669-1-imammedo@redhat.com/ > } > > static void rx_cpu_set_irq(void *opaque, int no, int request)
On Sat, 15 Feb 2025 at 02:17, Keith Packard via <qemu-devel@nongnu.org> wrote: > > This ensure that the CPU gets reset every time QEMU resets. > > Signed-off-by: Keith Packard <keithp@keithp.com> > --- > target/rx/cpu.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/target/rx/cpu.c b/target/rx/cpu.c > index 37a6fdd569..04dd34b310 100644 > --- a/target/rx/cpu.c > +++ b/target/rx/cpu.c > @@ -27,6 +27,7 @@ > #include "hw/loader.h" > #include "fpu/softfloat.h" > #include "tcg/debug-assert.h" > +#include "system/reset.h" > > static void rx_cpu_set_pc(CPUState *cs, vaddr value) > { > @@ -129,6 +130,13 @@ static ObjectClass *rx_cpu_class_by_name(const char *cpu_model) > return oc; > } > > +static void rx_cpu_reset(void *opaque) > +{ > + RXCPU *cpu = opaque; > + > + cpu_reset(CPU(cpu)); > +} > + > static void rx_cpu_realize(DeviceState *dev, Error **errp) > { > CPUState *cs = CPU(dev); > @@ -142,9 +150,10 @@ static void rx_cpu_realize(DeviceState *dev, Error **errp) > } > > qemu_init_vcpu(cs); > - cpu_reset(cs); > > rcc->parent_realize(dev, errp); > + > + qemu_register_reset(rx_cpu_reset, RX_CPU(cs)); > } Reset of devices not plugged into buses (of which CPUs are the most common kind) is a mess. But having them call qemu_register_reset() themselves in their own realize method isn't the usual workaround. Instead we get the board code to do it (usually in the same function that handles arranging to sort out the in-QEMU boot loader, see eg hw/arm/boot.c). thanks -- PMM
From: Peter Maydell <peter.maydell@linaro.org> Date: Mon, 17 Feb 2025 09:53:58 +0000 > Reset of devices not plugged into buses (of which CPUs > are the most common kind) is a mess. But having them > call qemu_register_reset() themselves in their own > realize method isn't the usual workaround. Instead we > get the board code to do it (usually in the same function > that handles arranging to sort out the in-QEMU boot > loader, see eg hw/arm/boot.c). Oh, thanks so much! I was confused about how to select the right starting PC value -- when loading a kernel image, the code wanted to set the PC to the kernel entry point, but otherwise it wanted to use the reset vector. That "worked" because the kernel loading code set the PC after the reset vector was loaded from ROM. With your advice, I've made the hardware layer register the reset handler which just calls cpu_reset and then sets the initial PC value, either from the loaded kernel or using the reset vector contents. So much cleaner and "obviously" doing what we want.
diff --git a/target/rx/cpu.c b/target/rx/cpu.c index 37a6fdd569..04dd34b310 100644 --- a/target/rx/cpu.c +++ b/target/rx/cpu.c @@ -27,6 +27,7 @@ #include "hw/loader.h" #include "fpu/softfloat.h" #include "tcg/debug-assert.h" +#include "system/reset.h" static void rx_cpu_set_pc(CPUState *cs, vaddr value) { @@ -129,6 +130,13 @@ static ObjectClass *rx_cpu_class_by_name(const char *cpu_model) return oc; } +static void rx_cpu_reset(void *opaque) +{ + RXCPU *cpu = opaque; + + cpu_reset(CPU(cpu)); +} + static void rx_cpu_realize(DeviceState *dev, Error **errp) { CPUState *cs = CPU(dev); @@ -142,9 +150,10 @@ static void rx_cpu_realize(DeviceState *dev, Error **errp) } qemu_init_vcpu(cs); - cpu_reset(cs); rcc->parent_realize(dev, errp); + + qemu_register_reset(rx_cpu_reset, RX_CPU(cs)); } static void rx_cpu_set_irq(void *opaque, int no, int request)
This ensure that the CPU gets reset every time QEMU resets. Signed-off-by: Keith Packard <keithp@keithp.com> --- target/rx/cpu.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)