On Mon, Aug 10, 2015 at 05:40:08PM +1000, Alexey Kardashevskiy wrote:
>On 08/06/2015 02:11 PM, Gavin Shan wrote:
>>There're 3 windows (IO, M32 and M64) for PHB, root port and upstream
>
>These are actually IO, non-prefetchable and prefetchable windows which happen
>to be IO, 32bit and 64bit windows but this has nothing to do with the M32/M64
>BAR registers in P7IOC/PHB3, do I understand this correctly?
>
In pci-ioda.c, we have below definiations that are defined when
developing the code, not from any specification:
IO - resources with IO property
M32 - 32-bits or non-prefetchable resources
M64 - 64-bits and prefetchable resources
>>port of the PCIE switch behind root port. In order to support PCI
>>hotplug, we extend the start/end address of those 3 windows of root
>>port or upstream port to the start/end address of the 3 PHB's windows.
>>The current implementation, assigning IO or M32 segment based on the
>>bridge's windows, isn't reliable.
>>
>>The patch fixes above issue by calculating PE's consumed IO or M32
>>segments from its contained devices, no PCI bridge windows involved
>>if the PE doesn't contain all the subordinate PCI buses.
>
>Please, rephrase it. How can PCI bridges be involved in PE consumption?
>
Ok. Will add something like below:
if the PE, corresponding to the PCI bus, doesn't contain all the subordinate
PCI buses.
>
>>Otherwise,
>>the PCI bridge windows still contribute to PE's consumed IO or M32
>>segments.
>
>PCI bridge windows themselves consume PEs? Is that correct?
>
PCI bridge windows consume IO, M32, M64 segments, not PEs.
>>
>>Signed-off-by: Gavin Shan <gwshan@xxxxxxxxxxxxxxxxxx>
>>---
>> arch/powerpc/platforms/powernv/pci-ioda.c | 136 +++++++++++++++++-------------
>> 1 file changed, 79 insertions(+), 57 deletions(-)
>>
>>diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>>index 488a53e..713f4b4 100644
>>--- a/arch/powerpc/platforms/powernv/pci-ioda.c
>>+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>>@@ -2844,75 +2844,97 @@ 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_setup_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;
>>- unsigned int segsize;
>>+ unsigned int index, segsize;
>> unsigned long *segmap, *pe_segmap;
>> uint16_t win;
>> 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)
>
>res->start >= res->end ?
>
No, res->start == res->end is valid.
>
>>+ 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) {
>>+ region.start = res->start - phb->ioda.io_pci_base;
>>+ region.end = res->end - phb->ioda.io_pci_base;
>>+ segsize = phb->ioda.io_segsize;
>>+ segmap = phb->ioda.io_segmap;
>>+ pe_segmap = pe->io_segmap;
>>+ win = OPAL_IO_WINDOW_TYPE;
>>+ } 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;
>>+ segsize = phb->ioda.m32_segsize;
>>+ segmap = phb->ioda.m32_segmap;
>>+ pe_segmap = pe->m32_segmap;
>>+ win = OPAL_M32_WINDOW_TYPE;
>>+ } else {
>>+ return 0;
>>+ }
>>
>>- if (res->flags & IORESOURCE_IO) {
>>- region.start = res->start - phb->ioda.io_pci_base;
>>- region.end = res->end - phb->ioda.io_pci_base;
>>- segsize = phb->ioda.io_segsize;
>>- segmap = phb->ioda.io_segmap;
>>- pe_segmap = pe->io_segmap;
>>- win = OPAL_IO_WINDOW_TYPE;
>>- } 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;
>>- segsize = phb->ioda.m32_segsize;
>>- segmap = phb->ioda.m32_segmap;
>>- pe_segmap = pe->m32_segmap;
>>- win = OPAL_M32_WINDOW_TYPE;
>>- } else {
>>- continue;
>>+ region.start = _ALIGN_DOWN(region.start, segsize);
>>+ region.end = _ALIGN_UP(region.end, segsize);
>>+ 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, 0, index);
>>+ if (rc != OPAL_SUCCESS) {
>>+ pr_warn("%s: Error %lld mapping (%d) seg#%d to PHB#%d-PE#%d\n",
>>+ __func__, rc, win, index,
>>+ pe->phb->hose->global_number,
>>+ pe->pe_number);
>>+ return -EIO;
>> }
>>
>>- index = region.start / phb->ioda.io_segsize;
>>- while (index < phb->ioda.total_pe &&
>>- region.start <= region.end) {
>>- set_bit(index, segmap);
>>- set_bit(index, pe_segmap);
>>- rc = opal_pci_map_pe_mmio_window(phb->opal_id,
>>- pe->pe_number, win, 0, index);
>>- if (rc != OPAL_SUCCESS) {
>>- pr_warn("%s: Error %lld mapping (%d) seg#%d to PHB#%d-PE#%d\n",
>>- __func__, rc, win, index,
>>- pe->phb->hose->global_number,
>>- pe->pe_number);
>>- break;
>>- }
>>+ set_bit(index, segmap);
>>+ set_bit(index, pe_segmap);
>>+ region.start += segsize;
>>+ index++;
>>+ }
>>+
>>+ return 0;
>>+}
>>+
>>+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;
>>+
>>+ /* 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_setup_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;
>>
>>- region.start += segsize;
>>- index++;
>>+ for (i = 0; i <= PCI_BRIDGE_RESOURCE_NUM; i++) {
>>+ res = &pdev->resource[PCI_BRIDGE_RESOURCES + i];
>>+ if (pnv_ioda_setup_one_res(hose, pe, res))
>>+ return;
>> }
>> }
>> }
>>
Thanks,
Gavin
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
