[PATCH 11/24] pnv/phb4: Bury unwanted "pnv-phb4-pec-stack" devices

Markus Armbruster posted 24 patches 5 years, 6 months ago
Maintainers: Sagar Karandikar <sagark@eecs.berkeley.edu>, Fabien Chouteau <chouteau@adacore.com>, Peter Maydell <peter.maydell@linaro.org>, Alistair Francis <Alistair.Francis@wdc.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Alistair Francis <alistair@alistair23.me>, Paolo Bonzini <pbonzini@redhat.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Andrzej Zaborowski <balrogg@gmail.com>, KONRAD Frederic <frederic.konrad@adacore.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>, Eduardo Habkost <ehabkost@redhat.com>, Laurent Vivier <laurent@vivier.eu>, David Gibson <david@gibson.dropbear.id.au>, Palmer Dabbelt <palmer@dabbelt.com>
There is a newer version of this series
[PATCH 11/24] pnv/phb4: Bury unwanted "pnv-phb4-pec-stack" devices
Posted by Markus Armbruster 5 years, 6 months ago
The number of stacks is controlled by property "num-stacks".
pnv_pec_instance_init() creates the maximum supported number, because
the property has not been set then.  pnv_pec_realize() realizes only
the wanted number.  Works, although it can leave unrealized devices
hanging around in the QOM composition tree.  Affects machine powernv9.

Bury the unwanted devices by making pnv_pec_realize() unparent them.
Visible in "info qom-tree":

     /machine (powernv9-machine)
       /chip[0] (power9_v2.0-pnv-chip)
         [...]
         /pec[0] (pnv-phb4-pec)
           /stack[0] (pnv-phb4-pec-stack)
             [...]
    -      /stack[1] (pnv-phb4-pec-stack)
    -        /phb (pnv-phb4)
    -          /pcie-mmcfg-mmio[0] (qemu:memory-region)
    -          /root (pnv-phb4-root-port)
    -          /source (xive-source)
    -      /stack[2] (pnv-phb4-pec-stack)
    -        /phb (pnv-phb4)
    -          /pcie-mmcfg-mmio[0] (qemu:memory-region)
    -          /root (pnv-phb4-root-port)
    -          /source (xive-source)
           /xscom-pec-0.0-nest[0] (qemu:memory-region)
           /xscom-pec-0.0-pci[0] (qemu:memory-region)
         /pec[1] (pnv-phb4-pec)
           /stack[0] (pnv-phb4-pec-stack)
             [...]
           /stack[1] (pnv-phb4-pec-stack)
             [...]
    -      /stack[2] (pnv-phb4-pec-stack)
    -        /phb (pnv-phb4)
    -          /pcie-mmcfg-mmio[0] (qemu:memory-region)
    -          /root (pnv-phb4-root-port)
    -          /source (xive-source)
           /xscom-pec-0.1-nest[0] (qemu:memory-region)
           /xscom-pec-0.1-pci[0] (qemu:memory-region)
         /pec[2] (pnv-phb4-pec)
           /stack[0] (pnv-phb4-pec-stack)
             [...]
           /stack[1] (pnv-phb4-pec-stack)
             [...]
           /stack[2] (pnv-phb4-pec-stack)
             [...]

Cc: Cédric Le Goater <clg@kaod.org>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: qemu-ppc@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/pci-host/pnv_phb4_pec.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
index 911d147ffd..565345a018 100644
--- a/hw/pci-host/pnv_phb4_pec.c
+++ b/hw/pci-host/pnv_phb4_pec.c
@@ -397,6 +397,9 @@ static void pnv_pec_realize(DeviceState *dev, Error **errp)
             return;
         }
     }
