On Thu, 4 Mar 2021, Philippe Mathieu-Daudé wrote: > On 3/2/21 10:11 PM, BALATON Zoltan wrote: >> To allow reusing ISA bridge emulation for vt8231_isa move the device >> state of vt82c686b_isa emulation in an abstract via_isa class. >> >> Signed-off-by: BALATON Zoltan >> --- >> hw/isa/vt82c686.c | 70 ++++++++++++++++++++++------------------ >> include/hw/pci/pci_ids.h | 2 +- >> 2 files changed, 40 insertions(+), 32 deletions(-) >> >> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c >> index 72234bc4d1..5137f97f37 100644 >> --- a/hw/isa/vt82c686.c >> +++ b/hw/isa/vt82c686.c >> @@ -609,24 +609,48 @@ static const TypeInfo vt8231_superio_info = { >> }; >> >> >> -OBJECT_DECLARE_SIMPLE_TYPE(VT82C686BISAState, VT82C686B_ISA) >> +#define TYPE_VIA_ISA "via-isa" >> +OBJECT_DECLARE_SIMPLE_TYPE(ViaISAState, VIA_ISA) >> >> -struct VT82C686BISAState { >> +struct ViaISAState { >> PCIDevice dev; >> qemu_irq cpu_intr; >> ViaSuperIOState *via_sio; >> }; >> >> +static const VMStateDescription vmstate_via = { >> + .name = "via-isa", > > You changed the migration stream name, so I think we have > a problem with migration... No clue how to do that properly. I don't think these machines support migration or state description of vt86c686b was not missing something before these patches that would make it not work anyway so I did not worry about this too much. I doubt anybody wants to migrate a fuloong2e machine so this should not be a problem in practice but maybe you can mention it in the release notes if you think that would be necessary. Regards, BALATON Zoltan > Otherwise the rest LGTM. > >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .fields = (VMStateField[]) { >> + VMSTATE_PCI_DEVICE(dev, ViaISAState), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + >> +static const TypeInfo via_isa_info = { >> + .name = TYPE_VIA_ISA, >> + .parent = TYPE_PCI_DEVICE, >> + .instance_size = sizeof(ViaISAState), >> + .abstract = true, >> + .interfaces = (InterfaceInfo[]) { >> + { INTERFACE_CONVENTIONAL_PCI_DEVICE }, >> + { }, >> + }, >> +}; >> + >> static void via_isa_request_i8259_irq(void *opaque, int irq, int level) >> { >> - VT82C686BISAState *s = opaque; >> + ViaISAState *s = opaque; >> qemu_set_irq(s->cpu_intr, level); >> } >> >> +/* TYPE_VT82C686B_ISA */ >> + >> static void vt82c686b_write_config(PCIDevice *d, uint32_t addr, >> uint32_t val, int len) >> { >> - VT82C686BISAState *s = VT82C686B_ISA(d); >> + ViaISAState *s = VIA_ISA(d); >> >> trace_via_isa_write(addr, val, len); >> pci_default_write_config(d, addr, val, len); >> @@ -636,19 +660,9 @@ static void vt82c686b_write_config(PCIDevice *d, uint32_t addr, >> } >> } >> >> -static const VMStateDescription vmstate_via = { >> - .name = "vt82c686b", >> - .version_id = 1, >> - .minimum_version_id = 1, >> - .fields = (VMStateField[]) { >> - VMSTATE_PCI_DEVICE(dev, VT82C686BISAState), >> - VMSTATE_END_OF_LIST() >> - } >> -}; >> - >> static void vt82c686b_isa_reset(DeviceState *dev) >> { >> - VT82C686BISAState *s = VT82C686B_ISA(dev); >> + ViaISAState *s = VIA_ISA(dev); >> uint8_t *pci_conf = s->dev.config; >> >> pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x000000c0); >> @@ -668,7 +682,7 @@ static void vt82c686b_isa_reset(DeviceState *dev) >> >> static void vt82c686b_realize(PCIDevice *d, Error **errp) >> { >> - VT82C686BISAState *s = VT82C686B_ISA(d); >> + ViaISAState *s = VIA_ISA(d); >> DeviceState *dev = DEVICE(d); >> ISABus *isa_bus; >> qemu_irq *isa_irq; >> @@ -692,7 +706,7 @@ static void vt82c686b_realize(PCIDevice *d, Error **errp) >> } >> } >> >> -static void via_class_init(ObjectClass *klass, void *data) >> +static void vt82c686b_class_init(ObjectClass *klass, void *data) >> { >> DeviceClass *dc = DEVICE_CLASS(klass); >> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); >> @@ -700,28 +714,21 @@ static void via_class_init(ObjectClass *klass, void *data) >> k->realize = vt82c686b_realize; >> k->config_write = vt82c686b_write_config; >> k->vendor_id = PCI_VENDOR_ID_VIA; >> - k->device_id = PCI_DEVICE_ID_VIA_ISA_BRIDGE; >> + k->device_id = PCI_DEVICE_ID_VIA_82C686B_ISA; >> k->class_id = PCI_CLASS_BRIDGE_ISA; >> k->revision = 0x40; >> dc->reset = vt82c686b_isa_reset; >> dc->desc = "ISA bridge"; >> dc->vmsd = &vmstate_via; >> - /* >> - * Reason: part of VIA VT82C686 southbridge, needs to be wired up, >> - * e.g. by mips_fuloong2e_init() >> - */ >> + /* Reason: part of VIA VT82C686 southbridge, needs to be wired up */ >> dc->user_creatable = false; >> } >> >> -static const TypeInfo via_info = { >> +static const TypeInfo vt82c686b_isa_info = { >> .name = TYPE_VT82C686B_ISA, >> - .parent = TYPE_PCI_DEVICE, >> - .instance_size = sizeof(VT82C686BISAState), >> - .class_init = via_class_init, >> - .interfaces = (InterfaceInfo[]) { >> - { INTERFACE_CONVENTIONAL_PCI_DEVICE }, >> - { }, >> - }, >> + .parent = TYPE_VIA_ISA, >> + .instance_size = sizeof(ViaISAState), >> + .class_init = vt82c686b_class_init, >> }; >> >> >> @@ -733,7 +740,8 @@ static void vt82c686b_register_types(void) >> type_register_static(&via_superio_info); >> type_register_static(&vt82c686b_superio_info); >> type_register_static(&vt8231_superio_info); >> - type_register_static(&via_info); >> + type_register_static(&via_isa_info); >> + type_register_static(&vt82c686b_isa_info); >> } >> >> type_init(vt82c686b_register_types) >> diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h >> index ea28dcc850..aa3f67eaa4 100644 >> --- a/include/hw/pci/pci_ids.h >> +++ b/include/hw/pci/pci_ids.h >> @@ -204,7 +204,7 @@ >> #define PCI_VENDOR_ID_XILINX 0x10ee >> >> #define PCI_VENDOR_ID_VIA 0x1106 >> -#define PCI_DEVICE_ID_VIA_ISA_BRIDGE 0x0686 >> +#define PCI_DEVICE_ID_VIA_82C686B_ISA 0x0686 >> #define PCI_DEVICE_ID_VIA_IDE 0x0571 >> #define PCI_DEVICE_ID_VIA_UHCI 0x3038 >> #define PCI_DEVICE_ID_VIA_82C686B_PM 0x3057 >> > >