| Message ID | 20220811163950.578927-6-danielhb413@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | enable pnv-phb user created devices | expand |
On 8/11/22 18:39, Daniel Henrique Barboza wrote: > When enabling user created PHBs (a change reverted by commit 9c10d86fee) > we were handling PHBs created by default versus by the user in different > manners. The only difference between these PHBs is that one will have a > valid phb3->chip that is assigned during pnv_chip_power8_realize(), > while the user created needs to search which chip it belongs to. > > Aside from that there shouldn't be any difference. Making the default > PHBs behave in line with the user created ones will make it easier to > re-introduce them later on. It will also make the code easier to follow > since we are dealing with them in equal manner. > > The first step is to turn chip8->phbs[] into a PnvPHB3 pointer array. > This will allow us to assign user created PHBs into it later on. The way > we initilize the default case is now more in line with that would happen > with the user created case: the object is created, parented by the chip > because pnv_xscom_dt() relies on it, and then assigned to the array. > > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Thanks, C. > --- > hw/ppc/pnv.c | 27 ++++++++++++++++++++++----- > include/hw/ppc/pnv.h | 6 +++++- > 2 files changed, 27 insertions(+), 6 deletions(-) > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index 2deaac17f7..e53e9e297d 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -294,6 +294,13 @@ static void pnv_dt_icp(PnvChip *chip, void *fdt, uint32_t pir, > Object *pnv_chip_add_phb(PnvChip *chip, PnvPHB *phb, Error **errp) > { > if (phb->version == 3) { > + Pnv8Chip *chip8 = PNV8_CHIP(chip); > + > + phb->chip = chip; > + > + chip8->phbs[chip8->num_phbs] = phb; > + chip8->num_phbs++; > + > return OBJECT(chip); > } else { > /* phb4 support will be added later */ > @@ -681,7 +688,7 @@ static void pnv_chip_power8_pic_print_info(PnvChip *chip, Monitor *mon) > ics_pic_print_info(&chip8->psi.ics, mon); > > for (i = 0; i < chip8->num_phbs; i++) { > - PnvPHB *phb = &chip8->phbs[i]; > + PnvPHB *phb = chip8->phbs[i]; > PnvPHB3 *phb3 = PNV_PHB3(phb->backend); > > pnv_phb3_msi_pic_print_info(&phb3->msis, mon); > @@ -1174,7 +1181,17 @@ static void pnv_chip_power8_instance_init(Object *obj) > chip8->num_phbs = pcc->num_phbs; > > for (i = 0; i < chip8->num_phbs; i++) { > - object_initialize_child(obj, "phb[*]", &chip8->phbs[i], TYPE_PNV_PHB); > + Object *phb = object_new(TYPE_PNV_PHB); > + > + /* > + * We need the chip to parent the PHB to allow the DT > + * to build correctly (via pnv_xscom_dt()). > + * > + * TODO: the PHB should be parented by a PEC device that, at > + * this moment, is not modelled powernv8/phb3. > + */ > + object_property_add_child(obj, "phb[*]", phb); > + chip8->phbs[i] = PNV_PHB(phb); > } > > } > @@ -1290,7 +1307,7 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp) > > /* PHB controllers */ > for (i = 0; i < chip8->num_phbs; i++) { > - PnvPHB *phb = &chip8->phbs[i]; > + PnvPHB *phb = chip8->phbs[i]; > > object_property_set_int(OBJECT(phb), "index", i, &error_fatal); > object_property_set_int(OBJECT(phb), "chip-id", chip->chip_id, > @@ -1958,7 +1975,7 @@ static ICSState *pnv_ics_get(XICSFabric *xi, int irq) > } > > for (j = 0; j < chip8->num_phbs; j++) { > - PnvPHB *phb = &chip8->phbs[j]; > + PnvPHB *phb = chip8->phbs[j]; > PnvPHB3 *phb3 = PNV_PHB3(phb->backend); > > if (ics_valid_irq(&phb3->lsis, irq)) { > @@ -1997,7 +2014,7 @@ static void pnv_ics_resend(XICSFabric *xi) > ics_resend(&chip8->psi.ics); > > for (j = 0; j < chip8->num_phbs; j++) { > - PnvPHB *phb = &chip8->phbs[j]; > + PnvPHB *phb = chip8->phbs[j]; > PnvPHB3 *phb3 = PNV_PHB3(phb->backend); > > ics_resend(&phb3->lsis); > diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h > index 781d0acffa..49433281d7 100644 > --- a/include/hw/ppc/pnv.h > +++ b/include/hw/ppc/pnv.h > @@ -81,7 +81,11 @@ struct Pnv8Chip { > PnvHomer homer; > > #define PNV8_CHIP_PHB3_MAX 4 > - PnvPHB phbs[PNV8_CHIP_PHB3_MAX]; > + /* > + * The array is used to allow quick access to the phbs by > + * pnv_ics_get_child() and pnv_ics_resend_child(). > + */ > + PnvPHB *phbs[PNV8_CHIP_PHB3_MAX]; > uint32_t num_phbs; > > XICSFabric *xics;
On 11/08/2022 18:39, Daniel Henrique Barboza wrote: > When enabling user created PHBs (a change reverted by commit 9c10d86fee) > we were handling PHBs created by default versus by the user in different > manners. The only difference between these PHBs is that one will have a > valid phb3->chip that is assigned during pnv_chip_power8_realize(), > while the user created needs to search which chip it belongs to. > > Aside from that there shouldn't be any difference. Making the default > PHBs behave in line with the user created ones will make it easier to > re-introduce them later on. It will also make the code easier to follow > since we are dealing with them in equal manner. > > The first step is to turn chip8->phbs[] into a PnvPHB3 pointer array. > This will allow us to assign user created PHBs into it later on. The way > we initilize the default case is now more in line with that would happen > with the user created case: the object is created, parented by the chip > because pnv_xscom_dt() relies on it, and then assigned to the array. > > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > --- Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com> Fred > hw/ppc/pnv.c | 27 ++++++++++++++++++++++----- > include/hw/ppc/pnv.h | 6 +++++- > 2 files changed, 27 insertions(+), 6 deletions(-) > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index 2deaac17f7..e53e9e297d 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -294,6 +294,13 @@ static void pnv_dt_icp(PnvChip *chip, void *fdt, uint32_t pir, > Object *pnv_chip_add_phb(PnvChip *chip, PnvPHB *phb, Error **errp) > { > if (phb->version == 3) { > + Pnv8Chip *chip8 = PNV8_CHIP(chip); > + > + phb->chip = chip; > + > + chip8->phbs[chip8->num_phbs] = phb; > + chip8->num_phbs++; > + > return OBJECT(chip); > } else { > /* phb4 support will be added later */ > @@ -681,7 +688,7 @@ static void pnv_chip_power8_pic_print_info(PnvChip *chip, Monitor *mon) > ics_pic_print_info(&chip8->psi.ics, mon); > > for (i = 0; i < chip8->num_phbs; i++) { > - PnvPHB *phb = &chip8->phbs[i]; > + PnvPHB *phb = chip8->phbs[i]; > PnvPHB3 *phb3 = PNV_PHB3(phb->backend); > > pnv_phb3_msi_pic_print_info(&phb3->msis, mon); > @@ -1174,7 +1181,17 @@ static void pnv_chip_power8_instance_init(Object *obj) > chip8->num_phbs = pcc->num_phbs; > > for (i = 0; i < chip8->num_phbs; i++) { > - object_initialize_child(obj, "phb[*]", &chip8->phbs[i], TYPE_PNV_PHB); > + Object *phb = object_new(TYPE_PNV_PHB); > + > + /* > + * We need the chip to parent the PHB to allow the DT > + * to build correctly (via pnv_xscom_dt()). > + * > + * TODO: the PHB should be parented by a PEC device that, at > + * this moment, is not modelled powernv8/phb3. > + */ > + object_property_add_child(obj, "phb[*]", phb); > + chip8->phbs[i] = PNV_PHB(phb); > } > > } > @@ -1290,7 +1307,7 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp) > > /* PHB controllers */ > for (i = 0; i < chip8->num_phbs; i++) { > - PnvPHB *phb = &chip8->phbs[i]; > + PnvPHB *phb = chip8->phbs[i]; > > object_property_set_int(OBJECT(phb), "index", i, &error_fatal); > object_property_set_int(OBJECT(phb), "chip-id", chip->chip_id, > @@ -1958,7 +1975,7 @@ static ICSState *pnv_ics_get(XICSFabric *xi, int irq) > } > > for (j = 0; j < chip8->num_phbs; j++) { > - PnvPHB *phb = &chip8->phbs[j]; > + PnvPHB *phb = chip8->phbs[j]; > PnvPHB3 *phb3 = PNV_PHB3(phb->backend); > > if (ics_valid_irq(&phb3->lsis, irq)) { > @@ -1997,7 +2014,7 @@ static void pnv_ics_resend(XICSFabric *xi) > ics_resend(&chip8->psi.ics); > > for (j = 0; j < chip8->num_phbs; j++) { > - PnvPHB *phb = &chip8->phbs[j]; > + PnvPHB *phb = chip8->phbs[j]; > PnvPHB3 *phb3 = PNV_PHB3(phb->backend); > > ics_resend(&phb3->lsis); > diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h > index 781d0acffa..49433281d7 100644 > --- a/include/hw/ppc/pnv.h > +++ b/include/hw/ppc/pnv.h > @@ -81,7 +81,11 @@ struct Pnv8Chip { > PnvHomer homer; > > #define PNV8_CHIP_PHB3_MAX 4 > - PnvPHB phbs[PNV8_CHIP_PHB3_MAX]; > + /* > + * The array is used to allow quick access to the phbs by > + * pnv_ics_get_child() and pnv_ics_resend_child(). > + */ > + PnvPHB *phbs[PNV8_CHIP_PHB3_MAX]; > uint32_t num_phbs; > > XICSFabric *xics;
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 2deaac17f7..e53e9e297d 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -294,6 +294,13 @@ static void pnv_dt_icp(PnvChip *chip, void *fdt, uint32_t pir, Object *pnv_chip_add_phb(PnvChip *chip, PnvPHB *phb, Error **errp) { if (phb->version == 3) { + Pnv8Chip *chip8 = PNV8_CHIP(chip); + + phb->chip = chip; + + chip8->phbs[chip8->num_phbs] = phb; + chip8->num_phbs++; + return OBJECT(chip); } else { /* phb4 support will be added later */ @@ -681,7 +688,7 @@ static void pnv_chip_power8_pic_print_info(PnvChip *chip, Monitor *mon) ics_pic_print_info(&chip8->psi.ics, mon); for (i = 0; i < chip8->num_phbs; i++) { - PnvPHB *phb = &chip8->phbs[i]; + PnvPHB *phb = chip8->phbs[i]; PnvPHB3 *phb3 = PNV_PHB3(phb->backend); pnv_phb3_msi_pic_print_info(&phb3->msis, mon); @@ -1174,7 +1181,17 @@ static void pnv_chip_power8_instance_init(Object *obj) chip8->num_phbs = pcc->num_phbs; for (i = 0; i < chip8->num_phbs; i++) { - object_initialize_child(obj, "phb[*]", &chip8->phbs[i], TYPE_PNV_PHB); + Object *phb = object_new(TYPE_PNV_PHB); + + /* + * We need the chip to parent the PHB to allow the DT + * to build correctly (via pnv_xscom_dt()). + * + * TODO: the PHB should be parented by a PEC device that, at + * this moment, is not modelled powernv8/phb3. + */ + object_property_add_child(obj, "phb[*]", phb); + chip8->phbs[i] = PNV_PHB(phb); } } @@ -1290,7 +1307,7 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp) /* PHB controllers */ for (i = 0; i < chip8->num_phbs; i++) { - PnvPHB *phb = &chip8->phbs[i]; + PnvPHB *phb = chip8->phbs[i]; object_property_set_int(OBJECT(phb), "index", i, &error_fatal); object_property_set_int(OBJECT(phb), "chip-id", chip->chip_id, @@ -1958,7 +1975,7 @@ static ICSState *pnv_ics_get(XICSFabric *xi, int irq) } for (j = 0; j < chip8->num_phbs; j++) { - PnvPHB *phb = &chip8->phbs[j]; + PnvPHB *phb = chip8->phbs[j]; PnvPHB3 *phb3 = PNV_PHB3(phb->backend); if (ics_valid_irq(&phb3->lsis, irq)) { @@ -1997,7 +2014,7 @@ static void pnv_ics_resend(XICSFabric *xi) ics_resend(&chip8->psi.ics); for (j = 0; j < chip8->num_phbs; j++) { - PnvPHB *phb = &chip8->phbs[j]; + PnvPHB *phb = chip8->phbs[j]; PnvPHB3 *phb3 = PNV_PHB3(phb->backend); ics_resend(&phb3->lsis); diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h index 781d0acffa..49433281d7 100644 --- a/include/hw/ppc/pnv.h +++ b/include/hw/ppc/pnv.h @@ -81,7 +81,11 @@ struct Pnv8Chip { PnvHomer homer; #define PNV8_CHIP_PHB3_MAX 4 - PnvPHB phbs[PNV8_CHIP_PHB3_MAX]; + /* + * The array is used to allow quick access to the phbs by + * pnv_ics_get_child() and pnv_ics_resend_child(). + */ + PnvPHB *phbs[PNV8_CHIP_PHB3_MAX]; uint32_t num_phbs; XICSFabric *xics;
When enabling user created PHBs (a change reverted by commit 9c10d86fee) we were handling PHBs created by default versus by the user in different manners. The only difference between these PHBs is that one will have a valid phb3->chip that is assigned during pnv_chip_power8_realize(), while the user created needs to search which chip it belongs to. Aside from that there shouldn't be any difference. Making the default PHBs behave in line with the user created ones will make it easier to re-introduce them later on. It will also make the code easier to follow since we are dealing with them in equal manner. The first step is to turn chip8->phbs[] into a PnvPHB3 pointer array. This will allow us to assign user created PHBs into it later on. The way we initilize the default case is now more in line with that would happen with the user created case: the object is created, parented by the chip because pnv_xscom_dt() relies on it, and then assigned to the array. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- hw/ppc/pnv.c | 27 ++++++++++++++++++++++----- include/hw/ppc/pnv.h | 6 +++++- 2 files changed, 27 insertions(+), 6 deletions(-)