Re: [PATCH v7 26/50] powerpc/powernv: Create PEs at PCI hot plugging time

Alexey Kardashevskiy
2015-11-17T07:57:28Z

On 11/05/2015 12:12 AM, Gavin Shan wrote:
> Currently, the PEs and their associated resources are assigned
> in ppc_md.pcibios_fixup() except those used by SRIOV VFs. The
> function is called for once after PCI probing and resources
> assignment is completed. So it isn't hotplug friendly.
>
> This creates PEs dynamically by ppc_md.pcibios_setup_bridge(), which
> is called on the event during system bootup and PCI hotplug: updating
> PCI bridge's windows after resource assignment/reassignment are done.
> For partial hotplug case, where not all PCI devices belonging to the
> PE are unplugged and plugged again, we just need unbinding/binding
> the affected PCI devices with the corresponding PE without creating
> new one.
>
> As there is no upstream bridge for root bus that needs to be covered
> by PE,

Does "that needs" part relate to a root bus or a an upstream bridge?

> we have to create PE for root bus in ppc_md.pcibios_setup_bridge()
> before any other PEs can be created, as PE for root bus is the ancestor
> to anyone else.
>
> On the other hand, the windows of root port or the upstream port

s/On the other hand, /Also/ ?

> of PCIe switch behind root port are extended to be PHB's aperatuses

apertures?

> to accommodate the additonal resources needed by newly plugged devices

s/additonal/additional

> based on the fact: hotpluggable slot is behind root port or downstream
> port of the PCIe switch behind root port. The extension for those
> PCI brdiges' windows is done in ppc_md.pcibios_setup_bridge() as
> well.

I find it quite difficult to separate "cut-n-paste" changes from functional
changes... May be it is just me.
I would suggest splitting this patch into several. First define the
setup_bridge() callback, then rework pnv_pci_ioda_setup_PEs(),
pnv_pci_ioda_setup_seg(), pnv_pci_ioda_setup_DMA(), and then add "partial
hotplug" handling may be.

Or just get "reviewed-by" from Ben :)

