1. Patchew
  2. QEMU
  3. [PATCH v5 0/3] pnv nest1 chiplet model
  4. View patch

[PATCH v5 3/3] hw/ppc: Nest1 chiplet wiring

Chalapathi V posted 3 patches 1 year ago
Maintainers: Nicholas Piggin <npiggin@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, "Cédric Le Goater" <clg@kaod.org>, "Frédéric Barrat" <fbarrat@linux.ibm.com>
There is a newer version of this series
[PATCH v5 3/3] hw/ppc: Nest1 chiplet wiring
Posted by Chalapathi V 1 year ago 
This part of the patchset connects the nest1 chiplet model to p10 chip.

Signed-off-by: Chalapathi V <chalapathi.v@linux.ibm.com>
---
 include/hw/ppc/pnv_chip.h |  2 ++
 hw/ppc/pnv.c              | 14 ++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
index 0ab5c42308..59a3158a6b 100644
--- a/include/hw/ppc/pnv_chip.h
+++ b/include/hw/ppc/pnv_chip.h
@@ -4,6 +4,7 @@
 #include "hw/pci-host/pnv_phb4.h"
 #include "hw/ppc/pnv_core.h"
 #include "hw/ppc/pnv_homer.h"
+#include "hw/ppc/pnv_nest_chiplet.h"
 #include "hw/ppc/pnv_lpc.h"
 #include "hw/ppc/pnv_occ.h"
 #include "hw/ppc/pnv_psi.h"
