[PATCH v4 04/21] powerpc/powernv: Improve IO and M32 mapping
Alexey Kardashevskiy
aik at ozlabs.ru
Sat May 9 20:53:38 AEST 2015
On 05/01/2015 04:02 PM, Gavin Shan wrote:
> The PHB's IO or M32 window is divided evenly to segments, each of
> them can be mapped to arbitrary PE# by IODT or M32DT. Current code
> figures out the consumed IO and M32 segments by one particular PE
> from the windows of the PE's upstream bridge. It won't be reliable
> once we extend M64 windows of root port, or the upstream port of
> the PCIE switch behind root port to PHB's IO or M32 window, in order
> to support PCI hotplug in future.
>
> The patch improves pnv_ioda_setup_pe_seg() to calculate PE's consumed
> IO or M32 segments from its contained devices, no bridge involved any
> more. Also, the logic to mapping IO and M32 segments are combined to
> simplify the code. Besides, it's always worthy to trace the IO and M32
> segments consumed by one PE, which can be released at PCI unplugging
> time.
>
> Signed-off-by: Gavin Shan <gwshan at linux.vnet.ibm.com>
> ---
> arch/powerpc/platforms/powernv/pci-ioda.c | 150 ++++++++++++++++--------------
> arch/powerpc/platforms/powernv/pci.h | 13 +--
> 2 files changed, 85 insertions(+), 78 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index a994882..7e6e266 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -2543,77 +2543,92 @@ static void pnv_pci_ioda_fixup_iov_resources(struct pci_dev *pdev)
> }
> #endif /* CONFIG_PCI_IOV */
>
> -/*
> - * This function is supposed to be called on basis of PE from top
> - * to bottom style. So the the I/O or MMIO segment assigned to
> - * parent PE could be overrided by its child PEs if necessary.
> - */
> -static void pnv_ioda_setup_pe_seg(struct pci_controller *hose,
> - struct pnv_ioda_pe *pe)
> +static int pnv_ioda_map_pe_one_res(struct pci_controller *hose,
> + struct pnv_ioda_pe *pe,
> + struct resource *res)
> {
> struct pnv_phb *phb = hose->private_data;
> struct pci_bus_region region;
> - struct resource *res;
> - int i, index;
> - int rc;
> + unsigned int segsize, index;
> + unsigned long *segmap, *pe_segmap;
> + uint16_t win_type;
> + int64_t rc;
>
> - /*
> - * NOTE: We only care PCI bus based PE for now. For PCI
> - * device based PE, for example SRIOV sensitive VF should
> - * be figured out later.
> - */
> - BUG_ON(!(pe->flags & (PNV_IODA_PE_BUS | PNV_IODA_PE_BUS_ALL)));
> + /* Check if we need map the resource */
> + if (!res->parent || !res->flags ||
> + res->start > res->end ||
> + pnv_pci_is_mem_pref_64(res->flags))
> + return 0;
>
> - pci_bus_for_each_resource(pe->pbus, res, i) {
> - if (!res || !res->flags ||
> - res->start > res->end)
> - continue;
> + if (res->flags & IORESOURCE_IO) {
> + segmap = phb->ioda.io_segmap;
> + pe_segmap = pe->io_segmap;
> + region.start = res->start - phb->ioda.io_pci_base;
> + region.end = res->end - phb->ioda.io_pci_base;
> + segsize = phb->ioda.io_segsize;
> + win_type = OPAL_IO_WINDOW_TYPE;
> + } else {
> + segmap = phb->ioda.m32_segmap;
> + pe_segmap = pe->m32_segmap;
> + region.start = res->start -
> + hose->mem_offset[0] -
> + phb->ioda.m32_pci_base;
> + region.end = res->end -
> + hose->mem_offset[0] -
> + phb->ioda.m32_pci_base;
> + segsize = phb->ioda.m32_segsize;
> + win_type = OPAL_M32_WINDOW_TYPE;
> + }
> +
> + index = region.start / segsize;
> + while (index < phb->ioda.total_pe &&
> + region.start <= region.end) {
> + rc = opal_pci_map_pe_mmio_window(phb->opal_id,
> + pe->pe_number, win_type, 0, index);
> + if (rc != OPAL_SUCCESS) {
> + pr_warn("%s: Error %lld mapping (%d) seg#%d to PE#%d\n",
> + __func__, rc, win_type, index, pe->pe_number);
> + return -EIO;
> + }
>
> - if (res->flags & IORESOURCE_IO) {
> - region.start = res->start - phb->ioda.io_pci_base;
> - region.end = res->end - phb->ioda.io_pci_base;
> - index = region.start / phb->ioda.io_segsize;
> + set_bit(index, segmap);
> + set_bit(index, pe_segmap);
> + region.start += segsize;
> + index++;
> + }
>
> - while (index < phb->ioda.total_pe &&
> - region.start <= region.end) {
> - phb->ioda.io_segmap[index] = pe->pe_number;
> - rc = opal_pci_map_pe_mmio_window(phb->opal_id,
> - pe->pe_number, OPAL_IO_WINDOW_TYPE, 0, index);
> - if (rc != OPAL_SUCCESS) {
> - pr_err("%s: OPAL error %d when mapping IO "
> - "segment #%d to PE#%d\n",
> - __func__, rc, index, pe->pe_number);
> - break;
> - }
> + return 0;
> +}
>
> - region.start += phb->ioda.io_segsize;
> - index++;
> - }
> - } else if ((res->flags & IORESOURCE_MEM) &&
> - !pnv_pci_is_mem_pref_64(res->flags)) {
> - region.start = res->start -
> - hose->mem_offset[0] -
> - phb->ioda.m32_pci_base;
> - region.end = res->end -
> - hose->mem_offset[0] -
> - phb->ioda.m32_pci_base;
> - index = region.start / phb->ioda.m32_segsize;
> -
> - while (index < phb->ioda.total_pe &&
> - region.start <= region.end) {
> - phb->ioda.m32_segmap[index] = pe->pe_number;
> - rc = opal_pci_map_pe_mmio_window(phb->opal_id,
> - pe->pe_number, OPAL_M32_WINDOW_TYPE, 0, index);
> - if (rc != OPAL_SUCCESS) {
> - pr_err("%s: OPAL error %d when mapping M32 "
> - "segment#%d to PE#%d",
> - __func__, rc, index, pe->pe_number);
> - break;
> - }
> +static void pnv_ioda_setup_pe_seg(struct pci_controller *hose,
> + struct pnv_ioda_pe *pe)
> +{
> + struct pci_dev *pdev;
> + struct resource *res;
> + int i;
>
> - region.start += phb->ioda.m32_segsize;
> - index++;
> - }
> + /* This function only works for bus dependent PE */
> + BUG_ON(!(pe->flags & (PNV_IODA_PE_BUS | PNV_IODA_PE_BUS_ALL)));
> +
> + list_for_each_entry(pdev, &pe->pbus->devices, bus_list) {
> + for (i = 0; i <= PCI_ROM_RESOURCE; i++) {
> + res = &pdev->resource[i];
> + if (pnv_ioda_map_pe_one_res(hose, pe, res))
> + return;
> + }
> +
> + /* If the PE contains all subordinate PCI buses, the
> + * resources of the child bridges should be mapped
> + * to the PE as well.
> + */
> + if (!(pe->flags & PNV_IODA_PE_BUS_ALL) ||
> + (pdev->class >> 8) != PCI_CLASS_BRIDGE_PCI)
> + continue;
> +
> + for (i = 0; i <= PCI_BRIDGE_RESOURCE_NUM; i++) {
> + res = &pdev->resource[PCI_BRIDGE_RESOURCES + i];
> + if (pnv_ioda_map_pe_one_res(hose, pe, res))
> + return;
This chunk is really hard to review. Looks like you completely
reimplemented the function instead of patching it. For review-ability and
bisect-ability it would help to split it to several simpler patches.
> }
> }
> }
> @@ -2780,7 +2795,7 @@ static void __init pnv_pci_init_ioda_phb(struct device_node *np,
> {
> struct pci_controller *hose;
> struct pnv_phb *phb;
> - unsigned long size, m32map_off, pemap_off, iomap_off = 0;
> + unsigned long size, pemap_off;
> const __be64 *prop64;
> const __be32 *prop32;
> int len;
> @@ -2865,19 +2880,10 @@ static void __init pnv_pci_init_ioda_phb(struct device_node *np,
>
> /* Allocate aux data & arrays. We don't have IO ports on PHB3 */
> size = _ALIGN_UP(phb->ioda.total_pe / 8, sizeof(unsigned long));
> - m32map_off = size;
> - size += phb->ioda.total_pe * sizeof(phb->ioda.m32_segmap[0]);
> - if (phb->type == PNV_PHB_IODA1) {
> - iomap_off = size;
> - size += phb->ioda.total_pe * sizeof(phb->ioda.io_segmap[0]);
> - }
> pemap_off = size;
> size += phb->ioda.total_pe * sizeof(struct pnv_ioda_pe);
> aux = memblock_virt_alloc(size, 0);
> phb->ioda.pe_alloc = aux;
> - phb->ioda.m32_segmap = aux + m32map_off;
> - if (phb->type == PNV_PHB_IODA1)
> - phb->ioda.io_segmap = aux + iomap_off;
> phb->ioda.pe_array = aux + pemap_off;
> set_bit(phb->ioda.reserved_pe, phb->ioda.pe_alloc);
>
> diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
> index 19022cf..f604bb7 100644
> --- a/arch/powerpc/platforms/powernv/pci.h
> +++ b/arch/powerpc/platforms/powernv/pci.h
> @@ -54,6 +54,8 @@ struct pnv_ioda_pe {
> * by slave PEs will be contributed to the master PE. One
> * PE can own multiple IO and M32 segments.
> */
> + unsigned long io_segmap[8];
> + unsigned long m32_segmap[8];
> unsigned long m64_segmap[8];
>
> /* "Weight" assigned to the PE for the sake of DMA resource
> @@ -154,16 +156,15 @@ struct pnv_phb {
> unsigned int io_segsize;
> unsigned int io_pci_base;
>
> - /* PE allocation bitmap */
> + /* PE allocation */
> + struct pnv_ioda_pe *pe_array;
> unsigned long *pe_alloc;
> - /* PE allocation mutex */
> struct mutex pe_alloc_mutex;
>
> - /* M32 & IO segment maps */
> + /* IO/M32/M64 segment bitmaps */
> + unsigned long io_segmap[8];
> + unsigned long m32_segmap[8];
> unsigned long m64_segmap[8];
Is this a copy of the same name fields above, in pnv_ioda_pe? Why 8?
> - unsigned int *m32_segmap;
> - unsigned int *io_segmap;
> - struct pnv_ioda_pe *pe_array;
>
Why moved this?
> /* IRQ chip */
> int irq_chip_init;
>
--
Alexey
More information about the Linuxppc-dev
mailing list