diff mbox series

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

Message ID 20211202144235.1276352-14-clg@kaod.org (mailing list archive)
State New, archived
Headers show
Series ppc/pnv: Add support for user created PHB3/PHB4 devices | expand

Commit Message

Cédric Le Goater Dec. 2, 2021, 2:42 p.m. UTC 
This change will help us providing support for user created PHB4
devices.

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

Comments

Daniel Henrique Barboza Dec. 2, 2021, 5:49 p.m. UTC | #1 
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 <clg@kaod.org>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   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(&stack->phb_regs_mr, OBJECT(&stack->phb),
>                             &pnv_phb4_xscom_ops, &stack->phb, name, 0x40);
>   
> -    /*
> -     * Let the machine/chip realize the PHB object to customize more
> -     * easily some fields
> -     */
> +    {
> +        Object *obj = OBJECT(&stack->phb);
> +
> +        object_property_set_int(obj, "chip-id", pec->chip_id, &error_fatal);
> +        object_property_set_int(obj, "version", pecc->version, &error_fatal);
> +        object_property_set_int(obj, "device-id", pecc->device_id,
> +                                &error_fatal);
> +        object_property_set_link(obj, "stack", OBJECT(stack),
> +                                 &error_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),
> +                            &stack->nest_regs_mr);
> +    pnv_xscom_add_subregion(chip,
> +                            pec_pci_base + 0x40 * (stack->stack_no + 1),
> +                            &stack->pci_regs_mr);
> +    pnv_xscom_add_subregion(chip,
> +                            pec_pci_base + PNV9_XSCOM_PEC_PCI_STK0 +
> +                            0x40 * stack->stack_no,
> +                            &stack->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 = &chip9->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, &pec->nest_regs_mr);
>           pnv_xscom_add_subregion(chip, pec_pci_base, &pec->pci_regs_mr);
> -
> -        for (j = 0; j < pec->num_stacks; j++) {
> -            PnvPhb4PecStack *stack = &pec->stacks[j];
> -            Object *obj = OBJECT(&stack->phb);
> -
> -            object_property_set_int(obj, "chip-id", chip->chip_id,
> -                                    &error_fatal);
> -            object_property_set_int(obj, "version", pecc->version,
> -                                    &error_fatal);
> -            object_property_set_int(obj, "device-id", pecc->device_id,
> -                                    &error_fatal);
> -            object_property_set_link(obj, "stack", OBJECT(stack),
> -                                     &error_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),
> -                                   &stack->nest_regs_mr);
> -            pnv_xscom_add_subregion(chip,
> -                                    pec_pci_base + 0x40 * (stack->stack_no + 1),
> -                                    &stack->pci_regs_mr);
> -            pnv_xscom_add_subregion(chip,
> -                                    pec_pci_base + PNV9_XSCOM_PEC_PCI_STK0 +
> -                                    0x40 * stack->stack_no,
> -                                    &stack->phb_regs_mr);
> -        }
>       }
>   }
>   
>
Frederic Barrat Dec. 7, 2021, 10:10 a.m. UTC | #2 
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 <clg@kaod.org>
> ---
>   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(&stack->phb_regs_mr, OBJECT(&stack->phb),
>                             &pnv_phb4_xscom_ops, &stack->phb, name, 0x40);
>   
> -    /*
> -     * Let the machine/chip realize the PHB object to customize more
> -     * easily some fields
> -     */
> +    {
> +        Object *obj = OBJECT(&stack->phb);
> +
> +        object_property_set_int(obj, "chip-id", pec->chip_id, &error_fatal);
> +        object_property_set_int(obj, "version", pecc->version, &error_fatal);
> +        object_property_set_int(obj, "device-id", pecc->device_id,
> +                                &error_fatal);
> +        object_property_set_link(obj, "stack", OBJECT(stack),
> +                                 &error_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),
> +                            &stack->nest_regs_mr);
> +    pnv_xscom_add_subregion(chip,
> +                            pec_pci_base + 0x40 * (stack->stack_no + 1),
> +                            &stack->pci_regs_mr);
> +    pnv_xscom_add_subregion(chip,
> +                            pec_pci_base + PNV9_XSCOM_PEC_PCI_STK0 +
> +                            0x40 * stack->stack_no,
> +                            &stack->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 = &chip9->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, &pec->nest_regs_mr);
>           pnv_xscom_add_subregion(chip, pec_pci_base, &pec->pci_regs_mr);
> -
> -        for (j = 0; j < pec->num_stacks; j++) {
> -            PnvPhb4PecStack *stack = &pec->stacks[j];
> -            Object *obj = OBJECT(&stack->phb);
> -
> -            object_property_set_int(obj, "chip-id", chip->chip_id,
> -                                    &error_fatal);
> -            object_property_set_int(obj, "version", pecc->version,
> -                                    &error_fatal);
> -            object_property_set_int(obj, "device-id", pecc->device_id,
> -                                    &error_fatal);
> -            object_property_set_link(obj, "stack", OBJECT(stack),
> -                                     &error_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),
> -                                   &stack->nest_regs_mr);
> -            pnv_xscom_add_subregion(chip,
> -                                    pec_pci_base + 0x40 * (stack->stack_no + 1),
> -                                    &stack->pci_regs_mr);
> -            pnv_xscom_add_subregion(chip,
> -                                    pec_pci_base + PNV9_XSCOM_PEC_PCI_STK0 +
> -                                    0x40 * stack->stack_no,
> -                                    &stack->phb_regs_mr);
> -        }
>       }
>   }
>   
>
Cédric Le Goater Dec. 7, 2021, 10:30 a.m. UTC | #3 
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 <clg@kaod.org>
>> ---
>>   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(&stack->phb_regs_mr, OBJECT(&stack->phb),
>>                             &pnv_phb4_xscom_ops, &stack->phb, name, 0x40);
>> -    /*
>> -     * Let the machine/chip realize the PHB object to customize more
>> -     * easily some fields
>> -     */
>> +    {
>> +        Object *obj = OBJECT(&stack->phb);
>> +
>> +        object_property_set_int(obj, "chip-id", pec->chip_id, &error_fatal);
>> +        object_property_set_int(obj, "version", pecc->version, &error_fatal);
>> +        object_property_set_int(obj, "device-id", pecc->device_id,
>> +                                &error_fatal);
>> +        object_property_set_link(obj, "stack", OBJECT(stack),
>> +                                 &error_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),
>> +                            &stack->nest_regs_mr);
>> +    pnv_xscom_add_subregion(chip,
>> +                            pec_pci_base + 0x40 * (stack->stack_no + 1),
>> +                            &stack->pci_regs_mr);
>> +    pnv_xscom_add_subregion(chip,
>> +                            pec_pci_base + PNV9_XSCOM_PEC_PCI_STK0 +
>> +                            0x40 * stack->stack_no,
>> +                            &stack->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 = &chip9->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, &pec->nest_regs_mr);
>>           pnv_xscom_add_subregion(chip, pec_pci_base, &pec->pci_regs_mr);
>> -
>> -        for (j = 0; j < pec->num_stacks; j++) {
>> -            PnvPhb4PecStack *stack = &pec->stacks[j];
>> -            Object *obj = OBJECT(&stack->phb);
>> -
>> -            object_property_set_int(obj, "chip-id", chip->chip_id,
>> -                                    &error_fatal);
>> -            object_property_set_int(obj, "version", pecc->version,
>> -                                    &error_fatal);
>> -            object_property_set_int(obj, "device-id", pecc->device_id,
>> -                                    &error_fatal);
>> -            object_property_set_link(obj, "stack", OBJECT(stack),
>> -                                     &error_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),
>> -                                   &stack->nest_regs_mr);
>> -            pnv_xscom_add_subregion(chip,
>> -                                    pec_pci_base + 0x40 * (stack->stack_no + 1),
>> -                                    &stack->pci_regs_mr);
>> -            pnv_xscom_add_subregion(chip,
>> -                                    pec_pci_base + PNV9_XSCOM_PEC_PCI_STK0 +
>> -                                    0x40 * stack->stack_no,
>> -                                    &stack->phb_regs_mr);
>> -        }
>>       }
>>   }
>>
diff mbox series

