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.
>
> Signed-off-by: Nikunj A Dadhania <nik...@linux.vnet.ibm.com>
> [ Squashed Michael's drc_index changes ]
> Signed-off-by: Michael Roth <mdr...@linux.vnet.ibm.com>
> Signed-off-by: Nikunj A Dadhania <nik...@linux.vnet.ibm.com>
> ---
> hw/ppc/spapr_pci.c | 167
> +++++++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 142 insertions(+), 25 deletions(-)
>
> 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"
>
> +#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 *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)));
> - _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));
> + }
>
> _FDT(fdt_setprop_cell(fdt, offset, "#address-cells",
> RESOURCE_CELLS_ADDRESS));
> @@ -964,30 +971,38 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev,
> void *fdt, int offset,
> return 0;
> }
>
> +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 *dev,
> - int drc_index, const char *drc_name,
> - 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 = PCI_SLOT(dev->devfn);
> int func = PCI_FUNC(dev->devfn);
> char nodename[512];
> + uint32_t drc_index = spapr_phb_get_pci_drc_index(p->sphb, dev);
>
> - fdt = create_device_tree(&fdt_size);
> if (func != 0) {
> sprintf(nodename, "pci@%d,%d", slot, func);
> } else {
> sprintf(nodename, "pci@%d", slot);
> }
> - offset = fdt_add_subnode(fdt, 0, nodename);
> - ret = spapr_populate_pci_child_dt(dev, fdt, offset, phb->index,
> drc_index,
> - drc_name);
> + offset = fdt_add_subnode(p->fdt, p->node_off, nodename);
> + ret = spapr_populate_pci_child_dt(dev, p->fdt, offset, p->sphb->index,
> + drc_index, drc_name);
> g_assert(!ret);
> -
> - *dt_offset = offset;
> - return fdt;
> + if (ret) {
> + return 0;
> + }
> + return offset;
> }
>
> static void spapr_phb_add_pci_device(sPAPRDRConnector *drc,
> @@ -997,24 +1012,26 @@ static void spapr_phb_add_pci_device(sPAPRDRConnector
> *drc,
> {
> sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> DeviceState *dev = DEVICE(pdev);
> - int drc_index = drck->get_index(drc);
> const char *drc_name = drck->get_name(drc);
> - void *fdt = NULL;
> - int fdt_start_offset = 0;
> + int fdt_start_offset = 0, fdt_size;
> + sPAPRFDT s_fdt = {NULL, 0, NULL};
>
> - /* 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);
> + s_fdt.fdt = create_device_tree(&fdt_size);
> + s_fdt.sphb = phb;
> + s_fdt.node_off = 0;
> + fdt_start_offset = spapr_create_pci_child_dt(pdev, &s_fdt, drc_name);
> + if (!fdt_start_offset) {
> + error_setg(errp, "Failed to create pci child device tree node");
> + goto out;
> + }
> }
>
> 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);
> }
> }
>
> @@ -1054,6 +1071,20 @@ static sPAPRDRConnector
> *spapr_phb_get_pci_drc(sPAPRPHBState *phb,
> pdev->devfn);
> }
>
> +static uint32_t spapr_phb_get_pci_drc_index(sPAPRPHBState *phb,
> + PCIDevice *pdev)
> +{
> + sPAPRDRConnector *drc = spapr_phb_get_pci_drc(phb, pdev);
> + sPAPRDRConnectorClass *drck;
> +
> + if (!drc) {
> + return 0;
> + }
> +
> + drck = 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 **errp)
> {
> @@ -1484,6 +1515,78 @@ PCIHostState *spapr_create_phb(sPAPRMachineState
> *spapr, int index)
> return PCI_HOST_BRIDGE(dev);
> }
>
> +static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev,
> + void *opaque)
> +{
> + PCIBus *sec_bus;
> + sPAPRFDT *p = opaque;
> + int offset;
> + sPAPRFDT s_fdt;
> +
> + offset = 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) !=
> + PCI_HEADER_TYPE_BRIDGE)) {
> + return;
> + }
> +
> + sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
> + if (!sec_bus) {
> + return;
> + }
> +
> + s_fdt.fdt = p->fdt;
> + s_fdt.node_off = offset;
> + s_fdt.sphb = 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 = opaque;
> + unsigned int primary = *bus_no;
> + unsigned int subordinate = 0xff;
> + PCIBus *sec_bus = NULL;
> +
> + if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) !=
> + 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 = 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 = PCI_HOST_BRIDGE(phb)->bus;
> + unsigned int 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)
> @@ -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 = PCI_HOST_BRIDGE(phb)->bus;
> + sPAPRFDT s_fdt;
>
> /* 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);
>
> + /* 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 = fdt;
> + s_fdt.node_off = bus_off;
> + s_fdt.sphb = phb;
> + pci_for_each_device(bus, pci_bus_num(bus),
> + spapr_populate_pci_devices_dt,
> + &s_fdt);
> +
> ret = spapr_drc_populate_dt(fdt, bus_off, OBJECT(phb),
> SPAPR_DR_CONNECTOR_TYPE_PCI);
> if (ret) {
--
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
pgprtEzBuhkyO.pgp
Description: PGP signature
