| Message ID | 20221125115240.3005559-6-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | virtio, pci, xics: 3-phase reset conversions | expand |
On 11/25/22 12:52, Peter Maydell wrote: > The realize method for the TYPE_ICS class uses qemu_register_reset() > to register a reset handler, as a workaround for the fact that > currently objects which directly inherit from TYPE_DEVICE don't get > automatically reset. However, the reset function directly calls > ics_reset(), which is the function that implements the legacy reset > method. This means that only the parent class's data gets reset, and > a subclass which also needs to handle reset, like TYPE_PHB3_MSI, has > to register its own reset function. > > Make the TYPE_ICS reset function call device_cold_reset() instead: > this will handle reset for both the parent class and the subclass, > and will work whether the classes are using legacy reset or 3-phase > reset. This allows us to remove the reset function that the subclass > currently has to set up. Nice ! > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Cédric Le Goater <clg@kaod.org> Thanks, C. > --- > hw/intc/xics.c | 2 +- > hw/pci-host/pnv_phb3_msi.c | 7 ------- > 2 files changed, 1 insertion(+), 8 deletions(-) > > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > index dcd021af668..dd130467ccc 100644 > --- a/hw/intc/xics.c > +++ b/hw/intc/xics.c > @@ -593,7 +593,7 @@ static void ics_reset(DeviceState *dev) > > static void ics_reset_handler(void *dev) > { > - ics_reset(dev); > + device_cold_reset(dev); > } > > static void ics_realize(DeviceState *dev, Error **errp) > diff --git a/hw/pci-host/pnv_phb3_msi.c b/hw/pci-host/pnv_phb3_msi.c > index 2f4112907b8..ae908fd9e41 100644 > --- a/hw/pci-host/pnv_phb3_msi.c > +++ b/hw/pci-host/pnv_phb3_msi.c > @@ -239,11 +239,6 @@ static void phb3_msi_reset(DeviceState *dev) > msi->rba_sum = 0; > } > > -static void phb3_msi_reset_handler(void *dev) > -{ > - phb3_msi_reset(dev); > -} > - > void pnv_phb3_msi_update_config(Phb3MsiState *msi, uint32_t base, > uint32_t count) > { > @@ -272,8 +267,6 @@ static void phb3_msi_realize(DeviceState *dev, Error **errp) > } > > msi->qirqs = qemu_allocate_irqs(phb3_msi_set_irq, msi, ics->nr_irqs); > - > - qemu_register_reset(phb3_msi_reset_handler, dev); > } > > static void phb3_msi_instance_init(Object *obj)
On Fri, 25 Nov 2022 13:24:00 +0100 Cédric Le Goater <clg@kaod.org> wrote: > On 11/25/22 12:52, Peter Maydell wrote: > > The realize method for the TYPE_ICS class uses qemu_register_reset() > > to register a reset handler, as a workaround for the fact that > > currently objects which directly inherit from TYPE_DEVICE don't get > > automatically reset. However, the reset function directly calls > > ics_reset(), which is the function that implements the legacy reset > > method. This means that only the parent class's data gets reset, and > > a subclass which also needs to handle reset, like TYPE_PHB3_MSI, has > > to register its own reset function. > > > > Make the TYPE_ICS reset function call device_cold_reset() instead: > > this will handle reset for both the parent class and the subclass, > > and will work whether the classes are using legacy reset or 3-phase > > reset. This allows us to remove the reset function that the subclass > > currently has to set up. > > Nice ! > Seconded. Reviewed-by: Greg Kurz <groug@kaod.org> > > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > Reviewed-by: Cédric Le Goater <clg@kaod.org> > > Thanks, > > C. > > > --- > > hw/intc/xics.c | 2 +- > > hw/pci-host/pnv_phb3_msi.c | 7 ------- > > 2 files changed, 1 insertion(+), 8 deletions(-) > > > > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > > index dcd021af668..dd130467ccc 100644 > > --- a/hw/intc/xics.c > > +++ b/hw/intc/xics.c > > @@ -593,7 +593,7 @@ static void ics_reset(DeviceState *dev) > > > > static void ics_reset_handler(void *dev) > > { > > - ics_reset(dev); > > + device_cold_reset(dev); > > } > > > > static void ics_realize(DeviceState *dev, Error **errp) > > diff --git a/hw/pci-host/pnv_phb3_msi.c b/hw/pci-host/pnv_phb3_msi.c > > index 2f4112907b8..ae908fd9e41 100644 > > --- a/hw/pci-host/pnv_phb3_msi.c > > +++ b/hw/pci-host/pnv_phb3_msi.c > > @@ -239,11 +239,6 @@ static void phb3_msi_reset(DeviceState *dev) > > msi->rba_sum = 0; > > } > > > > -static void phb3_msi_reset_handler(void *dev) > > -{ > > - phb3_msi_reset(dev); > > -} > > - > > void pnv_phb3_msi_update_config(Phb3MsiState *msi, uint32_t base, > > uint32_t count) > > { > > @@ -272,8 +267,6 @@ static void phb3_msi_realize(DeviceState *dev, Error **errp) > > } > > > > msi->qirqs = qemu_allocate_irqs(phb3_msi_set_irq, msi, ics->nr_irqs); > > - > > - qemu_register_reset(phb3_msi_reset_handler, dev); > > } > > > > static void phb3_msi_instance_init(Object *obj) >
On 25/11/22 14:45, Greg Kurz wrote: > On Fri, 25 Nov 2022 13:24:00 +0100 > Cédric Le Goater <clg@kaod.org> wrote: > >> On 11/25/22 12:52, Peter Maydell wrote: >>> The realize method for the TYPE_ICS class uses qemu_register_reset() >>> to register a reset handler, as a workaround for the fact that >>> currently objects which directly inherit from TYPE_DEVICE don't get >>> automatically reset. However, the reset function directly calls >>> ics_reset(), which is the function that implements the legacy reset >>> method. This means that only the parent class's data gets reset, and >>> a subclass which also needs to handle reset, like TYPE_PHB3_MSI, has >>> to register its own reset function. >>> >>> Make the TYPE_ICS reset function call device_cold_reset() instead: >>> this will handle reset for both the parent class and the subclass, >>> and will work whether the classes are using legacy reset or 3-phase >>> reset. This allows us to remove the reset function that the subclass >>> currently has to set up. >> >> Nice ! >> > > Seconded. > Thirded :) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > Reviewed-by: Greg Kurz <groug@kaod.org> > >>> >>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> >> Reviewed-by: Cédric Le Goater <clg@kaod.org>
diff --git a/hw/intc/xics.c b/hw/intc/xics.c index dcd021af668..dd130467ccc 100644 --- a/hw/intc/xics.c +++ b/hw/intc/xics.c @@ -593,7 +593,7 @@ static void ics_reset(DeviceState *dev) static void ics_reset_handler(void *dev) { - ics_reset(dev); + device_cold_reset(dev); } static void ics_realize(DeviceState *dev, Error **errp) diff --git a/hw/pci-host/pnv_phb3_msi.c b/hw/pci-host/pnv_phb3_msi.c index 2f4112907b8..ae908fd9e41 100644 --- a/hw/pci-host/pnv_phb3_msi.c +++ b/hw/pci-host/pnv_phb3_msi.c @@ -239,11 +239,6 @@ static void phb3_msi_reset(DeviceState *dev) msi->rba_sum = 0; } -static void phb3_msi_reset_handler(void *dev) -{ - phb3_msi_reset(dev); -} - void pnv_phb3_msi_update_config(Phb3MsiState *msi, uint32_t base, uint32_t count) { @@ -272,8 +267,6 @@ static void phb3_msi_realize(DeviceState *dev, Error **errp) } msi->qirqs = qemu_allocate_irqs(phb3_msi_set_irq, msi, ics->nr_irqs); - - qemu_register_reset(phb3_msi_reset_handler, dev); } static void phb3_msi_instance_init(Object *obj)
The realize method for the TYPE_ICS class uses qemu_register_reset() to register a reset handler, as a workaround for the fact that currently objects which directly inherit from TYPE_DEVICE don't get automatically reset. However, the reset function directly calls ics_reset(), which is the function that implements the legacy reset method. This means that only the parent class's data gets reset, and a subclass which also needs to handle reset, like TYPE_PHB3_MSI, has to register its own reset function. Make the TYPE_ICS reset function call device_cold_reset() instead: this will handle reset for both the parent class and the subclass, and will work whether the classes are using legacy reset or 3-phase reset. This allows us to remove the reset function that the subclass currently has to set up. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/xics.c | 2 +- hw/pci-host/pnv_phb3_msi.c | 7 ------- 2 files changed, 1 insertion(+), 8 deletions(-)