Patch

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(&stack->phb_regs_mr, OBJECT(&stack->phb),
                           &pnv_phb4_xscom_ops, &stack->phb, name, 0x40);
 
-    /*
-     * Let the machine/chip realize the PHB object to customize more
-     * easily some fields
-     */
+    {
+        Object *obj = OBJECT(&stack->phb);
+
+        object_property_set_int(obj, "chip-id", pec->chip_id, &error_fatal);
+        object_property_set_int(obj, "version", pecc->version, &error_fatal);
+        object_property_set_int(obj, "device-id", pecc->device_id,
+                                &error_fatal);
+        object_property_set_link(obj, "stack", OBJECT(stack),
+                                 &error_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),
+                            &stack->nest_regs_mr);
+    pnv_xscom_add_subregion(chip,
+                            pec_pci_base + 0x40 * (stack->stack_no + 1),
+                            &stack->pci_regs_mr);
+    pnv_xscom_add_subregion(chip,
+                            pec_pci_base + PNV9_XSCOM_PEC_PCI_STK0 +
+                            0x40 * stack->stack_no,
+                            &stack->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 = &chip9->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, &pec->nest_regs_mr);
         pnv_xscom_add_subregion(chip, pec_pci_base, &pec->pci_regs_mr);
-
-        for (j = 0; j < pec->num_stacks; j++) {
-            PnvPhb4PecStack *stack = &pec->stacks[j];
-            Object *obj = OBJECT(&stack->phb);
-
-            object_property_set_int(obj, "chip-id", chip->chip_id,
-                                    &error_fatal);
-            object_property_set_int(obj, "version", pecc->version,
-                                    &error_fatal);
-            object_property_set_int(obj, "device-id", pecc->device_id,
-                                    &error_fatal);
-            object_property_set_link(obj, "stack", OBJECT(stack),
-                                     &error_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),
-                                   &stack->nest_regs_mr);
-            pnv_xscom_add_subregion(chip,
-                                    pec_pci_base + 0x40 * (stack->stack_no + 1),
-                                    &stack->pci_regs_mr);
-            pnv_xscom_add_subregion(chip,
-                                    pec_pci_base + PNV9_XSCOM_PEC_PCI_STK0 +
-                                    0x40 * stack->stack_no,
-                                    &stack->phb_regs_mr);
-        }
     }
 }