Re: [PATCH 13/14] ppc/pnv: Move realize of PEC stacks under the PEC model

2021-12-07 Thread Cédric Le Goater 

On 12/7/21 11:10, Frederic Barrat wrote:



On 02/12/2021 15:42, Cédric Le Goater wrote:

This change will help us providing support for user created PHB4
devices.

Signed-off-by: Cédric Le Goater 
---
  hw/pci-host/pnv_phb4_pec.c | 36 
  hw/ppc/pnv.c   | 31 +--
  2 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
index dfed2af0f7df..9b081d543057 100644
--- a/hw/pci-host/pnv_phb4_pec.c
+++ b/hw/pci-host/pnv_phb4_pec.c
@@ -556,6 +556,10 @@ static void pnv_pec_stk_realize(DeviceState *dev, Error 
**errp)
  {
  PnvPhb4PecStack *stack = PNV_PHB4_PEC_STACK(dev);
  PnvPhb4PecState *pec = stack->pec;
+    PnvPhb4PecClass *pecc = PNV_PHB4_PEC_GET_CLASS(pec);
+    PnvChip *chip = pec->chip;
+    uint32_t pec_nest_base;
+    uint32_t pec_pci_base;
  char name[64];
  assert(pec);
@@ -579,10 +583,34 @@ static void pnv_pec_stk_realize(DeviceState *dev, Error 
**errp)
  pnv_xscom_region_init(>phb_regs_mr, OBJECT(>phb),
    _phb4_xscom_ops, >phb, name, 0x40);
-    /*
- * Let the machine/chip realize the PHB object to customize more
- * easily some fields
- */
+    {
+    Object *obj = OBJECT(>phb);
+
+    object_property_set_int(obj, "chip-id", pec->chip_id, _fatal);
+    object_property_set_int(obj, "version", pecc->version, _fatal);
+    object_property_set_int(obj, "device-id", pecc->device_id,
+    _fatal);
+    object_property_set_link(obj, "stack", OBJECT(stack),
+ _abort);
+    if (!sysbus_realize(SYS_BUS_DEVICE(obj), errp)) {
+    return;
+    }
+    }



Do we really need the extra sub-scope here? It looks off.


No. That's a left over from the initial patches I worked on.


+
+    pec_nest_base = pecc->xscom_nest_base(pec);
+    pec_pci_base = pecc->xscom_pci_base(pec);
+
+    /* Populate the XSCOM address space. */
+    pnv_xscom_add_subregion(chip,
+    pec_nest_base + 0x40 * (stack->stack_no + 1),
+    >nest_regs_mr);
+    pnv_xscom_add_subregion(chip,
+    pec_pci_base + 0x40 * (stack->stack_no + 1),
+    >pci_regs_mr);
+    pnv_xscom_add_subregion(chip,
+    pec_pci_base + PNV9_XSCOM_PEC_PCI_STK0 +
+    0x40 * stack->stack_no,
+    >phb_regs_mr);
  }
  static Property pnv_pec_stk_properties[] = {
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 3a550eed0f36..7e13b15241fd 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1370,7 +1370,7 @@ static void pnv_chip_quad_realize(Pnv9Chip *chip9, Error 
**errp)
  static void pnv_chip_power9_phb_realize(PnvChip *chip, Error **errp)
  {



With that change, we should really rename pnv_chip_power9_phb_realize() to 
pnv_chip_power9_pec_realize().


yes.

Thanks,

C.




   Fred



  Pnv9Chip *chip9 = PNV9_CHIP(chip);
-    int i, j;
+    int i;
  for (i = 0; i < chip->num_pecs; i++) {
  PnvPhb4PecState *pec = >pecs[i];
@@ -1392,35 +1392,6 @@ static void pnv_chip_power9_phb_realize(PnvChip *chip, 
Error **errp)
  pnv_xscom_add_subregion(chip, pec_nest_base, >nest_regs_mr);
  pnv_xscom_add_subregion(chip, pec_pci_base, >pci_regs_mr);
-
-    for (j = 0; j < pec->num_stacks; j++) {
-    PnvPhb4PecStack *stack = >stacks[j];
-    Object *obj = OBJECT(>phb);
-
-    object_property_set_int(obj, "chip-id", chip->chip_id,
-    _fatal);
-    object_property_set_int(obj, "version", pecc->version,
-    _fatal);
-    object_property_set_int(obj, "device-id", pecc->device_id,
-    _fatal);
-    object_property_set_link(obj, "stack", OBJECT(stack),
- _abort);
-    if (!sysbus_realize(SYS_BUS_DEVICE(obj), errp)) {
-    return;
-    }
-
-    /* Populate the XSCOM address space. */
-    pnv_xscom_add_subregion(chip,
-   pec_nest_base + 0x40 * (stack->stack_no + 
1),
-   >nest_regs_mr);
-    pnv_xscom_add_subregion(chip,
-    pec_pci_base + 0x40 * (stack->stack_no + 
1),
-    >pci_regs_mr);
-    pnv_xscom_add_subregion(chip,
-    pec_pci_base + PNV9_XSCOM_PEC_PCI_STK0 +
-    0x40 * stack->stack_no,
-    >phb_regs_mr);
-    }
  }
  }

Re: [PATCH 13/14] ppc/pnv: Move realize of PEC stacks under the PEC model

2021-12-07 Thread Frederic Barrat 




On 02/12/2021 15:42, Cédric Le Goater wrote:

This change will help us providing support for user created PHB4
devices.

Signed-off-by: Cédric Le Goater 
---
  hw/pci-host/pnv_phb4_pec.c | 36 
  hw/ppc/pnv.c   | 31 +--
  2 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
index dfed2af0f7df..9b081d543057 100644
--- a/hw/pci-host/pnv_phb4_pec.c
+++ b/hw/pci-host/pnv_phb4_pec.c
@@ -556,6 +556,10 @@ static void pnv_pec_stk_realize(DeviceState *dev, Error 
**errp)
  {
  PnvPhb4PecStack *stack = PNV_PHB4_PEC_STACK(dev);
  PnvPhb4PecState *pec = stack->pec;
+PnvPhb4PecClass *pecc = PNV_PHB4_PEC_GET_CLASS(pec);
+PnvChip *chip = pec->chip;
+uint32_t pec_nest_base;
+uint32_t pec_pci_base;
  char name[64];
  
  assert(pec);

@@ -579,10 +583,34 @@ static void pnv_pec_stk_realize(DeviceState *dev, Error 
**errp)
  pnv_xscom_region_init(>phb_regs_mr, OBJECT(>phb),
_phb4_xscom_ops, >phb, name, 0x40);
  
-/*

- * Let the machine/chip realize the PHB object to customize more
- * easily some fields
- */
+{
+Object *obj = OBJECT(>phb);
+
+object_property_set_int(obj, "chip-id", pec->chip_id, _fatal);
+object_property_set_int(obj, "version", pecc->version, _fatal);
+object_property_set_int(obj, "device-id", pecc->device_id,
+_fatal);
+object_property_set_link(obj, "stack", OBJECT(stack),
+ _abort);
+if (!sysbus_realize(SYS_BUS_DEVICE(obj), errp)) {
+return;
+}
+}



Do we really need the extra sub-scope here? It looks off.



+
+pec_nest_base = pecc->xscom_nest_base(pec);
+pec_pci_base = pecc->xscom_pci_base(pec);
+
+/* Populate the XSCOM address space. */
+pnv_xscom_add_subregion(chip,
+pec_nest_base + 0x40 * (stack->stack_no + 1),
+>nest_regs_mr);
+pnv_xscom_add_subregion(chip,
+pec_pci_base + 0x40 * (stack->stack_no + 1),
+>pci_regs_mr);
+pnv_xscom_add_subregion(chip,
+pec_pci_base + PNV9_XSCOM_PEC_PCI_STK0 +
+0x40 * stack->stack_no,
+>phb_regs_mr);
  }
  
  static Property pnv_pec_stk_properties[] = {

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 3a550eed0f36..7e13b15241fd 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1370,7 +1370,7 @@ static void pnv_chip_quad_realize(Pnv9Chip *chip9, Error 
**errp)
  static void pnv_chip_power9_phb_realize(PnvChip *chip, Error **errp)
  {



With that change, we should really rename pnv_chip_power9_phb_realize() 
to pnv_chip_power9_pec_realize().


  Fred



  Pnv9Chip *chip9 = PNV9_CHIP(chip);
-int i, j;
+int i;
  
  for (i = 0; i < chip->num_pecs; i++) {

  PnvPhb4PecState *pec = >pecs[i];
@@ -1392,35 +1392,6 @@ static void pnv_chip_power9_phb_realize(PnvChip *chip, 
Error **errp)
  
  pnv_xscom_add_subregion(chip, pec_nest_base, >nest_regs_mr);

  pnv_xscom_add_subregion(chip, pec_pci_base, >pci_regs_mr);
-
-for (j = 0; j < pec->num_stacks; j++) {
-PnvPhb4PecStack *stack = >stacks[j];
-Object *obj = OBJECT(>phb);
-
-object_property_set_int(obj, "chip-id", chip->chip_id,
-_fatal);
-object_property_set_int(obj, "version", pecc->version,
-_fatal);
-object_property_set_int(obj, "device-id", pecc->device_id,
-_fatal);
-object_property_set_link(obj, "stack", OBJECT(stack),
- _abort);
-if (!sysbus_realize(SYS_BUS_DEVICE(obj), errp)) {
-return;
-}
-
-/* Populate the XSCOM address space. */
-pnv_xscom_add_subregion(chip,
-   pec_nest_base + 0x40 * (stack->stack_no + 
1),
-   >nest_regs_mr);
-pnv_xscom_add_subregion(chip,
-pec_pci_base + 0x40 * (stack->stack_no + 
1),
->pci_regs_mr);
-pnv_xscom_add_subregion(chip,
-pec_pci_base + PNV9_XSCOM_PEC_PCI_STK0 +
-0x40 * stack->stack_no,
->phb_regs_mr);
-}
  }
  }

Re: [PATCH 13/14] ppc/pnv: Move realize of PEC stacks under the PEC model

2021-12-02 Thread Daniel Henrique Barboza 




On 12/2/21 11:42, Cédric Le Goater wrote:

This change will help us providing support for user created PHB4
devices.

Signed-off-by: Cédric Le Goater 
---


Reviewed-by: Daniel Henrique Barboza 


  hw/pci-host/pnv_phb4_pec.c | 36 
  hw/ppc/pnv.c   | 31 +--
  2 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
index dfed2af0f7df..9b081d543057 100644
--- a/hw/pci-host/pnv_phb4_pec.c
+++ b/hw/pci-host/pnv_phb4_pec.c
@@ -556,6 +556,10 @@ static void pnv_pec_stk_realize(DeviceState *dev, Error 
**errp)
  {
  PnvPhb4PecStack *stack = PNV_PHB4_PEC_STACK(dev);
  PnvPhb4PecState *pec = stack->pec;
+PnvPhb4PecClass *pecc = PNV_PHB4_PEC_GET_CLASS(pec);
+PnvChip *chip = pec->chip;
+uint32_t pec_nest_base;
+uint32_t pec_pci_base;
  char name[64];
  
  assert(pec);

@@ -579,10 +583,34 @@ static void pnv_pec_stk_realize(DeviceState *dev, Error 
**errp)
  pnv_xscom_region_init(>phb_regs_mr, OBJECT(>phb),
_phb4_xscom_ops, >phb, name, 0x40);
  
-/*

- * Let the machine/chip realize the PHB object to customize more
- * easily some fields
- */
+{
+Object *obj = OBJECT(>phb);
+
+object_property_set_int(obj, "chip-id", pec->chip_id, _fatal);
+object_property_set_int(obj, "version", pecc->version, _fatal);
+object_property_set_int(obj, "device-id", pecc->device_id,
+_fatal);
+object_property_set_link(obj, "stack", OBJECT(stack),
+ _abort);
+if (!sysbus_realize(SYS_BUS_DEVICE(obj), errp)) {
+return;
+}
+}
+
+pec_nest_base = pecc->xscom_nest_base(pec);
+pec_pci_base = pecc->xscom_pci_base(pec);
+
+/* Populate the XSCOM address space. */
+pnv_xscom_add_subregion(chip,
+pec_nest_base + 0x40 * (stack->stack_no + 1),
+>nest_regs_mr);
+pnv_xscom_add_subregion(chip,
+pec_pci_base + 0x40 * (stack->stack_no + 1),
+>pci_regs_mr);
+pnv_xscom_add_subregion(chip,
+pec_pci_base + PNV9_XSCOM_PEC_PCI_STK0 +
+0x40 * stack->stack_no,
+>phb_regs_mr);
  }
  
  static Property pnv_pec_stk_properties[] = {

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 3a550eed0f36..7e13b15241fd 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1370,7 +1370,7 @@ static void pnv_chip_quad_realize(Pnv9Chip *chip9, Error 
**errp)
  static void pnv_chip_power9_phb_realize(PnvChip *chip, Error **errp)
  {
  Pnv9Chip *chip9 = PNV9_CHIP(chip);
-int i, j;
+int i;
  
  for (i = 0; i < chip->num_pecs; i++) {

  PnvPhb4PecState *pec = >pecs[i];
@@ -1392,35 +1392,6 @@ static void pnv_chip_power9_phb_realize(PnvChip *chip, 
Error **errp)
  
  pnv_xscom_add_subregion(chip, pec_nest_base, >nest_regs_mr);

  pnv_xscom_add_subregion(chip, pec_pci_base, >pci_regs_mr);
-
-for (j = 0; j < pec->num_stacks; j++) {
-PnvPhb4PecStack *stack = >stacks[j];
-Object *obj = OBJECT(>phb);
-
-object_property_set_int(obj, "chip-id", chip->chip_id,
-_fatal);
-object_property_set_int(obj, "version", pecc->version,
-_fatal);
-object_property_set_int(obj, "device-id", pecc->device_id,
-_fatal);
-object_property_set_link(obj, "stack", OBJECT(stack),
- _abort);
-if (!sysbus_realize(SYS_BUS_DEVICE(obj), errp)) {
-return;
-}
-
-/* Populate the XSCOM address space. */
-pnv_xscom_add_subregion(chip,
-   pec_nest_base + 0x40 * (stack->stack_no + 
1),
-   >nest_regs_mr);
-pnv_xscom_add_subregion(chip,
-pec_pci_base + 0x40 * (stack->stack_no + 
1),
->pci_regs_mr);
-pnv_xscom_add_subregion(chip,
-pec_pci_base + PNV9_XSCOM_PEC_PCI_STK0 +
-0x40 * stack->stack_no,
->phb_regs_mr);
-}
  }
  }

[PATCH 13/14] ppc/pnv: Move realize of PEC stacks under the PEC model

2021-12-02 Thread Cédric Le Goater 
This change will help us providing support for user created PHB4
devices.

Signed-off-by: Cédric Le Goater 
---
 hw/pci-host/pnv_phb4_pec.c | 36 
 hw/ppc/pnv.c   | 31 +--
 2 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
index dfed2af0f7df..9b081d543057 100644
--- a/hw/pci-host/pnv_phb4_pec.c
+++ b/hw/pci-host/pnv_phb4_pec.c
@@ -556,6 +556,10 @@ static void pnv_pec_stk_realize(DeviceState *dev, Error 
**errp)
 {
 PnvPhb4PecStack *stack = PNV_PHB4_PEC_STACK(dev);
 PnvPhb4PecState *pec = stack->pec;
+PnvPhb4PecClass *pecc = PNV_PHB4_PEC_GET_CLASS(pec);
+PnvChip *chip = pec->chip;
+uint32_t pec_nest_base;
+uint32_t pec_pci_base;
 char name[64];
 
 assert(pec);
@@ -579,10 +583,34 @@ static void pnv_pec_stk_realize(DeviceState *dev, Error 
**errp)
 pnv_xscom_region_init(>phb_regs_mr, OBJECT(>phb),
   _phb4_xscom_ops, >phb, name, 0x40);
 
-/*
- * Let the machine/chip realize the PHB object to customize more
- * easily some fields
- */
+{
+Object *obj = OBJECT(>phb);
+
+object_property_set_int(obj, "chip-id", pec->chip_id, _fatal);
+object_property_set_int(obj, "version", pecc->version, _fatal);
+object_property_set_int(obj, "device-id", pecc->device_id,
+_fatal);
+object_property_set_link(obj, "stack", OBJECT(stack),
+ _abort);
+if (!sysbus_realize(SYS_BUS_DEVICE(obj), errp)) {
+return;
+}
+}
+
+pec_nest_base = pecc->xscom_nest_base(pec);
+pec_pci_base = pecc->xscom_pci_base(pec);
+
+/* Populate the XSCOM address space. */
+pnv_xscom_add_subregion(chip,
+pec_nest_base + 0x40 * (stack->stack_no + 1),
+>nest_regs_mr);
+pnv_xscom_add_subregion(chip,
+pec_pci_base + 0x40 * (stack->stack_no + 1),
+>pci_regs_mr);
+pnv_xscom_add_subregion(chip,
+pec_pci_base + PNV9_XSCOM_PEC_PCI_STK0 +
+0x40 * stack->stack_no,
+>phb_regs_mr);
 }
 
 static Property pnv_pec_stk_properties[] = {
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 3a550eed0f36..7e13b15241fd 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1370,7 +1370,7 @@ static void pnv_chip_quad_realize(Pnv9Chip *chip9, Error 
**errp)
 static void pnv_chip_power9_phb_realize(PnvChip *chip, Error **errp)
 {
 Pnv9Chip *chip9 = PNV9_CHIP(chip);
-int i, j;
+int i;
 
 for (i = 0; i < chip->num_pecs; i++) {
 PnvPhb4PecState *pec = >pecs[i];
@@ -1392,35 +1392,6 @@ static void pnv_chip_power9_phb_realize(PnvChip *chip, 
Error **errp)
 
 pnv_xscom_add_subregion(chip, pec_nest_base, >nest_regs_mr);
 pnv_xscom_add_subregion(chip, pec_pci_base, >pci_regs_mr);
-
-for (j = 0; j < pec->num_stacks; j++) {
-PnvPhb4PecStack *stack = >stacks[j];
-Object *obj = OBJECT(>phb);
-
-object_property_set_int(obj, "chip-id", chip->chip_id,
-_fatal);
-object_property_set_int(obj, "version", pecc->version,
-_fatal);
-object_property_set_int(obj, "device-id", pecc->device_id,
-_fatal);
-object_property_set_link(obj, "stack", OBJECT(stack),
- _abort);
-if (!sysbus_realize(SYS_BUS_DEVICE(obj), errp)) {
-return;
-}
-
-/* Populate the XSCOM address space. */
-pnv_xscom_add_subregion(chip,
-   pec_nest_base + 0x40 * (stack->stack_no + 
1),
-   >nest_regs_mr);
-pnv_xscom_add_subregion(chip,
-pec_pci_base + 0x40 * (stack->stack_no + 
1),
->pci_regs_mr);
-pnv_xscom_add_subregion(chip,
-pec_pci_base + PNV9_XSCOM_PEC_PCI_STK0 +
-0x40 * stack->stack_no,
->phb_regs_mr);
-}
 }
 }
 
-- 
2.31.1