@@ -113,6 +114,7 @@ struct Pnv10Chip {
     PnvOCC       occ;
     PnvSBE       sbe;
     PnvHomer     homer;
+    PnvNest1     nest1;
 
     uint32_t     nr_quads;
     PnvQuad      *quads;
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 0297871bdd..ba3dfab557 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1680,6 +1680,7 @@ static void pnv_chip_power10_instance_init(Object *obj)
     object_initialize_child(obj, "occ",  &chip10->occ, TYPE_PNV10_OCC);
     object_initialize_child(obj, "sbe",  &chip10->sbe, TYPE_PNV10_SBE);
     object_initialize_child(obj, "homer", &chip10->homer, TYPE_PNV10_HOMER);
+    object_initialize_child(obj, "nest1", &chip10->nest1, TYPE_PNV_NEST1);
 
     chip->num_pecs = pcc->num_pecs;
 
@@ -1849,6 +1850,19 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(get_system_memory(), PNV10_HOMER_BASE(chip),
                                 &chip10->homer.regs);
 
+    /* nest1 chiplet */
+    if (!qdev_realize(DEVICE(&chip10->nest1), NULL, errp)) {
+        return;
+    }
+    pnv_xscom_add_subregion(chip, PNV10_XSCOM_NEST1_CTRL_CHIPLET_BASE,
+             &chip10->nest1.perv.xscom_perv_ctrl_regs);
+
+    pnv_xscom_add_subregion(chip, PNV10_XSCOM_NEST1_PB_SCOM_EQ_BASE,
+                           &chip10->nest1.xscom_pb_eq_regs);
+
+    pnv_xscom_add_subregion(chip, PNV10_XSCOM_NEST1_PB_SCOM_ES_BASE,
+                           &chip10->nest1.xscom_pb_es_regs);
+
     /* PHBs */
     pnv_chip_power10_phb_realize(chip, &local_err);
     if (local_err) {
-- 
2.31.1
Re: [PATCH v5 3/3] hw/ppc: Nest1 chiplet wiring
Posted by Nicholas Piggin 1 year ago 
On Fri Nov 24, 2023 at 8:15 PM AEST, Chalapathi V wrote:
> This part of the patchset connects the nest1 chiplet model to p10 chip.

Seems fine to me. Should it just be squashed into patch 2?

Thanks,
Nick

>
> Signed-off-by: Chalapathi V <chalapathi.v@linux.ibm.com>
> ---
>  include/hw/ppc/pnv_chip.h |  2 ++
>  hw/ppc/pnv.c              | 14 ++++++++++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
> index 0ab5c42308..59a3158a6b 100644
> --- a/include/hw/ppc/pnv_chip.h
> +++ b/include/hw/ppc/pnv_chip.h
> @@ -4,6 +4,7 @@
>  #include "hw/pci-host/pnv_phb4.h"
>  #include "hw/ppc/pnv_core.h"
>  #include "hw/ppc/pnv_homer.h"
> +#include "hw/ppc/pnv_nest_chiplet.h"
>  #include "hw/ppc/pnv_lpc.h"
>  #include "hw/ppc/pnv_occ.h"
>  #include "hw/ppc/pnv_psi.h"
> @@ -113,6 +114,7 @@ struct Pnv10Chip {
>      PnvOCC       occ;
>      PnvSBE       sbe;
>      PnvHomer     homer;
> +    PnvNest1     nest1;
>  
>      uint32_t     nr_quads;
>      PnvQuad      *quads;
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 0297871bdd..ba3dfab557 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -1680,6 +1680,7 @@ static void pnv_chip_power10_instance_init(Object *obj)
>      object_initialize_child(obj, "occ",  &chip10->occ, TYPE_PNV10_OCC);
>      object_initialize_child(obj, "sbe",  &chip10->sbe, TYPE_PNV10_SBE);
>      object_initialize_child(obj, "homer", &chip10->homer, TYPE_PNV10_HOMER);
> +    object_initialize_child(obj, "nest1", &chip10->nest1, TYPE_PNV_NEST1);
>  
>      chip->num_pecs = pcc->num_pecs;
>  
> @@ -1849,6 +1850,19 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
>      memory_region_add_subregion(get_system_memory(), PNV10_HOMER_BASE(chip),
>                                  &chip10->homer.regs);
>  
> +    /* nest1 chiplet */
> +    if (!qdev_realize(DEVICE(&chip10->nest1), NULL, errp)) {
> +        return;
> +    }
> +    pnv_xscom_add_subregion(chip, PNV10_XSCOM_NEST1_CTRL_CHIPLET_BASE,
> +             &chip10->nest1.perv.xscom_perv_ctrl_regs);
> +
> +    pnv_xscom_add_subregion(chip, PNV10_XSCOM_NEST1_PB_SCOM_EQ_BASE,
> +                           &chip10->nest1.xscom_pb_eq_regs);
> +
> +    pnv_xscom_add_subregion(chip, PNV10_XSCOM_NEST1_PB_SCOM_ES_BASE,
> +                           &chip10->nest1.xscom_pb_es_regs);
> +
>      /* PHBs */
>      pnv_chip_power10_phb_realize(chip, &local_err);
>      if (local_err) {
Re: [PATCH v5 3/3] hw/ppc: Nest1 chiplet wiring
Posted by Cédric Le Goater 1 year ago 
On 11/24/23 12:28, Nicholas Piggin wrote:
> On Fri Nov 24, 2023 at 8:15 PM AEST, Chalapathi V wrote:
>> This part of the patchset connects the nest1 chiplet model to p10 chip.
> 
> Seems fine to me. Should it just be squashed into patch 2?

It is better to keep the model a part from the wiring because the
same model could be plugged in different board/machine. It clarifies
the interfaces, which should be limited to irq connects and memory
mappings and it makes modeling shortcuts more visible: backpointers,
looping on the machine mappings to find a core, etc.

I didn't comment on the PnvChiptod proposal but it could/should
be done the same.

This patch proposal is nice and clean.

Thanks,

C.




> 
> Thanks,
> Nick
> 
>>
>> Signed-off-by: Chalapathi V <chalapathi.v@linux.ibm.com>
>> ---
>>   include/hw/ppc/pnv_chip.h |  2 ++
>>   hw/ppc/pnv.c              | 14 ++++++++++++++
>>   2 files changed, 16 insertions(+)
>>
>> diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
>> index 0ab5c42308..59a3158a6b 100644
>> --- a/include/hw/ppc/pnv_chip.h
>> +++ b/include/hw/ppc/pnv_chip.h
>> @@ -4,6 +4,7 @@
>>   #include "hw/pci-host/pnv_phb4.h"
>>   #include "hw/ppc/pnv_core.h"
>>   #include "hw/ppc/pnv_homer.h"
>> +#include "hw/ppc/pnv_nest_chiplet.h"
>>   #include "hw/ppc/pnv_lpc.h"
>>   #include "hw/ppc/pnv_occ.h"
>>   #include "hw/ppc/pnv_psi.h"
>> @@ -113,6 +114,7 @@ struct Pnv10Chip {
>>       PnvOCC       occ;
>>       PnvSBE       sbe;
>>       PnvHomer     homer;
>> +    PnvNest1     nest1;
>>   
>>       uint32_t     nr_quads;
>>       PnvQuad      *quads;
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index 0297871bdd..ba3dfab557 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -1680,6 +1680,7 @@ static void pnv_chip_power10_instance_init(Object *obj)
>>       object_initialize_child(obj, "occ",  &chip10->occ, TYPE_PNV10_OCC);
>>       object_initialize_child(obj, "sbe",  &chip10->sbe, TYPE_PNV10_SBE);
>>       object_initialize_child(obj, "homer", &chip10->homer, TYPE_PNV10_HOMER);
>> +    object_initialize_child(obj, "nest1", &chip10->nest1, TYPE_PNV_NEST1);
>>   
>>       chip->num_pecs = pcc->num_pecs;
>>   
>> @@ -1849,6 +1850,19 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
>>       memory_region_add_subregion(get_system_memory(), PNV10_HOMER_BASE(chip),
>>                                   &chip10->homer.regs);
>>   
>> +    /* nest1 chiplet */
>> +    if (!qdev_realize(DEVICE(&chip10->nest1), NULL, errp)) {
>> +        return;
>> +    }
>> +    pnv_xscom_add_subregion(chip, PNV10_XSCOM_NEST1_CTRL_CHIPLET_BASE,
>> +             &chip10->nest1.perv.xscom_perv_ctrl_regs);
>> +
>> +    pnv_xscom_add_subregion(chip, PNV10_XSCOM_NEST1_PB_SCOM_EQ_BASE,
>> +                           &chip10->nest1.xscom_pb_eq_regs);
>> +
>> +    pnv_xscom_add_subregion(chip, PNV10_XSCOM_NEST1_PB_SCOM_ES_BASE,
>> +                           &chip10->nest1.xscom_pb_es_regs);
>> +
>>       /* PHBs */
>>       pnv_chip_power10_phb_realize(chip, &local_err);
>>       if (local_err) {
>
Re: [PATCH v5 3/3] hw/ppc: Nest1 chiplet wiring
Posted by Nicholas Piggin 1 year ago 
On Fri Nov 24, 2023 at 10:26 PM AEST, Cédric Le Goater wrote:
> On 11/24/23 12:28, Nicholas Piggin wrote:
> > On Fri Nov 24, 2023 at 8:15 PM AEST, Chalapathi V wrote:
> >> This part of the patchset connects the nest1 chiplet model to p10 chip.
> > 
> > Seems fine to me. Should it just be squashed into patch 2?
>
> It is better to keep the model a part from the wiring because the
> same model could be plugged in different board/machine. It clarifies
> the interfaces, which should be limited to irq connects and memory
> mappings and it makes modeling shortcuts more visible: backpointers,
> looping on the machine mappings to find a core, etc.

Okay that makes sense.

> I didn't comment on the PnvChiptod proposal but it could/should
> be done the same.

I'll look at splitting it too then.

Thanks,
Nick

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.