>
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> ---
> arch/powerpc/platforms/powernv/pci-ioda.c | 240 +++++++++++++++++-------------
> arch/powerpc/platforms/powernv/pci.h | 1 +
> 2 files changed, 138 insertions(+), 103 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index 5e6745f..0bb0056 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -975,6 +975,15 @@ static void pnv_ioda_setup_same_PE(struct pci_bus *bus, struct pnv_ioda_pe *pe)
> pci_name(dev));
> continue;
> }
> +
> + /*
> + * In partial hotplug case, the PCI device might be still
> + * associated with the PE and needn't be attached to the
> + * PE again.
> + */
> + if (pdn->pe_number != IODA_INVALID_PE)
> + continue;
> +
> pdn->pe_number = pe->pe_number;
> if ((pe->flags & PNV_IODA_PE_BUS_ALL) && dev->subordinate)
> pnv_ioda_setup_same_PE(dev->subordinate, pe);
> @@ -992,9 +1001,26 @@ static struct pnv_ioda_pe *pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all)
> struct pci_controller *hose = pci_bus_to_host(bus);
> struct pnv_phb *phb = hose->private_data;
> struct pnv_ioda_pe *pe = NULL;
> + int pe_num;
> +
> + /*
> + * In partial hotplug case, the PE instance might be still alive.
> + * We should reuse it instead of allocating a new one.
> + */
> + pe_num = phb->ioda.pe_rmap[bus->number << 8];
> + if (pe_num != IODA_INVALID_PE) {
> + pe = &phb->ioda.pe_array[pe_num];
> + pnv_ioda_setup_same_PE(bus, pe);
> + return NULL;
> + }
> +
> + /* PE number for root bus should have been reserved */
> + if (pci_is_root_bus(bus) &&
> + phb->ioda.root_pe_idx != IODA_INVALID_PE)
> + pe = &phb->ioda.pe_array[phb->ioda.root_pe_idx];
>
> /* Check if PE is determined by M64 */
> - if (phb->pick_m64_pe)
> + if (!pe && phb->pick_m64_pe)
> pe = phb->pick_m64_pe(bus, all);
>
> /* The PE number isn't pinned by M64 */
> @@ -1036,46 +1062,6 @@ static struct pnv_ioda_pe *pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all)
> return pe;
> }
>
> -static void pnv_ioda_setup_PEs(struct pci_bus *bus)
> -{
> - struct pci_dev *dev;
> -
> - pnv_ioda_setup_bus_PE(bus, false);
> -
> - list_for_each_entry(dev, &bus->devices, bus_list) {
> - if (dev->subordinate) {
> - if (pci_pcie_type(dev) == PCI_EXP_TYPE_PCI_BRIDGE)
> - pnv_ioda_setup_bus_PE(dev->subordinate, true);
> - else
> - pnv_ioda_setup_PEs(dev->subordinate);
> - }
> - }
> -}
> -
> -/*
> - * Configure PEs so that the downstream PCI buses and devices
> - * could have their associated PE#. Unfortunately, we didn't
> - * figure out the way to identify the PLX bridge yet. So we
> - * simply put the PCI bus and the subordinate behind the root
> - * port to PE# here. The game rule here is expected to be changed
> - * as soon as we can detected PLX bridge correctly.
> - */
> -static void pnv_pci_ioda_setup_PEs(void)
> -{
> - struct pci_controller *hose, *tmp;
> - struct pnv_phb *phb;
> -
> - list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
> - phb = hose->private_data;
> -
> - /* M64 layout might affect PE allocation */
> - if (phb->reserve_m64_pe)
> - phb->reserve_m64_pe(hose->bus, NULL, true);
> -
> - pnv_ioda_setup_PEs(hose->bus);
> - }
> -}
> -
> #ifdef CONFIG_PCI_IOV
> static int pnv_pci_vf_release_m64(struct pci_dev *pdev, u16 num_vfs)
> {
> @@ -2391,8 +2377,13 @@ static void pnv_pci_ioda2_table_free_pages(struct iommu_table *tbl)
> static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,
> struct pnv_ioda_pe *pe)
> {
> + unsigned int weight;
> int64_t rc;
>
> + weight = pnv_pci_ioda_pe_dma_weight(pe);
> + if (!weight)
> + return;
> +
> /* TVE #1 is selected by PCI address bit 59 */
> pe->tce_bypass_base = 1ull << 59;
>
> @@ -2424,33 +2415,6 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,
> pnv_ioda_setup_bus_dma(pe, pe->pbus);
> }
>
> -static void pnv_pci_ioda1_setup_dma(struct pnv_phb *phb)
> -{
> - struct pnv_ioda_pe *pe;
> -
> - pnv_pci_ioda_setup_opal_tce_kill(phb);
> -
> - list_for_each_entry(pe, &phb->ioda.pe_list, list)
> - pnv_pci_ioda1_setup_dma_pe(phb, pe);
> -}
> -
> -static void pnv_pci_ioda2_setup_dma(struct pnv_phb *phb)
> -{
> - struct pnv_ioda_pe *pe;
> - unsigned int weight;
> -
> - pnv_pci_ioda_setup_opal_tce_kill(phb);
> -
> - list_for_each_entry(pe, &phb->ioda.pe_list, list) {
> - weight = pnv_pci_ioda_pe_dma_weight(pe);
> - if (!weight)
> - continue;
> -
> - pe_info(pe, "Assign DMA32 space\n");
> - pnv_pci_ioda2_setup_dma_pe(phb, pe);
> - }
> -}
> -
> #ifdef CONFIG_PCI_MSI
> static void pnv_ioda2_msi_eoi(struct irq_data *d)
> {
> @@ -2914,37 +2878,6 @@ static void pnv_ioda_setup_pe_seg(struct pnv_ioda_pe *pe)
> }
> }
>
> -static void pnv_pci_ioda_setup_seg(void)
> -{
> - struct pci_controller *tmp, *hose;
> - struct pnv_phb *phb;
> - struct pnv_ioda_pe *pe;
> -
> - list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
> - phb = hose->private_data;
> - list_for_each_entry(pe, &phb->ioda.pe_list, list) {
> - pnv_ioda_setup_pe_seg(pe);
> - }
> - }
> -}
> -
> -static void pnv_pci_ioda_setup_DMA(void)
> -{
> - struct pci_controller *hose, *tmp;
> - struct pnv_phb *phb;
> -
> - list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
> - phb = hose->private_data;
> - if (phb->type == PNV_PHB_IODA1)
> - pnv_pci_ioda1_setup_dma(phb);
> - else
> - pnv_pci_ioda2_setup_dma(phb);
> -
> - /* Mark the PHB initialization done */
> - phb->initialized = 1;
> - }
> -}
> -
> static void pnv_pci_ioda_create_dbgfs(void)
> {
> #ifdef CONFIG_DEBUG_FS
> @@ -2955,6 +2888,9 @@ static void pnv_pci_ioda_create_dbgfs(void)
> list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
> phb = hose->private_data;
>
> + /* Notify initialization of PHB done */
> + phb->initialized = 1;
> +
> sprintf(name, "PCI%04x", hose->global_number);
> phb->dbgfs = debugfs_create_dir(name, powerpc_debugfs_root);
> if (!phb->dbgfs)
> @@ -2966,10 +2902,6 @@ static void pnv_pci_ioda_create_dbgfs(void)
>
> static void pnv_pci_ioda_fixup(void)
> {
> - pnv_pci_ioda_setup_PEs();
> - pnv_pci_ioda_setup_seg();
> - pnv_pci_ioda_setup_DMA();
> -
> pnv_pci_ioda_create_dbgfs();
>
> #ifdef CONFIG_EEH
> @@ -3019,6 +2951,104 @@ static resource_size_t pnv_pci_window_alignment(struct pci_bus *bus,
> return phb->ioda.io_segsize;
> }
>
> +/*
> + * We are updating root port or the upstream port of the
> + * bridge behind the root port with PHB's windows in order
> + * to accommodate the changes on required resources during
> + * PCI (slot) hotplug, which is connected to either root
> + * port or the downstream ports of PCIe switch behind the
> + * root port.
> + */
> +static void pnv_pci_fixup_bridge_resources(struct pci_bus *bus,
> + unsigned long type)
> +{
> + struct pci_controller *hose = pci_bus_to_host(bus);
> + struct pnv_phb *phb = hose->private_data;
> + struct pci_dev *bridge = bus->self;
> + struct resource *r, *w;
> + int i;
> +
> + /* Check if we need apply fixup to the bridge's windows */
> + if (!pci_is_root_bus(bridge->bus) &&
> + !pci_is_root_bus(bridge->bus->self->bus))
> + return;
> +
> + /* Fixup the resoureces */

s/resoureces/resources/

> + for (i = 0; i < PCI_BRIDGE_RESOURCE_NUM; i++) {
> + r = &bridge->resource[PCI_BRIDGE_RESOURCES + i];
> + if (!r->flags || !r->parent)
> + continue;
> +
> + w = NULL;
> + if (r->flags & type & IORESOURCE_IO)
> + w = &hose->io_resource;
> + else if (pnv_pci_is_mem_pref_64(r->flags) &&
> + (type & IORESOURCE_PREFETCH) &&
> + phb->ioda.m64_segsize)
> + w = &hose->mem_resources[1];
> + else if (r->flags & type & IORESOURCE_MEM)
> + w = &hose->mem_resources[0];
> +
> + r->start = w->start;
> + r->end = w->end;
> + }
> +}
> +
> +static void pnv_pci_setup_bridge(struct pci_bus *bus,
> + unsigned long type)
> +{
> + struct pci_controller *hose = pci_bus_to_host(bus);
> + struct pnv_phb *phb = hose->private_data;
> + struct pci_dev *bridge = bus->self;
> + struct pnv_ioda_pe *pe;
> + bool all = (pci_pcie_type(bridge) == PCI_EXP_TYPE_PCI_BRIDGE);
> +
> + /* The PE for root bus should be realized before any one else */
> + if (!phb->ioda.root_pe_populated) {
> + pe = pnv_ioda_setup_bus_PE(phb->hose->bus, false);
> + if (pe) {
> + phb->ioda.root_pe_idx = pe->pe_number;
> + phb->ioda.root_pe_populated = true;
> + }
> + }
> +
> + /* Extend bridge's windows if necessary */
> + pnv_pci_fixup_bridge_resources(bus, type);
> +
> + /* Don't assign PE to PCI bus, which doesn't have subordinate devices */
> + if (list_empty(&bus->devices))
> + return;
> +
> + /* Reserve PEs according to used M64 resources */
> + if (phb->reserve_m64_pe)
> + phb->reserve_m64_pe(bus, NULL, all);
> +
> + /*
> + * Assign PE. We might run here because of partial hotplug.
> + * For the case, we just pick up the existing PE and should
> + * not allocate resources again.
> + */
> + pe = pnv_ioda_setup_bus_PE(bus, all);
> + if (!pe)
> + return;
> +
> + /* Setup MMIO mapping */
> + pnv_ioda_setup_pe_seg(pe);
> +
> + /* Setup DMA */
> + switch (phb->type) {
> + case PNV_PHB_IODA1:
> + pnv_pci_ioda1_setup_dma_pe(phb, pe);
> + break;
> + case PNV_PHB_IODA2:
> + pnv_pci_ioda2_setup_dma_pe(phb, pe);
> + break;
> + default:
> + pr_warn("%s: No DMA for PHB#%d (type %d)\n",
> + __func__, phb->hose->global_number, phb->type);
> + }
> +}
> +
> #ifdef CONFIG_PCI_IOV
> static resource_size_t pnv_pci_iov_resource_alignment(struct pci_dev *pdev,
> int resno)
> @@ -3095,6 +3125,7 @@ static const struct pci_controller_ops pnv_pci_ioda_controller_ops = {
> #endif
> .enable_device_hook = pnv_pci_enable_device_hook,
> .window_alignment = pnv_pci_window_alignment,
> + .setup_bridge = pnv_pci_setup_bridge,
> .reset_secondary_bus = pnv_pci_reset_secondary_bus,
> .dma_set_mask = pnv_pci_ioda_dma_set_mask,
> .dma_get_required_mask = pnv_pci_ioda_dma_get_required_mask,
> @@ -3168,6 +3199,9 @@ static void __init pnv_pci_init_ioda_phb(struct device_node *np,
> if (phb->regs == NULL)
> pr_err(" Failed to map registers !\n");
>
> + /* Initialize TCE kill register */
> + pnv_pci_ioda_setup_opal_tce_kill(phb);
> +
> /* Initialize more IODA stuff */
> phb->ioda.total_pe_num = 1;
> prop32 = of_get_property(np, "ibm,opal-num-pes", NULL);
> diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
> index a8ba97f..ef5271a 100644
> --- a/arch/powerpc/platforms/powernv/pci.h
> +++ b/arch/powerpc/platforms/powernv/pci.h
> @@ -121,6 +121,7 @@ struct pnv_phb {
> unsigned int total_pe_num;
> unsigned int reserved_pe_idx;
> unsigned int root_pe_idx;
> + bool root_pe_populated;
>
> /* 32-bit MMIO window */
> unsigned int m32_size;
>

--
Alexey