| Message ID | 20211202144235.1276352-5-clg@kaod.org |
|---|---|
| State | New |
| Headers | show |
| Series | ppc/pnv: Add support for user created PHB3/PHB4 devices | expand |
On 12/2/21 11:42, Cédric Le Goater wrote: > PHB3 devices and PCI devices can now be added to the powernv8 machine > using : > > -device pnv-phb3,chip-id=0,index=1 \ > -device nec-usb-xhci,bus=pci.1,addr=0x0 > > The 'index' property identifies the PHB3 in the chip. In case of user > created devices, a lookup on 'chip-id' is required to assign the > owning chip. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> > include/hw/ppc/pnv.h | 2 ++ > hw/pci-host/pnv_phb3.c | 11 ++++++++++- > hw/ppc/pnv.c | 23 ++++++++++++++++++----- > 3 files changed, 30 insertions(+), 6 deletions(-) > > diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h > index 6f498c8f1b5f..0710673a7fd8 100644 > --- a/include/hw/ppc/pnv.h > +++ b/include/hw/ppc/pnv.h > @@ -215,6 +215,8 @@ struct PnvMachineState { > hwaddr fw_load_addr; > }; > > +PnvChip *pnv_get_chip(PnvMachineState *pnv, uint32_t chip_id); > + > #define PNV_FDT_ADDR 0x01000000 > #define PNV_TIMEBASE_FREQ 512000000ULL > > diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c > index 3aa42ef9d4b9..dd1cf37288a0 100644 > --- a/hw/pci-host/pnv_phb3.c > +++ b/hw/pci-host/pnv_phb3.c > @@ -998,6 +998,15 @@ static void pnv_phb3_realize(DeviceState *dev, Error **errp) > return; > } > > + /* User created devices */ > + if (!phb->chip) { > + phb->chip = pnv_get_chip(pnv, phb->chip_id); > + if (!phb->chip) { > + error_setg(errp, "invalid chip id: %d", phb->chip_id); > + return; > + } > + } > + > /* LSI sources */ > object_property_set_link(OBJECT(&phb->lsis), "xics", OBJECT(pnv), > &error_abort); > @@ -1105,7 +1114,7 @@ static void pnv_phb3_class_init(ObjectClass *klass, void *data) > dc->realize = pnv_phb3_realize; > device_class_set_props(dc, pnv_phb3_properties); > set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); > - dc->user_creatable = false; > + dc->user_creatable = true; > } > > static const TypeInfo pnv_phb3_type_info = { > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index de277c457838..d7fe92cb082d 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -1097,14 +1097,14 @@ static void pnv_chip_power8_instance_init(Object *obj) > > object_initialize_child(obj, "homer", &chip8->homer, TYPE_PNV8_HOMER); > > - for (i = 0; i < pcc->num_phbs; i++) { > + if (defaults_enabled()) { > + chip->num_phbs = pcc->num_phbs; > + } > + > + for (i = 0; i < chip->num_phbs; i++) { > object_initialize_child(obj, "phb[*]", &chip8->phbs[i], TYPE_PNV_PHB3); > } > > - /* > - * Number of PHBs is the chip default > - */ > - chip->num_phbs = pcc->num_phbs; > } > > static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp) > @@ -1784,6 +1784,19 @@ PowerPCCPU *pnv_chip_find_cpu(PnvChip *chip, uint32_t pir) > return NULL; > } > > +PnvChip *pnv_get_chip(PnvMachineState *pnv, uint32_t chip_id) > +{ > + int i; > + > + for (i = 0; i < pnv->num_chips; i++) { > + PnvChip *chip = pnv->chips[i]; > + if (chip->chip_id == chip_id) { > + return chip; > + } > + } > + return NULL; > +} > + > static ICSState *pnv_ics_get(XICSFabric *xi, int irq) > { > PnvMachineState *pnv = PNV_MACHINE(xi); >
On 02/12/2021 15:42, Cédric Le Goater wrote: > PHB3 devices and PCI devices can now be added to the powernv8 machine > using : > > -device pnv-phb3,chip-id=0,index=1 \ > -device nec-usb-xhci,bus=pci.1,addr=0x0 > > The 'index' property identifies the PHB3 in the chip. In case of user > created devices, a lookup on 'chip-id' is required to assign the > owning chip. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index de277c457838..d7fe92cb082d 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -1097,14 +1097,14 @@ static void pnv_chip_power8_instance_init(Object *obj) > > object_initialize_child(obj, "homer", &chip8->homer, TYPE_PNV8_HOMER); > > - for (i = 0; i < pcc->num_phbs; i++) { > + if (defaults_enabled()) { > + chip->num_phbs = pcc->num_phbs; > + } > + > + for (i = 0; i < chip->num_phbs; i++) { > object_initialize_child(obj, "phb[*]", &chip8->phbs[i], TYPE_PNV_PHB3); > } > > - /* > - * Number of PHBs is the chip default > - */ > - chip->num_phbs = pcc->num_phbs; > } So if "-nodefaults" is mentioned on the command line, then chip->num_phbs is not set. It seems the intention is to have only the PHBs defined on the CLI, which is fine. However, I don't see where chip->num_phbs is incremented in that case. Fred > > static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp) > @@ -1784,6 +1784,19 @@ PowerPCCPU *pnv_chip_find_cpu(PnvChip *chip, uint32_t pir) > return NULL; > } > > +PnvChip *pnv_get_chip(PnvMachineState *pnv, uint32_t chip_id) > +{ > + int i; > + > + for (i = 0; i < pnv->num_chips; i++) { > + PnvChip *chip = pnv->chips[i]; > + if (chip->chip_id == chip_id) { > + return chip; > + } > + } > + return NULL; > +} > + > static ICSState *pnv_ics_get(XICSFabric *xi, int irq) > { > PnvMachineState *pnv = PNV_MACHINE(xi); >
On 12/7/21 10:47, Frederic Barrat wrote: > > > On 02/12/2021 15:42, Cédric Le Goater wrote: >> PHB3 devices and PCI devices can now be added to the powernv8 machine >> using : >> >> -device pnv-phb3,chip-id=0,index=1 \ >> -device nec-usb-xhci,bus=pci.1,addr=0x0 >> >> The 'index' property identifies the PHB3 in the chip. In case of user >> created devices, a lookup on 'chip-id' is required to assign the >> owning chip. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- > >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c >> index de277c457838..d7fe92cb082d 100644 >> --- a/hw/ppc/pnv.c >> +++ b/hw/ppc/pnv.c >> @@ -1097,14 +1097,14 @@ static void pnv_chip_power8_instance_init(Object *obj) >> object_initialize_child(obj, "homer", &chip8->homer, TYPE_PNV8_HOMER); >> - for (i = 0; i < pcc->num_phbs; i++) { >> + if (defaults_enabled()) { >> + chip->num_phbs = pcc->num_phbs; >> + } >> + >> + for (i = 0; i < chip->num_phbs; i++) { >> object_initialize_child(obj, "phb[*]", &chip8->phbs[i], TYPE_PNV_PHB3); >> } >> - /* >> - * Number of PHBs is the chip default >> - */ >> - chip->num_phbs = pcc->num_phbs; >> } > > > So if "-nodefaults" is mentioned on the command line, then chip->num_phbs is not set. It seems the intention is to have only the PHBs defined on the CLI, which is fine. However, I don't see where chip->num_phbs is incremented in that case. Good catch :) That's why we need another patch fixing all this because it is breaking the XICS fabric handlers, ics_get and ics_resend. 'info pic' is impacted also. Here is the proposed fix for v2 : https://github.com/legoater/qemu/commit/b47bce3109f316a65aa2fa2a46651b2960e93fca I chose to loop on the children of the chip to find the user created devices and leave the PnvChip model with empty defaults. 'info pic' is impacted the same on P9 https://github.com/legoater/qemu/commit/d4733edca94c95f717f4ee35bbea6dc085365286 Thanks, C.
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h index 6f498c8f1b5f..0710673a7fd8 100644 --- a/include/hw/ppc/pnv.h +++ b/include/hw/ppc/pnv.h @@ -215,6 +215,8 @@ struct PnvMachineState { hwaddr fw_load_addr; }; +PnvChip *pnv_get_chip(PnvMachineState *pnv, uint32_t chip_id); + #define PNV_FDT_ADDR 0x01000000 #define PNV_TIMEBASE_FREQ 512000000ULL diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c index 3aa42ef9d4b9..dd1cf37288a0 100644 --- a/hw/pci-host/pnv_phb3.c +++ b/hw/pci-host/pnv_phb3.c @@ -998,6 +998,15 @@ static void pnv_phb3_realize(DeviceState *dev, Error **errp) return; } + /* User created devices */ + if (!phb->chip) { + phb->chip = pnv_get_chip(pnv, phb->chip_id); + if (!phb->chip) { + error_setg(errp, "invalid chip id: %d", phb->chip_id); + return; + } + } + /* LSI sources */ object_property_set_link(OBJECT(&phb->lsis), "xics", OBJECT(pnv), &error_abort); @@ -1105,7 +1114,7 @@ static void pnv_phb3_class_init(ObjectClass *klass, void *data) dc->realize = pnv_phb3_realize; device_class_set_props(dc, pnv_phb3_properties); set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); - dc->user_creatable = false; + dc->user_creatable = true; } static const TypeInfo pnv_phb3_type_info = { diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index de277c457838..d7fe92cb082d 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -1097,14 +1097,14 @@ static void pnv_chip_power8_instance_init(Object *obj) object_initialize_child(obj, "homer", &chip8->homer, TYPE_PNV8_HOMER); - for (i = 0; i < pcc->num_phbs; i++) { + if (defaults_enabled()) { + chip->num_phbs = pcc->num_phbs; + } + + for (i = 0; i < chip->num_phbs; i++) { object_initialize_child(obj, "phb[*]", &chip8->phbs[i], TYPE_PNV_PHB3); } - /* - * Number of PHBs is the chip default - */ - chip->num_phbs = pcc->num_phbs; } static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp) @@ -1784,6 +1784,19 @@ PowerPCCPU *pnv_chip_find_cpu(PnvChip *chip, uint32_t pir) return NULL; } +PnvChip *pnv_get_chip(PnvMachineState *pnv, uint32_t chip_id) +{ + int i; + + for (i = 0; i < pnv->num_chips; i++) { + PnvChip *chip = pnv->chips[i]; + if (chip->chip_id == chip_id) { + return chip; + } + } + return NULL; +} + static ICSState *pnv_ics_get(XICSFabric *xi, int irq) { PnvMachineState *pnv = PNV_MACHINE(xi);
PHB3 devices and PCI devices can now be added to the powernv8 machine using : -device pnv-phb3,chip-id=0,index=1 \ -device nec-usb-xhci,bus=pci.1,addr=0x0 The 'index' property identifies the PHB3 in the chip. In case of user created devices, a lookup on 'chip-id' is required to assign the owning chip. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- include/hw/ppc/pnv.h | 2 ++ hw/pci-host/pnv_phb3.c | 11 ++++++++++- hw/ppc/pnv.c | 23 ++++++++++++++++++----- 3 files changed, 30 insertions(+), 6 deletions(-)