+    for (; i < PHB4_PEC_MAX_STACKS; i++) {
+        object_unparent(OBJECT(&pec->stacks[i]));
+    }
 
     /* Initialize the XSCOM regions for the PEC registers */
     snprintf(name, sizeof(name), "xscom-pec-%d.%d-nest", pec->chip_id,
-- 
2.21.1


Re: [PATCH 11/24] pnv/phb4: Bury unwanted "pnv-phb4-pec-stack" devices
Posted by Cédric Le Goater 5 years, 6 months ago
On 5/18/20 7:03 AM, Markus Armbruster wrote:
> The number of stacks is controlled by property "num-stacks".
> pnv_pec_instance_init() creates the maximum supported number, because
> the property has not been set then.  pnv_pec_realize() realizes only
> the wanted number.  Works, although it can leave unrealized devices
> hanging around in the QOM composition tree.  Affects machine powernv9.



> Bury the unwanted devices by making pnv_pec_realize() unparent them.
> Visible in "info qom-tree":
> 
>      /machine (powernv9-machine)
>        /chip[0] (power9_v2.0-pnv-chip)
>          [...]
>          /pec[0] (pnv-phb4-pec)
>            /stack[0] (pnv-phb4-pec-stack)
>              [...]
>     -      /stack[1] (pnv-phb4-pec-stack)
>     -        /phb (pnv-phb4)
>     -          /pcie-mmcfg-mmio[0] (qemu:memory-region)
>     -          /root (pnv-phb4-root-port)
>     -          /source (xive-source)
>     -      /stack[2] (pnv-phb4-pec-stack)
>     -        /phb (pnv-phb4)
>     -          /pcie-mmcfg-mmio[0] (qemu:memory-region)
>     -          /root (pnv-phb4-root-port)
>     -          /source (xive-source)
>            /xscom-pec-0.0-nest[0] (qemu:memory-region)
>            /xscom-pec-0.0-pci[0] (qemu:memory-region)
>          /pec[1] (pnv-phb4-pec)
>            /stack[0] (pnv-phb4-pec-stack)
>              [...]
>            /stack[1] (pnv-phb4-pec-stack)
>              [...]
>     -      /stack[2] (pnv-phb4-pec-stack)
>     -        /phb (pnv-phb4)
>     -          /pcie-mmcfg-mmio[0] (qemu:memory-region)
>     -          /root (pnv-phb4-root-port)
>     -          /source (xive-source)
>            /xscom-pec-0.1-nest[0] (qemu:memory-region)
>            /xscom-pec-0.1-pci[0] (qemu:memory-region)
>          /pec[2] (pnv-phb4-pec)
>            /stack[0] (pnv-phb4-pec-stack)
>              [...]
>            /stack[1] (pnv-phb4-pec-stack)
>              [...]
>            /stack[2] (pnv-phb4-pec-stack)
>              [...]
> 
> Cc: Cédric Le Goater <clg@kaod.org>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

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

Thanks,

C. 

> ---
>  hw/pci-host/pnv_phb4_pec.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
> index 911d147ffd..565345a018 100644
> --- a/hw/pci-host/pnv_phb4_pec.c
> +++ b/hw/pci-host/pnv_phb4_pec.c
> @@ -397,6 +397,9 @@ static void pnv_pec_realize(DeviceState *dev, Error **errp)
>              return;
>          }
>      }
> +    for (; i < PHB4_PEC_MAX_STACKS; i++) {
> +        object_unparent(OBJECT(&pec->stacks[i]));
> +    }
>  
>      /* Initialize the XSCOM regions for the PEC registers */
>      snprintf(name, sizeof(name), "xscom-pec-%d.%d-nest", pec->chip_id,
> 


Re: [PATCH 11/24] pnv/phb4: Bury unwanted "pnv-phb4-pec-stack" devices
Posted by Cédric Le Goater 5 years, 5 months ago
On 5/18/20 7:03 AM, Markus Armbruster wrote:
> The number of stacks is controlled by property "num-stacks".
> pnv_pec_instance_init() creates the maximum supported number, because
> the property has not been set then.  pnv_pec_realize() realizes only
> the wanted number.  Works, although it can leave unrealized devices
> hanging around in the QOM composition tree.  Affects machine powernv9.

I have used this pattern in many models. Is there a better one ?

Thanks,

C.

Re: [PATCH 11/24] pnv/phb4: Bury unwanted "pnv-phb4-pec-stack" devices
Posted by Markus Armbruster 5 years, 5 months ago
Cédric Le Goater <clg@kaod.org> writes:

> On 5/18/20 7:03 AM, Markus Armbruster wrote:
>> The number of stacks is controlled by property "num-stacks".
>> pnv_pec_instance_init() creates the maximum supported number, because
>> the property has not been set then.  pnv_pec_realize() realizes only
>> the wanted number.  Works, although it can leave unrealized devices
>> hanging around in the QOM composition tree.  Affects machine powernv9.
>
> I have used this pattern in many models. Is there a better one ?

The pattern is just fine, we just need to unparent any devices that turn
out to be unwanted.

Of course, when we already know what's wanted at instance_init time,
there's no reason for creating more.


Re: [PATCH 11/24] pnv/phb4: Bury unwanted "pnv-phb4-pec-stack" devices
Posted by Greg Kurz 5 years, 6 months ago
On Mon, 18 May 2020 07:03:55 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> The number of stacks is controlled by property "num-stacks".
> pnv_pec_instance_init() creates the maximum supported number, because
> the property has not been set then.  pnv_pec_realize() realizes only
> the wanted number.  Works, although it can leave unrealized devices
> hanging around in the QOM composition tree.  Affects machine powernv9.
> 
> Bury the unwanted devices by making pnv_pec_realize() unparent them.
> Visible in "info qom-tree":
> 
>      /machine (powernv9-machine)
>        /chip[0] (power9_v2.0-pnv-chip)
>          [...]
>          /pec[0] (pnv-phb4-pec)
>            /stack[0] (pnv-phb4-pec-stack)
>              [...]
>     -      /stack[1] (pnv-phb4-pec-stack)
>     -        /phb (pnv-phb4)
>     -          /pcie-mmcfg-mmio[0] (qemu:memory-region)
>     -          /root (pnv-phb4-root-port)
>     -          /source (xive-source)
>     -      /stack[2] (pnv-phb4-pec-stack)
>     -        /phb (pnv-phb4)
>     -          /pcie-mmcfg-mmio[0] (qemu:memory-region)
>     -          /root (pnv-phb4-root-port)
>     -          /source (xive-source)
>            /xscom-pec-0.0-nest[0] (qemu:memory-region)
>            /xscom-pec-0.0-pci[0] (qemu:memory-region)
>          /pec[1] (pnv-phb4-pec)
>            /stack[0] (pnv-phb4-pec-stack)
>              [...]
>            /stack[1] (pnv-phb4-pec-stack)
>              [...]
>     -      /stack[2] (pnv-phb4-pec-stack)
>     -        /phb (pnv-phb4)
>     -          /pcie-mmcfg-mmio[0] (qemu:memory-region)
>     -          /root (pnv-phb4-root-port)
>     -          /source (xive-source)
>            /xscom-pec-0.1-nest[0] (qemu:memory-region)
>            /xscom-pec-0.1-pci[0] (qemu:memory-region)
>          /pec[2] (pnv-phb4-pec)
>            /stack[0] (pnv-phb4-pec-stack)
>              [...]
>            /stack[1] (pnv-phb4-pec-stack)
>              [...]
>            /stack[2] (pnv-phb4-pec-stack)
>              [...]
> 
> Cc: Cédric Le Goater <clg@kaod.org>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---

Makes sense :)

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/pci-host/pnv_phb4_pec.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
> index 911d147ffd..565345a018 100644
> --- a/hw/pci-host/pnv_phb4_pec.c
> +++ b/hw/pci-host/pnv_phb4_pec.c
> @@ -397,6 +397,9 @@ static void pnv_pec_realize(DeviceState *dev, Error **errp)
>              return;
>          }
>      }
> +    for (; i < PHB4_PEC_MAX_STACKS; i++) {
> +        object_unparent(OBJECT(&pec->stacks[i]));
> +    }
>  
>      /* Initialize the XSCOM regions for the PEC registers */
>      snprintf(name, sizeof(name), "xscom-pec-%d.%d-nest", pec->chip_id,