From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45289) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z57oP-00085N-2r for qemu-devel@nongnu.org; Wed, 17 Jun 2015 03:31:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z57oM-0005Pl-AA for qemu-devel@nongnu.org; Wed, 17 Jun 2015 03:31:05 -0400 Date: Wed, 17 Jun 2015 16:49:36 +1000 From: David Gibson Message-ID: <20150617064936.GS13352@voom.redhat.com> References: <1434020549-26362-1-git-send-email-nikunj@linux.vnet.ibm.com> <1434020549-26362-4-git-send-email-nikunj@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="R+WA+n3pAWWgNWUs" Content-Disposition: inline In-Reply-To: <1434020549-26362-4-git-send-email-nikunj@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH v7 3/6] spapr_pci: enumerate and add PCI device tree List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikunj A Dadhania Cc: agraf@suse.de, thuth@redhat.com, aik@ozlabs.ru, mdroth@linux.vnet.ibm.com, qemu-devel@nongnu.org, qemu-ppc@nongnu.org --R+WA+n3pAWWgNWUs Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jun 11, 2015 at 04:32:26PM +0530, Nikunj A Dadhania wrote: > All the PCI enumeration and device node creation was off-loaded to > SLOF. With PCI hotplug support, code needed to be added to add device > node. This creates multiple copy of the code one in SLOF and other in > hotplug code. To unify this, the patch adds the pci device node > creation in Qemu. For backward compatibility, a flag > "qemu,phb-enumerated" is added to the phb, suggesting to SLOF to not > do device node creation. >=20 > Signed-off-by: Nikunj A Dadhania > [ Squashed Michael's drc_index changes ] > Signed-off-by: Michael Roth > Signed-off-by: Nikunj A Dadhania > --- > hw/ppc/spapr_pci.c | 167 +++++++++++++++++++++++++++++++++++++++++++++--= ------ > 1 file changed, 142 insertions(+), 25 deletions(-) >=20 > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 33254b3..6ef7f44 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -23,6 +23,7 @@ > * THE SOFTWARE. > */ > #include "hw/hw.h" > +#include "hw/sysbus.h" > #include "hw/pci/pci.h" > #include "hw/pci/msi.h" > #include "hw/pci/msix.h" > @@ -35,6 +36,7 @@ > #include "qemu/error-report.h" > #include "qapi/qmp/qerror.h" > =20 > +#include "hw/pci/pci_bridge.h" > #include "hw/pci/pci_bus.h" > #include "hw/ppc/spapr_drc.h" > #include "sysemu/device_tree.h" > @@ -946,8 +948,13 @@ static int spapr_populate_pci_child_dt(PCIDevice *de= v, void *fdt, int offset, > * processed by OF beforehand > */ > _FDT(fdt_setprop_string(fdt, offset, "name", "pci")); > - _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, strlen(drc_n= ame))); > - _FDT(fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index)); > + if (drc_name) { > + _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, > + strlen(drc_name))); > + } > + if (drc_index) { > + _FDT(fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index= )); > + } > =20 > _FDT(fdt_setprop_cell(fdt, offset, "#address-cells", > RESOURCE_CELLS_ADDRESS)); > @@ -964,30 +971,38 @@ static int spapr_populate_pci_child_dt(PCIDevice *d= ev, void *fdt, int offset, > return 0; > } > =20 > +typedef struct sPAPRFDT { > + void *fdt; > + int node_off; > + sPAPRPHBState *sphb; > +} sPAPRFDT; I don't really like this structure - it seems a very ad-hoc collection of things. Even though it means there will be a lot of parameters to the function, I'd prefer passing them separately to spapr_create_pci_child_dt() rather than using this structure. > +static uint32_t spapr_phb_get_pci_drc_index(sPAPRPHBState *phb, > + PCIDevice *pdev); > + > /* create OF node for pci device and required OF DT properties */ > -static void *spapr_create_pci_child_dt(sPAPRPHBState *phb, PCIDevice *de= v, > - int drc_index, const char *drc_na= me, > - int *dt_offset) > +static int spapr_create_pci_child_dt(PCIDevice *dev, sPAPRFDT *p, > + const char *drc_name) > { > - void *fdt; > - int offset, ret, fdt_size; > + int offset, ret; > int slot =3D PCI_SLOT(dev->devfn); > int func =3D PCI_FUNC(dev->devfn); > char nodename[512]; > + uint32_t drc_index =3D spapr_phb_get_pci_drc_index(p->sphb, dev); > =20 > - fdt =3D create_device_tree(&fdt_size); > if (func !=3D 0) { > sprintf(nodename, "pci@%d,%d", slot, func); > } else { > sprintf(nodename, "pci@%d", slot); > } > - offset =3D fdt_add_subnode(fdt, 0, nodename); > - ret =3D spapr_populate_pci_child_dt(dev, fdt, offset, phb->index, dr= c_index, > - drc_name); > + offset =3D fdt_add_subnode(p->fdt, p->node_off, nodename); > + ret =3D spapr_populate_pci_child_dt(dev, p->fdt, offset, p->sphb->in= dex, > + drc_index, drc_name); > g_assert(!ret); > - > - *dt_offset =3D offset; > - return fdt; > + if (ret) { > + return 0; > + } > + return offset; > } > =20 > static void spapr_phb_add_pci_device(sPAPRDRConnector *drc, > @@ -997,24 +1012,26 @@ static void spapr_phb_add_pci_device(sPAPRDRConnec= tor *drc, > { > sPAPRDRConnectorClass *drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); > DeviceState *dev =3D DEVICE(pdev); > - int drc_index =3D drck->get_index(drc); > const char *drc_name =3D drck->get_name(drc); > - void *fdt =3D NULL; > - int fdt_start_offset =3D 0; > + int fdt_start_offset =3D 0, fdt_size; > + sPAPRFDT s_fdt =3D {NULL, 0, NULL}; > =20 > - /* boot-time devices get their device tree node created by SLOF, but= for > - * hotplugged devices we need QEMU to generate it so the guest can f= etch > - * it via RTAS > - */ > if (dev->hotplugged) { > - fdt =3D spapr_create_pci_child_dt(phb, pdev, drc_index, drc_name, > - &fdt_start_offset); > + s_fdt.fdt =3D create_device_tree(&fdt_size); > + s_fdt.sphb =3D phb; > + s_fdt.node_off =3D 0; > + fdt_start_offset =3D spapr_create_pci_child_dt(pdev, &s_fdt, drc= _name); > + if (!fdt_start_offset) { > + error_setg(errp, "Failed to create pci child device tree nod= e"); > + goto out; > + } > } > =20 > drck->attach(drc, DEVICE(pdev), > - fdt, fdt_start_offset, !dev->hotplugged, errp); > + s_fdt.fdt, fdt_start_offset, !dev->hotplugged, errp); > +out: > if (*errp) { > - g_free(fdt); > + g_free(s_fdt.fdt); > } > } > =20 > @@ -1054,6 +1071,20 @@ static sPAPRDRConnector *spapr_phb_get_pci_drc(sPA= PRPHBState *phb, > pdev->devfn); > } > =20 > +static uint32_t spapr_phb_get_pci_drc_index(sPAPRPHBState *phb, > + PCIDevice *pdev) > +{ > + sPAPRDRConnector *drc =3D spapr_phb_get_pci_drc(phb, pdev); > + sPAPRDRConnectorClass *drck; > + > + if (!drc) { > + return 0; > + } > + > + drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); > + return drck->get_index(drc); > +} > + > static void spapr_phb_hot_plug_child(HotplugHandler *plug_handler, > DeviceState *plugged_dev, Error **e= rrp) > { > @@ -1484,6 +1515,78 @@ PCIHostState *spapr_create_phb(sPAPRMachineState *= spapr, int index) > return PCI_HOST_BRIDGE(dev); > } > =20 > +static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev, > + void *opaque) > +{ > + PCIBus *sec_bus; > + sPAPRFDT *p =3D opaque; > + int offset; > + sPAPRFDT s_fdt; > + > + offset =3D spapr_create_pci_child_dt(pdev, p, NULL); > + if (!offset) { > + error_report("Failed to create pci child device tree node"); > + return; > + } > + > + if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) !=3D > + PCI_HEADER_TYPE_BRIDGE)) { > + return; > + } > + > + sec_bus =3D pci_bridge_get_sec_bus(PCI_BRIDGE(pdev)); > + if (!sec_bus) { > + return; > + } > + > + s_fdt.fdt =3D p->fdt; > + s_fdt.node_off =3D offset; > + s_fdt.sphb =3D p->sphb; > + pci_for_each_device(sec_bus, pci_bus_num(sec_bus), > + spapr_populate_pci_devices_dt, > + &s_fdt); > +} > + > +static void spapr_phb_pci_enumerate_bridge(PCIBus *bus, PCIDevice *pdev, > + void *opaque) > +{ > + unsigned int *bus_no =3D opaque; > + unsigned int primary =3D *bus_no; > + unsigned int subordinate =3D 0xff; > + PCIBus *sec_bus =3D NULL; > + > + if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) !=3D > + PCI_HEADER_TYPE_BRIDGE)) { > + return; > + } > + > + (*bus_no)++; > + pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1); > + pci_default_write_config(pdev, PCI_SECONDARY_BUS, *bus_no, 1); > + pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, *bus_no, 1); > + > + sec_bus =3D pci_bridge_get_sec_bus(PCI_BRIDGE(pdev)); > + if (!sec_bus) { > + return; > + } > + > + pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, subordinate, 1); > + pci_for_each_device(sec_bus, pci_bus_num(sec_bus), > + spapr_phb_pci_enumerate_bridge, bus_no); > + pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, *bus_no, 1); > +} > + > +static void spapr_phb_pci_enumerate(sPAPRPHBState *phb) > +{ > + PCIBus *bus =3D PCI_HOST_BRIDGE(phb)->bus; > + unsigned int bus_no =3D 0; > + > + pci_for_each_device(bus, pci_bus_num(bus), > + spapr_phb_pci_enumerate_bridge, > + &bus_no); > + > +} > + > int spapr_populate_pci_dt(sPAPRPHBState *phb, > uint32_t xics_phandle, > void *fdt) > @@ -1523,6 +1626,8 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, > cpu_to_be32(b_ddddd(-1)|b_fff(0)), 0x0, 0x0, cpu_to_be32(-1)}; > uint32_t interrupt_map[PCI_SLOT_MAX * PCI_NUM_PINS][7]; > sPAPRTCETable *tcet; > + PCIBus *bus =3D PCI_HOST_BRIDGE(phb)->bus; > + sPAPRFDT s_fdt; > =20 > /* Start populating the FDT */ > sprintf(nodename, "pci@%" PRIx64, phb->buid); > @@ -1572,6 +1677,18 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, > tcet->liobn, tcet->bus_offset, > tcet->nb_table << tcet->page_shift); > =20 > + /* Walk the bridges and program the bus numbers*/ > + spapr_phb_pci_enumerate(phb); > + _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,phb-enumerated", 0x1)); > + > + /* Populate tree nodes with PCI devices attached */ > + s_fdt.fdt =3D fdt; > + s_fdt.node_off =3D bus_off; > + s_fdt.sphb =3D phb; > + pci_for_each_device(bus, pci_bus_num(bus), > + spapr_populate_pci_devices_dt, > + &s_fdt); > + > ret =3D spapr_drc_populate_dt(fdt, bus_off, OBJECT(phb), > SPAPR_DR_CONNECTOR_TYPE_PCI); > if (ret) { --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --R+WA+n3pAWWgNWUs Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVgRiAAAoJEGw4ysog2bOSm+4QAMDHbKDbxCb/Niobo1GlVBLK vE6dtqHhBKQaBMIzkMzt6WlYrluJlyAM8A4cg8uoKd7GxL7W0bTH1cUnr83kmP8e idwjAGGxsCtOaWxyVFg4UXPoL3cnrQsYs3FP4CyfHxOAcLOPJpuXS5k+sMy2jXnb wv9pPZr4k1kUE+wR/bDbJbNxRZBy+VfOaprtUV0Yn5Wswz0v3rsqzJj/U8cA0/Ve gRrGYaKtoRszv2XKyylgKBEAJwLMC/6cHO0N9C1rTWbHT9Qc9VhMozEaqrvsPTmp 3Ur3vJQ4AeUlHABPqCIH+gntNXAqiVBaTaaOswEARuzim+2NQCujiLiTigwWvxBi I8ux4dPsDm/aihi/SUvhIodyQxzNnn/6Hjo0Wwua2gCT3GY0cHVF95foCskiBGyN tTNQy8bQOtHWRQaJFBEKJKNqZ4RqDgK3RQ957lxclxdlFCp8IgDFAFEm4JLPidhR EBtqizl7fxqYoSNUKyVssCu4oJOWKDBtYwY9GYfWHWtsxdlIQevHxcnH2GXTayYl B/686QUDze5SXsYBN/YapxIUIEJesuc8N5PRGE4EaT8U+eAlyiVMoalIqgeb8nLR dSxI2irY8O4GWKOACHmddkNMQvFy/WPD5YfwihUxsMCW71aFeHxz8aCItEp1i/v3 /446mlnUtODM8qWFGIEj =/RYF -----END PGP SIGNATURE----- --R+WA+n3pAWWgNWUs--