Re: [PATCH for-7.2 06/10] ppc/pnv: enable user created pnv-phb for powernv8

From: Frederic Barrat <fbarrat@linux.ibm.com>
To: Daniel Henrique Barboza <danielhb413@gmail.com>, qemu-devel@nongnu.org
Cc: qemu-ppc@nongnu.org, clg@kaod.org
Subject: Re: [PATCH for-7.2 06/10] ppc/pnv: enable user created pnv-phb for powernv8
Date: Fri, 5 Aug 2022 12:50:42 +0200	[thread overview]
Message-ID: <5f3a7053-ebfe-ef3d-093a-d5a593ad88e5@linux.ibm.com> (raw)
In-Reply-To: <20220803134459.2633902-7-danielhb413@gmail.com>

> diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c
> index 077f391d59..953c384bf6 100644
> --- a/hw/pci-host/pnv_phb.c
> +++ b/hw/pci-host/pnv_phb.c
> @@ -17,6 +17,7 @@
>   #include "hw/ppc/pnv.h"
>   #include "hw/qdev-properties.h"
>   #include "qom/object.h"
> +#include "sysemu/sysemu.h"
>   
>   
>   /*
> @@ -171,6 +172,10 @@ static void pnv_phb_realize(DeviceState *dev, Error **errp)
>           pnv_phb4_bus_init(dev, PNV_PHB4(phb->backend));
>       }
>   
> +    if (phb->version == 3 && !defaults_enabled()) {
> +        return;
> +    }
> +
>       pnv_phb_attach_root_port(pci);
>   }

So we skip the call to pnv_phb_attach_root_port() for the user created PHBs.

I seem to remember that when the user creates a PHB on the command line, 
they also need to create the root port and we can't just create one by 
default with the PHB. It seems heavy when just using the qemu CLI but 
that was a requirement from libvirt. Did I get that right?

In any case, I can see a problem. pnv_phb_attach_root_port() creates the 
root port object, so if the above is correct, we want to skip that. But 
it also creates the PHB->root port relationship in QOM. That we don't do 
anywhere for the user-created case.

Calling qemu with "-nodefaults -device pnv-phb -device 
pnv-phb-root-port", here is what I see in QOM, which shows the wrong 
parenting of the root port:
   /peripheral-anon (container)
     /device[1] (pnv-phb-root-port)
       /bus master container[0] (memory-region)
       /bus master[0] (memory-region)
       /pci_bridge_io[0] (memory-region)
       /pci_bridge_io[1] (memory-region)
       /pci_bridge_mem[0] (memory-region)
       /pci_bridge_pci[0] (memory-region)
       /pci_bridge_pref_mem[0] (memory-region)
       /pci_bridge_vga_io_hi[0] (memory-region)
       /pci_bridge_vga_io_lo[0] (memory-region)
       /pci_bridge_vga_mem[0] (memory-region)
       /pcie.0 (PCIE)

With the full series, same problem on P8/P9/P10

   Fred