[PATCH 01/11] ppc/pnv: move root port attach to pnv_phb4_realize()

Daniel Henrique Barboza posted 11 patches 3 years, 7 months ago
Maintainers: "Cédric Le Goater" <clg@kaod.org>
There is a newer version of this series
[PATCH 01/11] ppc/pnv: move root port attach to pnv_phb4_realize()
Posted by Daniel Henrique Barboza 3 years, 7 months ago
Creating a root port is something related to the PHB, not the PEC. It
also makes the logic more in line with what pnv-phb3 does.

Signed-off-by: Daniel Henrique Barboza <danielhb@linux.ibm.com>
---
 hw/pci-host/pnv_phb4.c     | 4 ++++
 hw/pci-host/pnv_phb4_pec.c | 3 ---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index 6594016121..23ad8de7ee 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -1547,6 +1547,7 @@ static void pnv_phb4_instance_init(Object *obj)
 static void pnv_phb4_realize(DeviceState *dev, Error **errp)
 {
     PnvPHB4 *phb = PNV_PHB4(dev);
+    PnvPhb4PecClass *pecc = PNV_PHB4_PEC_GET_CLASS(phb->pec);
     PCIHostState *pci = PCI_HOST_BRIDGE(dev);
     XiveSource *xsrc = &phb->xsrc;
     int nr_irqs;
@@ -1583,6 +1584,9 @@ static void pnv_phb4_realize(DeviceState *dev, Error **errp)
     pci_setup_iommu(pci->bus, pnv_phb4_dma_iommu, phb);
     pci->bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE;
 
+    /* Add a single Root port if running with defaults */
+    pnv_phb_attach_root_port(pci, pecc->rp_model);
+
     /* Setup XIVE Source */
     if (phb->big_phb) {
         nr_irqs = PNV_PHB4_MAX_INTs;
diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
index 8b7e823fa5..c9aaf1c28e 100644
--- a/hw/pci-host/pnv_phb4_pec.c
+++ b/hw/pci-host/pnv_phb4_pec.c
@@ -130,9 +130,6 @@ static void pnv_pec_default_phb_realize(PnvPhb4PecState *pec,
     if (!sysbus_realize(SYS_BUS_DEVICE(phb), errp)) {
         return;
     }
-
-    /* Add a single Root port if running with defaults */
-    pnv_phb_attach_root_port(PCI_HOST_BRIDGE(phb), pecc->rp_model);
 }
 
 static void pnv_pec_realize(DeviceState *dev, Error **errp)
-- 
2.36.1
Re: [PATCH 01/11] ppc/pnv: move root port attach to pnv_phb4_realize()
Posted by Cédric Le Goater 3 years, 7 months ago
On 6/13/22 17:44, Daniel Henrique Barboza wrote:
> Creating a root port is something related to the PHB, not the PEC. It
> also makes the logic more in line with what pnv-phb3 does.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.ibm.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

So the root port is back where it was.

Could we avoid the pci_new() and use object_initialize_child() instead ?

Thanks,

C.


> ---
>   hw/pci-host/pnv_phb4.c     | 4 ++++
>   hw/pci-host/pnv_phb4_pec.c | 3 ---
>   2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index 6594016121..23ad8de7ee 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -1547,6 +1547,7 @@ static void pnv_phb4_instance_init(Object *obj)
>   static void pnv_phb4_realize(DeviceState *dev, Error **errp)
>   {
>       PnvPHB4 *phb = PNV_PHB4(dev);
> +    PnvPhb4PecClass *pecc = PNV_PHB4_PEC_GET_CLASS(phb->pec);
>       PCIHostState *pci = PCI_HOST_BRIDGE(dev);
>       XiveSource *xsrc = &phb->xsrc;
>       int nr_irqs;
> @@ -1583,6 +1584,9 @@ static void pnv_phb4_realize(DeviceState *dev, Error **errp)
>       pci_setup_iommu(pci->bus, pnv_phb4_dma_iommu, phb);
>       pci->bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE;
>   
> +    /* Add a single Root port if running with defaults */
> +    pnv_phb_attach_root_port(pci, pecc->rp_model);
> +
>       /* Setup XIVE Source */
>       if (phb->big_phb) {
>           nr_irqs = PNV_PHB4_MAX_INTs;
> diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
> index 8b7e823fa5..c9aaf1c28e 100644
> --- a/hw/pci-host/pnv_phb4_pec.c
> +++ b/hw/pci-host/pnv_phb4_pec.c
> @@ -130,9 +130,6 @@ static void pnv_pec_default_phb_realize(PnvPhb4PecState *pec,
>       if (!sysbus_realize(SYS_BUS_DEVICE(phb), errp)) {
>           return;
>       }
> -
> -    /* Add a single Root port if running with defaults */
> -    pnv_phb_attach_root_port(PCI_HOST_BRIDGE(phb), pecc->rp_model);
>   }
>   
>   static void pnv_pec_realize(DeviceState *dev, Error **errp)


Re: [PATCH 01/11] ppc/pnv: move root port attach to pnv_phb4_realize()
Posted by Daniel Henrique Barboza 3 years, 7 months ago

On 6/14/22 09:02, Cédric Le Goater wrote:
> On 6/13/22 17:44, Daniel Henrique Barboza wrote:
>> Creating a root port is something related to the PHB, not the PEC. It
>> also makes the logic more in line with what pnv-phb3 does.
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.ibm.com>
> 
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> 
> So the root port is back where it was.
> 
> Could we avoid the pci_new() and use object_initialize_child() instead ?


We could but then we would need to deal with yet another difference with
default versus user created devices, given that for user devices we can't
initialize_child(). And since we're also unifying the root ports later on
I'd rather wait to see how it turns out when everything is finished.


Tanks,

Daniel

> 
> Thanks,
> 
> C.
> 
> 
>> ---
>>   hw/pci-host/pnv_phb4.c     | 4 ++++
>>   hw/pci-host/pnv_phb4_pec.c | 3 ---
>>   2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
>> index 6594016121..23ad8de7ee 100644
>> --- a/hw/pci-host/pnv_phb4.c
>> +++ b/hw/pci-host/pnv_phb4.c
>> @@ -1547,6 +1547,7 @@ static void pnv_phb4_instance_init(Object *obj)
>>   static void pnv_phb4_realize(DeviceState *dev, Error **errp)
>>   {
>>       PnvPHB4 *phb = PNV_PHB4(dev);
>> +    PnvPhb4PecClass *pecc = PNV_PHB4_PEC_GET_CLASS(phb->pec);
>>       PCIHostState *pci = PCI_HOST_BRIDGE(dev);
>>       XiveSource *xsrc = &phb->xsrc;
>>       int nr_irqs;
>> @@ -1583,6 +1584,9 @@ static void pnv_phb4_realize(DeviceState *dev, Error **errp)
>>       pci_setup_iommu(pci->bus, pnv_phb4_dma_iommu, phb);
>>       pci->bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE;
>> +    /* Add a single Root port if running with defaults */
>> +    pnv_phb_attach_root_port(pci, pecc->rp_model);
>> +
>>       /* Setup XIVE Source */
>>       if (phb->big_phb) {
>>           nr_irqs = PNV_PHB4_MAX_INTs;
>> diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
>> index 8b7e823fa5..c9aaf1c28e 100644
>> --- a/hw/pci-host/pnv_phb4_pec.c
>> +++ b/hw/pci-host/pnv_phb4_pec.c
>> @@ -130,9 +130,6 @@ static void pnv_pec_default_phb_realize(PnvPhb4PecState *pec,
>>       if (!sysbus_realize(SYS_BUS_DEVICE(phb), errp)) {
>>           return;
>>       }
>> -
>> -    /* Add a single Root port if running with defaults */
>> -    pnv_phb_attach_root_port(PCI_HOST_BRIDGE(phb), pecc->rp_model);
>>   }
>>   static void pnv_pec_realize(DeviceState *dev, Error **errp)
> 

Re: [PATCH 01/11] ppc/pnv: move root port attach to pnv_phb4_realize()
Posted by Frederic Barrat 3 years, 7 months ago

On 13/06/2022 17:44, Daniel Henrique Barboza wrote:
> Creating a root port is something related to the PHB, not the PEC. It
> also makes the logic more in line with what pnv-phb3 does.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.ibm.com>
> ---

LGTM,
Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>

   Fred


>   hw/pci-host/pnv_phb4.c     | 4 ++++
>   hw/pci-host/pnv_phb4_pec.c | 3 ---
>   2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index 6594016121..23ad8de7ee 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -1547,6 +1547,7 @@ static void pnv_phb4_instance_init(Object *obj)
>   static void pnv_phb4_realize(DeviceState *dev, Error **errp)
>   {
>       PnvPHB4 *phb = PNV_PHB4(dev);
> +    PnvPhb4PecClass *pecc = PNV_PHB4_PEC_GET_CLASS(phb->pec);
>       PCIHostState *pci = PCI_HOST_BRIDGE(dev);
>       XiveSource *xsrc = &phb->xsrc;
>       int nr_irqs;
> @@ -1583,6 +1584,9 @@ static void pnv_phb4_realize(DeviceState *dev, Error **errp)
>       pci_setup_iommu(pci->bus, pnv_phb4_dma_iommu, phb);
>       pci->bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE;
>   
> +    /* Add a single Root port if running with defaults */
> +    pnv_phb_attach_root_port(pci, pecc->rp_model);
> +
>       /* Setup XIVE Source */
>       if (phb->big_phb) {
>           nr_irqs = PNV_PHB4_MAX_INTs;
> diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
> index 8b7e823fa5..c9aaf1c28e 100644
> --- a/hw/pci-host/pnv_phb4_pec.c
> +++ b/hw/pci-host/pnv_phb4_pec.c
> @@ -130,9 +130,6 @@ static void pnv_pec_default_phb_realize(PnvPhb4PecState *pec,
>       if (!sysbus_realize(SYS_BUS_DEVICE(phb), errp)) {
>           return;
>       }
> -
> -    /* Add a single Root port if running with defaults */
> -    pnv_phb_attach_root_port(PCI_HOST_BRIDGE(phb), pecc->rp_model);
>   }
>   
>   static void pnv_pec_realize(DeviceState *dev, Error **errp)