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. > > Signed-off-by: Nikunj A Dadhania <[email protected]> > --- > hw/ppc/spapr_pci.c | 93 > ++++++++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 84 insertions(+), 9 deletions(-) > > 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" > > +#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_name))); > + if (drc_name) { > + _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, > strlen(drc_name))); > + } > _FDT(fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index)); > > _FDT(fdt_setprop_cell(fdt, offset, "#address-cells", > @@ -994,10 +998,6 @@ static void spapr_phb_add_pci_device(sPAPRDRConnector > *drc, > void *fdt = NULL; > int fdt_start_offset = 0; > > - /* 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 fetch > - * it via RTAS > - */ > if (dev->hotplugged) { > fdt = 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); > } > > -typedef struct sPAPRTCEDT { > +typedef struct sPAPRFDT { > void *fdt; > int node_off; > -} sPAPRTCEDT; > + uint32_t index; > +} sPAPRFDT; > > static int spapr_phb_children_dt(Object *child, void *opaque) > { > - sPAPRTCEDT *p = opaque; > + sPAPRFDT *p = opaque; > sPAPRTCETable *tcet; > > tcet = (sPAPRTCETable *) object_dynamic_cast(child, > TYPE_SPAPR_TCE_TABLE); > @@ -1496,6 +1497,73 @@ static int spapr_phb_children_dt(Object *child, void > *opaque) > return 1; > } > > +static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev, void > *opaque) > +{ > + sPAPRFDT *p = opaque; > + int ret, offset; > + int slot = PCI_SLOT(pdev->devfn); > + int func = PCI_FUNC(pdev->devfn); > + char nodename[512]; > + > + if (func) { > + sprintf(nodename, "pci@%d,%d", slot, func); > + } else { > + sprintf(nodename, "pci@%d", slot); > + } > + offset = fdt_add_subnode(p->fdt, p->node_off, nodename); > + ret = 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) == > + PCI_HEADER_TYPE_BRIDGE)) { > + PCIBus *sec_bus = 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 = p->fdt, .node_off =
> offset , .index = 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 = (unsigned short *) opaque;
> + unsigned short primary = *bus_no;
> + unsigned short secondary;
> + unsigned short subordinate = 0xff;
> +
> + if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) ==
> + PCI_HEADER_TYPE_BRIDGE)) {
> + PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
> + secondary = *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 = *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, 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 = PCI_HOST_BRIDGE(phb)->bus;
> + unsigned short bus_no = 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[] = {
> 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 = PCI_HOST_BRIDGE(phb)->bus;
>
> /* Start populating the FDT */
> sprintf(nodename, "pci@%" PRIx64, phb->buid);
> @@ -1579,7 +1648,13 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
> sizeof(interrupt_map)));
>
> object_child_foreach(OBJECT(phb), spapr_phb_children_dt,
> - &((sPAPRTCEDT){ .fdt = fdt, .node_off = bus_off }));
> + &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off }));
> +
> + 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 = fdt, .node_off = bus_off,
> .index = phb->index }));
>
> ret = spapr_drc_populate_dt(fdt, bus_off, OBJECT(phb),
> SPAPR_DR_CONNECTOR_TYPE_PCI);
--
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
pgp0w3qwhoXBn.pgp
Description: PGP signature
