| Message ID | 20230422150728.176512-6-shentey@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Clean up PCI IDE device models | expand |
On Sat, 22 Apr 2023, Bernhard Beschow wrote: > Resolves redundant code in every PCI IDE device model. This patch could be broken up a bit more as it seems to do unrelated changes. Such as setting DEVICE_CATEGORY_STORAGE in a different way could be a separate patch to make it simpler to review. Regards, BALATON Zoltan > --- > include/hw/ide/pci.h | 1 - > hw/ide/cmd646.c | 15 --------------- > hw/ide/pci.c | 25 ++++++++++++++++++++++++- > hw/ide/piix.c | 19 ------------------- > hw/ide/sii3112.c | 3 ++- > hw/ide/via.c | 15 --------------- > 6 files changed, 26 insertions(+), 52 deletions(-) > > diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h > index 74c127e32f..7bc4e53d02 100644 > --- a/include/hw/ide/pci.h > +++ b/include/hw/ide/pci.h > @@ -61,7 +61,6 @@ void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val); > extern MemoryRegionOps bmdma_addr_ioport_ops; > void pci_ide_create_devs(PCIDevice *dev); > > -extern const VMStateDescription vmstate_ide_pci; > extern const MemoryRegionOps pci_ide_cmd_le_ops; > extern const MemoryRegionOps pci_ide_data_le_ops; > #endif > diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c > index a094a6e12a..9aabf80e52 100644 > --- a/hw/ide/cmd646.c > +++ b/hw/ide/cmd646.c > @@ -301,17 +301,6 @@ static void pci_cmd646_ide_realize(PCIDevice *dev, Error **errp) > } > } > > -static void pci_cmd646_ide_exitfn(PCIDevice *dev) > -{ > - PCIIDEState *d = PCI_IDE(dev); > - unsigned i; > - > - for (i = 0; i < 2; ++i) { > - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io); > - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport); > - } > -} > - > static Property cmd646_ide_properties[] = { > DEFINE_PROP_UINT32("secondary", PCIIDEState, secondary, 0), > DEFINE_PROP_END_OF_LIST(), > @@ -323,17 +312,13 @@ static void cmd646_ide_class_init(ObjectClass *klass, void *data) > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > dc->reset = cmd646_reset; > - dc->vmsd = &vmstate_ide_pci; > k->realize = pci_cmd646_ide_realize; > - k->exit = pci_cmd646_ide_exitfn; > k->vendor_id = PCI_VENDOR_ID_CMD; > k->device_id = PCI_DEVICE_ID_CMD_646; > k->revision = 0x07; > - k->class_id = PCI_CLASS_STORAGE_IDE; > k->config_read = cmd646_pci_config_read; > k->config_write = cmd646_pci_config_write; > device_class_set_props(dc, cmd646_ide_properties); > - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); > } > > static const TypeInfo cmd646_ide_info = { > diff --git a/hw/ide/pci.c b/hw/ide/pci.c > index 67e0998ff0..8bea92e394 100644 > --- a/hw/ide/pci.c > +++ b/hw/ide/pci.c > @@ -467,7 +467,7 @@ static int ide_pci_post_load(void *opaque, int version_id) > return 0; > } > > -const VMStateDescription vmstate_ide_pci = { > +static const VMStateDescription vmstate_ide_pci = { > .name = "ide", > .version_id = 3, > .minimum_version_id = 0, > @@ -530,11 +530,34 @@ static void pci_ide_init(Object *obj) > qdev_init_gpio_out(DEVICE(d), d->isa_irq, ARRAY_SIZE(d->isa_irq)); > } > > +static void pci_ide_exitfn(PCIDevice *dev) > +{ > + PCIIDEState *d = PCI_IDE(dev); > + unsigned i; > + > + for (i = 0; i < ARRAY_SIZE(d->bmdma); ++i) { > + memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io); > + memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport); > + } > +} > + > +static void pci_ide_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > + > + dc->vmsd = &vmstate_ide_pci; > + k->exit = pci_ide_exitfn; > + k->class_id = PCI_CLASS_STORAGE_IDE; > + set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); > +} > + > static const TypeInfo pci_ide_type_info = { > .name = TYPE_PCI_IDE, > .parent = TYPE_PCI_DEVICE, > .instance_size = sizeof(PCIIDEState), > .instance_init = pci_ide_init, > + .class_init = pci_ide_class_init, > .abstract = true, > .interfaces = (InterfaceInfo[]) { > { INTERFACE_CONVENTIONAL_PCI_DEVICE }, > diff --git a/hw/ide/piix.c b/hw/ide/piix.c > index a32f7ccece..4e6ca99123 100644 > --- a/hw/ide/piix.c > +++ b/hw/ide/piix.c > @@ -159,8 +159,6 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp) > bmdma_setup_bar(d); > pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar); > > - vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d); > - > for (unsigned i = 0; i < 2; i++) { > if (!pci_piix_init_bus(d, i, errp)) { > return; > @@ -168,17 +166,6 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp) > } > } > > -static void pci_piix_ide_exitfn(PCIDevice *dev) > -{ > - PCIIDEState *d = PCI_IDE(dev); > - unsigned i; > - > - for (i = 0; i < 2; ++i) { > - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io); > - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport); > - } > -} > - > /* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */ > static void piix3_ide_class_init(ObjectClass *klass, void *data) > { > @@ -187,11 +174,8 @@ static void piix3_ide_class_init(ObjectClass *klass, void *data) > > dc->reset = piix_ide_reset; > k->realize = pci_piix_ide_realize; > - k->exit = pci_piix_ide_exitfn; > k->vendor_id = PCI_VENDOR_ID_INTEL; > k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1; > - k->class_id = PCI_CLASS_STORAGE_IDE; > - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); > dc->hotpluggable = false; > } > > @@ -209,11 +193,8 @@ static void piix4_ide_class_init(ObjectClass *klass, void *data) > > dc->reset = piix_ide_reset; > k->realize = pci_piix_ide_realize; > - k->exit = pci_piix_ide_exitfn; > k->vendor_id = PCI_VENDOR_ID_INTEL; > k->device_id = PCI_DEVICE_ID_INTEL_82371AB; > - k->class_id = PCI_CLASS_STORAGE_IDE; > - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); > dc->hotpluggable = false; > } > > diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c > index 5dd3d03c29..0af897a9ef 100644 > --- a/hw/ide/sii3112.c > +++ b/hw/ide/sii3112.c > @@ -301,9 +301,10 @@ static void sii3112_pci_class_init(ObjectClass *klass, void *data) > pd->class_id = PCI_CLASS_STORAGE_RAID; > pd->revision = 1; > pd->realize = sii3112_pci_realize; > + pd->exit = NULL; > dc->reset = sii3112_reset; > + dc->vmsd = NULL; > dc->desc = "SiI3112A SATA controller"; > - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); > } > > static const TypeInfo sii3112_pci_info = { > diff --git a/hw/ide/via.c b/hw/ide/via.c > index 91253fa4ef..287143a005 100644 > --- a/hw/ide/via.c > +++ b/hw/ide/via.c > @@ -200,34 +200,19 @@ static void via_ide_realize(PCIDevice *dev, Error **errp) > } > } > > -static void via_ide_exitfn(PCIDevice *dev) > -{ > - PCIIDEState *d = PCI_IDE(dev); > - unsigned i; > - > - for (i = 0; i < ARRAY_SIZE(d->bmdma); ++i) { > - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io); > - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport); > - } > -} > - > static void via_ide_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > dc->reset = via_ide_reset; > - dc->vmsd = &vmstate_ide_pci; > /* Reason: only works as function of VIA southbridge */ > dc->user_creatable = false; > > k->realize = via_ide_realize; > - k->exit = via_ide_exitfn; > k->vendor_id = PCI_VENDOR_ID_VIA; > k->device_id = PCI_DEVICE_ID_VIA_IDE; > k->revision = 0x06; > - k->class_id = PCI_CLASS_STORAGE_IDE; > - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); > } > > static const TypeInfo via_ide_info = { >
Am 22. April 2023 17:34:30 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >On Sat, 22 Apr 2023, Bernhard Beschow wrote: >> Resolves redundant code in every PCI IDE device model. > >This patch could be broken up a bit more as it seems to do unrelated changes. Such as setting DEVICE_CATEGORY_STORAGE in a different way could be a separate patch to make it simpler to review. Okay, I'll slice this patch in the next iteration, moving each assignment separately. Best regards, Bernhard > >Regards, >BALATON Zoltan > >> --- >> include/hw/ide/pci.h | 1 - >> hw/ide/cmd646.c | 15 --------------- >> hw/ide/pci.c | 25 ++++++++++++++++++++++++- >> hw/ide/piix.c | 19 ------------------- >> hw/ide/sii3112.c | 3 ++- >> hw/ide/via.c | 15 --------------- >> 6 files changed, 26 insertions(+), 52 deletions(-) >> >> diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h >> index 74c127e32f..7bc4e53d02 100644 >> --- a/include/hw/ide/pci.h >> +++ b/include/hw/ide/pci.h >> @@ -61,7 +61,6 @@ void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val); >> extern MemoryRegionOps bmdma_addr_ioport_ops; >> void pci_ide_create_devs(PCIDevice *dev); >> >> -extern const VMStateDescription vmstate_ide_pci; >> extern const MemoryRegionOps pci_ide_cmd_le_ops; >> extern const MemoryRegionOps pci_ide_data_le_ops; >> #endif >> diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c >> index a094a6e12a..9aabf80e52 100644 >> --- a/hw/ide/cmd646.c >> +++ b/hw/ide/cmd646.c >> @@ -301,17 +301,6 @@ static void pci_cmd646_ide_realize(PCIDevice *dev, Error **errp) >> } >> } >> >> -static void pci_cmd646_ide_exitfn(PCIDevice *dev) >> -{ >> - PCIIDEState *d = PCI_IDE(dev); >> - unsigned i; >> - >> - for (i = 0; i < 2; ++i) { >> - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io); >> - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport); >> - } >> -} >> - >> static Property cmd646_ide_properties[] = { >> DEFINE_PROP_UINT32("secondary", PCIIDEState, secondary, 0), >> DEFINE_PROP_END_OF_LIST(), >> @@ -323,17 +312,13 @@ static void cmd646_ide_class_init(ObjectClass *klass, void *data) >> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); >> >> dc->reset = cmd646_reset; >> - dc->vmsd = &vmstate_ide_pci; >> k->realize = pci_cmd646_ide_realize; >> - k->exit = pci_cmd646_ide_exitfn; >> k->vendor_id = PCI_VENDOR_ID_CMD; >> k->device_id = PCI_DEVICE_ID_CMD_646; >> k->revision = 0x07; >> - k->class_id = PCI_CLASS_STORAGE_IDE; >> k->config_read = cmd646_pci_config_read; >> k->config_write = cmd646_pci_config_write; >> device_class_set_props(dc, cmd646_ide_properties); >> - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); >> } >> >> static const TypeInfo cmd646_ide_info = { >> diff --git a/hw/ide/pci.c b/hw/ide/pci.c >> index 67e0998ff0..8bea92e394 100644 >> --- a/hw/ide/pci.c >> +++ b/hw/ide/pci.c >> @@ -467,7 +467,7 @@ static int ide_pci_post_load(void *opaque, int version_id) >> return 0; >> } >> >> -const VMStateDescription vmstate_ide_pci = { >> +static const VMStateDescription vmstate_ide_pci = { >> .name = "ide", >> .version_id = 3, >> .minimum_version_id = 0, >> @@ -530,11 +530,34 @@ static void pci_ide_init(Object *obj) >> qdev_init_gpio_out(DEVICE(d), d->isa_irq, ARRAY_SIZE(d->isa_irq)); >> } >> >> +static void pci_ide_exitfn(PCIDevice *dev) >> +{ >> + PCIIDEState *d = PCI_IDE(dev); >> + unsigned i; >> + >> + for (i = 0; i < ARRAY_SIZE(d->bmdma); ++i) { >> + memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io); >> + memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport); >> + } >> +} >> + >> +static void pci_ide_class_init(ObjectClass *klass, void *data) >> +{ >> + DeviceClass *dc = DEVICE_CLASS(klass); >> + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); >> + >> + dc->vmsd = &vmstate_ide_pci; >> + k->exit = pci_ide_exitfn; >> + k->class_id = PCI_CLASS_STORAGE_IDE; >> + set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); >> +} >> + >> static const TypeInfo pci_ide_type_info = { >> .name = TYPE_PCI_IDE, >> .parent = TYPE_PCI_DEVICE, >> .instance_size = sizeof(PCIIDEState), >> .instance_init = pci_ide_init, >> + .class_init = pci_ide_class_init, >> .abstract = true, >> .interfaces = (InterfaceInfo[]) { >> { INTERFACE_CONVENTIONAL_PCI_DEVICE }, >> diff --git a/hw/ide/piix.c b/hw/ide/piix.c >> index a32f7ccece..4e6ca99123 100644 >> --- a/hw/ide/piix.c >> +++ b/hw/ide/piix.c >> @@ -159,8 +159,6 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp) >> bmdma_setup_bar(d); >> pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar); >> >> - vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d); >> - >> for (unsigned i = 0; i < 2; i++) { >> if (!pci_piix_init_bus(d, i, errp)) { >> return; >> @@ -168,17 +166,6 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp) >> } >> } >> >> -static void pci_piix_ide_exitfn(PCIDevice *dev) >> -{ >> - PCIIDEState *d = PCI_IDE(dev); >> - unsigned i; >> - >> - for (i = 0; i < 2; ++i) { >> - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io); >> - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport); >> - } >> -} >> - >> /* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */ >> static void piix3_ide_class_init(ObjectClass *klass, void *data) >> { >> @@ -187,11 +174,8 @@ static void piix3_ide_class_init(ObjectClass *klass, void *data) >> >> dc->reset = piix_ide_reset; >> k->realize = pci_piix_ide_realize; >> - k->exit = pci_piix_ide_exitfn; >> k->vendor_id = PCI_VENDOR_ID_INTEL; >> k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1; >> - k->class_id = PCI_CLASS_STORAGE_IDE; >> - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); >> dc->hotpluggable = false; >> } >> >> @@ -209,11 +193,8 @@ static void piix4_ide_class_init(ObjectClass *klass, void *data) >> >> dc->reset = piix_ide_reset; >> k->realize = pci_piix_ide_realize; >> - k->exit = pci_piix_ide_exitfn; >> k->vendor_id = PCI_VENDOR_ID_INTEL; >> k->device_id = PCI_DEVICE_ID_INTEL_82371AB; >> - k->class_id = PCI_CLASS_STORAGE_IDE; >> - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); >> dc->hotpluggable = false; >> } >> >> diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c >> index 5dd3d03c29..0af897a9ef 100644 >> --- a/hw/ide/sii3112.c >> +++ b/hw/ide/sii3112.c >> @@ -301,9 +301,10 @@ static void sii3112_pci_class_init(ObjectClass *klass, void *data) >> pd->class_id = PCI_CLASS_STORAGE_RAID; >> pd->revision = 1; >> pd->realize = sii3112_pci_realize; >> + pd->exit = NULL; >> dc->reset = sii3112_reset; >> + dc->vmsd = NULL; >> dc->desc = "SiI3112A SATA controller"; >> - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); >> } >> >> static const TypeInfo sii3112_pci_info = { >> diff --git a/hw/ide/via.c b/hw/ide/via.c >> index 91253fa4ef..287143a005 100644 >> --- a/hw/ide/via.c >> +++ b/hw/ide/via.c >> @@ -200,34 +200,19 @@ static void via_ide_realize(PCIDevice *dev, Error **errp) >> } >> } >> >> -static void via_ide_exitfn(PCIDevice *dev) >> -{ >> - PCIIDEState *d = PCI_IDE(dev); >> - unsigned i; >> - >> - for (i = 0; i < ARRAY_SIZE(d->bmdma); ++i) { >> - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io); >> - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport); >> - } >> -} >> - >> static void via_ide_class_init(ObjectClass *klass, void *data) >> { >> DeviceClass *dc = DEVICE_CLASS(klass); >> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); >> >> dc->reset = via_ide_reset; >> - dc->vmsd = &vmstate_ide_pci; >> /* Reason: only works as function of VIA southbridge */ >> dc->user_creatable = false; >> >> k->realize = via_ide_realize; >> - k->exit = via_ide_exitfn; >> k->vendor_id = PCI_VENDOR_ID_VIA; >> k->device_id = PCI_DEVICE_ID_VIA_IDE; >> k->revision = 0x06; >> - k->class_id = PCI_CLASS_STORAGE_IDE; >> - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); >> } >> >> static const TypeInfo via_ide_info = { >>
On 22/4/23 17:07, Bernhard Beschow wrote: > Resolves redundant code in every PCI IDE device model. > --- > include/hw/ide/pci.h | 1 - > hw/ide/cmd646.c | 15 --------------- > hw/ide/pci.c | 25 ++++++++++++++++++++++++- > hw/ide/piix.c | 19 ------------------- > hw/ide/sii3112.c | 3 ++- > hw/ide/via.c | 15 --------------- > 6 files changed, 26 insertions(+), 52 deletions(-) > diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c > index 5dd3d03c29..0af897a9ef 100644 > --- a/hw/ide/sii3112.c > +++ b/hw/ide/sii3112.c > @@ -301,9 +301,10 @@ static void sii3112_pci_class_init(ObjectClass *klass, void *data) > pd->class_id = PCI_CLASS_STORAGE_RAID; > pd->revision = 1; > pd->realize = sii3112_pci_realize; > + pd->exit = NULL; > dc->reset = sii3112_reset; > + dc->vmsd = NULL; > dc->desc = "SiI3112A SATA controller"; The SiI3112A doesn't have these regions?
Am 23. April 2023 17:41:33 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>: >On 22/4/23 17:07, Bernhard Beschow wrote: >> Resolves redundant code in every PCI IDE device model. >> --- >> include/hw/ide/pci.h | 1 - >> hw/ide/cmd646.c | 15 --------------- >> hw/ide/pci.c | 25 ++++++++++++++++++++++++- >> hw/ide/piix.c | 19 ------------------- >> hw/ide/sii3112.c | 3 ++- >> hw/ide/via.c | 15 --------------- >> 6 files changed, 26 insertions(+), 52 deletions(-) > > >> diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c >> index 5dd3d03c29..0af897a9ef 100644 >> --- a/hw/ide/sii3112.c >> +++ b/hw/ide/sii3112.c >> @@ -301,9 +301,10 @@ static void sii3112_pci_class_init(ObjectClass *klass, void *data) >> pd->class_id = PCI_CLASS_STORAGE_RAID; >> pd->revision = 1; >> pd->realize = sii3112_pci_realize; >> + pd->exit = NULL; >> dc->reset = sii3112_reset; >> + dc->vmsd = NULL; >> dc->desc = "SiI3112A SATA controller"; > >The SiI3112A doesn't have these regions? Yeah, it ignores a lot of stuff in the base class. This gets changed in the last part of this series though. This seems why there is no exit method. Furthermore -- probably due to additional custom fields -- there is no migration description. Best regards, Bernhard
On Sun, 23 Apr 2023, Bernhard Beschow wrote: > Am 23. April 2023 17:41:33 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>: >> On 22/4/23 17:07, Bernhard Beschow wrote: >>> Resolves redundant code in every PCI IDE device model. >>> --- >>> include/hw/ide/pci.h | 1 - >>> hw/ide/cmd646.c | 15 --------------- >>> hw/ide/pci.c | 25 ++++++++++++++++++++++++- >>> hw/ide/piix.c | 19 ------------------- >>> hw/ide/sii3112.c | 3 ++- >>> hw/ide/via.c | 15 --------------- >>> 6 files changed, 26 insertions(+), 52 deletions(-) >> >> >>> diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c >>> index 5dd3d03c29..0af897a9ef 100644 >>> --- a/hw/ide/sii3112.c >>> +++ b/hw/ide/sii3112.c >>> @@ -301,9 +301,10 @@ static void sii3112_pci_class_init(ObjectClass *klass, void *data) >>> pd->class_id = PCI_CLASS_STORAGE_RAID; >>> pd->revision = 1; >>> pd->realize = sii3112_pci_realize; >>> + pd->exit = NULL; >>> dc->reset = sii3112_reset; >>> + dc->vmsd = NULL; >>> dc->desc = "SiI3112A SATA controller"; >> >> The SiI3112A doesn't have these regions? > > Yeah, it ignores a lot of stuff in the base class. This gets changed in > the last part of this series though. This seems why there is no exit > method. Furthermore -- probably due to additional custom fields -- there > is no migration description. Probably there's no state descriptor because I did not bother to implement it back then when I did not even know how that worked. I've considered extending this to a 4 port version before adding migration/save support but that did not happen. This is only used on sam460ex by default and likely nobody wants to migrate that anyway. However why do you need to explicitly set these to NULL? Aren't those structs allocated 0 filled so you'd only need to set non-NULL members. As for ignoting stuff in the base class, this isn't really a PCI IDE controller. It's a SATA controller that for compatibility with older drivers looks a lot like an IDE controller but handles only one device per channel and has some additional SATA stuff that we mostly don't model. This way I could reuse code that was already there but still had some duplication that you're now resolving. Regards, BALATON Zoltan
On 22/04/2023 16:07, Bernhard Beschow wrote: > Resolves redundant code in every PCI IDE device model. I think this needs to mention that it's moving the PCIDeviceClass::exit() function from all of the PCI IDE controller implementations to a common implementation in the parent PCI_IDE type. > --- > include/hw/ide/pci.h | 1 - > hw/ide/cmd646.c | 15 --------------- > hw/ide/pci.c | 25 ++++++++++++++++++++++++- > hw/ide/piix.c | 19 ------------------- > hw/ide/sii3112.c | 3 ++- > hw/ide/via.c | 15 --------------- > 6 files changed, 26 insertions(+), 52 deletions(-) > > diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h > index 74c127e32f..7bc4e53d02 100644 > --- a/include/hw/ide/pci.h > +++ b/include/hw/ide/pci.h > @@ -61,7 +61,6 @@ void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val); > extern MemoryRegionOps bmdma_addr_ioport_ops; > void pci_ide_create_devs(PCIDevice *dev); > > -extern const VMStateDescription vmstate_ide_pci; > extern const MemoryRegionOps pci_ide_cmd_le_ops; > extern const MemoryRegionOps pci_ide_data_le_ops; > #endif > diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c > index a094a6e12a..9aabf80e52 100644 > --- a/hw/ide/cmd646.c > +++ b/hw/ide/cmd646.c > @@ -301,17 +301,6 @@ static void pci_cmd646_ide_realize(PCIDevice *dev, Error **errp) > } > } > > -static void pci_cmd646_ide_exitfn(PCIDevice *dev) > -{ > - PCIIDEState *d = PCI_IDE(dev); > - unsigned i; > - > - for (i = 0; i < 2; ++i) { > - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io); > - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport); > - } > -} > - > static Property cmd646_ide_properties[] = { > DEFINE_PROP_UINT32("secondary", PCIIDEState, secondary, 0), > DEFINE_PROP_END_OF_LIST(), > @@ -323,17 +312,13 @@ static void cmd646_ide_class_init(ObjectClass *klass, void *data) > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > dc->reset = cmd646_reset; > - dc->vmsd = &vmstate_ide_pci; > k->realize = pci_cmd646_ide_realize; > - k->exit = pci_cmd646_ide_exitfn; > k->vendor_id = PCI_VENDOR_ID_CMD; > k->device_id = PCI_DEVICE_ID_CMD_646; > k->revision = 0x07; > - k->class_id = PCI_CLASS_STORAGE_IDE; > k->config_read = cmd646_pci_config_read; > k->config_write = cmd646_pci_config_write; > device_class_set_props(dc, cmd646_ide_properties); > - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); > } > > static const TypeInfo cmd646_ide_info = { > diff --git a/hw/ide/pci.c b/hw/ide/pci.c > index 67e0998ff0..8bea92e394 100644 > --- a/hw/ide/pci.c > +++ b/hw/ide/pci.c > @@ -467,7 +467,7 @@ static int ide_pci_post_load(void *opaque, int version_id) > return 0; > } > > -const VMStateDescription vmstate_ide_pci = { > +static const VMStateDescription vmstate_ide_pci = { > .name = "ide", > .version_id = 3, > .minimum_version_id = 0, > @@ -530,11 +530,34 @@ static void pci_ide_init(Object *obj) > qdev_init_gpio_out(DEVICE(d), d->isa_irq, ARRAY_SIZE(d->isa_irq)); > } > > +static void pci_ide_exitfn(PCIDevice *dev) > +{ > + PCIIDEState *d = PCI_IDE(dev); > + unsigned i; > + > + for (i = 0; i < ARRAY_SIZE(d->bmdma); ++i) { > + memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io); > + memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport); > + } > +} > + > +static void pci_ide_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > + > + dc->vmsd = &vmstate_ide_pci; > + k->exit = pci_ide_exitfn; > + k->class_id = PCI_CLASS_STORAGE_IDE; > + set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); > +} > + > static const TypeInfo pci_ide_type_info = { > .name = TYPE_PCI_IDE, > .parent = TYPE_PCI_DEVICE, > .instance_size = sizeof(PCIIDEState), > .instance_init = pci_ide_init, > + .class_init = pci_ide_class_init, > .abstract = true, > .interfaces = (InterfaceInfo[]) { > { INTERFACE_CONVENTIONAL_PCI_DEVICE }, > diff --git a/hw/ide/piix.c b/hw/ide/piix.c > index a32f7ccece..4e6ca99123 100644 > --- a/hw/ide/piix.c > +++ b/hw/ide/piix.c > @@ -159,8 +159,6 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp) > bmdma_setup_bar(d); > pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar); > > - vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d); > - Presumably this still survives migration between a pre-series and post-series QEMU using the PIIX IDE controller? > for (unsigned i = 0; i < 2; i++) { > if (!pci_piix_init_bus(d, i, errp)) { > return; > @@ -168,17 +166,6 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp) > } > } > > -static void pci_piix_ide_exitfn(PCIDevice *dev) > -{ > - PCIIDEState *d = PCI_IDE(dev); > - unsigned i; > - > - for (i = 0; i < 2; ++i) { > - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io); > - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport); > - } > -} > - > /* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */ > static void piix3_ide_class_init(ObjectClass *klass, void *data) > { > @@ -187,11 +174,8 @@ static void piix3_ide_class_init(ObjectClass *klass, void *data) > > dc->reset = piix_ide_reset; > k->realize = pci_piix_ide_realize; > - k->exit = pci_piix_ide_exitfn; > k->vendor_id = PCI_VENDOR_ID_INTEL; > k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1; > - k->class_id = PCI_CLASS_STORAGE_IDE; > - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); > dc->hotpluggable = false; > } > > @@ -209,11 +193,8 @@ static void piix4_ide_class_init(ObjectClass *klass, void *data) > > dc->reset = piix_ide_reset; > k->realize = pci_piix_ide_realize; > - k->exit = pci_piix_ide_exitfn; > k->vendor_id = PCI_VENDOR_ID_INTEL; > k->device_id = PCI_DEVICE_ID_INTEL_82371AB; > - k->class_id = PCI_CLASS_STORAGE_IDE; > - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); > dc->hotpluggable = false; > } > > diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c > index 5dd3d03c29..0af897a9ef 100644 > --- a/hw/ide/sii3112.c > +++ b/hw/ide/sii3112.c > @@ -301,9 +301,10 @@ static void sii3112_pci_class_init(ObjectClass *klass, void *data) > pd->class_id = PCI_CLASS_STORAGE_RAID; > pd->revision = 1; > pd->realize = sii3112_pci_realize; > + pd->exit = NULL; > dc->reset = sii3112_reset; > + dc->vmsd = NULL; > dc->desc = "SiI3112A SATA controller"; > - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); > } No need to set explicit NULLs here: class/object structures are all zeroed before init (unless you're deliberately trying to prevent the common PCIDeviceClass::exit() function from being called here temporarily?) > static const TypeInfo sii3112_pci_info = { > diff --git a/hw/ide/via.c b/hw/ide/via.c > index 91253fa4ef..287143a005 100644 > --- a/hw/ide/via.c > +++ b/hw/ide/via.c > @@ -200,34 +200,19 @@ static void via_ide_realize(PCIDevice *dev, Error **errp) > } > } > > -static void via_ide_exitfn(PCIDevice *dev) > -{ > - PCIIDEState *d = PCI_IDE(dev); > - unsigned i; > - > - for (i = 0; i < ARRAY_SIZE(d->bmdma); ++i) { > - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io); > - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport); > - } > -} > - > static void via_ide_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > dc->reset = via_ide_reset; > - dc->vmsd = &vmstate_ide_pci; > /* Reason: only works as function of VIA southbridge */ > dc->user_creatable = false; > > k->realize = via_ide_realize; > - k->exit = via_ide_exitfn; > k->vendor_id = PCI_VENDOR_ID_VIA; > k->device_id = PCI_DEVICE_ID_VIA_IDE; > k->revision = 0x06; > - k->class_id = PCI_CLASS_STORAGE_IDE; > - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); > } > > static const TypeInfo via_ide_info = { A couple of queries, but generally looks good to me. ATB, Mark.
Am 26. April 2023 11:04:30 UTC schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>: >On 22/04/2023 16:07, Bernhard Beschow wrote: > >> Resolves redundant code in every PCI IDE device model. > >I think this needs to mention that it's moving the PCIDeviceClass::exit() function from all of the PCI IDE controller implementations to a common implementation in the parent PCI_IDE type. I'll completely rework this patch. > >> --- >> include/hw/ide/pci.h | 1 - >> hw/ide/cmd646.c | 15 --------------- >> hw/ide/pci.c | 25 ++++++++++++++++++++++++- >> hw/ide/piix.c | 19 ------------------- >> hw/ide/sii3112.c | 3 ++- >> hw/ide/via.c | 15 --------------- >> 6 files changed, 26 insertions(+), 52 deletions(-) >> >> diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h >> index 74c127e32f..7bc4e53d02 100644 >> --- a/include/hw/ide/pci.h >> +++ b/include/hw/ide/pci.h >> @@ -61,7 +61,6 @@ void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val); >> extern MemoryRegionOps bmdma_addr_ioport_ops; >> void pci_ide_create_devs(PCIDevice *dev); >> -extern const VMStateDescription vmstate_ide_pci; >> extern const MemoryRegionOps pci_ide_cmd_le_ops; >> extern const MemoryRegionOps pci_ide_data_le_ops; >> #endif >> diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c >> index a094a6e12a..9aabf80e52 100644 >> --- a/hw/ide/cmd646.c >> +++ b/hw/ide/cmd646.c >> @@ -301,17 +301,6 @@ static void pci_cmd646_ide_realize(PCIDevice *dev, Error **errp) >> } >> } >> -static void pci_cmd646_ide_exitfn(PCIDevice *dev) >> -{ >> - PCIIDEState *d = PCI_IDE(dev); >> - unsigned i; >> - >> - for (i = 0; i < 2; ++i) { >> - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io); >> - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport); >> - } >> -} >> - >> static Property cmd646_ide_properties[] = { >> DEFINE_PROP_UINT32("secondary", PCIIDEState, secondary, 0), >> DEFINE_PROP_END_OF_LIST(), >> @@ -323,17 +312,13 @@ static void cmd646_ide_class_init(ObjectClass *klass, void *data) >> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); >> dc->reset = cmd646_reset; >> - dc->vmsd = &vmstate_ide_pci; >> k->realize = pci_cmd646_ide_realize; >> - k->exit = pci_cmd646_ide_exitfn; >> k->vendor_id = PCI_VENDOR_ID_CMD; >> k->device_id = PCI_DEVICE_ID_CMD_646; >> k->revision = 0x07; >> - k->class_id = PCI_CLASS_STORAGE_IDE; >> k->config_read = cmd646_pci_config_read; >> k->config_write = cmd646_pci_config_write; >> device_class_set_props(dc, cmd646_ide_properties); >> - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); >> } >> static const TypeInfo cmd646_ide_info = { >> diff --git a/hw/ide/pci.c b/hw/ide/pci.c >> index 67e0998ff0..8bea92e394 100644 >> --- a/hw/ide/pci.c >> +++ b/hw/ide/pci.c >> @@ -467,7 +467,7 @@ static int ide_pci_post_load(void *opaque, int version_id) >> return 0; >> } >> -const VMStateDescription vmstate_ide_pci = { >> +static const VMStateDescription vmstate_ide_pci = { >> .name = "ide", >> .version_id = 3, >> .minimum_version_id = 0, >> @@ -530,11 +530,34 @@ static void pci_ide_init(Object *obj) >> qdev_init_gpio_out(DEVICE(d), d->isa_irq, ARRAY_SIZE(d->isa_irq)); >> } >> +static void pci_ide_exitfn(PCIDevice *dev) >> +{ >> + PCIIDEState *d = PCI_IDE(dev); >> + unsigned i; >> + >> + for (i = 0; i < ARRAY_SIZE(d->bmdma); ++i) { >> + memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io); >> + memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport); >> + } >> +} >> + >> +static void pci_ide_class_init(ObjectClass *klass, void *data) >> +{ >> + DeviceClass *dc = DEVICE_CLASS(klass); >> + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); >> + >> + dc->vmsd = &vmstate_ide_pci; >> + k->exit = pci_ide_exitfn; >> + k->class_id = PCI_CLASS_STORAGE_IDE; >> + set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); >> +} >> + >> static const TypeInfo pci_ide_type_info = { >> .name = TYPE_PCI_IDE, >> .parent = TYPE_PCI_DEVICE, >> .instance_size = sizeof(PCIIDEState), >> .instance_init = pci_ide_init, >> + .class_init = pci_ide_class_init, >> .abstract = true, >> .interfaces = (InterfaceInfo[]) { >> { INTERFACE_CONVENTIONAL_PCI_DEVICE }, >> diff --git a/hw/ide/piix.c b/hw/ide/piix.c >> index a32f7ccece..4e6ca99123 100644 >> --- a/hw/ide/piix.c >> +++ b/hw/ide/piix.c >> @@ -159,8 +159,6 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp) >> bmdma_setup_bar(d); >> pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar); >> - vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d); >> - > >Presumably this still survives migration between a pre-series and post-series QEMU using the PIIX IDE controller? > >> for (unsigned i = 0; i < 2; i++) { >> if (!pci_piix_init_bus(d, i, errp)) { >> return; >> @@ -168,17 +166,6 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp) >> } >> } >> -static void pci_piix_ide_exitfn(PCIDevice *dev) >> -{ >> - PCIIDEState *d = PCI_IDE(dev); >> - unsigned i; >> - >> - for (i = 0; i < 2; ++i) { >> - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io); >> - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport); >> - } >> -} >> - >> /* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */ >> static void piix3_ide_class_init(ObjectClass *klass, void *data) >> { >> @@ -187,11 +174,8 @@ static void piix3_ide_class_init(ObjectClass *klass, void *data) >> dc->reset = piix_ide_reset; >> k->realize = pci_piix_ide_realize; >> - k->exit = pci_piix_ide_exitfn; >> k->vendor_id = PCI_VENDOR_ID_INTEL; >> k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1; >> - k->class_id = PCI_CLASS_STORAGE_IDE; >> - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); >> dc->hotpluggable = false; >> } >> @@ -209,11 +193,8 @@ static void piix4_ide_class_init(ObjectClass *klass, void *data) >> dc->reset = piix_ide_reset; >> k->realize = pci_piix_ide_realize; >> - k->exit = pci_piix_ide_exitfn; >> k->vendor_id = PCI_VENDOR_ID_INTEL; >> k->device_id = PCI_DEVICE_ID_INTEL_82371AB; >> - k->class_id = PCI_CLASS_STORAGE_IDE; >> - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); >> dc->hotpluggable = false; >> } >> diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c >> index 5dd3d03c29..0af897a9ef 100644 >> --- a/hw/ide/sii3112.c >> +++ b/hw/ide/sii3112.c >> @@ -301,9 +301,10 @@ static void sii3112_pci_class_init(ObjectClass *klass, void *data) >> pd->class_id = PCI_CLASS_STORAGE_RAID; >> pd->revision = 1; >> pd->realize = sii3112_pci_realize; >> + pd->exit = NULL; >> dc->reset = sii3112_reset; >> + dc->vmsd = NULL; >> dc->desc = "SiI3112A SATA controller"; >> - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); >> } > >No need to set explicit NULLs here: class/object structures are all zeroed before init (unless you're deliberately trying to prevent the common PCIDeviceClass::exit() function from being called here temporarily?) > >> static const TypeInfo sii3112_pci_info = { >> diff --git a/hw/ide/via.c b/hw/ide/via.c >> index 91253fa4ef..287143a005 100644 >> --- a/hw/ide/via.c >> +++ b/hw/ide/via.c >> @@ -200,34 +200,19 @@ static void via_ide_realize(PCIDevice *dev, Error **errp) >> } >> } >> -static void via_ide_exitfn(PCIDevice *dev) >> -{ >> - PCIIDEState *d = PCI_IDE(dev); >> - unsigned i; >> - >> - for (i = 0; i < ARRAY_SIZE(d->bmdma); ++i) { >> - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io); >> - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport); >> - } >> -} >> - >> static void via_ide_class_init(ObjectClass *klass, void *data) >> { >> DeviceClass *dc = DEVICE_CLASS(klass); >> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); >> dc->reset = via_ide_reset; >> - dc->vmsd = &vmstate_ide_pci; >> /* Reason: only works as function of VIA southbridge */ >> dc->user_creatable = false; >> k->realize = via_ide_realize; >> - k->exit = via_ide_exitfn; >> k->vendor_id = PCI_VENDOR_ID_VIA; >> k->device_id = PCI_DEVICE_ID_VIA_IDE; >> k->revision = 0x06; >> - k->class_id = PCI_CLASS_STORAGE_IDE; >> - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); >> } >> static const TypeInfo via_ide_info = { > >A couple of queries, but generally looks good to me. > > >ATB, > >Mark.
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h index 74c127e32f..7bc4e53d02 100644 --- a/include/hw/ide/pci.h +++ b/include/hw/ide/pci.h @@ -61,7 +61,6 @@ void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val); extern MemoryRegionOps bmdma_addr_ioport_ops; void pci_ide_create_devs(PCIDevice *dev); -extern const VMStateDescription vmstate_ide_pci; extern const MemoryRegionOps pci_ide_cmd_le_ops; extern const MemoryRegionOps pci_ide_data_le_ops; #endif diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c index a094a6e12a..9aabf80e52 100644 --- a/hw/ide/cmd646.c +++ b/hw/ide/cmd646.c @@ -301,17 +301,6 @@ static void pci_cmd646_ide_realize(PCIDevice *dev, Error **errp) } } -static void pci_cmd646_ide_exitfn(PCIDevice *dev) -{ - PCIIDEState *d = PCI_IDE(dev); - unsigned i; - - for (i = 0; i < 2; ++i) { - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io); - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport); - } -} - static Property cmd646_ide_properties[] = { DEFINE_PROP_UINT32("secondary", PCIIDEState, secondary, 0), DEFINE_PROP_END_OF_LIST(), @@ -323,17 +312,13 @@ static void cmd646_ide_class_init(ObjectClass *klass, void *data) PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); dc->reset = cmd646_reset; - dc->vmsd = &vmstate_ide_pci; k->realize = pci_cmd646_ide_realize; - k->exit = pci_cmd646_ide_exitfn; k->vendor_id = PCI_VENDOR_ID_CMD; k->device_id = PCI_DEVICE_ID_CMD_646; k->revision = 0x07; - k->class_id = PCI_CLASS_STORAGE_IDE; k->config_read = cmd646_pci_config_read; k->config_write = cmd646_pci_config_write; device_class_set_props(dc, cmd646_ide_properties); - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); } static const TypeInfo cmd646_ide_info = { diff --git a/hw/ide/pci.c b/hw/ide/pci.c index 67e0998ff0..8bea92e394 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -467,7 +467,7 @@ static int ide_pci_post_load(void *opaque, int version_id) return 0; } -const VMStateDescription vmstate_ide_pci = { +static const VMStateDescription vmstate_ide_pci = { .name = "ide", .version_id = 3, .minimum_version_id = 0, @@ -530,11 +530,34 @@ static void pci_ide_init(Object *obj) qdev_init_gpio_out(DEVICE(d), d->isa_irq, ARRAY_SIZE(d->isa_irq)); } +static void pci_ide_exitfn(PCIDevice *dev) +{ + PCIIDEState *d = PCI_IDE(dev); + unsigned i; + + for (i = 0; i < ARRAY_SIZE(d->bmdma); ++i) { + memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io); + memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport); + } +} + +static void pci_ide_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); + + dc->vmsd = &vmstate_ide_pci; + k->exit = pci_ide_exitfn; + k->class_id = PCI_CLASS_STORAGE_IDE; + set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); +} + static const TypeInfo pci_ide_type_info = { .name = TYPE_PCI_IDE, .parent = TYPE_PCI_DEVICE, .instance_size = sizeof(PCIIDEState), .instance_init = pci_ide_init, + .class_init = pci_ide_class_init, .abstract = true, .interfaces = (InterfaceInfo[]) { { INTERFACE_CONVENTIONAL_PCI_DEVICE }, diff --git a/hw/ide/piix.c b/hw/ide/piix.c index a32f7ccece..4e6ca99123 100644 --- a/hw/ide/piix.c +++ b/hw/ide/piix.c @@ -159,8 +159,6 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp) bmdma_setup_bar(d); pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar); - vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d); - for (unsigned i = 0; i < 2; i++) { if (!pci_piix_init_bus(d, i, errp)) { return; @@ -168,17 +166,6 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp) } } -static void pci_piix_ide_exitfn(PCIDevice *dev) -{ - PCIIDEState *d = PCI_IDE(dev); - unsigned i; - - for (i = 0; i < 2; ++i) { - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io); - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport); - } -} - /* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */ static void piix3_ide_class_init(ObjectClass *klass, void *data) { @@ -187,11 +174,8 @@ static void piix3_ide_class_init(ObjectClass *klass, void *data) dc->reset = piix_ide_reset; k->realize = pci_piix_ide_realize; - k->exit = pci_piix_ide_exitfn; k->vendor_id = PCI_VENDOR_ID_INTEL; k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1; - k->class_id = PCI_CLASS_STORAGE_IDE; - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); dc->hotpluggable = false; } @@ -209,11 +193,8 @@ static void piix4_ide_class_init(ObjectClass *klass, void *data) dc->reset = piix_ide_reset; k->realize = pci_piix_ide_realize; - k->exit = pci_piix_ide_exitfn; k->vendor_id = PCI_VENDOR_ID_INTEL; k->device_id = PCI_DEVICE_ID_INTEL_82371AB; - k->class_id = PCI_CLASS_STORAGE_IDE; - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); dc->hotpluggable = false; } diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c index 5dd3d03c29..0af897a9ef 100644 --- a/hw/ide/sii3112.c +++ b/hw/ide/sii3112.c @@ -301,9 +301,10 @@ static void sii3112_pci_class_init(ObjectClass *klass, void *data) pd->class_id = PCI_CLASS_STORAGE_RAID; pd->revision = 1; pd->realize = sii3112_pci_realize; + pd->exit = NULL; dc->reset = sii3112_reset; + dc->vmsd = NULL; dc->desc = "SiI3112A SATA controller"; - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); } static const TypeInfo sii3112_pci_info = { diff --git a/hw/ide/via.c b/hw/ide/via.c index 91253fa4ef..287143a005 100644 --- a/hw/ide/via.c +++ b/hw/ide/via.c @@ -200,34 +200,19 @@ static void via_ide_realize(PCIDevice *dev, Error **errp) } } -static void via_ide_exitfn(PCIDevice *dev) -{ - PCIIDEState *d = PCI_IDE(dev); - unsigned i; - - for (i = 0; i < ARRAY_SIZE(d->bmdma); ++i) { - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io); - memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport); - } -} - static void via_ide_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); dc->reset = via_ide_reset; - dc->vmsd = &vmstate_ide_pci; /* Reason: only works as function of VIA southbridge */ dc->user_creatable = false; k->realize = via_ide_realize; - k->exit = via_ide_exitfn; k->vendor_id = PCI_VENDOR_ID_VIA; k->device_id = PCI_DEVICE_ID_VIA_IDE; k->revision = 0x06; - k->class_id = PCI_CLASS_STORAGE_IDE; - set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); } static const TypeInfo via_ide_info = {