From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58846) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yn2ZQ-0005KI-PV for qemu-devel@nongnu.org; Tue, 28 Apr 2015 06:16:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yn2ZN-0002cK-9k for qemu-devel@nongnu.org; Tue, 28 Apr 2015 06:16:52 -0400 Date: Tue, 28 Apr 2015 17:46:41 +1000 From: David Gibson Message-ID: <20150428074641.GE24753@voom.redhat.com> References: <1429698934-28915-1-git-send-email-nikunj@linux.vnet.ibm.com> <1429698934-28915-2-git-send-email-nikunj@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="h56sxpGKRmy85csR" Content-Disposition: inline In-Reply-To: <1429698934-28915-2-git-send-email-nikunj@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [RFC PATCH 1/2] spapr: enumerate and add PCI device tree List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikunj A Dadhania Cc: aik@ozlabs.ru, qemu-ppc@nongnu.org, qemu-devel@nongnu.org, agraf@suse.de --h56sxpGKRmy85csR Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Apr 22, 2015 at 04:05:33PM +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 > --- > hw/ppc/spapr_pci.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++= ------ > 1 file changed, 84 insertions(+), 9 deletions(-) >=20 > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 3796d54..abf71f7 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" > @@ -938,7 +940,9 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev= , 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))); > + if (drc_name) { > + _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, strlen(d= rc_name))); > + } > _FDT(fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index)); > =20 > _FDT(fdt_setprop_cell(fdt, offset, "#address-cells", > @@ -994,10 +998,6 @@ static void spapr_phb_add_pci_device(sPAPRDRConnecto= r *drc, > void *fdt =3D NULL; > int fdt_start_offset =3D 0; > =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); > @@ -1473,14 +1473,15 @@ PCIHostState *spapr_create_phb(sPAPREnvironment *= spapr, int index) > return PCI_HOST_BRIDGE(dev); > } > =20 > -typedef struct sPAPRTCEDT { > +typedef struct sPAPRFDT { > void *fdt; > int node_off; > -} sPAPRTCEDT; > + uint32_t index; > +} sPAPRFDT; > =20 > static int spapr_phb_children_dt(Object *child, void *opaque) > { > - sPAPRTCEDT *p =3D opaque; > + sPAPRFDT *p =3D opaque; > sPAPRTCETable *tcet; > =20 > tcet =3D (sPAPRTCETable *) object_dynamic_cast(child, TYPE_SPAPR_TCE= _TABLE); > @@ -1496,6 +1497,73 @@ static int spapr_phb_children_dt(Object *child, vo= id *opaque) > return 1; > } > =20 > +static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev, = void *opaque) > +{ > + sPAPRFDT *p =3D opaque; > + int ret, offset; > + int slot =3D PCI_SLOT(pdev->devfn); > + int func =3D PCI_FUNC(pdev->devfn); > + char nodename[512]; > + > + if (func) { > + sprintf(nodename, "pci@%d,%d", slot, func); > + } else { > + sprintf(nodename, "pci@%d", slot); > + } > + offset =3D fdt_add_subnode(p->fdt, p->node_off, nodename); > + ret =3D spapr_populate_pci_child_dt(pdev, p->fdt, offset, p->index, = 0, NULL); > + g_assert(!ret); > + > + if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) =3D=3D > + PCI_HEADER_TYPE_BRIDGE)) { > + PCIBus *sec_bus =3D pci_bridge_get_sec_bus(PCI_BRIDGE(pdev)); > + if(sec_bus) { You have several minor indent style problems - please run through scripts/checkpatch.pl and correct accordingly. > + pci_for_each_device(sec_bus, pci_bus_num(sec_bus), > + spapr_populate_pci_devices_dt, > + &((sPAPRFDT){ .fdt =3D p->fdt, .node_off= =3D offset , .index =3D p->index })); The inline structure definition is pretty hard to read. I'd prefer to see that declared as a local. Otherwise, this looks ok. > + } > + } > + return; > +} > + > +static void spapr_phb_pci_enumerate_bridge(PCIBus *bus, PCIDevice *pdev,= void *opaque) > +{ > + unsigned short *bus_no =3D (unsigned short *) opaque; > + unsigned short primary =3D *bus_no; > + unsigned short secondary; > + unsigned short subordinate =3D 0xff; > + > + if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) =3D=3D > + PCI_HEADER_TYPE_BRIDGE)) { > + PCIBus *sec_bus =3D pci_bridge_get_sec_bus(PCI_BRIDGE(pdev)); > + secondary =3D *bus_no + 1; > + pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1); > + pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary, 1); > + pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, secondary, 1= ); > + *bus_no =3D *bus_no + 1; > + if (sec_bus) { > + pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1); > + pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary,= 1); > + pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, subordin= ate, 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 short 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) > @@ -1534,6 +1602,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, > uint32_t interrupt_map_mask[] =3D { > 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]; > + PCIBus *bus =3D PCI_HOST_BRIDGE(phb)->bus; > =20 > /* Start populating the FDT */ > sprintf(nodename, "pci@%" PRIx64, phb->buid); > @@ -1579,7 +1648,13 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, > sizeof(interrupt_map))); > =20 > object_child_foreach(OBJECT(phb), spapr_phb_children_dt, > - &((sPAPRTCEDT){ .fdt =3D fdt, .node_off =3D bus= _off })); > + &((sPAPRFDT){ .fdt =3D fdt, .node_off =3D bus_o= ff })); > + > + spapr_phb_pci_enumerate(phb); > + _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,phb-enumerated", 0x1)); > + pci_for_each_device(bus, pci_bus_num(bus), > + spapr_populate_pci_devices_dt, > + &((sPAPRFDT){ .fdt =3D fdt, .node_off =3D bus_of= f, .index =3D phb->index })); > =20 > ret =3D spapr_drc_populate_dt(fdt, bus_off, OBJECT(phb), > SPAPR_DR_CONNECTOR_TYPE_PCI); --=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 --h56sxpGKRmy85csR Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVPzrgAAoJEGw4ysog2bOSwtgP/1iYEgqNXnOKkZgE9TVSLOHZ 5gscddgLoTlsxc75Ho5MrAJ9kZ7dVVICqv9iL4hF0HGuEFO3jExPA6hTo9NE73yz yhyOKOy9Nw70vyRPBvZAVSMXbRLlXmm8FqxnO5H2asIIB6qQLcGCEu7iymgn6U8c b2Is69krK+RGgC+HHjynV/RdHuxEDxSgFIVINGgV0xQyXO2uesBOLIY3fk0FDCsq BREqXnppiP1CL4pkgMhW93O13peAJJJIl4om8CwnohJkIysAQi1e+eZsBGz+U6mU Lg5/2QrJlckuH6K0ATafsI0SsS6foCOf/wHKCLPE4Hr87P9RrQ8BI/ZhkiYJTmZ+ upo4RNGY6V9EafpsfkTfMNeL6bBbLJPOfe4HE2SBx2PY6M/x7NM4MgFiLmE8FP3K iTpM5ico6qcDYmK21k8puZ+a4XWyly/leL7UtY1vOPr+0GA7FWXXFTN5bewvhQn+ MYiZykmldIPOsfACw9a6WiziQBAm0ozojVEKq2Bl5wdpYpAiGkEH55MS0OK+kcZO je9R3H6fRcjEb2iG/jEbq7rtcxw/UlchhZCMtFPUNaOJmw0Mtjlksr23fWkOjhQj CXJ0Jv5B7EZORCIHzsji3GhuMKo0x+PT6No355C7wnKlh4rnJFWOqM4RPAyg1sJx NrYa2/JueAS1zpsTrGoL =HWS8 -----END PGP SIGNATURE----- --h56sxpGKRmy85csR--