| Message ID | 1501285073-2215-5-git-send-email-zuban32s@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 29/07/2017 2:37, Aleksandr Bezzubikov wrote: > From: Aleksandr Bezzubikov <abezzubikov@ispras.ru> > > To enable hotplugging of a newly created pcie-pci-bridge, > we need to tell firmware (SeaBIOS in this case) Not only SeaBIOS, also OVMF - so all guest firmware to reserve > additional buses for pcie-root-port, that allows us to > hotplug pcie-pci-bridge into this root port. > The number of buses to reserve is provided to the device via a corresponding > property, and to the firmware via new PCI capability. > The property's default value is 0 to keep default behavior unchanged. > > Signed-off-by: Aleksandr Bezzubikov <zuban32s@gmail.com> > --- > hw/pci-bridge/gen_pcie_root_port.c | 23 +++++++++++++++++++++++ > hw/pci-bridge/pcie_root_port.c | 2 +- > include/hw/pci/pcie_port.h | 2 ++ > 3 files changed, 26 insertions(+), 1 deletion(-) > > diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c > index cb694d6..da3caa1 100644 > --- a/hw/pci-bridge/gen_pcie_root_port.c > +++ b/hw/pci-bridge/gen_pcie_root_port.c > @@ -16,6 +16,8 @@ > #include "hw/pci/pcie_port.h" > > #define TYPE_GEN_PCIE_ROOT_PORT "pcie-root-port" > +#define GEN_PCIE_ROOT_PORT(obj) \ > + OBJECT_CHECK(GenPCIERootPort, (obj), TYPE_GEN_PCIE_ROOT_PORT) > > #define GEN_PCIE_ROOT_PORT_AER_OFFSET 0x100 > #define GEN_PCIE_ROOT_PORT_MSIX_NR_VECTOR 1 > @@ -26,6 +28,9 @@ typedef struct GenPCIERootPort { > /*< public >*/ > > bool migrate_msix; > + > + /* additional buses to reserve on firmware init */ > + uint8_t bus_reserve; > } GenPCIERootPort; > > static uint8_t gen_rp_aer_vector(const PCIDevice *d) > @@ -60,6 +65,21 @@ static bool gen_rp_test_migrate_msix(void *opaque, int version_id) > return rp->migrate_msix; > } > > +static void gen_rp_realize(PCIDevice *d, Error **errp) > +{ > + rp_realize(d, errp); > + PCIESlot *s = PCIE_SLOT(d); > + GenPCIERootPort *grp = GEN_PCIE_ROOT_PORT(d); > + > + int rc = pci_bridge_qemu_cap_init(d, 0, grp->bus_reserve, 0, 0, 0, errp); > + if (rc < 0) { > + pcie_chassis_del_slot(s); > + pcie_cap_exit(d); > + gen_rp_interrupts_uninit(d); > + pci_bridge_exitfn(d); > + } > +} > + > static const VMStateDescription vmstate_rp_dev = { > .name = "pcie-root-port", > .version_id = 1, > @@ -78,6 +98,7 @@ static const VMStateDescription vmstate_rp_dev = { > > static Property gen_rp_props[] = { > DEFINE_PROP_BOOL("x-migrate-msix", GenPCIERootPort, migrate_msix, true), > + DEFINE_PROP_UINT8("bus-reserve", GenPCIERootPort, bus_reserve, 0), > DEFINE_PROP_END_OF_LIST() > }; > > @@ -89,6 +110,8 @@ static void gen_rp_dev_class_init(ObjectClass *klass, void *data) > > k->vendor_id = PCI_VENDOR_ID_REDHAT; > k->device_id = PCI_DEVICE_ID_REDHAT_PCIE_RP; > + k->realize = gen_rp_realize; > + > dc->desc = "PCI Express Root Port"; > dc->vmsd = &vmstate_rp_dev; > dc->props = gen_rp_props; > diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c > index 4d588cb..2f3bcb1 100644 > --- a/hw/pci-bridge/pcie_root_port.c > +++ b/hw/pci-bridge/pcie_root_port.c > @@ -52,7 +52,7 @@ static void rp_reset(DeviceState *qdev) > pci_bridge_disable_base_limit(d); > } > > -static void rp_realize(PCIDevice *d, Error **errp) > +void rp_realize(PCIDevice *d, Error **errp) > { > PCIEPort *p = PCIE_PORT(d); > PCIESlot *s = PCIE_SLOT(d); > diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h > index 1333266..febd96a 100644 > --- a/include/hw/pci/pcie_port.h > +++ b/include/hw/pci/pcie_port.h > @@ -63,6 +63,8 @@ void pcie_chassis_del_slot(PCIESlot *s); > #define PCIE_ROOT_PORT_GET_CLASS(obj) \ > OBJECT_GET_CLASS(PCIERootPortClass, (obj), TYPE_PCIE_ROOT_PORT) > > +void rp_realize(PCIDevice *d, Error **errp); This is not how QEMU re-uses parent's realize function. You can grep for "parent_realize" in the project, it goes something like this: 1. You add "DeviceRealize parent_realize" to GenPCIERootPort class. 2. In class_init you save parent's realize and replace it with your own: grpc->parent_realize = dc->realize; dc->realize = gen_rp_realize; 3. In gen_rp_realize call first parent_realize: rpc->parent_realize(dev, errp); - your code here - if (err) rpc-> exit() Thanks, Marcel > + > typedef struct PCIERootPortClass { > PCIDeviceClass parent_class; > >
2017-07-31 14:43 GMT+03:00 Marcel Apfelbaum <marcel@redhat.com>: > On 29/07/2017 2:37, Aleksandr Bezzubikov wrote: >> >> From: Aleksandr Bezzubikov <abezzubikov@ispras.ru> >> >> To enable hotplugging of a newly created pcie-pci-bridge, >> we need to tell firmware (SeaBIOS in this case) > > > Not only SeaBIOS, also OVMF - so all guest firmware > But it can mislead some users - it will look like we've already implemented this for SeaBIOS and OVMF both, but we haven't. > > to reserve >> >> additional buses for pcie-root-port, that allows us to >> hotplug pcie-pci-bridge into this root port. >> The number of buses to reserve is provided to the device via a >> corresponding >> property, and to the firmware via new PCI capability. >> The property's default value is 0 to keep default behavior unchanged. >> >> Signed-off-by: Aleksandr Bezzubikov <zuban32s@gmail.com> >> --- >> hw/pci-bridge/gen_pcie_root_port.c | 23 +++++++++++++++++++++++ >> hw/pci-bridge/pcie_root_port.c | 2 +- >> include/hw/pci/pcie_port.h | 2 ++ >> 3 files changed, 26 insertions(+), 1 deletion(-) >> >> diff --git a/hw/pci-bridge/gen_pcie_root_port.c >> b/hw/pci-bridge/gen_pcie_root_port.c >> index cb694d6..da3caa1 100644 >> --- a/hw/pci-bridge/gen_pcie_root_port.c >> +++ b/hw/pci-bridge/gen_pcie_root_port.c >> @@ -16,6 +16,8 @@ >> #include "hw/pci/pcie_port.h" >> #define TYPE_GEN_PCIE_ROOT_PORT "pcie-root-port" >> +#define GEN_PCIE_ROOT_PORT(obj) \ >> + OBJECT_CHECK(GenPCIERootPort, (obj), TYPE_GEN_PCIE_ROOT_PORT) >> #define GEN_PCIE_ROOT_PORT_AER_OFFSET 0x100 >> #define GEN_PCIE_ROOT_PORT_MSIX_NR_VECTOR 1 >> @@ -26,6 +28,9 @@ typedef struct GenPCIERootPort { >> /*< public >*/ >> bool migrate_msix; >> + >> + /* additional buses to reserve on firmware init */ >> + uint8_t bus_reserve; >> } GenPCIERootPort; >> static uint8_t gen_rp_aer_vector(const PCIDevice *d) >> @@ -60,6 +65,21 @@ static bool gen_rp_test_migrate_msix(void *opaque, int >> version_id) >> return rp->migrate_msix; >> } >> +static void gen_rp_realize(PCIDevice *d, Error **errp) >> +{ >> + rp_realize(d, errp); >> + PCIESlot *s = PCIE_SLOT(d); >> + GenPCIERootPort *grp = GEN_PCIE_ROOT_PORT(d); >> + >> + int rc = pci_bridge_qemu_cap_init(d, 0, grp->bus_reserve, 0, 0, 0, >> errp); >> + if (rc < 0) { >> + pcie_chassis_del_slot(s); >> + pcie_cap_exit(d); >> + gen_rp_interrupts_uninit(d); >> + pci_bridge_exitfn(d); >> + } >> +} >> + >> static const VMStateDescription vmstate_rp_dev = { >> .name = "pcie-root-port", >> .version_id = 1, >> @@ -78,6 +98,7 @@ static const VMStateDescription vmstate_rp_dev = { >> static Property gen_rp_props[] = { >> DEFINE_PROP_BOOL("x-migrate-msix", GenPCIERootPort, migrate_msix, >> true), >> + DEFINE_PROP_UINT8("bus-reserve", GenPCIERootPort, bus_reserve, 0), >> DEFINE_PROP_END_OF_LIST() >> }; >> @@ -89,6 +110,8 @@ static void gen_rp_dev_class_init(ObjectClass *klass, >> void *data) >> k->vendor_id = PCI_VENDOR_ID_REDHAT; >> k->device_id = PCI_DEVICE_ID_REDHAT_PCIE_RP; >> + k->realize = gen_rp_realize; >> + >> dc->desc = "PCI Express Root Port"; >> dc->vmsd = &vmstate_rp_dev; >> dc->props = gen_rp_props; >> diff --git a/hw/pci-bridge/pcie_root_port.c >> b/hw/pci-bridge/pcie_root_port.c >> index 4d588cb..2f3bcb1 100644 >> --- a/hw/pci-bridge/pcie_root_port.c >> +++ b/hw/pci-bridge/pcie_root_port.c >> @@ -52,7 +52,7 @@ static void rp_reset(DeviceState *qdev) >> pci_bridge_disable_base_limit(d); >> } >> -static void rp_realize(PCIDevice *d, Error **errp) >> +void rp_realize(PCIDevice *d, Error **errp) >> { >> PCIEPort *p = PCIE_PORT(d); >> PCIESlot *s = PCIE_SLOT(d); >> diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h >> index 1333266..febd96a 100644 >> --- a/include/hw/pci/pcie_port.h >> +++ b/include/hw/pci/pcie_port.h >> @@ -63,6 +63,8 @@ void pcie_chassis_del_slot(PCIESlot *s); >> #define PCIE_ROOT_PORT_GET_CLASS(obj) \ >> OBJECT_GET_CLASS(PCIERootPortClass, (obj), TYPE_PCIE_ROOT_PORT) >> +void rp_realize(PCIDevice *d, Error **errp); > > > This is not how QEMU re-uses parent's realize function. > You can grep for "parent_realize" in the project, it > goes something like this: > 1. You add "DeviceRealize parent_realize" to GenPCIERootPort class. > 2. In class_init you save parent's realize and replace it with > your own: > grpc->parent_realize = dc->realize; > dc->realize = gen_rp_realize; > 3. In gen_rp_realize call first parent_realize: > rpc->parent_realize(dev, errp); > - your code here - > if (err) > rpc-> exit() > OK, this is really much more pretty. > Thanks, > Marcel > > >> + >> typedef struct PCIERootPortClass { >> PCIDeviceClass parent_class; >> > >
diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c index cb694d6..da3caa1 100644 --- a/hw/pci-bridge/gen_pcie_root_port.c +++ b/hw/pci-bridge/gen_pcie_root_port.c @@ -16,6 +16,8 @@ #include "hw/pci/pcie_port.h" #define TYPE_GEN_PCIE_ROOT_PORT "pcie-root-port" +#define GEN_PCIE_ROOT_PORT(obj) \ + OBJECT_CHECK(GenPCIERootPort, (obj), TYPE_GEN_PCIE_ROOT_PORT) #define GEN_PCIE_ROOT_PORT_AER_OFFSET 0x100 #define GEN_PCIE_ROOT_PORT_MSIX_NR_VECTOR 1 @@ -26,6 +28,9 @@ typedef struct GenPCIERootPort { /*< public >*/ bool migrate_msix; + + /* additional buses to reserve on firmware init */ + uint8_t bus_reserve; } GenPCIERootPort; static uint8_t gen_rp_aer_vector(const PCIDevice *d) @@ -60,6 +65,21 @@ static bool gen_rp_test_migrate_msix(void *opaque, int version_id) return rp->migrate_msix; } +static void gen_rp_realize(PCIDevice *d, Error **errp) +{ + rp_realize(d, errp); + PCIESlot *s = PCIE_SLOT(d); + GenPCIERootPort *grp = GEN_PCIE_ROOT_PORT(d); + + int rc = pci_bridge_qemu_cap_init(d, 0, grp->bus_reserve, 0, 0, 0, errp); + if (rc < 0) { + pcie_chassis_del_slot(s); + pcie_cap_exit(d); + gen_rp_interrupts_uninit(d); + pci_bridge_exitfn(d); + } +} + static const VMStateDescription vmstate_rp_dev = { .name = "pcie-root-port", .version_id = 1, @@ -78,6 +98,7 @@ static const VMStateDescription vmstate_rp_dev = { static Property gen_rp_props[] = { DEFINE_PROP_BOOL("x-migrate-msix", GenPCIERootPort, migrate_msix, true), + DEFINE_PROP_UINT8("bus-reserve", GenPCIERootPort, bus_reserve, 0), DEFINE_PROP_END_OF_LIST() }; @@ -89,6 +110,8 @@ static void gen_rp_dev_class_init(ObjectClass *klass, void *data) k->vendor_id = PCI_VENDOR_ID_REDHAT; k->device_id = PCI_DEVICE_ID_REDHAT_PCIE_RP; + k->realize = gen_rp_realize; + dc->desc = "PCI Express Root Port"; dc->vmsd = &vmstate_rp_dev; dc->props = gen_rp_props; diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c index 4d588cb..2f3bcb1 100644 --- a/hw/pci-bridge/pcie_root_port.c +++ b/hw/pci-bridge/pcie_root_port.c @@ -52,7 +52,7 @@ static void rp_reset(DeviceState *qdev) pci_bridge_disable_base_limit(d); } -static void rp_realize(PCIDevice *d, Error **errp) +void rp_realize(PCIDevice *d, Error **errp) { PCIEPort *p = PCIE_PORT(d); PCIESlot *s = PCIE_SLOT(d); diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h index 1333266..febd96a 100644 --- a/include/hw/pci/pcie_port.h +++ b/include/hw/pci/pcie_port.h @@ -63,6 +63,8 @@ void pcie_chassis_del_slot(PCIESlot *s); #define PCIE_ROOT_PORT_GET_CLASS(obj) \ OBJECT_GET_CLASS(PCIERootPortClass, (obj), TYPE_PCIE_ROOT_PORT) +void rp_realize(PCIDevice *d, Error **errp); + typedef struct PCIERootPortClass { PCIDeviceClass parent_class;