| Message ID | 20220901162613.6939-6-shentey@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Consolidate PIIX south bridges | expand |
On 1/9/22 18:25, Bernhard Beschow wrote: > The USB controller is an integral part of PIIX3 (function 2). So create > it as part of the south bridge. > > Note that the USB function is optional in QEMU. This is why it gets > object_initialize_child()'ed in realize rather than in instance_init. > > Signed-off-by: Bernhard Beschow <shentey@gmail.com> > --- > hw/i386/pc_piix.c | 6 ++---- > hw/isa/Kconfig | 1 + > hw/isa/piix3.c | 17 +++++++++++++++++ > include/hw/southbridge/piix.h | 4 ++++ > 4 files changed, 24 insertions(+), 4 deletions(-) > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index b08d946992..76ac8b2035 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -219,6 +219,8 @@ static void pc_init1(MachineState *machine, > pcms->bus = pci_bus; > > pci_dev = pci_new_multifunction(-1, true, type); > + object_property_set_bool(OBJECT(pci_dev), "has-usb", > + machine_usb(machine), &error_abort); > pci_realize_and_unref(pci_dev, pci_bus, &error_fatal); > piix3 = PIIX3_PCI_DEVICE(pci_dev); > piix3->pic = x86ms->gsi; > @@ -297,10 +299,6 @@ static void pc_init1(MachineState *machine, > } > #endif > > - if (pcmc->pci_enabled && machine_usb(machine)) { > - pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci"); > - } > - > if (pcmc->pci_enabled && x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) { > PCIDevice *piix4_pm; > > diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig > index 6e8f9cac54..f02eca3c3e 100644 > --- a/hw/isa/Kconfig > +++ b/hw/isa/Kconfig > @@ -36,6 +36,7 @@ config PIIX3 > select I8257 > select ISA_BUS > select MC146818RTC > + select USB_UHCI > > config PIIX4 > bool > diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c > index 96ab7107e2..27052a5546 100644 > --- a/hw/isa/piix3.c > +++ b/hw/isa/piix3.c > @@ -297,6 +297,7 @@ static const MemoryRegionOps rcr_ops = { > static void pci_piix3_realize(PCIDevice *dev, Error **errp) > { > PIIX3State *d = PIIX3_PCI_DEVICE(dev); > + PCIBus *pci_bus = pci_get_bus(dev); > ISABus *isa_bus; > > isa_bus = isa_bus_new(DEVICE(d), get_system_memory(), > @@ -319,6 +320,16 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp) > if (!qdev_realize(DEVICE(&d->rtc), BUS(isa_bus), errp)) { > return; > } > + > + /* USB */ > + if (d->has_usb) { > + object_initialize_child(OBJECT(dev), "uhci", &d->uhci, > + "piix3-usb-uhci"); > + qdev_prop_set_int32(DEVICE(&d->uhci), "addr", dev->devfn + 2); > + if (!qdev_realize(DEVICE(&d->uhci), BUS(pci_bus), errp)) { > + return; > + } > + } > } > > static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope) > @@ -341,6 +352,11 @@ static void pci_piix3_init(Object *obj) > object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC); > } > > +static Property pci_piix3_props[] = { > + DEFINE_PROP_BOOL("has-usb", PIIX3State, has_usb, true), Maybe s/has-usb/usb-enabled/? > + DEFINE_PROP_END_OF_LIST(), > +}; > + > static void pci_piix3_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > @@ -359,6 +375,7 @@ static void pci_piix3_class_init(ObjectClass *klass, void *data) > * pc_piix.c's pc_init1() > */ > dc->user_creatable = false; > + device_class_set_props(dc, pci_piix3_props); > adevc->build_dev_aml = build_pci_isa_aml; > } > > diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h > index b1fa08dd2b..5367917182 100644 > --- a/include/hw/southbridge/piix.h > +++ b/include/hw/southbridge/piix.h > @@ -15,6 +15,7 @@ > #include "hw/pci/pci.h" > #include "qom/object.h" > #include "hw/rtc/mc146818rtc.h" > +#include "hw/usb/hcd-uhci.h" > > /* PIRQRC[A:D]: PIRQx Route Control Registers */ > #define PIIX_PIRQCA 0x60 > @@ -54,12 +55,15 @@ struct PIIXState { > int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS]; > > RTCState rtc; > + UHCIState uhci; > > /* Reset Control Register contents */ > uint8_t rcr; > > /* IO memory region for Reset Control Register (PIIX_RCR_IOPORT) */ > MemoryRegion rcr_mem; > + > + bool has_usb; > }; > typedef struct PIIXState PIIX3State; >
Am 1. September 2022 19:13:22 UTC schrieb "Philippe Mathieu-Daudé" <f4bug@amsat.org>: >On 1/9/22 18:25, Bernhard Beschow wrote: >> The USB controller is an integral part of PIIX3 (function 2). So create >> it as part of the south bridge. >> >> Note that the USB function is optional in QEMU. This is why it gets >> object_initialize_child()'ed in realize rather than in instance_init. >> >> Signed-off-by: Bernhard Beschow <shentey@gmail.com> >> --- >> hw/i386/pc_piix.c | 6 ++---- >> hw/isa/Kconfig | 1 + >> hw/isa/piix3.c | 17 +++++++++++++++++ >> include/hw/southbridge/piix.h | 4 ++++ >> 4 files changed, 24 insertions(+), 4 deletions(-) >> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c >> index b08d946992..76ac8b2035 100644 >> --- a/hw/i386/pc_piix.c >> +++ b/hw/i386/pc_piix.c >> @@ -219,6 +219,8 @@ static void pc_init1(MachineState *machine, >> pcms->bus = pci_bus; >> pci_dev = pci_new_multifunction(-1, true, type); >> + object_property_set_bool(OBJECT(pci_dev), "has-usb", >> + machine_usb(machine), &error_abort); >> pci_realize_and_unref(pci_dev, pci_bus, &error_fatal); >> piix3 = PIIX3_PCI_DEVICE(pci_dev); >> piix3->pic = x86ms->gsi; >> @@ -297,10 +299,6 @@ static void pc_init1(MachineState *machine, >> } >> #endif >> - if (pcmc->pci_enabled && machine_usb(machine)) { >> - pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci"); >> - } >> - >> if (pcmc->pci_enabled && x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) { >> PCIDevice *piix4_pm; >> diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig >> index 6e8f9cac54..f02eca3c3e 100644 >> --- a/hw/isa/Kconfig >> +++ b/hw/isa/Kconfig >> @@ -36,6 +36,7 @@ config PIIX3 >> select I8257 >> select ISA_BUS >> select MC146818RTC >> + select USB_UHCI >> config PIIX4 >> bool >> diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c >> index 96ab7107e2..27052a5546 100644 >> --- a/hw/isa/piix3.c >> +++ b/hw/isa/piix3.c >> @@ -297,6 +297,7 @@ static const MemoryRegionOps rcr_ops = { >> static void pci_piix3_realize(PCIDevice *dev, Error **errp) >> { >> PIIX3State *d = PIIX3_PCI_DEVICE(dev); >> + PCIBus *pci_bus = pci_get_bus(dev); >> ISABus *isa_bus; >> isa_bus = isa_bus_new(DEVICE(d), get_system_memory(), >> @@ -319,6 +320,16 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp) >> if (!qdev_realize(DEVICE(&d->rtc), BUS(isa_bus), errp)) { >> return; >> } >> + >> + /* USB */ >> + if (d->has_usb) { >> + object_initialize_child(OBJECT(dev), "uhci", &d->uhci, >> + "piix3-usb-uhci"); >> + qdev_prop_set_int32(DEVICE(&d->uhci), "addr", dev->devfn + 2); >> + if (!qdev_realize(DEVICE(&d->uhci), BUS(pci_bus), errp)) { >> + return; >> + } >> + } >> } >> static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope) >> @@ -341,6 +352,11 @@ static void pci_piix3_init(Object *obj) >> object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC); >> } >> +static Property pci_piix3_props[] = { >> + DEFINE_PROP_BOOL("has-usb", PIIX3State, has_usb, true), > >Maybe s/has-usb/usb-enabled/? I consisered that. I chose has-foo for now since I took inspiration from the board. Also, "enabled" may refer to a device being present but its enable bit is cleared in PCI configuration space, c.f. vt82c686. Let me know if you still suggest a change here, in which case I'll consult you for naming suggestions once I consolidate piix and vt82c686 ;) Best regards, Bernhard > >> + DEFINE_PROP_END_OF_LIST(), >> +}; >> + >> static void pci_piix3_class_init(ObjectClass *klass, void *data) >> { >> DeviceClass *dc = DEVICE_CLASS(klass); >> @@ -359,6 +375,7 @@ static void pci_piix3_class_init(ObjectClass *klass, void *data) >> * pc_piix.c's pc_init1() >> */ >> dc->user_creatable = false; >> + device_class_set_props(dc, pci_piix3_props); >> adevc->build_dev_aml = build_pci_isa_aml; >> } >> diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h >> index b1fa08dd2b..5367917182 100644 >> --- a/include/hw/southbridge/piix.h >> +++ b/include/hw/southbridge/piix.h >> @@ -15,6 +15,7 @@ >> #include "hw/pci/pci.h" >> #include "qom/object.h" >> #include "hw/rtc/mc146818rtc.h" >> +#include "hw/usb/hcd-uhci.h" >> /* PIRQRC[A:D]: PIRQx Route Control Registers */ >> #define PIIX_PIRQCA 0x60 >> @@ -54,12 +55,15 @@ struct PIIXState { >> int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS]; >> RTCState rtc; >> + UHCIState uhci; >> /* Reset Control Register contents */ >> uint8_t rcr; >> /* IO memory region for Reset Control Register (PIIX_RCR_IOPORT) */ >> MemoryRegion rcr_mem; >> + >> + bool has_usb; >> }; >> typedef struct PIIXState PIIX3State; >> >
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index b08d946992..76ac8b2035 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -219,6 +219,8 @@ static void pc_init1(MachineState *machine, pcms->bus = pci_bus; pci_dev = pci_new_multifunction(-1, true, type); + object_property_set_bool(OBJECT(pci_dev), "has-usb", + machine_usb(machine), &error_abort); pci_realize_and_unref(pci_dev, pci_bus, &error_fatal); piix3 = PIIX3_PCI_DEVICE(pci_dev); piix3->pic = x86ms->gsi; @@ -297,10 +299,6 @@ static void pc_init1(MachineState *machine, } #endif - if (pcmc->pci_enabled && machine_usb(machine)) { - pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci"); - } - if (pcmc->pci_enabled && x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) { PCIDevice *piix4_pm; diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig index 6e8f9cac54..f02eca3c3e 100644 --- a/hw/isa/Kconfig +++ b/hw/isa/Kconfig @@ -36,6 +36,7 @@ config PIIX3 select I8257 select ISA_BUS select MC146818RTC + select USB_UHCI config PIIX4 bool diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c index 96ab7107e2..27052a5546 100644 --- a/hw/isa/piix3.c +++ b/hw/isa/piix3.c @@ -297,6 +297,7 @@ static const MemoryRegionOps rcr_ops = { static void pci_piix3_realize(PCIDevice *dev, Error **errp) { PIIX3State *d = PIIX3_PCI_DEVICE(dev); + PCIBus *pci_bus = pci_get_bus(dev); ISABus *isa_bus; isa_bus = isa_bus_new(DEVICE(d), get_system_memory(), @@ -319,6 +320,16 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp) if (!qdev_realize(DEVICE(&d->rtc), BUS(isa_bus), errp)) { return; } + + /* USB */ + if (d->has_usb) { + object_initialize_child(OBJECT(dev), "uhci", &d->uhci, + "piix3-usb-uhci"); + qdev_prop_set_int32(DEVICE(&d->uhci), "addr", dev->devfn + 2); + if (!qdev_realize(DEVICE(&d->uhci), BUS(pci_bus), errp)) { + return; + } + } } static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope) @@ -341,6 +352,11 @@ static void pci_piix3_init(Object *obj) object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC); } +static Property pci_piix3_props[] = { + DEFINE_PROP_BOOL("has-usb", PIIX3State, has_usb, true), + DEFINE_PROP_END_OF_LIST(), +}; + static void pci_piix3_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -359,6 +375,7 @@ static void pci_piix3_class_init(ObjectClass *klass, void *data) * pc_piix.c's pc_init1() */ dc->user_creatable = false; + device_class_set_props(dc, pci_piix3_props); adevc->build_dev_aml = build_pci_isa_aml; } diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h index b1fa08dd2b..5367917182 100644 --- a/include/hw/southbridge/piix.h +++ b/include/hw/southbridge/piix.h @@ -15,6 +15,7 @@ #include "hw/pci/pci.h" #include "qom/object.h" #include "hw/rtc/mc146818rtc.h" +#include "hw/usb/hcd-uhci.h" /* PIRQRC[A:D]: PIRQx Route Control Registers */ #define PIIX_PIRQCA 0x60 @@ -54,12 +55,15 @@ struct PIIXState { int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS]; RTCState rtc; + UHCIState uhci; /* Reset Control Register contents */ uint8_t rcr; /* IO memory region for Reset Control Register (PIIX_RCR_IOPORT) */ MemoryRegion rcr_mem; + + bool has_usb; }; typedef struct PIIXState PIIX3State;
The USB controller is an integral part of PIIX3 (function 2). So create it as part of the south bridge. Note that the USB function is optional in QEMU. This is why it gets object_initialize_child()'ed in realize rather than in instance_init. Signed-off-by: Bernhard Beschow <shentey@gmail.com> --- hw/i386/pc_piix.c | 6 ++---- hw/isa/Kconfig | 1 + hw/isa/piix3.c | 17 +++++++++++++++++ include/hw/southbridge/piix.h | 4 ++++ 4 files changed, 24 insertions(+), 4 deletions(-)