The series of patches intend to support PCI slot for PowerPC PowerNV platform,
which is running on top of skiboot firmware. The patchset requires corresponding
changes from skiboot firmware, which is sent to skiboot@lists.ozlabs.org
for review. The PCI slots are exposed by skiboot with device node properties,
and kernel utilizes those properties to populated PCI slots accordingly.
The original PCI infrastructure on PowerNV platform can't support hotplug
because the PE is assigned during PHB fixup time, which is called for once
during system boot time. For this, the PCI infrastructure on PowerNV platform
has been reworked for a lot. After that, the PE and its corresponding resources
(IODT, M32DT, M64 segments, DMA32 and bypass window) are assigned upon updating
PCI bridge's resources, which might decide PE# assigned to the PE (e.g. M64
resources, on P8 strictly speaking). Each PE will maintain a reference count,
which is (number of child PCI devices + 1). That indicates when last child PCI
device leaves the PE, the PE and its included resources will be relased and put
back into free pool again. With this design, the PE will be released when EEH PE
is released. PATCH[1 - 23] are related to this part.
From skiboot perspective, PCI slot is providing (hot/fundamental/complete)
resets to EEH. The kernel gets to know if skiboot supports various reset on one
particular PCI slot through device-tree node. If it does, EEH will utilize the
functionality provided by skiboot. Besides, the device-tree nodes have to change
in order to support PCI hotplug. For example, when one PCI adapter inserted to
one slot, its device-tree node should be added to the system dynamically. Conversely,
the device-tree node should be removed from the system when the PCI adapter is going
to be offline. Since pci_dn and eeh_dev have same life cyle as PCI device nodes,
they should be added/removed accordingly during PCI hotplug. PATCH[24 - 36] are
doing the related work.
The OF driver is changed to support unflattening FDT blob for sub-stree, which
is covered by PATCH[37 - 41].
The last patch is the standalone PCI hotplug driver for PowerNV platform. When
removing PCI adapter from one PCI slot, which is invoked by command in userland,
the skiboot will power off the slot to save power and remove all device-tree
nodes for all PCI devices behind the slot. Conversely, the Power to the slot
is turned on, the PCI devices behind the slot is rescanned, and the device-tree
nodes for those newly detected PCI devices will be built in skiboot. For both
of cases, one message will be sent to kernel by skiboot so that the kernel
can adjust the device-tree accordingly. At the same time, the kernel also have
to deallocate or allocate PE# and its related resources (PE# and so on) for the
removed/added PCI devices.
Changelog
=========
v6:
* Patch reorder, split, squash - Alexey.
* Minor coding style - Alexey.
* Better function names for pcibios_{add,remove}_pci_devices - Bjorn
* Replace pr_warn() with dev_warn() in PowerNV hotplug driver - Bjorn
* Concurrent depth as paramter passed to __unflatten_dt_node() - Grant / Alexey
* Replace overlay with of_changeset - Grant
v5:
* Rebased to 4.1.rc6 and some unmerged patches as below:
Alexey's DDW patchset (v11);
Gavin's EEH error injection support (in mpe's next branch);
Richard's EEH cleanup patches (in mpe's next branch);
Richard's EEH support for VF (v7);
Gavin's misc EEH fixes for 4.2;
* The revision bases on skiboot corresponding patches (v7):
https://patchwork.ozlabs.org/patch/480437/
* Utilize OF overlay to update device-tree with help of newly introduced
OPAL API opal_get_overlay_dt().
* Split patches for easy review according to aik's comments.
* Fix coding style from checkpatchc.pl as pointed by aik.
* Code cleanup and misc fixup according to aik's input.
v4:
* Rebased to 4.1.RC1
* Added API to unflatten FDT blob to device node sub-tree, which is attached
the indicated parent device node. The original mechanism based on formatted
string stream has been dropped.
* The PATCH[v3 09/21] ("powerpc/eeh: Delay probing EEH device during hotplug")
was picked up sent to linux-ppc@ separately for review as Richard's "VF EEH
Support" depends on that.
v3:
* Rebased to 4.1.RC0
* PowerNV PCI infrasturcture is total refactored in order to support PCI
hotplug. The PowerNV hotplug driver is also reworked a lot because of
the changes in skiboot in order to support PCI hotplug.
Gavin Shan (42):
PCI: Add pcibios_setup_bridge()
powerpc/powernv: Drop pnv_ioda_setup_dev_PE()
powerpc/powernv: Enable M64 on P7IOC
powerpc/powernv: Reorder fields in struct pnv_phb
powerpc/powernv: Track IO/M32/M64 segments from PE
powerpc/powernv: Simplify pnv_ioda_setup_pe_seg()
powerpc/powernv: Improve IO and M32 mapping
powerpc/powernv: Calculate PHB's DMA weight dynamically
powerpc/powernv: DMA32 cleanup
powerpc/powernv: pnv_ioda_setup_dma() configure one PE only
powerpc/powernv: Trace DMA32 segments consumed by PE
powerpc/powernv: Increase PE# capacity
powerpc/pci: Cleanup on pci_controller_ops
powerpc/pci: Override pcibios_setup_bridge()
powerpc/powernv: PE oriented during configuration
powerpc/powernv: Helper function pnv_ioda_init_pe()
powerpc/powernv: Rename PE# fields in PHB
powerpc/powernv: Allocate PE# in deasending order
powerpc/powernv: Reserve PE# for root bus
powerpc/powernv: Create PEs dynamically
powerpc/powernv: Remove DMA32 list of PEs
powerpc/powernv: Move functions around
powerpc/powernv: Release PEs dynamically
powerpc/powernv: Supports slot ID
powerpc/powernv: Use PCI slot reset infrastructure
powerpc/powernv: Simplify pnv_eeh_reset()
powerpc/powernv: Don't cover root bus in pnv_pci_reset_secondary_bus()
powerpc/powernv: Fundamental reset in pnv_pci_reset_secondary_bus()
powerpc/pci: Don't scan empty slot
powerpc/pci: Move pcibios_find_pci_bus() around
powerpc/pci: Rename pcibios_{add,remove}_pci_devices
powerpc/powernv: Introduce pnv_pci_poll()
powerpc/powernv: Functions to get/reset PCI slot status
powerpc/pci: Delay creating pci_dn
powerpc/pci: Export traverse_pci_device_nodes()
powerpc/pci: Update bridge windows on PCI plugging
powerpc/powernv: Select OF_DYNAMIC
drivers/of: Unflatten subordinate nodes after specified level
drivers/of: Allow to specify root node in of_fdt_unflatten_tree()
drivers/of: Return allocated memory chunk from of_fdt_unflatten_tree()
drivers/of: Export OF changeset functions
pci/hotplug: PowerPC PowerNV PCI hotplug driver
MAINTAINERS | 6 +
arch/powerpc/include/asm/eeh.h | 2 +-
arch/powerpc/include/asm/opal-api.h | 8 +-
arch/powerpc/include/asm/opal.h | 9 +-
arch/powerpc/include/asm/pci-bridge.h | 25 +-
arch/powerpc/include/asm/pnv-pci.h | 7 +
arch/powerpc/include/asm/ppc-pci.h | 9 +-
arch/powerpc/kernel/eeh_dev.c | 19 +-
arch/powerpc/kernel/eeh_driver.c | 12 +-
arch/powerpc/kernel/pci-common.c | 16 +-
arch/powerpc/kernel/pci-hotplug.c | 48 +-
arch/powerpc/kernel/pci_dn.c | 71 +-
arch/powerpc/platforms/maple/pci.c | 34 +-
arch/powerpc/platforms/pasemi/pci.c | 3 -
arch/powerpc/platforms/powermac/pci.c | 40 +-
arch/powerpc/platforms/powernv/Kconfig | 1 +
arch/powerpc/platforms/powernv/eeh-powernv.c | 181 +--
arch/powerpc/platforms/powernv/opal-wrappers.S | 4 +
arch/powerpc/platforms/powernv/pci-ioda.c | 1661 ++++++++++++++----------
arch/powerpc/platforms/powernv/pci.c | 92 +-
arch/powerpc/platforms/powernv/pci.h | 63 +-
arch/powerpc/platforms/pseries/pci_dlpar.c | 32 -
arch/powerpc/platforms/pseries/setup.c | 9 +-
drivers/of/dynamic.c | 65 +-
drivers/of/fdt.c | 69 +-
drivers/of/overlay.c | 8 +-
drivers/of/unittest.c | 6 +-
drivers/pci/hotplug/Kconfig | 12 +
drivers/pci/hotplug/Makefile | 4 +
drivers/pci/hotplug/powernv_php.c | 140 ++
drivers/pci/hotplug/powernv_php.h | 92 ++
drivers/pci/hotplug/powernv_php_slot.c | 722 ++++++++++
drivers/pci/hotplug/rpadlpar_core.c | 8 +-
drivers/pci/hotplug/rpaphp_core.c | 4 +-
drivers/pci/hotplug/rpaphp_pci.c | 4 +-
drivers/pci/setup-bus.c | 5 +
include/linux/of.h | 2 +
include/linux/of_fdt.h | 3 +-
include/linux/pci.h | 1 +
39 files changed, 2536 insertions(+), 961 deletions(-)
create mode 100644 drivers/pci/hotplug/powernv_php.c
create mode 100644 drivers/pci/hotplug/powernv_php.h
create mode 100644 drivers/pci/hotplug/powernv_php_slot.c
--
2.1.0
Currently, PowerPC PowerNV platform utilizes ppc_md.pcibios_fixup(),
which is called for once after PCI probing and resource assignment
are completed, to allocate platform required resources for PCI devices:
PE#, IO and MMIO mapping, DMA address translation (TCE) table etc.
Obviously, it's not hotplug friendly.
The patch adds weak function pcibios_setup_bridge(), which is called
by pci_setup_bridge(). PowerPC PowerNV platform will reuse the function
to assign above platform required resources to newly added PCI devices,
in order to support PCI hotplug in subsequent patches.
Signed-off-by: Gavin Shan <redacted>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/setup-bus.c | 5 +++++
include/linux/pci.h | 1 +
2 files changed, 6 insertions(+)
Nobody is using the this function. The patch drops it.
Signed-off-by: Gavin Shan <redacted>
Reviewed-by: Alexey Kardashevskiy <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 71 -------------------------------
1 file changed, 71 deletions(-)
@@ -923,77 +923,6 @@ static int pnv_pci_vf_resource_shift(struct pci_dev *dev, int offset)}#endif /* CONFIG_PCI_IOV */-#if 0-staticstructpnv_ioda_pe*pnv_ioda_setup_dev_PE(structpci_dev*dev)-{-structpci_controller*hose=pci_bus_to_host(dev->bus);-structpnv_phb*phb=hose->private_data;-structpci_dn*pdn=pci_get_pdn(dev);-structpnv_ioda_pe*pe;-intpe_num;--if(!pdn){-pr_err("%s: Device tree node not associated properly\n",-pci_name(dev));-returnNULL;-}-if(pdn->pe_number!=IODA_INVALID_PE)-returnNULL;--/* PE#0 has been pre-set */-if(dev->bus->number==0)-pe_num=0;-else-pe_num=pnv_ioda_alloc_pe(phb);-if(pe_num==IODA_INVALID_PE){-pr_warning("%s: Not enough PE# available, disabling device\n",-pci_name(dev));-returnNULL;-}--/* NOTE: We get only one ref to the pci_dev for the pdn, not for the-*pointerinthePEdatastructure,bothshouldbedestroyedatthe-*sametime.However,thisneedstobelookedatmorecloselyagain-*onceweactuallystartremovingthings(Hotplug,SR-IOV,...)-*-*AtsomepointwewanttoremovethePDNcompletelyanyways-*/-pe=&phb->ioda.pe_array[pe_num];-pci_dev_get(dev);-pdn->pcidev=dev;-pdn->pe_number=pe_num;-pe->pdev=dev;-pe->pbus=NULL;-pe->tce32_seg=-1;-pe->mve_number=-1;-pe->rid=dev->bus->number<<8|pdn->devfn;--pe_info(pe,"Associated device to PE\n");--if(pnv_ioda_configure_pe(phb,pe)){-/* XXX What do we do here ? */-if(pe_num)-pnv_ioda_free_pe(phb,pe_num);-pdn->pe_number=IODA_INVALID_PE;-pe->pdev=NULL;-pci_dev_put(dev);-returnNULL;-}--/* Assign a DMA weight to the device */-pe->dma_weight=pnv_ioda_dma_weight(dev);-if(pe->dma_weight!=0){-phb->ioda.dma_weight+=pe->dma_weight;-phb->ioda.dma_pe_count++;-}--/* Link the PE */-pnv_ioda_link_pe_by_weight(phb,pe);--returnpe;-}-#endif /* Useful for SRIOV case */-staticvoidpnv_ioda_setup_same_PE(structpci_bus*bus,structpnv_ioda_pe*pe){structpci_dev*dev;
The patch enables M64 window on P7IOC, which has been enabled on
PHB3. Different from PHB3 where 16 M64 BARs are supported and each
of them can be owned by one particular PE# exclusively or divided
evenly to 256 segments, each P7IOC PHB has 16 M64 BARs and each
of them are divided into 8 segments. So each P7IOC PHB can support
128 M64 segments only. Also, P7IOC has M64DT, which helps mapping
one particular M64 segment# to arbitrary PE#. PHB3 doesn't have
M64DT, indicating that one M64 segment can only be pinned to the
fixed PE#. In order to have similar logic to support M64 for PHB3
and P7IOC, we just provide 128 M64 (16 BARs) segments and fixed
mapping between PE# and M64 segment# on P7IOC. In turn, we just
need different phb->init_m64() hooks for P7IOC and PHB3 to support
M64.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 116 ++++++++++++++++++++++++++----
1 file changed, 104 insertions(+), 12 deletions(-)
@@ -172,6 +172,69 @@ static void pnv_ioda_free_pe(struct pnv_phb *phb, int pe)clear_bit(pe,phb->ioda.pe_alloc);}+staticintpnv_ioda1_init_m64(structpnv_phb*phb)+{+structresource*r;+intseg;++/* There are as many M64 segments as the maximum number+*ofPEs,whichis128.+*/+for(seg=0;seg<phb->ioda.total_pe;seg+=8){+unsignedlongbase;+int64_trc;++base=phb->ioda.m64_base+seg*phb->ioda.m64_segsize;+rc=opal_pci_set_phb_mem_window(phb->opal_id,+OPAL_M64_WINDOW_TYPE,+seg/8,+base,+0,/* unused */+8*phb->ioda.m64_segsize);+if(rc!=OPAL_SUCCESS){+pr_warn(" Error %lld setting M64 PHB#%d-BAR#%d\n",+rc,phb->hose->global_number,seg/8);+gotofail;+}++rc=opal_pci_phb_mmio_enable(phb->opal_id,+OPAL_M64_WINDOW_TYPE,+seg/8,+OPAL_ENABLE_M64_SPLIT);+if(rc!=OPAL_SUCCESS){+pr_warn(" Error %lld enabling M64 PHB#%d-BAR#%d\n",+rc,phb->hose->global_number,seg/8);+gotofail;+}+}++/* Strip off the segment used by the reserved PE, which+*isexpectedtobe0orlastsupportedPE#.ThePHB's+*firstmemorywindowtracesthe32-bitsMMIOrange+*whilethesecondonetracesthe64-bitsprefetchable+*MMIOrangethatthePHBsupports.+*/+r=&phb->hose->mem_resources[1];+if(phb->ioda.reserved_pe==0)+r->start+=phb->ioda.m64_segsize;+elseif(phb->ioda.reserved_pe==(phb->ioda.total_pe-1))+r->end-=phb->ioda.m64_segsize;+else+pr_warn(" Cannot strip M64 segment for reserved PE#%d\n",+phb->ioda.reserved_pe);++return0;++fail:+for(;seg>=0;seg-=8)+opal_pci_phb_mmio_enable(phb->opal_id,+OPAL_M64_WINDOW_TYPE,+seg/8,+OPAL_DISABLE_M64);++return-EIO;+}+/* The default M64 BAR is shared by all PEs */staticintpnv_ioda2_init_m64(structpnv_phb*phb){
@@ -293,7 +356,7 @@ static int pnv_ioda2_pick_m64_pe(struct pci_bus *bus, bool all)}/* Figure out reserved PE numbers by the PE */-pnv_ioda2_reserve_m64_pe(bus,pe_alloc,all);+pnv_ioda_reserve_m64_pe(bus,pe_alloc,all);/**thecurrentbusmightnotownM64windowandthat'sall
@@ -324,6 +387,26 @@ static int pnv_ioda2_pick_m64_pe(struct pci_bus *bus, bool all)pe->master=master_pe;list_add_tail(&pe->list,&master_pe->slaves);}++/* P7IOC supports M64DT, which helps mapping M64 segment+*tooneparticularPE#.However,PHB3hasfixedmapping+*betweenM64segmentandPE#.Inordertohavesamelogic+*forP7IOCandPHB3,weenforcefixedmappingbetweenM64+*segmentandPE#onP7IOC.+*/+if(phb->type==PNV_PHB_IODA1){+int64_trc;++rc=opal_pci_map_pe_mmio_window(phb->opal_id,+pe->pe_number,+OPAL_M64_WINDOW_TYPE,+pe->pe_number/8,+pe->pe_number%8);+if(rc!=OPAL_SUCCESS)+pr_warn("%s: Error %lld mapping M64 for PHB#%d-PE#%d\n",+__func__,rc,phb->hose->global_number,+pe->pe_number);+}}kfree(pe_alloc);
@@ -338,8 +421,8 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb)constu32*r;u64pci_addr;-/* FIXME: Support M64 for P7IOC */-if(phb->type!=PNV_PHB_IODA2){+if(phb->type!=PNV_PHB_IODA1&&+phb->type!=PNV_PHB_IODA2){pr_info(" Not support M64 window\n");return;}
@@ -372,9 +455,18 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb)/* Use last M64 BAR to cover M64 window */phb->ioda.m64_bar_idx=15;-phb->init_m64=pnv_ioda2_init_m64;-phb->reserve_m64_pe=pnv_ioda2_reserve_m64_pe;-phb->pick_m64_pe=pnv_ioda2_pick_m64_pe;+phb->reserve_m64_pe=pnv_ioda_reserve_m64_pe;+phb->pick_m64_pe=pnv_ioda_pick_m64_pe;+switch(phb->type){+casePNV_PHB_IODA1:+phb->init_m64=pnv_ioda1_init_m64;+break;+casePNV_PHB_IODA2:+phb->init_m64=pnv_ioda2_init_m64;+break;+default:+pr_debug(" M64 not supported\n");+}}staticvoidpnv_ioda_freeze_pe(structpnv_phb*phb,intpe_no)
The patches moves those fields of struct pnv_phb that are related
to PE# allocation around. No logical change.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci.h | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
The patch is adding 6 bitmaps, three to PE and three to PHB, to track
the consumed by one particular PE, which can be released once the PE
is destroyed during PCI unplugging time. Also, we're using fixed
quantity of bits to trace the used IO, M32 and M64 segments by PEs
in one particular PHB.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 29 +++++++++++++++--------------
arch/powerpc/platforms/powernv/pci.h | 18 ++++++++++++++----
2 files changed, 29 insertions(+), 18 deletions(-)
@@ -3175,19 +3185,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(unsignedlong));-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(structpnv_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);
@@ -49,6 +49,15 @@ struct pnv_ioda_pe {/* PE number */unsignedintpe_number;+/* IO/M32/M64 segments consumed by the PE. Each PE can+*haveoneM64segmentatmost,butM64segmentsconsumed+*byslavePEswillbecontributedtothemasterPE.One+*PEcanownmultipleIOandM32segments.+*/+unsignedlongio_segmap[8];+unsignedlongm32_segmap[8];+unsignedlongm64_segmap[8];+/* "Weight" assigned to the PE for the sake of DMA resource*allocations*/
The original implementation of pnv_ioda_setup_pe_seg() configures
IO and M32 segments by separate logics, which can be merged by
by caching @seg_bitmap, @seg_size, @win in advance. The patch
shouldn't cause any behavioural changes.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 68 ++++++++++++++-----------------
1 file changed, 31 insertions(+), 37 deletions(-)
There're 3 windows (IO, M32 and M64) for PHB, root port and upstream
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. Otherwise,
the PCI bridge windows still contribute to PE's consumed IO or M32
segments.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 136 +++++++++++++++++-------------
1 file changed, 79 insertions(+), 57 deletions(-)
@@ -2844,75 +2844,97 @@ static void pnv_pci_ioda_fixup_iov_resources(struct pci_dev *pdev)}#endif /* CONFIG_PCI_IOV */-/*-*ThisfunctionissupposedtobecalledonbasisofPEfromtop-*tobottomstyle.SothetheI/OorMMIOsegmentassignedto-*parentPEcouldbeoverridedbyitschildPEsifnecessary.-*/-staticvoidpnv_ioda_setup_pe_seg(structpci_controller*hose,-structpnv_ioda_pe*pe)+staticintpnv_ioda_setup_one_res(structpci_controller*hose,+structpnv_ioda_pe*pe,+structresource*res){structpnv_phb*phb=hose->private_data;structpci_bus_regionregion;-structresource*res;-inti,index;-unsignedintsegsize;+unsignedintindex,segsize;unsignedlong*segmap,*pe_segmap;uint16_twin;int64_trc;-/*-*NOTE:WeonlycarePCIbusbasedPEfornow.ForPCI-*devicebasedPE,forexampleSRIOVsensitiveVFshould-*befiguredoutlater.-*/-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)+return0;-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;+}elseif((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{+return0;+}-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;-}elseif((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++;+}++return0;+}++staticvoidpnv_ioda_setup_pe_seg(structpci_controller*hose,+structpnv_ioda_pe*pe)+{+structpci_dev*pdev;+structresource*res;+inti;++/* 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+*resourcesofthechildbridgesshouldbemapped+*tothePEaswell.+*/+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;}}}
For P7IOC, the whole available DMA32 space, which is below the
MEM32 space, is divided evenly into 256MB segments. The number
of continuous segments assigned to one particular PE depends on
the PE's DMA weight that is calculated based on the type of each
PCI devices contained in the PE, and PHB's DMA weight which is
accumulative DMA weight of PEs contained in the PHB. It means
that the PHB's DMA weight calculation depends on existing PEs,
which works perfectly now, but not hotplug friendly. As the
whole available DMA32 space can be assigned to one PE on PHB3,
so we don't have the issue on PHB3.
The patch calculates PHB's DMA weight based on the PCI devices
contained in the PHB dynamically so that it's hotplug friendly.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 88 +++++++++++++++----------------
arch/powerpc/platforms/powernv/pci.h | 6 ---
2 files changed, 43 insertions(+), 51 deletions(-)
@@ -927,6 +927,9 @@ static void pnv_ioda_link_pe_by_weight(struct pnv_phb *phb,staticunsignedintpnv_ioda_dma_weight(structpci_dev*dev){+structpci_controller*hose=pci_bus_to_host(dev->bus);+structpnv_phb*phb=hose->private_data;+/* This is quite simplistic. The "base" weight of a device*is10.0meansnoDMAistobeaccountedforit.*/
@@ -939,14 +942,34 @@ static unsigned int pnv_ioda_dma_weight(struct pci_dev *dev)if(dev->class==PCI_CLASS_SERIAL_USB_UHCI||dev->class==PCI_CLASS_SERIAL_USB_OHCI||dev->class==PCI_CLASS_SERIAL_USB_EHCI)-return3;+return3*phb->ioda.tce32_count;/* Increase the weight of RAID (includes Obsidian) */if((dev->class>>8)==PCI_CLASS_STORAGE_RAID)-return15;+return15*phb->ioda.tce32_count;/* Default */-return10;+return10*phb->ioda.tce32_count;+}++staticint__pnv_ioda_phb_dma_weight(structpci_dev*pdev,void*data)+{+unsignedint*dma_weight=data;++*dma_weight+=pnv_ioda_dma_weight(pdev);+return0;+}++staticunsignedintpnv_ioda_phb_dma_weight(structpnv_phb*phb)+{+unsignedintdma_weight=0;++if(!phb->hose->bus)+return0;++pci_walk_bus(phb->hose->bus,+__pnv_ioda_phb_dma_weight,&dma_weight);+returndma_weight;}#ifdef CONFIG_PCI_IOV
@@ -1097,14 +1120,6 @@ static void pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all)/* Put PE to the list */list_add_tail(&pe->list,&phb->ioda.pe_list);-/* Account for one DMA PE if at least one DMA capable device exist-*belowthebridge-*/-if(pe->dma_weight!=0){-phb->ioda.dma_weight+=pe->dma_weight;-phb->ioda.dma_pe_count++;-}-/* Link the PE */pnv_ioda_link_pe_by_weight(phb,pe);}
@@ -2431,24 +2446,13 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,staticvoidpnv_ioda_setup_dma(structpnv_phb*phb){structpci_controller*hose=phb->hose;-unsignedintresidual,remaining,segs,tw,base;structpnv_ioda_pe*pe;+unsignedintdma_weight;-/* If we have more PE# than segments available, hand out one-*perPEuntilwerunoutandlettherestfail.Ifnot,-*thenweassignatleastonesegmentperPE,plusmorebased-*ontheamountofdevicesunderthatPE-*/-if(phb->ioda.dma_pe_count>phb->ioda.tce32_count)-residual=0;-else-residual=phb->ioda.tce32_count--phb->ioda.dma_pe_count;--pr_info("PCI: Domain %04x has %ld available 32-bit DMA segments\n",-hose->global_number,phb->ioda.tce32_count);-pr_info("PCI: %d PE# for a total weight of %d\n",-phb->ioda.dma_pe_count,phb->ioda.dma_weight);+/* Calculate the PHB's DMA weight */+dma_weight=pnv_ioda_phb_dma_weight(phb);+pr_info("PCI%04x has %ld DMA32 segments, total weight %d\n",+hose->global_number,phb->ioda.tce32_count,dma_weight);pnv_pci_ioda_setup_opal_tce_kill(phb);
@@ -183,12 +183,6 @@ struct pnv_phb {/* 32-bit TCE tables allocation */unsignedlongtce32_count;-/* Total "weight" for the sake of DMA resources-*allocation-*/-unsignedintdma_weight;-unsignedintdma_pe_count;-/* Sorted list of used PE's, sorted at*bootforresourceallocationpurposes*/
The patch cleans up DMA32 in pci-ioda.c. It shouldn't introduce
behavioural changes:
* Rename various fields in "struct pnv_phb" and "struct pnv_ioda_pe"
as 32-bits DMA should be related to "DMA", not "TCE".
* Removed struct pnv_ioda_pe::tce32_segcount.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 48 +++++++++++++++----------------
arch/powerpc/platforms/powernv/pci.h | 7 ++---
2 files changed, 27 insertions(+), 28 deletions(-)
@@ -1936,7 +1936,7 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,/* XXX FIXME: Allocate multi-level tables on PHB3 *//* We shouldn't already have a 32-bit DMA associated */-if(WARN_ON(pe->tce32_seg>=0))+if(WARN_ON(pe->dma32_seg>=0))return;tbl=pnv_pci_table_alloc(phb->hose->node);
@@ -1945,7 +1945,7 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,pnv_pci_link_table_and_group(phb->hose->node,0,tbl,&pe->table_group);/* Grab a 32-bit TCE table */-pe->tce32_seg=base;+pe->dma32_seg=base;pe_info(pe," Setting up 32-bit TCE table at %08x..%08x\n",(base<<28),((base+segs)<<28)-1);
@@ -2006,8 +2006,8 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,return;fail:/* XXX Failure: Try to fallback to 64-bit only ? */-if(pe->tce32_seg>=0)-pe->tce32_seg=-1;+if(pe->dma32_seg>=0)+pe->dma32_seg=-1;if(tce_mem)__free_pages(tce_mem,get_order(TCE32_TABLE_SIZE*segs));if(tbl){
@@ -2405,7 +2405,7 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,int64_trc;/* We shouldn't already have a 32-bit DMA associated */-if(WARN_ON(pe->tce32_seg>=0))+if(WARN_ON(pe->dma32_seg>=0))return;/* TVE #1 is selected by PCI address bit 59 */
@@ -2415,7 +2415,7 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,pe->pe_number);/* The PE will reserve all possible 32-bits space */-pe->tce32_seg=0;+pe->dma32_seg=0;pe_info(pe,"Setting up 32-bit TCE table at 0..%08x\n",phb->ioda.m32_pci_base);
@@ -3211,7 +3211,7 @@ static void __init pnv_pci_init_ioda_phb(struct device_node *np,mutex_init(&phb->ioda.pe_list_mutex);/* Calculate how many 32-bit TCE segments we have */-phb->ioda.tce32_count=phb->ioda.m32_pci_base>>28;+phb->ioda.dma32_segcount=phb->ioda.m32_pci_base>>28;#if 0 /* We should really do that ... */rc=opal_pci_set_phb_mem_window(opal->phb_id,
@@ -61,11 +61,10 @@ struct pnv_ioda_pe {/* "Weight" assigned to the PE for the sake of DMA resource*allocations*/-unsignedintdma_weight;+unsignedintdma32_weight;/* "Base" iommu table, ie, 4K TCEs, 32-bit DMA */-inttce32_seg;-inttce32_segcount;+intdma32_seg;structiommu_table_grouptable_group;/* 64-bit TCE bypass region */
@@ -181,7 +180,7 @@ struct pnv_phb {unsignedcharpe_rmap[0x10000];/* 32-bit TCE tables allocation */-unsignedlongtce32_count;+unsignedlongdma32_segcount;/* Sorted list of used PE's, sorted at*bootforresourceallocationpurposes
The original implementation of pnv_ioda_setup_dma() iterates the
list of PEs and configures the DMA32 space for them one by one.
The function was designed to be called during PHB fixup time.
When configuring PE's DMA32 space in pcibios_setup_bridge(), in
order to support PCI hotplug, we have to have the function PE
oriented.
This renames pnv_ioda_setup_dma() to pnv_ioda1_setup_dma() and
adds one more argument "struct pnv_ioda_pe *pe" to it. The caller,
pnv_pci_ioda_setup_DMA(), gets PE from the list and passes to it
or pnv_pci_ioda2_setup_dma_pe(). The patch shouldn't cause behavioral
changes.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 75 +++++++++++++++----------------
1 file changed, 36 insertions(+), 39 deletions(-)
@@ -2443,52 +2443,29 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,pnv_ioda_setup_bus_dma(pe,pe->pbus);}-staticvoidpnv_ioda_setup_dma(structpnv_phb*phb)+staticunsignedintpnv_ioda1_setup_dma(structpnv_phb*phb,+structpnv_ioda_pe*pe,+unsignedintbase){structpci_controller*hose=phb->hose;-structpnv_ioda_pe*pe;-unsignedintdma_weight;+unsignedintdma_weight,segs;/* Calculate the PHB's DMA weight */dma_weight=pnv_ioda_phb_dma_weight(phb);pr_info("PCI%04x has %ld DMA32 segments, total weight %d\n",hose->global_number,phb->ioda.dma32_segcount,dma_weight);-pnv_pci_ioda_setup_opal_tce_kill(phb);--/* Walk our PE list and configure their DMA segments, hand them-*outonebasesegmentplusanyresidualsegmentsbasedon-*weight-*/-list_for_each_entry(pe,&phb->ioda.pe_dma_list,dma_link){-if(!pe->dma32_weight)-continue;--/*-*ForIODA2compliantPHB3,weneedn'tcareabouttheweight.-*Theallavailable32-bitsDMAspacewillbeassignedto-*thespecificPE.-*/-if(phb->type==PNV_PHB_IODA1){-unsignedintsegs,base=0;--if(pe->dma32_weight<-dma_weight/phb->ioda.dma32_segcount)-segs=1;-else-segs=(pe->dma32_weight*-phb->ioda.dma32_segcount)/dma_weight;--pe_info(pe,"DMA32 weight %d, assigned %d segments\n",-pe->dma32_weight,segs);-pnv_pci_ioda_setup_dma_pe(phb,pe,base,segs);+if(pe->dma32_weight<+dma_weight/phb->ioda.dma32_segcount)+segs=1;+else+segs=(pe->dma32_weight*+phb->ioda.dma32_segcount)/dma_weight;+pe_info(pe,"DMA weight %d, assigned %d segments\n",+pe->dma32_weight,segs);+pnv_pci_ioda_setup_dma_pe(phb,pe,base,segs);-base+=segs;-}else{-pe_info(pe,"Assign DMA32 space\n");-pnv_pci_ioda2_setup_dma_pe(phb,pe);-}-}+returnsegs;}#ifdef CONFIG_PCI_MSI
@@ -2955,12 +2932,32 @@ static void pnv_pci_ioda_setup_DMA(void){structpci_controller*hose,*tmp;structpnv_phb*phb;+structpnv_ioda_pe*pe;+unsignedintbase;list_for_each_entry_safe(hose,tmp,&hose_list,list_node){-pnv_ioda_setup_dma(hose->private_data);+phb=hose->private_data;+pnv_pci_ioda_setup_opal_tce_kill(phb);++base=0;+list_for_each_entry(pe,&phb->ioda.pe_dma_list,dma_link){+if(!pe->dma32_weight)+continue;++switch(phb->type){+casePNV_PHB_IODA1:+base+=pnv_ioda1_setup_dma(phb,pe,base);+break;+casePNV_PHB_IODA2:+pnv_pci_ioda2_setup_dma_pe(phb,pe);+break;+default:+pr_warn("%s: No DMA for PHB type %d\n",+__func__,phb->type);+}+}/* Mark the PHB initialization done */-phb=hose->private_data;phb->initialized=1;}}
On P7IOC, the whole DMA32 space is divided evenly to 256MB segments.
Each PE can consume one or multiple DMA32 segments. Current code
doesn't trace the available DMA32 segments and those consumed by
one particular PE. It's conflicting with PCI hotplug.
The patch introduces one bitmap to PHB to trace the available
DMA32 segments for allocation, more fields to "struct pnv_ioda_pe"
to trace the consumed DMA32 segments by the PE, which is going to
be released when the PE is destroyed at PCI unplugging time.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 40 +++++++++++++++++++++++--------
arch/powerpc/platforms/powernv/pci.h | 4 +++-
2 files changed, 33 insertions(+), 11 deletions(-)
Each PHB maintains one instance of "struct pci_controller_ops",
which includes various callbacks called by PCI subsystem. In the
definition of this struct, some callbacks have explicit names for
its arguments, but the left don't have.
This adds all explicit names of the arguments to the callbacks in
"struct pci_controller_ops" so that the code looks consistent.
Cc: Daniel Axtens <redacted>
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/include/asm/pci-bridge.h | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
@@ -21,18 +21,19 @@ struct pci_controller_ops {void(*dma_dev_setup)(structpci_dev*dev);void(*dma_bus_setup)(structpci_bus*bus);-int(*probe_mode)(structpci_bus*);+int(*probe_mode)(structpci_bus*bus);/* Called when pci_enable_device() is called. Returns true to*allowassignment/enablingofthedevice.*/-bool(*enable_device_hook)(structpci_dev*);+bool(*enable_device_hook)(structpci_dev*dev);-void(*disable_device)(structpci_dev*);+void(*disable_device)(structpci_dev*dev);-void(*release_device)(structpci_dev*);+void(*release_device)(structpci_dev*dev);/* Called during PCI resource reassignment */-resource_size_t(*window_alignment)(structpci_bus*,unsignedlongtype);+resource_size_t(*window_alignment)(structpci_bus*bus,+unsignedlongtype);void(*reset_secondary_bus)(structpci_dev*dev);#ifdef CONFIG_PCI_MSI
The patch overrides pcibios_setup_bridge(), called to update PCI
bridge windows at completion of PCI resource assignment, to assign
PE and setup various (resource) mapping in next patch.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/include/asm/pci-bridge.h | 2 ++
arch/powerpc/kernel/pci-common.c | 8 ++++++++
2 files changed, 10 insertions(+)
Several functions used to configure PE take pe_number to indentify
PE instance. As the pe_number is included in PE instance after it
is reserved or allocated. It's convienent for those functions to
return PE instance which includes the required pe_number.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 51 ++++++++++++++++---------------
arch/powerpc/platforms/powernv/pci.h | 2 +-
2 files changed, 27 insertions(+), 26 deletions(-)
@@ -132,12 +132,12 @@ static inline bool pnv_pci_is_mem_pref_64(unsigned long flags)(IORESOURCE_MEM_64|IORESOURCE_PREFETCH));}-staticvoidpnv_ioda_reserve_pe(structpnv_phb*phb,intpe_no)+staticstructpnv_ioda_pe*pnv_ioda_reserve_pe(structpnv_phb*phb,intpe_no){if(!(pe_no>=0&&pe_no<phb->ioda.total_pe)){pr_warn("%s: Invalid PE %d on PHB#%x\n",__func__,pe_no,phb->hose->global_number);-return;+returnNULL;}if(test_and_set_bit(pe_no,phb->ioda.pe_alloc))
@@ -344,7 +346,7 @@ static int pnv_ioda_pick_m64_pe(struct pci_bus *bus, bool all)/* Root bus shouldn't use M64 */if(pci_is_root_bus(bus))-returnIODA_INVALID_PE;+returnNULL;/* Allocate bitmap */size=_ALIGN_UP(phb->ioda.total_pe/8,sizeof(unsignedlong));
@@ -352,7 +354,7 @@ static int pnv_ioda_pick_m64_pe(struct pci_bus *bus, bool all)if(!pe_alloc){pr_warn("%s: Out of memory !\n",__func__);-returnIODA_INVALID_PE;+returnNULL;}/* Figure out reserved PE numbers by the PE */
@@ -1069,28 +1071,26 @@ static void pnv_ioda_setup_same_PE(struct pci_bus *bus, struct pnv_ioda_pe *pe)*subordinatePCIdevicesandbuses.ThesecondtypeofPEisnormally*orgiriatedbyPCIe-to-PCIbridgeorPLXswitchdownstreamports.*/-staticvoidpnv_ioda_setup_bus_PE(structpci_bus*bus,boolall)+staticstructpnv_ioda_pe*pnv_ioda_setup_bus_PE(structpci_bus*bus,boolall){structpci_controller*hose=pci_bus_to_host(bus);structpnv_phb*phb=hose->private_data;-structpnv_ioda_pe*pe;-intpe_num=IODA_INVALID_PE;+structpnv_ioda_pe*pe=NULL;/* Check if PE is determined by M64 */if(phb->pick_m64_pe)-pe_num=phb->pick_m64_pe(bus,all);+pe=phb->pick_m64_pe(bus,all);/* The PE number isn't pinned by M64 */-if(pe_num==IODA_INVALID_PE)-pe_num=pnv_ioda_alloc_pe(phb);+if(!pe)+pe=pnv_ioda_alloc_pe(phb);-if(pe_num==IODA_INVALID_PE){-pr_warning("%s: Not enough PE# available for PCI bus %04x:%02x\n",+if(!pe){+pr_warning("%s: No enough PE# for PCI bus %04x:%02x\n",__func__,pci_domain_nr(bus),bus->number);-return;+returnNULL;}-pe=&phb->ioda.pe_array[pe_num];pe->flags|=(all?PNV_IODA_PE_BUS_ALL:PNV_IODA_PE_BUS);pe->pbus=bus;pe->pdev=NULL;
@@ -1101,17 +1101,16 @@ static void pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all)if(all)pe_info(pe,"Secondary bus %d..%d associated with PE#%d\n",-bus->busn_res.start,bus->busn_res.end,pe_num);+bus->busn_res.start,bus->busn_res.end,pe->pe_number);elsepe_info(pe,"Secondary bus %d associated with PE#%d\n",-bus->busn_res.start,pe_num);+bus->busn_res.start,pe->pe_number);if(pnv_ioda_configure_pe(phb,pe)){/* XXX What do we do here ? */-if(pe_num)-pnv_ioda_free_pe(phb,pe_num);+pnv_ioda_free_pe(phb,pe->pe_number);pe->pbus=NULL;-return;+returnNULL;}/* Associate it with all child devices */
@@ -1122,6 +1121,8 @@ static void pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all)/* Link the PE */pnv_ioda_link_pe_by_weight(phb,pe);++returnpe;}staticvoidpnv_ioda_setup_PEs(structpci_bus*bus)
The patch introduces helper function pnv_ioda_init_pe(), which
initialize PE instance after reserving or allocating PE#, to
simplify the code. The patch doesn't introduce behavioural
changes.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
@@ -132,6 +132,17 @@ static inline bool pnv_pci_is_mem_pref_64(unsigned long flags)(IORESOURCE_MEM_64|IORESOURCE_PREFETCH));}+staticstructpnv_ioda_pe*pnv_ioda_init_pe(structpnv_phb*phb,intpe_no)+{+structpnv_ioda_pe*pe=&phb->ioda.pe_array[pe_no];++pe->phb=phb;+pe->pe_number=pe_no;+INIT_LIST_HEAD(&pe->list);++returnpe;+}+staticstructpnv_ioda_pe*pnv_ioda_reserve_pe(structpnv_phb*phb,intpe_no){if(!(pe_no>=0&&pe_no<phb->ioda.total_pe)){
@@ -144,10 +155,7 @@ static struct pnv_ioda_pe *pnv_ioda_reserve_pe(struct pnv_phb *phb, int pe_no)pr_debug("%s: PE %d was reserved on PHB#%x\n",__func__,pe_no,phb->hose->global_number);-phb->ioda.pe_array[pe_no].phb=phb;-phb->ioda.pe_array[pe_no].pe_number=pe_no;--return&phb->ioda.pe_array[pe_no];+returnpnv_ioda_init_pe(phb,pe_no);}staticstructpnv_ioda_pe*pnv_ioda_alloc_pe(structpnv_phb*phb)
The available PE#, represented by a bitmap in the PHB, is allocated
in ascending order. It conflicts with the fact that M64 segments are
assigned in same order. In order to avoid the conflict, the patch
allocates PE# in descending order.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
pcibios_setup_bridge() is normally called to update PCI bridge
windows. It allocates PE for PCI buses. However it is not called
on a root bus which does not have an upstream bridge.
This reserves PE# for a root bus in advance. This will be used in
the subsequent patch to do setup.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 30 +++++++++++++++++++++++++++++-
arch/powerpc/platforms/powernv/pci.h | 1 +
2 files changed, 30 insertions(+), 1 deletion(-)
@@ -236,6 +236,13 @@ static int pnv_ioda1_init_m64(struct pnv_phb *phb)pr_warn(" Cannot strip M64 segment for reserved PE#%d\n",phb->ioda.reserved_pe_idx);+/* Strip off the M64 segment corresponding to the PE#+*forPCIrootbus,whichislastsupportedPE#or+*(reservedPE#-1).+*/+if(phb->ioda.root_pe_idx!=IODA_INVALID_PE)+r->end-=phb->ioda.m64_segsize;+return0;fail:
@@ -293,6 +300,13 @@ static int pnv_ioda2_init_m64(struct pnv_phb *phb)pr_warn(" Cannot strip M64 segment for reserved PE#%d\n",phb->ioda.reserved_pe_idx);+/* Strip off the M64 segment corresponding to the PE#+*forPCIrootbus,whichislastsupportedPE#or+*(reservedPE#-1).+*/+if(phb->ioda.root_pe_idx!=IODA_INVALID_PE)+r->end-=phb->ioda.m64_segsize;+return0;fail:
@@ -3237,7 +3251,21 @@ static void __init pnv_pci_init_ioda_phb(struct device_node *np,aux=memblock_virt_alloc(size,0);phb->ioda.pe_alloc=aux;phb->ioda.pe_array=aux+pemap_off;-set_bit(phb->ioda.reserved_pe_idx,phb->ioda.pe_alloc);++/* Choose number of PE for root bus, which shouldn't consume+*anyM64resource.Soweavoidpickinglow-endPE#,which+*isusuallyboundwithM64resourcesclosely.+*/+pnv_ioda_reserve_pe(phb,phb->ioda.reserved_pe_idx);+if(phb->ioda.reserved_pe_idx==0){+phb->ioda.root_pe_idx=phb->ioda.total_pe_num-1;+pnv_ioda_reserve_pe(phb,phb->ioda.root_pe_idx);+}elseif(phb->ioda.reserved_pe_idx==(phb->ioda.total_pe_num-1)){+phb->ioda.root_pe_idx=phb->ioda.reserved_pe_idx-1;+pnv_ioda_reserve_pe(phb,phb->ioda.root_pe_idx);+}else{+phb->ioda.root_pe_idx=IODA_INVALID_PE;+}INIT_LIST_HEAD(&phb->ioda.pe_dma_list);INIT_LIST_HEAD(&phb->ioda.pe_list);
Currently, the PEs and their associated resources are assigned
in ppc_md.pcibios_fixup() except those consumed by SRIOV VFs.
The function is called for once after PCI probing and resources
assignment is finished which isn't hotplug friendly.
The patch 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 finished. 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.
Besides, it might require additional resources (e.g. M32) to the
windows of the PCI bridge when unplugging current adapter, and
insert a different adapter if there is one PCI slot, which is
assumed behind root port, or the downstream bridge of the PCIE
switch behind root port. The parent bridge of the newly plugged
adapter would reject the request to add more resources, leading
to hotplug failure. For the issue, the patch extends the windows
of root port, or the upstream port of the PCIe switch behind root
port to PHB's windows when ppc_md.pcibios_setup_bridge() is called.
There is no upstream bridge for root bus, so we have to fix it up
before any PE is created because the root bus PE is the ancestor
to anyone else.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 226 ++++++++++++++++++------------
arch/powerpc/platforms/powernv/pci.h | 1 +
2 files changed, 137 insertions(+), 90 deletions(-)
@@ -1083,6 +1083,13 @@ static void pnv_ioda_setup_same_PE(struct pci_bus *bus, struct pnv_ioda_pe *pe)pci_name(dev));continue;}++/* The PCI device might be not detached from the+*PEinpartialhotplugcase.+*/+if(pdn->pe_number!=IODA_INVALID_PE)+continue;+pdn->pe_number=pe->pe_number;pe->dma32_weight+=pnv_ioda_dma_weight(dev);if((pe->flags&PNV_IODA_PE_BUS_ALL)&&dev->subordinate)
@@ -1101,9 +1108,27 @@ static struct pnv_ioda_pe *pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all)structpci_controller*hose=pci_bus_to_host(bus);structpnv_phb*phb=hose->private_data;structpnv_ioda_pe*pe=NULL;+intpe_num;++/* For partial hotplug case, the PE instance hasn't been destroyed+*yet.Weshouldn'tallocatedanewoneandassignresourcesto+*it.TheexistingPEinstanceshouldbereused,butweshould+*associatethedevicestothePE.+*/+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);+returnNULL;+}++/* 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 */
@@ -2962,52 +2947,6 @@ static void pnv_ioda_setup_pe_seg(struct pci_controller *hose,}}-staticvoidpnv_pci_ioda_setup_seg(void)-{-structpci_controller*tmp,*hose;-structpnv_phb*phb;-structpnv_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(hose,pe);-}-}-}--staticvoidpnv_pci_ioda_setup_DMA(void)-{-structpci_controller*hose,*tmp;-structpnv_phb*phb;-structpnv_ioda_pe*pe;--list_for_each_entry_safe(hose,tmp,&hose_list,list_node){-phb=hose->private_data;-pnv_pci_ioda_setup_opal_tce_kill(phb);--list_for_each_entry(pe,&phb->ioda.pe_dma_list,dma_link){-if(!pe->dma32_weight)-continue;--switch(phb->type){-casePNV_PHB_IODA1:-pnv_ioda1_setup_dma(phb,pe);-break;-casePNV_PHB_IODA2:-pnv_pci_ioda2_setup_dma_pe(phb,pe);-break;-default:-pr_warn("%s: No DMA for PHB type %d\n",-__func__,phb->type);-}-}--/* Mark the PHB initialization done */-phb->initialized=1;-}-}-staticvoidpnv_pci_ioda_create_dbgfs(void){#ifdef CONFIG_DEBUG_FS
@@ -3082,6 +3026,105 @@ static resource_size_t pnv_pci_window_alignment(struct pci_bus *bus,returnphb->ioda.io_segsize;}+/*+*Weareupdatingrootportortheupstreambridgebehindthe+*rootportwithPHB'swindowsinordertoaccommodatethe+*changesonrequiredresourcesduringPCI(slot)hotplug,+*whichisconnectedtoeitherrootportorthedownstream+*portsofPCIeswitchbehindtherootport.+*/+staticvoidpnv_pci_fixup_bridge_resources(structpci_bus*bus,+unsignedlongtype)+{+structpci_controller*hose=pci_bus_to_host(bus);+structpnv_phb*phb=hose->private_data;+structpci_dev*bridge=bus->self;+structresource*r,*w;+inti;++/* 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 */+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;+elseif(pnv_pci_is_mem_pref_64(r->flags)&&+(type&IORESOURCE_PREFETCH)&&+phb->ioda.m64_segsize)+w=&hose->mem_resources[1];+elseif(r->flags&type&IORESOURCE_MEM)+w=&hose->mem_resources[0];++r->start=w->start;+r->end=w->end;+}+}++staticvoidpnv_pci_setup_bridge(structpci_bus*bus,+unsignedlongtype)+{+structpci_controller*hose=pci_bus_to_host(bus);+structpnv_phb*phb=hose->private_data;+structpci_dev*bridge=bus->self;+structpnv_ioda_pe*pe;+boolall=(pci_pcie_type(bridge)==PCI_EXP_TYPE_PCI_BRIDGE);++/* The root bus (ancestor PE) should be finalized+*beforeanyoneelse+*/+if(!phb->ioda.root_pe_is_populated){+pe=pnv_ioda_setup_bus_PE(phb->hose->bus,false);+if(pe&&phb->ioda.root_pe_idx==IODA_INVALID_PE)+phb->ioda.root_pe_idx=pe->pe_number;+phb->ioda.root_pe_is_populated=true;+}++/* Extend bridge's windows if necessary */+pnv_pci_fixup_bridge_resources(bus,type);++/* Don't assign PE to bus which doesn't have any+*subordinatePCIdevices.+*/+if(list_empty(&bus->devices))+return;++/* Reserve PEs for M64 resource */+if(phb->reserve_m64_pe)+phb->reserve_m64_pe(bus,NULL,all);++/* Assign PE. We might run here because of partial hotplug.+*Forthecase,wejustpickuptheexistingPEandshould+*notallocateresourcesagain.+*/+pe=pnv_ioda_setup_bus_PE(bus,all);+if(!pe)+return;++/* Setup MMIO mapping */+pnv_ioda_setup_pe_seg(hose,pe);++/* Setup DMA */+switch(phb->type){+casePNV_PHB_IODA1:+pnv_ioda1_setup_dma(phb,pe);+break;+casePNV_PHB_IODA2:+pnv_pci_ioda2_setup_dma_pe(phb,pe);+break;+default:+pr_warn("%s: No DMA for PHB type %d\n",+__func__,phb->type);+}+}+#ifdef CONFIG_PCI_IOVstaticresource_size_tpnv_pci_iov_resource_alignment(structpci_dev*pdev,intresno)
Every PHB maintains a list of PEs based on their DMA32 weight. After
patch "powerpc/powernv: Create PEs dynamically", the list is useless
and it's safe to remove it.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 18 ------------------
arch/powerpc/platforms/powernv/pci.h | 6 ------
2 files changed, 24 deletions(-)
@@ -1169,9 +1155,6 @@ static struct pnv_ioda_pe *pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all)/* Put PE to the list */list_add_tail(&pe->list,&phb->ioda.pe_list);-/* Link the PE */-pnv_ioda_link_pe_by_weight(phb,pe);-returnpe;}
The patch moves functions related to releasing PE around so that
we don't need extra declaration for them in subsequent patches.
Also, it fixes warnings from scripts/checkpatch.pl. It doesn't
introduce any behavioural changes.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 743 +++++++++++++++---------------
1 file changed, 377 insertions(+), 366 deletions(-)
@@ -132,6 +132,295 @@ static inline bool pnv_pci_is_mem_pref_64(unsigned long flags)(IORESOURCE_MEM_64|IORESOURCE_PREFETCH));}+staticinlinevoidpnv_pci_ioda2_tce_invalidate_entire(structpnv_ioda_pe*pe)+{+/* 01xb - invalidate TCEs that match the specified PE# */+unsignedlongval=(0x4ull<<60)|(pe->pe_number&0xFF);+structpnv_phb*phb=pe->phb;++if(!phb->ioda.tce_inval_reg)+return;++mb();/* Ensure above stores are visible */+__raw_writeq(cpu_to_be64(val),phb->ioda.tce_inval_reg);+}++#if defined(CONFIG_IOMMU_API) || defined(CONFIG_PCI_IOV)+staticlongpnv_pci_ioda2_unset_window(structiommu_table_group*table_group,+intnum)+{+structpnv_ioda_pe*pe=container_of(table_group,structpnv_ioda_pe,+table_group);+structpnv_phb*phb=pe->phb;+longret;++pe_info(pe,"Removing DMA window #%d\n",num);++ret=opal_pci_map_pe_dma_window(phb->opal_id,pe->pe_number,+(pe->pe_number<<1)+num,+0/* levels */,0/* table address */,+0/* table size */,0/* page size */);+if(ret)+pe_warn(pe,"Unmapping failed, ret = %ld\n",ret);+else+pnv_pci_ioda2_tce_invalidate_entire(pe);++pnv_pci_unlink_table_and_group(table_group->tables[num],table_group);++returnret;+}+#endif /* CONFIG_IOMMU_API || CONFIG_PCI_IOV */++staticvoidpnv_pci_ioda2_set_bypass(structpnv_ioda_pe*pe,boolenable)+{+uint16_twindow_id=(pe->pe_number<<1)+1;+int64_trc;++pe_info(pe,"%sabling 64-bit DMA bypass\n",enable?"En":"Dis");+if(enable){+phys_addr_ttop=memblock_end_of_DRAM();++top=roundup_pow_of_two(top);+rc=opal_pci_map_pe_dma_window_real(pe->phb->opal_id,+pe->pe_number,+window_id,+pe->tce_bypass_base,+top);+}else{+rc=opal_pci_map_pe_dma_window_real(pe->phb->opal_id,+pe->pe_number,+window_id,+pe->tce_bypass_base,+0);+}+if(rc)+pe_err(pe,"OPAL error %lld configuring bypass window\n",rc);+else+pe->tce_bypass_enabled=enable;+}++#ifdef CONFIG_PCI_IOV+staticvoidpnv_pci_ioda2_release_dma_pe(structpci_dev*dev,+structpnv_ioda_pe*pe)+{+structiommu_table*tbl;+int64_trc;++tbl=pe->table_group.tables[0];+rc=pnv_pci_ioda2_unset_window(&pe->table_group,0);+if(rc)+pe_warn(pe,"OPAL error %ld release DMA window\n",rc);++pnv_pci_ioda2_set_bypass(pe,false);+if(pe->table_group.group){+iommu_group_put(pe->table_group.group);+BUG_ON(pe->table_group.group);+}+pnv_pci_ioda2_table_free_pages(tbl);+iommu_free_table(tbl,of_node_full_name(dev->dev.of_node));+}+#endif /* CONFIG_PCI_IOV */++staticintpnv_ioda_set_one_peltv(structpnv_phb*phb,+structpnv_ioda_pe*parent,+structpnv_ioda_pe*child,+boolis_add)+{+constchar*desc=is_add?"adding":"removing";+uint8_top=is_add?OPAL_ADD_PE_TO_DOMAIN:+OPAL_REMOVE_PE_FROM_DOMAIN;+structpnv_ioda_pe*slave;+longrc;++/* Parent PE affects child PE */+rc=opal_pci_set_peltv(phb->opal_id,parent->pe_number,+child->pe_number,op);+if(rc!=OPAL_SUCCESS){+pe_warn(child,"OPAL error %ld %s to parent PELTV\n",+rc,desc);+return-ENXIO;+}++if(!(child->flags&PNV_IODA_PE_MASTER))+return0;++/* Compound case: parent PE affects slave PEs */+list_for_each_entry(slave,&child->slaves,list){+rc=opal_pci_set_peltv(phb->opal_id,parent->pe_number,+slave->pe_number,op);+if(rc!=OPAL_SUCCESS){+pe_warn(slave,"OPAL error %ld %s to parent PELTV\n",+rc,desc);+return-ENXIO;+}+}++return0;+}++staticintpnv_ioda_set_peltv(structpnv_phb*phb,+structpnv_ioda_pe*pe,+boolis_add)+{+structpnv_ioda_pe*slave;+structpci_dev*pdev=NULL;+intret;++/*+*ClearPEfrozenstate.Ifit'smasterPE,weneed+*clearslavePEfrozenstateaswell.+*/+if(is_add){+opal_pci_eeh_freeze_clear(phb->opal_id,pe->pe_number,+OPAL_EEH_ACTION_CLEAR_FREEZE_ALL);+if(pe->flags&PNV_IODA_PE_MASTER){+list_for_each_entry(slave,&pe->slaves,list)+opal_pci_eeh_freeze_clear(phb->opal_id,+slave->pe_number,+OPAL_EEH_ACTION_CLEAR_FREEZE_ALL);+}+}++/*+*AssociatePEinPELT.WeneedaddthePEintothe+*correspondingPELT-Vaswell.Otherwise,theerror+*originatedfromthePEmightcontributetoother+*PEs.+*/+ret=pnv_ioda_set_one_peltv(phb,pe,pe,is_add);+if(ret)+returnret;++/* For compound PEs, any one affects all of them */+if(pe->flags&PNV_IODA_PE_MASTER){+list_for_each_entry(slave,&pe->slaves,list){+ret=pnv_ioda_set_one_peltv(phb,slave,pe,is_add);+if(ret)+returnret;+}+}++if(pe->flags&(PNV_IODA_PE_BUS_ALL|PNV_IODA_PE_BUS))+pdev=pe->pbus->self;+elseif(pe->flags&PNV_IODA_PE_DEV)+pdev=pe->pdev->bus->self;+#ifdef CONFIG_PCI_IOV+elseif(pe->flags&PNV_IODA_PE_VF)+pdev=pe->parent_dev;+#endif /* CONFIG_PCI_IOV */+while(pdev){+structpci_dn*pdn=pci_get_pdn(pdev);+structpnv_ioda_pe*parent;++if(pdn&&pdn->pe_number!=IODA_INVALID_PE){+parent=&phb->ioda.pe_array[pdn->pe_number];+ret=pnv_ioda_set_one_peltv(phb,parent,pe,is_add);+if(ret)+returnret;+}++pdev=pdev->bus->self;+}++return0;+}++#ifdef CONFIG_PCI_IOV+staticintpnv_ioda_deconfigure_pe(structpnv_phb*phb,structpnv_ioda_pe*pe)+{+structpci_dev*parent;+uint8_tbcomp,dcomp,fcomp;+int64_trc;+longrid_end,rid;++/* Currently, we just deconfigure VF PE. Bus PE will always there.*/+if(pe->pbus){+intcount;++dcomp=OPAL_IGNORE_RID_DEVICE_NUMBER;+fcomp=OPAL_IGNORE_RID_FUNCTION_NUMBER;+parent=pe->pbus->self;+if(pe->flags&PNV_IODA_PE_BUS_ALL)+count=pe->pbus->busn_res.end-+pe->pbus->busn_res.start+1;+else+count=1;++switch(count){+case1:+bcomp=OpalPciBusAll;+break;+case2:+bcomp=OpalPciBus7Bits;+break;+case4:+bcomp=OpalPciBus6Bits;+break;+case8:+bcomp=OpalPciBus5Bits;+break;+case16:+bcomp=OpalPciBus4Bits;+break;+case32:+bcomp=OpalPciBus3Bits;+break;+default:+dev_err(&pe->pbus->dev,"Subordinate buses %d unsupported\n",+count);+/* Do an exact match only */+bcomp=OpalPciBusAll;+}+rid_end=pe->rid+(count<<8);+}else{+if(pe->flags&PNV_IODA_PE_VF)+parent=pe->parent_dev;+else+parent=pe->pdev->bus->self;+bcomp=OpalPciBusAll;+dcomp=OPAL_COMPARE_RID_DEVICE_NUMBER;+fcomp=OPAL_COMPARE_RID_FUNCTION_NUMBER;+rid_end=pe->rid+1;+}++/* Clear the reverse map */+for(rid=pe->rid;rid<rid_end;rid++)+phb->ioda.pe_rmap[rid]=IODA_INVALID_PE;++/* Release from all parents PELT-V */+while(parent){+structpci_dn*pdn=pci_get_pdn(parent);++if(pdn&&pdn->pe_number!=IODA_INVALID_PE){+rc=opal_pci_set_peltv(phb->opal_id,+pdn->pe_number,pe->pe_number,+OPAL_REMOVE_PE_FROM_DOMAIN);+/* XXX What to do in case of error ? */+}+parent=parent->bus->self;+}++opal_pci_eeh_freeze_clear(phb->opal_id,pe->pe_number,+OPAL_EEH_ACTION_CLEAR_FREEZE_ALL);++/* Disassociate PE in PELT */+rc=opal_pci_set_peltv(phb->opal_id,pe->pe_number,+pe->pe_number,OPAL_REMOVE_PE_FROM_DOMAIN);+if(rc)+pe_warn(pe,"OPAL error %ld remove self from PELTV\n",rc);+rc=opal_pci_set_pe(phb->opal_id,pe->pe_number,pe->rid,+bcomp,dcomp,fcomp,OPAL_UNMAP_PE);+if(rc)+pe_err(pe,"OPAL error %ld trying to setup PELT table\n",rc);++pe->pbus=NULL;+pe->pdev=NULL;+pe->parent_dev=NULL;++return0;+}+#endif /* CONFIG_PCI_IOV */+staticstructpnv_ioda_pe*pnv_ioda_init_pe(structpnv_phb*phb,intpe_no){structpnv_ioda_pe*pe=&phb->ioda.pe_array[pe_no];
@@ -547,305 +836,117 @@ static int pnv_ioda_unfreeze_pe(struct pnv_phb *phb, int pe_no, int opt)structpnv_ioda_pe*pe,*slave;s64rc;-/* Find master PE */-pe=&phb->ioda.pe_array[pe_no];-if(pe->flags&PNV_IODA_PE_SLAVE){-pe=pe->master;-WARN_ON(!pe||!(pe->flags&PNV_IODA_PE_MASTER));-pe_no=pe->pe_number;-}--/* Clear frozen state for master PE */-rc=opal_pci_eeh_freeze_clear(phb->opal_id,pe_no,opt);-if(rc!=OPAL_SUCCESS){-pr_warn("%s: Failure %lld clear %d on PHB#%x-PE#%x\n",-__func__,rc,opt,phb->hose->global_number,pe_no);-return-EIO;-}--if(!(pe->flags&PNV_IODA_PE_MASTER))-return0;--/* Clear frozen state for slave PEs */-list_for_each_entry(slave,&pe->slaves,list){-rc=opal_pci_eeh_freeze_clear(phb->opal_id,-slave->pe_number,-opt);-if(rc!=OPAL_SUCCESS){-pr_warn("%s: Failure %lld clear %d on PHB#%x-PE#%x\n",-__func__,rc,opt,phb->hose->global_number,-slave->pe_number);-return-EIO;-}-}--return0;-}--staticintpnv_ioda_get_pe_state(structpnv_phb*phb,intpe_no)-{-structpnv_ioda_pe*slave,*pe;-u8fstate,state;-__be16pcierr;-s64rc;--/* Sanity check on PE number */-if(pe_no<0||pe_no>=phb->ioda.total_pe_num)-returnOPAL_EEH_STOPPED_PERM_UNAVAIL;--/*-*FetchthemasterPEandthePEinstancemightbe-*notinitializedyet.-*/-pe=&phb->ioda.pe_array[pe_no];-if(pe->flags&PNV_IODA_PE_SLAVE){-pe=pe->master;-WARN_ON(!pe||!(pe->flags&PNV_IODA_PE_MASTER));-pe_no=pe->pe_number;-}--/* Check the master PE */-rc=opal_pci_eeh_freeze_status(phb->opal_id,pe_no,-&state,&pcierr,NULL);-if(rc!=OPAL_SUCCESS){-pr_warn("%s: Failure %lld getting "-"PHB#%x-PE#%x state\n",-__func__,rc,-phb->hose->global_number,pe_no);-returnOPAL_EEH_STOPPED_TEMP_UNAVAIL;-}--/* Check the slave PE */-if(!(pe->flags&PNV_IODA_PE_MASTER))-returnstate;--list_for_each_entry(slave,&pe->slaves,list){-rc=opal_pci_eeh_freeze_status(phb->opal_id,-slave->pe_number,-&fstate,-&pcierr,-NULL);-if(rc!=OPAL_SUCCESS){-pr_warn("%s: Failure %lld getting "-"PHB#%x-PE#%x state\n",-__func__,rc,-phb->hose->global_number,slave->pe_number);-returnOPAL_EEH_STOPPED_TEMP_UNAVAIL;-}--/*-*Overridetheresultbasedontheascending-*priority.-*/-if(fstate>state)-state=fstate;-}--returnstate;-}--/* Currently those 2 are only used when MSIs are enabled, this will change-*butinthemeantime,weneedtoprotectthemtoavoidwarnings-*/-#ifdef CONFIG_PCI_MSI-staticstructpnv_ioda_pe*pnv_ioda_get_pe(structpci_dev*dev)-{-structpci_controller*hose=pci_bus_to_host(dev->bus);-structpnv_phb*phb=hose->private_data;-structpci_dn*pdn=pci_get_pdn(dev);--if(!pdn)-returnNULL;-if(pdn->pe_number==IODA_INVALID_PE)-returnNULL;-return&phb->ioda.pe_array[pdn->pe_number];-}-#endif /* CONFIG_PCI_MSI */--staticintpnv_ioda_set_one_peltv(structpnv_phb*phb,-structpnv_ioda_pe*parent,-structpnv_ioda_pe*child,-boolis_add)-{-constchar*desc=is_add?"adding":"removing";-uint8_top=is_add?OPAL_ADD_PE_TO_DOMAIN:-OPAL_REMOVE_PE_FROM_DOMAIN;-structpnv_ioda_pe*slave;-longrc;--/* Parent PE affects child PE */-rc=opal_pci_set_peltv(phb->opal_id,parent->pe_number,-child->pe_number,op);-if(rc!=OPAL_SUCCESS){-pe_warn(child,"OPAL error %ld %s to parent PELTV\n",-rc,desc);-return-ENXIO;-}--if(!(child->flags&PNV_IODA_PE_MASTER))-return0;--/* Compound case: parent PE affects slave PEs */-list_for_each_entry(slave,&child->slaves,list){-rc=opal_pci_set_peltv(phb->opal_id,parent->pe_number,-slave->pe_number,op);-if(rc!=OPAL_SUCCESS){-pe_warn(slave,"OPAL error %ld %s to parent PELTV\n",-rc,desc);-return-ENXIO;-}-}--return0;-}--staticintpnv_ioda_set_peltv(structpnv_phb*phb,-structpnv_ioda_pe*pe,-boolis_add)-{-structpnv_ioda_pe*slave;-structpci_dev*pdev=NULL;-intret;--/*-*ClearPEfrozenstate.Ifit'smasterPE,weneed-*clearslavePEfrozenstateaswell.-*/-if(is_add){-opal_pci_eeh_freeze_clear(phb->opal_id,pe->pe_number,-OPAL_EEH_ACTION_CLEAR_FREEZE_ALL);-if(pe->flags&PNV_IODA_PE_MASTER){-list_for_each_entry(slave,&pe->slaves,list)-opal_pci_eeh_freeze_clear(phb->opal_id,-slave->pe_number,-OPAL_EEH_ACTION_CLEAR_FREEZE_ALL);-}-}--/*-*AssociatePEinPELT.WeneedaddthePEintothe-*correspondingPELT-Vaswell.Otherwise,theerror-*originatedfromthePEmightcontributetoother-*PEs.-*/-ret=pnv_ioda_set_one_peltv(phb,pe,pe,is_add);-if(ret)-returnret;+/* Find master PE */+pe=&phb->ioda.pe_array[pe_no];+if(pe->flags&PNV_IODA_PE_SLAVE){+pe=pe->master;+WARN_ON(!pe||!(pe->flags&PNV_IODA_PE_MASTER));+pe_no=pe->pe_number;+}-/* For compound PEs, any one affects all of them */-if(pe->flags&PNV_IODA_PE_MASTER){-list_for_each_entry(slave,&pe->slaves,list){-ret=pnv_ioda_set_one_peltv(phb,slave,pe,is_add);-if(ret)-returnret;-}+/* Clear frozen state for master PE */+rc=opal_pci_eeh_freeze_clear(phb->opal_id,pe_no,opt);+if(rc!=OPAL_SUCCESS){+pr_warn("%s: Failure %lld clear %d on PHB#%x-PE#%x\n",+__func__,rc,opt,phb->hose->global_number,pe_no);+return-EIO;}-if(pe->flags&(PNV_IODA_PE_BUS_ALL|PNV_IODA_PE_BUS))-pdev=pe->pbus->self;-elseif(pe->flags&PNV_IODA_PE_DEV)-pdev=pe->pdev->bus->self;-#ifdef CONFIG_PCI_IOV-elseif(pe->flags&PNV_IODA_PE_VF)-pdev=pe->parent_dev;-#endif /* CONFIG_PCI_IOV */-while(pdev){-structpci_dn*pdn=pci_get_pdn(pdev);-structpnv_ioda_pe*parent;+if(!(pe->flags&PNV_IODA_PE_MASTER))+return0;-if(pdn&&pdn->pe_number!=IODA_INVALID_PE){-parent=&phb->ioda.pe_array[pdn->pe_number];-ret=pnv_ioda_set_one_peltv(phb,parent,pe,is_add);-if(ret)-returnret;+/* Clear frozen state for slave PEs */+list_for_each_entry(slave,&pe->slaves,list){+rc=opal_pci_eeh_freeze_clear(phb->opal_id,+slave->pe_number,+opt);+if(rc!=OPAL_SUCCESS){+pr_warn("%s: Failure %lld clear %d on PHB#%x-PE#%x\n",+__func__,rc,opt,phb->hose->global_number,+slave->pe_number);+return-EIO;}--pdev=pdev->bus->self;}return0;}-#ifdef CONFIG_PCI_IOV-staticintpnv_ioda_deconfigure_pe(structpnv_phb*phb,structpnv_ioda_pe*pe)+staticintpnv_ioda_get_pe_state(structpnv_phb*phb,intpe_no){-structpci_dev*parent;-uint8_tbcomp,dcomp,fcomp;-int64_trc;-longrid_end,rid;+structpnv_ioda_pe*slave,*pe;+u8fstate,state;+__be16pcierr;+s64rc;-/* Currently, we just deconfigure VF PE. Bus PE will always there.*/-if(pe->pbus){-intcount;+/* Sanity check on PE number */+if(pe_no<0||pe_no>=phb->ioda.total_pe_num)+returnOPAL_EEH_STOPPED_PERM_UNAVAIL;-dcomp=OPAL_IGNORE_RID_DEVICE_NUMBER;-fcomp=OPAL_IGNORE_RID_FUNCTION_NUMBER;-parent=pe->pbus->self;-if(pe->flags&PNV_IODA_PE_BUS_ALL)-count=pe->pbus->busn_res.end-pe->pbus->busn_res.start+1;-else-count=1;+/*+*FetchthemasterPEandthePEinstancemightbe+*notinitializedyet.+*/+pe=&phb->ioda.pe_array[pe_no];+if(pe->flags&PNV_IODA_PE_SLAVE){+pe=pe->master;+WARN_ON(!pe||!(pe->flags&PNV_IODA_PE_MASTER));+pe_no=pe->pe_number;+}-switch(count){-case1:bcomp=OpalPciBusAll;break;-case2:bcomp=OpalPciBus7Bits;break;-case4:bcomp=OpalPciBus6Bits;break;-case8:bcomp=OpalPciBus5Bits;break;-case16:bcomp=OpalPciBus4Bits;break;-case32:bcomp=OpalPciBus3Bits;break;-default:-dev_err(&pe->pbus->dev,"Number of subordinate buses %d unsupported\n",-count);-/* Do an exact match only */-bcomp=OpalPciBusAll;-}-rid_end=pe->rid+(count<<8);-}else{-if(pe->flags&PNV_IODA_PE_VF)-parent=pe->parent_dev;-else-parent=pe->pdev->bus->self;-bcomp=OpalPciBusAll;-dcomp=OPAL_COMPARE_RID_DEVICE_NUMBER;-fcomp=OPAL_COMPARE_RID_FUNCTION_NUMBER;-rid_end=pe->rid+1;+/* Check the master PE */+rc=opal_pci_eeh_freeze_status(phb->opal_id,pe_no,+&state,&pcierr,NULL);+if(rc!=OPAL_SUCCESS){+pr_warn("%s: Failure %lld getting PHB#%x-PE#%x state\n",+__func__,rc,phb->hose->global_number,pe_no);+returnOPAL_EEH_STOPPED_TEMP_UNAVAIL;}-/* Clear the reverse map */-for(rid=pe->rid;rid<rid_end;rid++)-phb->ioda.pe_rmap[rid]=IODA_INVALID_PE;+/* Check the slave PE */+if(!(pe->flags&PNV_IODA_PE_MASTER))+returnstate;-/* Release from all parents PELT-V */-while(parent){-structpci_dn*pdn=pci_get_pdn(parent);-if(pdn&&pdn->pe_number!=IODA_INVALID_PE){-rc=opal_pci_set_peltv(phb->opal_id,pdn->pe_number,-pe->pe_number,OPAL_REMOVE_PE_FROM_DOMAIN);-/* XXX What to do in case of error ? */+list_for_each_entry(slave,&pe->slaves,list){+rc=opal_pci_eeh_freeze_status(phb->opal_id,+slave->pe_number,+&fstate,+&pcierr,+NULL);+if(rc!=OPAL_SUCCESS){+pr_warn("%s: Failure %lld getting PHB#%x-PE#%x state\n",+__func__,rc,phb->hose->global_number,+slave->pe_number);+returnOPAL_EEH_STOPPED_TEMP_UNAVAIL;}-parent=parent->bus->self;-}-opal_pci_eeh_freeze_clear(phb->opal_id,pe->pe_number,-OPAL_EEH_ACTION_CLEAR_FREEZE_ALL);+/*+*Overridetheresultbasedontheascending+*priority.+*/+if(fstate>state)+state=fstate;+}-/* Disassociate PE in PELT */-rc=opal_pci_set_peltv(phb->opal_id,pe->pe_number,-pe->pe_number,OPAL_REMOVE_PE_FROM_DOMAIN);-if(rc)-pe_warn(pe,"OPAL error %ld remove self from PELTV\n",rc);-rc=opal_pci_set_pe(phb->opal_id,pe->pe_number,pe->rid,-bcomp,dcomp,fcomp,OPAL_UNMAP_PE);-if(rc)-pe_err(pe,"OPAL error %ld trying to setup PELT table\n",rc);+returnstate;+}-pe->pbus=NULL;-pe->pdev=NULL;-pe->parent_dev=NULL;+/* Currently those 2 are only used when MSIs are enabled, this will change+*butinthemeantime,weneedtoprotectthemtoavoidwarnings+*/+#ifdef CONFIG_PCI_MSI+staticstructpnv_ioda_pe*pnv_ioda_get_pe(structpci_dev*dev)+{+structpci_controller*hose=pci_bus_to_host(dev->bus);+structpnv_phb*phb=hose->private_data;+structpci_dn*pdn=pci_get_pdn(dev);-return0;+if(!pdn)+returnNULL;+if(pdn->pe_number==IODA_INVALID_PE)+returnNULL;+return&phb->ioda.pe_array[pdn->pe_number];}-#endif /* CONFIG_PCI_IOV */+#endif /* CONFIG_PCI_MSI */staticintpnv_ioda_configure_pe(structpnv_phb*phb,structpnv_ioda_pe*pe){
This adds the refcount to PE, which represents number of PCI
devices contained in the PE. When last device leaves from the
PE, the PE together with its consumed resources (IO, DMA, PELTM,
PELTV) are released, to support PCI hotplug.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 233 +++++++++++++++++++++++++++---
arch/powerpc/platforms/powernv/pci.h | 3 +
2 files changed, 217 insertions(+), 19 deletions(-)
@@ -415,11 +546,72 @@ static int pnv_ioda_deconfigure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)pe->pbus=NULL;pe->pdev=NULL;+#ifdef CONFIG_PCI_IOVpe->parent_dev=NULL;+#endifreturn0;}-#endif /* CONFIG_PCI_IOV */++staticvoidpnv_ioda_release_pe(structpnv_ioda_pe*pe)+{+structpnv_phb*phb=pe->phb;+structpnv_ioda_pe*tmp,*slave;++/* Release slave PEs in compound PE */+if(pe->flags&PNV_IODA_PE_MASTER){+list_for_each_entry_safe(slave,tmp,&pe->slaves,list)+pnv_ioda_release_pe(pe);+}++/* Remove the PE from the list */+list_del(&pe->list);++/* Release resources */+pnv_ioda_release_pe_dma(pe);+pnv_ioda_release_pe_seg(pe);+pnv_ioda_deconfigure_pe(pe->phb,pe);++/* Release PE number */+clear_bit(pe->pe_number,phb->ioda.pe_alloc);+}++staticinlinestructpnv_ioda_pe*pnv_ioda_pe_get(structpnv_ioda_pe*pe)+{+if(!pe)+returnNULL;++pe->device_count++;+returnpe;+}++staticinlinevoidpnv_ioda_pe_put(structpnv_ioda_pe*pe)+{+if(!pe)+return;++pe->device_count--;+BUG_ON(pe->device_count<0);+if(pe->device_count==0)+pnv_ioda_release_pe(pe);+}++staticvoidpnv_pci_release_device(structpci_dev*pdev)+{+structpci_controller*hose=pci_bus_to_host(pdev->bus);+structpnv_phb*phb=hose->private_data;+structpci_dn*pdn=pci_get_pdn(pdev);+structpnv_ioda_pe*pe;++if(pdev->is_virtfn)+return;++if(!pdn||pdn->pe_number==IODA_INVALID_PE)+return;++pe=&phb->ioda.pe_array[pdn->pe_number];+pnv_ioda_pe_put(pe);+}staticstructpnv_ioda_pe*pnv_ioda_init_pe(structpnv_phb*phb,intpe_no){
@@ -1244,9 +1439,8 @@ static struct pnv_ioda_pe *pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all)bus->busn_res.start,pe->pe_number);if(pnv_ioda_configure_pe(phb,pe)){-/* XXX What do we do here ? */-pnv_ioda_free_pe(phb,pe->pe_number);pe->pbus=NULL;+pnv_ioda_release_pe(pe);returnNULL;}
@@ -1449,14 +1643,14 @@ static void pnv_ioda_release_vf_PE(struct pci_dev *pdev, u16 num_vfs)if((pe->flags&PNV_IODA_PE_MASTER)&&(pe->flags&PNV_IODA_PE_VF)){list_for_each_entry_safe(s,sn,&pe->slaves,list){-pnv_pci_ioda2_release_dma_pe(pdev,s);+pnv_pci_ioda2_release_dma_pe(s);list_del(&s->list);pnv_ioda_deconfigure_pe(phb,s);pnv_ioda_free_pe(phb,s->pe_number);}}-pnv_pci_ioda2_release_dma_pe(pdev,pe);+pnv_pci_ioda2_release_pe_dma(pe);/* Remove from list */mutex_lock(&phb->ioda.pe_list_mutex);
@@ -1995,7 +2189,7 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,/* XXX FIXME: Allocate multi-level tables on PHB3 *//* We shouldn't already have a 32-bit DMA associated */-if(WARN_ON(pe->dma32_seg>=0))+if(WARN_ON(pe->dma32_seg!=PNV_INVALID_SEGMENT))return;tbl=pnv_pci_table_alloc(phb->hose->node);
@@ -2066,10 +2260,10 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,return;fail:/* XXX Failure: Try to fallback to 64-bit only ? */-if(pe->dma32_seg>=0){+if(pe->dma32_seg!=PNV_INVALID_SEGMENT){bitmap_clear(phb->ioda.dma32_segmap,pe->dma32_seg,pe->dma32_segcount);-pe->dma32_seg=-1;+pe->dma32_seg=PNV_INVALID_SEGMENT;pe->dma32_segcount=0;}
@@ -2416,7 +2610,7 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,int64_trc;/* We shouldn't already have a 32-bit DMA associated */-if(WARN_ON(pe->dma32_seg>=0))+if(WARN_ON(pe->dma32_seg!=PNV_INVALID_SEGMENT))return;/* TVE #1 is selected by PCI address bit 59 */
@@ -25,11 +25,14 @@ enum pnv_phb_model {#define PNV_IODA_PE_SLAVE (1 << 4) /* Slave PE in compound case */#define PNV_IODA_PE_VF (1 << 5) /* PE for one VF */+#define PNV_INVALID_SEGMENT (-1)+/* Data associated with a PE, including IOMMU tracking etc.. */structpnv_phb;structpnv_ioda_pe{unsignedlongflags;structpnv_phb*phb;+intdevice_count;/* A PE can be associated with a single device or an*entirebus(&children).Intheformercase,pdev
PowerNV platform is running on top of skiboot firmware, which has
changes to support PCI slots. PCI slots are identified by PHB's
OPAL ID (PHB slot) or combo of that and PCI slot ID. The patch
changes argument names of opal_pci_reset() and opal_pci_poll()
to reflect the firmware's change. pnv_eeh_phb_poll() is also
renamed to pnv_eeh_poll() to reflect the firmware's change.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/include/asm/opal.h | 4 ++--
arch/powerpc/platforms/powernv/eeh-powernv.c | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
@@ -745,12 +745,12 @@ static int pnv_eeh_get_state(struct eeh_pe *pe, int *delay)returnret;}-statics64pnv_eeh_phb_poll(structpnv_phb*phb)+statics64pnv_eeh_poll(uint64_tid){s64rc=OPAL_HARDWARE;while(1){-rc=opal_pci_poll(phb->opal_id);+rc=opal_pci_poll(id,NULL);if(rc<=0)break;
@@ -790,7 +790,7 @@ int pnv_eeh_phb_reset(struct pci_controller *hose, int option)*resetfollowedbyhotresetonrootbus.Sowealso*needthePCIbussettlementdelay.*/-rc=pnv_eeh_phb_poll(phb);+rc=pnv_eeh_poll(phb->opal_id);if(option==EEH_RESET_DEACTIVATE){if(system_state<SYSTEM_RUNNING)udelay(1000*EEH_PE_RST_SETTLE_TIME);
@@ -833,7 +833,7 @@ static int pnv_eeh_root_reset(struct pci_controller *hose, int option)gotoout;/* Poll state of the PHB until the request is done */-rc=pnv_eeh_phb_poll(phb);+rc=pnv_eeh_poll(phb->opal_id);if(option==EEH_RESET_DEACTIVATE)msleep(EEH_PE_RST_SETTLE_TIME);out:
The skiboot firmware might provide the capability of resetting PCI
slot by property "ibm,reset-by-firmware" on the PCI slot associated
device node. The patch checks on the property and route the reset
to firmware if the property exists. Otherwise, we fail back to the
old path as before.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/eeh-powernv.c | 44 +++++++++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
@@ -894,6 +894,48 @@ static int pnv_eeh_bridge_reset(struct pci_dev *dev, int option)return0;}+staticintpnv_eeh_bridge_reset(structpci_dev*pdev,intoption)+{+structpci_controller*hose;+structpnv_phb*phb;+structdevice_node*dn=pdev?pci_device_to_OF_node(pdev):NULL;+uint64_tid=(0x1ul<<60);+uint8_tscope;+int64_trc;++/*+*Ifthefirmwarecan'thandleit,wewillissuehotreset+*onthesecondarybusdespitetherequestedresettype.+*/+if(!dn||!of_get_property(dn,"ibm,reset-by-firmware",NULL))+return__pnv_eeh_bridge_reset(pdev,option);++/* The firmware can handle the request */+switch(option){+caseEEH_RESET_HOT:+scope=OPAL_RESET_PCI_HOT;+break;+caseEEH_RESET_FUNDAMENTAL:+scope=OPAL_RESET_PCI_FUNDAMENTAL;+break;+caseEEH_RESET_DEACTIVATE:+return0;+default:+dev_warn(&pdev->dev,"%s: Unsupported reset %d\n",+__func__,option);+return-EINVAL;+}++hose=pci_bus_to_host(pdev->bus);+phb=hose->private_data;+id|=(pdev->bus->number<<24)|(pdev->devfn<<16)|phb->opal_id;+rc=opal_pci_reset(id,scope,OPAL_ASSERT_RESET);+if(rc>0)+rc=pnv_eeh_poll(id);++return(rc==OPAL_SUCCESS)?0:-EIO;+}+staticvoidpnv_eeh_wait_for_pending(structpci_dn*pdn,intpos,u16mask,boolaf_flr_rst){
pnv_pci_reset_secondary_bus(), invoked by pcibios_reset_secondary_bus()
on PowerNV platform. The latter can't be called on root bus. So the
former needn't cover root bus as well.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/eeh-powernv.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
In hotplug case, function pcibios_add_pci_devices() is called to
rescan the specified PCI bus, which might not have any child devices.
Access to the PCI bus's child device node will cause kernel crash
without exception.
This adds condition of skipping scanning PCI bus without child devices
in order to avoid kernel crash.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/kernel/pci-hotplug.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
This moves pcibios_find_pci_bus() to PowerPC kernel directory
so that it can be reused by hotplug code for pSeries and PowerNV
platform at the same time. Also, the function is renamed to
of_node_to_pci_bus().
Signed-off-by: Gavin Shan <redacted>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
arch/powerpc/include/asm/pci-bridge.h | 2 +-
arch/powerpc/kernel/pci-hotplug.c | 30 ++++++++++++++++++++++++++++
arch/powerpc/platforms/pseries/pci_dlpar.c | 32 ------------------------------
drivers/pci/hotplug/rpadlpar_core.c | 6 +++---
drivers/pci/hotplug/rpaphp_pci.c | 2 +-
5 files changed, 35 insertions(+), 37 deletions(-)
@@ -259,7 +259,7 @@ static inline struct eeh_dev *pdn_to_eeh_dev(struct pci_dn *pdn)#endif/** Find the bus corresponding to the indicated device node */-externstructpci_bus*pcibios_find_pci_bus(structdevice_node*dn);+externstructpci_bus*of_node_to_pci_bus(structdevice_node*dn);/** Remove all of the PCI devices under this bus */externvoidpcibios_remove_pci_devices(structpci_bus*bus);
@@ -176,7 +176,7 @@ static int dlpar_add_pci_slot(char *drc_name, struct device_node *dn)structpci_dev*dev;structpci_controller*phb;-if(pcibios_find_pci_bus(dn))+if(of_node_to_pci_bus(dn))return-EINVAL;/* Add pci bus */
@@ -213,7 +213,7 @@ static int dlpar_remove_phb(char *drc_name, struct device_node *dn)structpci_dn*pdn;intrc=0;-if(!pcibios_find_pci_bus(dn))+if(!of_node_to_pci_bus(dn))return-EINVAL;/* If pci slot is hotpluggable, use hotplug to remove it */
@@ -357,7 +357,7 @@ int dlpar_remove_pci_slot(char *drc_name, struct device_node *dn)pci_lock_rescan_remove();-bus=pcibios_find_pci_bus(dn);+bus=of_node_to_pci_bus(dn);if(!bus){ret=-EINVAL;gotoout;
@@ -93,7 +93,7 @@ int rpaphp_enable_slot(struct slot *slot)if(rc)returnrc;-bus=pcibios_find_pci_bus(slot->dn);+bus=of_node_to_pci_bus(slot->dn);if(!bus){err("%s: no pci_bus for dn %s\n",__func__,slot->dn->full_name);return-EINVAL;
@@ -262,10 +262,10 @@ static inline struct eeh_dev *pdn_to_eeh_dev(struct pci_dn *pdn)externstructpci_bus*of_node_to_pci_bus(structdevice_node*dn);/** Remove all of the PCI devices under this bus */-externvoidpcibios_remove_pci_devices(structpci_bus*bus);+externvoidpci_remove_pci_devices(structpci_bus*bus);/** Discover new pci devices under this bus, and add them */-externvoidpcibios_add_pci_devices(structpci_bus*bus);+externvoidpci_add_pci_devices(structpci_bus*bus);externvoidisa_bridge_find_early(structpci_controller*hose);
@@ -68,20 +68,20 @@ void pcibios_release_device(struct pci_dev *dev)}/**-*pcibios_remove_pci_devices-removealldevicesunderthisbus+*pci_remove_pci_devices-removealldevicesunderthisbus*@bus:theindicatedPCIbus**RemoveallofthePCIdevicesunderthisbusbothfromthe*linuxpcidevicetree,andfromthepowerpcEEHaddresscache.*/-voidpcibios_remove_pci_devices(structpci_bus*bus)+voidpci_remove_pci_devices(structpci_bus*bus){structpci_dev*dev,*tmp;structpci_bus*child_bus;/* First go down child busses */list_for_each_entry(child_bus,&bus->children,node)-pcibios_remove_pci_devices(child_bus);+pci_remove_pci_devices(child_bus);pr_debug("PCI: Removing devices on bus %04x:%02x\n",pci_domain_nr(bus),bus->number);
Previously we wouldn't remove pdn because PCI hotplug isn't
supported. update_dn_pci_info() is called at system booting
time to create pdn for PCI device nodes. However, it's going
to be changed later because of PCI hotplug.
This converts update_dn_pci_info() to add_pci_device_node_info(),
traverse_pci_devices() to traverse_pci_device_nodes(). This also
adds remove_pci_device_node_info() which will be used in subsequent
patch at the moment of unplugging PCI devices. All those functions
are exported for PowerNV hotplug driver to use.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/include/asm/pci-bridge.h | 4 ++-
arch/powerpc/include/asm/ppc-pci.h | 8 ++++--
arch/powerpc/kernel/pci_dn.c | 51 +++++++++++++++++++++++++++++-----
arch/powerpc/platforms/pseries/setup.c | 2 +-
4 files changed, 53 insertions(+), 12 deletions(-)
@@ -33,9 +33,11 @@ extern struct pci_dev *isa_bridge_pcidev; /* may be NULL if no ISA bus */structdevice_node;structpci_dn;-typedefvoid*(*traverse_func)(structdevice_node*me,void*data);-void*traverse_pci_devices(structdevice_node*start,traverse_funcpre,-void*data);+typedefvoid*(*traverse_func)(structdevice_node*me,+structpci_controller*phb);+void*traverse_pci_device_nodes(structdevice_node*start,+traverse_funcpre,+structpci_controller*phb);void*traverse_pci_dn(structpci_dn*root,void*(*fn)(structpci_dn*,void*),void*data);
@@ -377,7 +413,7 @@ void *traverse_pci_devices(struct device_node *start, traverse_func pre,if(classp)class=of_read_number(classp,1);-if(pre&&((ret=pre(dn,data))!=NULL))+if(pre&&((ret=pre(dn,phb))!=NULL))returnret;/* If we are a PCI bridge, go down */
@@ -455,7 +492,7 @@ void pci_devs_phb_init_dynamic(struct pci_controller *phb)structpci_dn*pdn;/* PHB nodes themselves must not match */-update_dn_pci_info(dn,phb);+add_pci_device_node_info(dn,phb);pdn=dn->data;if(pdn){pdn->devfn=pdn->busno=-1;
@@ -465,7 +502,7 @@ void pci_devs_phb_init_dynamic(struct pci_controller *phb)}/* Update dn->phb ptrs for new phb and children devices */-traverse_pci_devices(dn,update_dn_pci_info,phb);+traverse_pci_device_nodes(dn,add_pci_device_node_info,phb);}/**
During the PCI plugging event, the PCI devices are rescanned and
their IO and MMIO resources are reassigned. However, the PowerNV
platform will assign PE# based on that, which depends on updating
to window of bridge of the PE's primary bus.
The patch updates the windows of bridge of PE's primary bus if
we have valid bridge. Otherwise, we assume it's root bus or SRIOV
virtual bus and PE won't be assigned during PCI plugging time.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/kernel/pci-common.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
unflatten_dt_node() is called recursively to unflatten FDT nodes
with the assumption that FDT blob has only one root node, which
isn't true when the FDT blob represents device sub-tree. This
improves the function to supporting device sub-tree that have
multiple nodes in the first level:
* Rename original unflatten_dt_node() to __unflatten_dt_node().
* Wrapper unflatten_dt_node() calls __unflatten_dt_node() with
adjusted current node depth to 1 to avoid underflow.
Signed-off-by: Gavin Shan <redacted>
---
drivers/of/fdt.c | 53 ++++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 40 insertions(+), 13 deletions(-)
@@ -408,7 +434,8 @@ static void __unflatten_device_tree(const void *blob,/* First pass, scan for size */start=0;-size=(unsignedlong)unflatten_dt_node(blob,NULL,&start,NULL,NULL,0,true);+size=(unsignedlong)unflatten_dt_node(blob,NULL,&start,+NULL,NULL,true);size=ALIGN(size,4);pr_debug(" size is %lx, allocating...\n",size);
@@ -423,7 +450,7 @@ static void __unflatten_device_tree(const void *blob,/* Second pass, do actual unflattening */start=0;-unflatten_dt_node(blob,mem,&start,NULL,mynodes,0,false);+unflatten_dt_node(blob,mem,&start,NULL,mynodes,false);if(be32_to_cpup(mem+size)!=0xdeadbeef)pr_warning("End of tree marker overwritten: %08x\n",be32_to_cpup(mem+size));
This introduces one more argument to of_fdt_unflatten_tree()
to specify the root node for the FDT blob, which is going to be
unflattened. In the result, the function can be used to unflatten
FDT blob, which represents device sub-tree in PowerNV hotplug
driver.
Signed-off-by: Gavin Shan <redacted>
---
drivers/of/fdt.c | 13 ++++++++-----
drivers/of/unittest.c | 2 +-
include/linux/of_fdt.h | 1 +
3 files changed, 10 insertions(+), 6 deletions(-)
@@ -435,7 +437,7 @@ static void __unflatten_device_tree(const void *blob,/* First pass, scan for size */start=0;size=(unsignedlong)unflatten_dt_node(blob,NULL,&start,-NULL,NULL,true);+dad,NULL,true);size=ALIGN(size,4);pr_debug(" size is %lx, allocating...\n",size);
@@ -450,7 +452,7 @@ static void __unflatten_device_tree(const void *blob,/* Second pass, do actual unflattening */start=0;-unflatten_dt_node(blob,mem,&start,NULL,mynodes,false);+unflatten_dt_node(blob,mem,&start,dad,mynodes,false);if(be32_to_cpup(mem+size)!=0xdeadbeef)pr_warning("End of tree marker overwritten: %08x\n",be32_to_cpup(mem+size));
@@ -1125,7 +1128,7 @@ bool __init early_init_dt_scan(void *params)*/void__initunflatten_device_tree(void){-__unflatten_device_tree(initial_boot_params,&of_root,+__unflatten_device_tree(initial_boot_params,NULL,&of_root,early_init_dt_alloc_memory_arch);/* Get pointer to "/chosen" and "/aliases" nodes for use everywhere */
@@ -907,7 +907,7 @@ static int __init unittest_data_add(void)"not running tests\n",__func__);return-ENOMEM;}-of_fdt_unflatten_tree(unittest_data,&unittest_data_node);+of_fdt_unflatten_tree(unittest_data,NULL,&unittest_data_node);if(!unittest_data_node){pr_warn("%s: No tree to attach; not running tests\n",__func__);return-ENODATA;
This changes of_fdt_unflatten_tree() so that it returns the allocated
memory chunk for unflattened device-tree, which can be released once
it's obsoleted.
Signed-off-by: Gavin Shan <redacted>
---
drivers/of/fdt.c | 11 ++++++-----
include/linux/of_fdt.h | 2 +-
2 files changed, 7 insertions(+), 6 deletions(-)
The PowerNV PCI hotplug driver is going to use the OF changeset
to manage the changed device sub-tree, which requires those OF
changeset functions are exported.
Signed-off-by: Gavin Shan <redacted>
---
drivers/of/dynamic.c | 65 ++++++++++++++++++++++++++++++++++++---------------
drivers/of/overlay.c | 8 +++----
drivers/of/unittest.c | 4 ++--
include/linux/of.h | 2 ++
4 files changed, 54 insertions(+), 25 deletions(-)
@@ -378,9 +378,9 @@ int of_overlay_create(struct device_node *tree)}/* apply the changeset */-err=of_changeset_apply(&ov->cset);+err=__of_changeset_apply(&ov->cset);if(err){-pr_err("%s: of_changeset_apply() failed for tree@%s\n",+pr_err("%s: __of_changeset_apply() failed for tree@%s\n",__func__,tree->full_name);gotoerr_revert_overlay;}
@@ -508,7 +508,7 @@ int of_overlay_destroy(int id)list_del(&ov->node);-of_changeset_revert(&ov->cset);+__of_changeset_revert(&ov->cset);of_free_overlay_info(ov);idr_remove(&ov_idr,id);of_changeset_destroy(&ov->cset);
@@ -539,7 +539,7 @@ int of_overlay_destroy_all(void)/* the tail of list is guaranteed to be safe to remove */list_for_each_entry_safe_reverse(ov,ovn,&ov_list,node){list_del(&ov->node);-of_changeset_revert(&ov->cset);+__of_changeset_revert(&ov->cset);of_free_overlay_info(ov);idr_remove(&ov_idr,ov->id);kfree(ov);
@@ -1001,7 +1001,9 @@ extern int of_reconfig_get_state_change(unsigned long action,externvoidof_changeset_init(structof_changeset*ocs);externvoidof_changeset_destroy(structof_changeset*ocs);+externint__of_changeset_apply(structof_changeset*ocs);externintof_changeset_apply(structof_changeset*ocs);+externint__of_changeset_revert(structof_changeset*ocs);externintof_changeset_revert(structof_changeset*ocs);externintof_changeset_action(structof_changeset*ocs,unsignedlongaction,structdevice_node*np,
The patch intends to add standalone driver to support PCI hotplug
for PowerPC PowerNV platform, which runs on top of skiboot firmware.
The firmware identified hotpluggable slots and marked their device
tree node with proper "ibm,slot-pluggable" and "ibm,reset-by-firmware".
The driver simply scans device-tree to create/register PCI hotplug slot
accordingly.
If the skiboot firmware doesn't support slot status retrieval, the PCI
slot device node shouldn't have property "ibm,reset-by-firmware". In
that case, none of valid PCI slots will be detected from device tree.
The skiboot firmware doesn't export the capability to access attention
LEDs yet and it's something for TBD.
Signed-off-by: Gavin Shan <redacted>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
MAINTAINERS | 6 +
drivers/pci/hotplug/Kconfig | 12 +
drivers/pci/hotplug/Makefile | 4 +
drivers/pci/hotplug/powernv_php.c | 140 +++++++
drivers/pci/hotplug/powernv_php.h | 92 +++++
drivers/pci/hotplug/powernv_php_slot.c | 722 +++++++++++++++++++++++++++++++++
6 files changed, 976 insertions(+)
create mode 100644 drivers/pci/hotplug/powernv_php.c
create mode 100644 drivers/pci/hotplug/powernv_php.h
create mode 100644 drivers/pci/hotplug/powernv_php_slot.c
@@ -0,0 +1,722 @@+/*+*PCIHotplugDriverforPowerPCPowerNVplatform.+*+*CopyrightGavinShan,IBMCorporation2015.+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseaspublishedby+*theFreeSoftwareFoundation;eitherversion2oftheLicense,or+*(atyouroption)anylaterversion.+*/++#include<linux/module.h>++#include<asm/opal.h>+#include<asm/pnv-pci.h>+#include<asm/ppc-pci.h>++#include"powernv_php.h"++staticLIST_HEAD(php_slot_list);+staticDEFINE_SPINLOCK(php_slot_lock);++/*+*Removefirmwaredataforallchilddevicenodesofthe+*indicatedone.+*/+staticvoidremove_child_pdn(structdevice_node*np)+{+structdevice_node*child;++for_each_child_of_node(np,child){+/* In depth first */+remove_child_pdn(child);++remove_pci_device_node_info(child);+}+}++/*+*Removeallsubordinatedevicenodesoftheindicatedone.+*Thosedevicenodesindeepestpathshouldbereleasedfirstly.+*/+staticintremove_child_device_nodes(structdevice_node*parent)+{+structdevice_node*np,*child;+intret=0;++/* If the device node has children, remove them firstly */+for_each_child_of_node(parent,np){+ret=remove_child_device_nodes(np);+if(ret)+returnret;++/* The device shouldn't have alive children */+child=of_get_next_child(np,NULL);+if(child){+of_node_put(child);+of_node_put(np);+pr_err("%s: Alive children of node <%s>\n",+__func__,of_node_full_name(np));+return-EBUSY;+}++/* Detach the device node */+of_detach_node(np);+of_node_put(np);+}++return0;+}++/*+*Thefunctionprocessesthemessagesentbyfirmware+*toremovealldevicetreenodesbeneaththeslot's+*nodes,andtheassociatedauxillarydata.+*/+staticvoidslot_power_off_handler(structpowernv_php_slot*slot)+{+intret,status=1;++/* Release the firmware data for the child device nodes */+remove_child_pdn(slot->dn);++/*+*Releasethechilddevicenodes.Ifthesub-treewas+*builtwiththehelpofchangeset,wejustneeddestroy+*thechanges.+*/+if(slot->fdt){+of_changeset_destroy(&slot->ocs);+kfree(slot->dt);+slot->dt=NULL;+slot->dn->child=NULL;+kfree(slot->fdt);+slot->fdt=NULL;+}else{+ret=remove_child_device_nodes(slot->dn);+if(ret){+status=2;+dev_warn(&slot->pdev->dev,"Error %d freeing nodes\n",+ret);+}+}++/* Confirm status change */+slot->status_confirmed=status;+wake_up_interruptible(&slot->queue);+}++staticintslot_populate_changeset(structof_changeset*ocs,+structdevice_node*dn)+{+structdevice_node*child;+intret=0;++for_each_child_of_node(dn,child){+ret=of_changeset_attach_node(ocs,child);+if(ret)+returnret;++ret=slot_populate_changeset(ocs,child);+}++returnret;+}++staticvoidslot_power_on_handler(structpowernv_php_slot*slot)+{+void*fdt,*dt;+uint64_tlen;+intret,status=1;++/* We don't know the FDT blob size. It tries with incremental+*sizedmemorychunk.+*/+for(len=0x2000;len<=0x10000;len+=0x2000){+fdt=kzalloc(len,GFP_KERNEL);+if(!fdt)+break;++ret=pnv_pci_get_device_tree(slot->dn->phandle,fdt,len);+if(!ret)+break;++kfree(fdt);+}++if(len>0x10000){+dev_warn(&slot->pdev->dev,"Cannot alloc FDT blob\n");+gotoout;+}++/* Unflatten device tree blob */+dt=of_fdt_unflatten_tree(fdt,slot->dn,NULL);+if(!dt){+dev_warn(&slot->pdev->dev,"Cannot unflatten FDT\n");+gotofree_fdt;+}++/* Initialize and apply the changeset */+of_changeset_init(&slot->ocs);+ret=slot_populate_changeset(&slot->ocs,slot->dn);+if(ret){+dev_warn(&slot->pdev->dev,"Error %d populating changeset\n",+ret);+gotofree_dt;+}++slot->dn->child=NULL;+ret=of_changeset_apply(&slot->ocs);+if(ret){+dev_warn(&slot->pdev->dev,"Error %d applying changeset\n",+ret);+gotodestroy_changeset;+}++/* Add device node firmware data */+traverse_pci_device_nodes(slot->dn,+add_pci_device_node_info,+pci_bus_to_host(slot->bus));+slot->fdt=fdt;+slot->dt=dt;+gotoout;++destroy_changeset:+of_changeset_destroy(&slot->ocs);+free_dt:+kfree(dt);+slot->dn->child=NULL;+free_fdt:+kfree(fdt);+status=2;+out:+/* Confirm status change */+slot->status_confirmed=status;+wake_up_interruptible(&slot->queue);+}++staticvoidpowernv_php_slot_work(structwork_struct*data)+{+structpowernv_php_slot*slot=container_of(data,+structpowernv_php_slot,+work);+uint64_tphp_event=be64_to_cpu(slot->msg->params[0]);++switch(php_event){+case0:/* Slot power off */+slot_power_off_handler(slot);+break;+case1:/* Slot power on */+slot_power_on_handler(slot);+break;+default:+dev_warn(&slot->pdev->dev,"Unsupported hotplug event %lld\n",+php_event);+}++of_node_put(slot->dn);+}++intpowernv_php_msg_handler(structnotifier_block*nb,+unsignedlongtype,void*message)+{+phandleh;+structdevice_node*np;+structpowernv_php_slot*slot;+structopal_msg*msg=message;++/* Check the message type */+if(type!=OPAL_MSG_PCI_HOTPLUG){+pr_warn("%s: Wrong message type %ld received!\n",+__func__,type);+returnNOTIFY_DONE;+}++/* Find the device node */+h=(phandle)be64_to_cpu(msg->params[1]);+np=of_find_node_by_phandle(h);+if(!np){+pr_warn("%s: No device node for phandle 0x%08x\n",+__func__,h);+returnNOTIFY_DONE;+}++/* Find the slot */+slot=powernv_php_slot_find(np);+if(!slot){+pr_warn("%s: No slot found for node <%s>\n",+__func__,of_node_full_name(np));+of_node_put(np);+returnNOTIFY_DONE;+}++/* Schedule the work */+slot->msg=msg;+schedule_work(&slot->work);+returnNOTIFY_OK;+}++staticintset_power_status(structhotplug_slot*php_slot,u8val)+{+structpowernv_php_slot*slot=php_slot->private;+intret;++/* Set power status */+slot->status_confirmed=0;+ret=pnv_pci_set_power_status(slot->id,val);+if(ret){+dev_warn(&slot->pdev->dev,"Error %d powering %s slot\n",+ret,val?"on":"off");+returnret;+}++/* Continue to PCI probing after finalized device-tree. The+*device-treemighthavebeenupdatedcompletelyatthis+*point.Thuswedon'thavetoalwayswaitingforthat.+*/+if(slot->status_confirmed==1)+return0;+elseif(slot->status_confirmed>0)+return-EBUSY;++ret=wait_event_timeout(slot->queue,slot->status_confirmed,10*HZ);+if(!ret){+dev_warn(&slot->pdev->dev,"Error %d waiting for power-%s\n",+ret,val?"on":"off");+return-EBUSY;+}++/* Check the result */+if(slot->status_confirmed==1)+return0;++dev_warn(&slot->pdev->dev,"Error status %d for power-%s\n",+slot->status_confirmed,val?"on":"off");+return-EBUSY;+}++staticintget_power_status(structhotplug_slot*php_slot,u8*val)+{+structpowernv_php_slot*slot=php_slot->private;+uint8_tstate;+intret;++/*+*Retrievepowerstatusfromfirmware.Ifwefail+*gettingthat,thepowerstatusfailsbackto+*beon.+*/+ret=pnv_pci_get_power_status(slot->id,&state);+if(ret){+*val=POWERNV_PHP_SLOT_POWER_ON;+dev_warn(&slot->pdev->dev,"Error %d getting power status\n",+ret);+}else{+*val=state?POWERNV_PHP_SLOT_POWER_ON:+POWERNV_PHP_SLOT_POWER_OFF;+php_slot->info->power_status=*val;+}++return0;+}++staticintget_adapter_status(structhotplug_slot*php_slot,u8*val)+{+structpowernv_php_slot*slot=php_slot->private;+uint8_tstate;+intret;++/*+*Retrievepresencestatusfromfirmware.Ifwecan't+*getthat,itwillfailbacktobeempty.+*/+ret=pnv_pci_get_presence_status(slot->id,&state);+if(ret>=0){+ret=0;+*val=state?POWERNV_PHP_SLOT_PRESENT:+POWERNV_PHP_SLOT_EMPTY;+php_slot->info->adapter_status=*val;+ret=0;+}else{+*val=POWERNV_PHP_SLOT_EMPTY;+dev_warn(&slot->pdev->dev,"Error %d getting presence\n",+ret);+}++returnret;+}++staticintset_attention_status(structhotplug_slot*php_slot,u8val)+{+structpowernv_php_slot*slot=php_slot->private;++/* The default operation would to turn on the attention */+switch(val){+casePOWERNV_PHP_SLOT_ATTEN_OFF:+casePOWERNV_PHP_SLOT_ATTEN_ON:+casePOWERNV_PHP_SLOT_ATTEN_IND:+casePOWERNV_PHP_SLOT_ATTEN_ACT:+break;+default:+dev_warn(&slot->pdev->dev,"Invalid attention %d\n",val);+return-EINVAL;+}++/* FIXME: Make it real once firmware supports it */+php_slot->info->attention_status=val;++return0;+}++intpowernv_php_slot_enable(structhotplug_slot*php_slot,boolrescan)+{+structpowernv_php_slot*slot=php_slot->private;+uint8_tpresence,power_status;+intret;++/* Check if the slot has been configured */+if(slot->state!=POWERNV_PHP_SLOT_STATE_REGISTER)+return0;++/* Retrieve slot presence status */+ret=php_slot->ops->get_adapter_status(php_slot,&presence);+if(ret)+returnret;++/* Proceed if there have nothing behind the slot */+if(presence==POWERNV_PHP_SLOT_EMPTY)+gotoscan;++/*+*Ifwedon'tdetectsomethingbehindtheslot,weneed+*makesurethepowersuplytotheslotison.Otherwise,+*theslotdownstreamPCIelinkturnshouldbedown.+*+*Onthefirsttime,wedon'tchangethepowerstatusto+*boostsystembootwithassumptionthatthefirmware+*suppliesconsistentslotpowerstatus:emptyslotalways+*hasitspoweroffandnon-emptyslothasitspoweron.+*/+if(!slot->check_power_status){+slot->check_power_status=1;+gotoscan;+}++/* Check the power status. Scan the slot if that's already on */+ret=php_slot->ops->get_power_status(php_slot,&power_status);+if(ret)+returnret;++if(power_status==POWERNV_PHP_SLOT_POWER_ON)+gotoscan;++/* Power is off, turn it on and then scan the slot */+ret=set_power_status(php_slot,POWERNV_PHP_SLOT_POWER_ON);+if(ret)+returnret;++scan:+switch(presence){+casePOWERNV_PHP_SLOT_PRESENT:+if(rescan){+pci_lock_rescan_remove();+pci_add_pci_devices(slot->bus);+pci_unlock_rescan_remove();+}++/* Rescan for child hotpluggable slots */+slot->state=POWERNV_PHP_SLOT_STATE_POPULATED;+if(rescan)+powernv_php_register(slot->dn);+break;+casePOWERNV_PHP_SLOT_EMPTY:+slot->state=POWERNV_PHP_SLOT_STATE_POPULATED;+break;+default:+dev_warn(&slot->pdev->dev,"Invalid presence status %d\n",+presence);+return-EINVAL;+}++return0;+}++staticintenable_slot(structhotplug_slot*php_slot)+{+returnpowernv_php_slot_enable(php_slot,true);+}++staticintdisable_slot(structhotplug_slot*php_slot)+{+structpowernv_php_slot*slot=php_slot->private;+uint8_tpower_status;+intret;++if(slot->state!=POWERNV_PHP_SLOT_STATE_POPULATED)+return0;++/* Remove all devices behind the slot */+pci_lock_rescan_remove();+pci_remove_pci_devices(slot->bus);+pci_unlock_rescan_remove();++/* Detach the child hotpluggable slots */+powernv_php_unregister(slot->dn);++/*+*Checkthepowerstatusandturnitoffifnecessary.Ifwe+*failtogetthepowerstatus,thepowerwillbeforcedto+*beoff.+*/+ret=php_slot->ops->get_power_status(php_slot,&power_status);+if(ret||power_status==POWERNV_PHP_SLOT_POWER_ON){+ret=set_power_status(php_slot,POWERNV_PHP_SLOT_POWER_OFF);+if(ret)+dev_warn(&slot->pdev->dev,"Error %d powering off\n",+ret);+}++/* Update slot state */+slot->state=POWERNV_PHP_SLOT_STATE_REGISTER;+return0;+}++staticstructhotplug_slot_opsphp_slot_ops={+.get_power_status=get_power_status,+.get_adapter_status=get_adapter_status,+.set_attention_status=set_attention_status,+.enable_slot=enable_slot,+.disable_slot=disable_slot,+};++staticstructpowernv_php_slot*php_slot_match(structdevice_node*dn,+structpowernv_php_slot*slot)+{+structpowernv_php_slot*target,*tmp;++if(slot->dn==dn)+returnslot;++list_for_each_entry(tmp,&slot->children,link){+target=php_slot_match(dn,tmp);+if(target)+returntarget;+}++returnNULL;+}++structpowernv_php_slot*powernv_php_slot_find(structdevice_node*dn)+{+structpowernv_php_slot*slot,*tmp;+unsignedlongflags;++spin_lock_irqsave(&php_slot_lock,flags);+list_for_each_entry(tmp,&php_slot_list,link){+slot=php_slot_match(dn,tmp);+if(slot){+spin_unlock_irqrestore(&php_slot_lock,flags);+returnslot;+}+}+spin_unlock_irqrestore(&php_slot_lock,flags);++returnNULL;+}++voidpowernv_php_slot_free(structkref*kref)+{+structpowernv_php_slot*slot=to_powernv_php_slot(kref);++WARN_ON(!list_empty(&slot->children));+kfree(slot->name);+kfree(slot);+}++staticvoidphp_slot_release(structhotplug_slot*hp_slot)+{+structpowernv_php_slot*slot=hp_slot->private;+unsignedlongflags;++/* Remove from global or child list */+spin_lock_irqsave(&php_slot_lock,flags);+list_del(&slot->link);+spin_unlock_irqrestore(&php_slot_lock,flags);++/* Detach from parent */+powernv_php_slot_put(slot);+powernv_php_slot_put(slot->parent);+}++staticboolphp_slot_get_id(structdevice_node*dn,+uint64_t*id)+{+structdevice_node*parent=dn;+const__be64*prop64;+const__be32*prop32;++/*+*ThehotpluggableslotalwayshasacompoundId,which+*consistsof16-bitsPHBId,16bitsbus/slot/function+*number,andcompoundindicator+*/+*id=(0x1ul<<63);++/* Bus/Slot/Function number */+prop32=of_get_property(dn,"reg",NULL);+if(!prop32)+returnfalse;+*id|=((of_read_number(prop32,1)&0x00ffff00)<<8);++/* PHB Id */+while((parent=of_get_parent(parent))){+if(!PCI_DN(parent)){+of_node_put(parent);+break;+}++if(!of_device_is_compatible(parent,"ibm,ioda2-phb")&&+!of_device_is_compatible(parent,"ibm,ioda-phb")){+of_node_put(parent);+continue;+}++prop64=of_get_property(parent,"ibm,opal-phbid",NULL);+if(!prop64){+of_node_put(parent);+returnfalse;+}++*id|=be64_to_cpup(prop64);+of_node_put(parent);+returntrue;+}++returnfalse;+}++structpowernv_php_slot*powernv_php_slot_alloc(structdevice_node*dn)+{+structeeh_dev*edev=pdn_to_eeh_dev(PCI_DN(dn));+structpci_bus*bus;+structpowernv_php_slot*slot;+constchar*label;+uint64_tid;+intslot_no;+size_tsize;+void*pmem;++/* Slot name */+label=of_get_property(dn,"ibm,slot-label",NULL);+if(!label)+returnNULL;++/* Slot identifier */+if(!php_slot_get_id(dn,&id))+returnNULL;++/* PCI bus */+bus=of_node_to_pci_bus(dn);+if(!bus)+returnNULL;++/* Slot number */+if(dn->child&&PCI_DN(dn->child))+slot_no=PCI_SLOT(PCI_DN(dn->child)->devfn);+else+slot_no=-1;++/* Allocate slot */+size=sizeof(structpowernv_php_slot)++sizeof(structhotplug_slot)++sizeof(structhotplug_slot_info);+pmem=kzalloc(size,GFP_KERNEL);+if(!pmem){+pr_warn("%s: Cannot allocate slot for node %s\n",+__func__,dn->full_name);+returnNULL;+}++/* Assign memory blocks */+slot=pmem;+slot->php_slot=pmem+sizeof(structpowernv_php_slot);+slot->php_slot->info=pmem+sizeof(structpowernv_php_slot)++sizeof(structhotplug_slot);+slot->name=kstrdup(label,GFP_KERNEL);+if(!slot->name){+pr_warn("%s: Cannot populate name for node %s\n",+__func__,dn->full_name);+kfree(pmem);+returnNULL;+}++/* Initialize slot */+kref_init(&slot->kref);+slot->state=POWERNV_PHP_SLOT_STATE_INIT;+slot->dn=dn;+slot->pdev=eeh_dev_to_pci_dev(edev);+slot->bus=bus;+slot->id=id;+slot->slot_no=slot_no;+INIT_WORK(&slot->work,powernv_php_slot_work);+init_waitqueue_head(&slot->queue);+slot->check_power_status=0;+slot->status_confirmed=0;+slot->php_slot->ops=&php_slot_ops;+slot->php_slot->release=php_slot_release;+slot->php_slot->private=slot;+INIT_LIST_HEAD(&slot->children);+INIT_LIST_HEAD(&slot->link);++returnslot;+}++intpowernv_php_slot_register(structpowernv_php_slot*slot)+{+structpowernv_php_slot*parent;+structdevice_node*dn=slot->dn;+unsignedlongflags;+intret;++/* Avoid register same slot for twice */+if(powernv_php_slot_find(slot->dn))+return-EEXIST;++/* Register slot */+ret=pci_hp_register(slot->php_slot,slot->bus,+slot->slot_no,slot->name);+if(ret){+dev_warn(&slot->pdev->dev,"Error %d registering slot\n",+ret);+returnret;+}++/* Put into global or parent list */+while((dn=of_get_parent(dn))){+if(!PCI_DN(dn)){+of_node_put(dn);+break;+}++parent=powernv_php_slot_find(dn);+if(parent){+of_node_put(dn);+break;+}+}++spin_lock_irqsave(&php_slot_lock,flags);+if(parent){+powernv_php_slot_get(parent);+slot->parent=parent;+list_add_tail(&slot->link,&parent->children);+}else{+list_add_tail(&slot->link,&php_slot_list);+}+spin_unlock_irqrestore(&php_slot_lock,flags);++/* Update slot state */+slot->state=POWERNV_PHP_SLOT_STATE_REGISTER;+return0;+}
From: Rob Herring <hidden> Date: 2015-08-06 13:48:10
On Wed, Aug 5, 2015 at 11:11 PM, Gavin Shan [off-list ref] wrote:
quoted hunk
The PowerNV PCI hotplug driver is going to use the OF changeset
to manage the changed device sub-tree, which requires those OF
changeset functions are exported.
Signed-off-by: Gavin Shan <redacted>
---
drivers/of/dynamic.c | 65 ++++++++++++++++++++++++++++++++++++---------------
drivers/of/overlay.c | 8 +++----
drivers/of/unittest.c | 4 ++--
include/linux/of.h | 2 ++
4 files changed, 54 insertions(+), 25 deletions(-)
@@ -662,20 +663,9 @@ void of_changeset_destroy(struct of_changeset *ocs) list_for_each_entry_safe_reverse(ce, cen, &ocs->entries, node) __of_changeset_entry_destroy(ce); }+EXPORT_SYMBOL(of_changeset_destroy);-/**- * of_changeset_apply - Applies a changeset- *- * @ocs: changeset pointer- *- * Applies a changeset to the live tree.- * Any side-effects of live tree state changes are applied here on- * sucess, like creation/destruction of devices and side-effects- * like creation of sysfs properties and directories.- * Returns 0 on success, a negative error value in case of an error.- * On error the partially applied effects are reverted.- */-int of_changeset_apply(struct of_changeset *ocs)+int __of_changeset_apply(struct of_changeset *ocs) { struct of_changeset_entry *ce; int ret;
@@ -704,17 +694,30 @@ int of_changeset_apply(struct of_changeset *ocs) } /**- * of_changeset_revert - Reverts an applied changeset+ * of_changeset_apply - Applies a changeset * * @ocs: changeset pointer *- * Reverts a changeset returning the state of the tree to what it- * was before the application.- * Any side-effects like creation/destruction of devices and- * removal of sysfs properties and directories are applied.+ * Applies a changeset to the live tree.+ * Any side-effects of live tree state changes are applied here on+ * sucess, like creation/destruction of devices and side-effects
s/sucess/success/
quoted hunk
+ * like creation of sysfs properties and directories.
* Returns 0 on success, a negative error value in case of an error.
+ * On error the partially applied effects are reverted.
*/
-int of_changeset_revert(struct of_changeset *ocs)
+int of_changeset_apply(struct of_changeset *ocs)
+{
+ int ret;
+
+ mutex_lock(&of_mutex);
+ ret = __of_changeset_apply(ocs);
+ mutex_unlock(&of_mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL(of_changeset_apply);
+
+int __of_changeset_revert(struct of_changeset *ocs)
{
struct of_changeset_entry *ce;
int ret;
@@ -742,6 +745,29 @@ int of_changeset_revert(struct of_changeset *ocs) } /**+ * of_changeset_revert - Reverts an applied changeset+ *+ * @ocs: changeset pointer+ *+ * Reverts a changeset returning the state of the tree to what it+ * was before the application.+ * Any side-effects like creation/destruction of devices and+ * removal of sysfs properties and directories are applied.+ * Returns 0 on success, a negative error value in case of an error.+ */+int of_changeset_revert(struct of_changeset *ocs)+{+ int ret;++ mutex_lock(&of_mutex);+ ret = __of_changeset_revert(ocs);+ mutex_unlock(&of_mutex);++ return ret;+}+EXPORT_SYMBOL(of_changeset_revert);++/** * of_changeset_action - Perform a changeset action * * @ocs: changeset pointer
@@ -779,3 +805,4 @@ int of_changeset_action(struct of_changeset *ocs, unsigned long action, list_add_tail(&ce->node, &ocs->entries); return 0; }+EXPORT_SYMBOL(of_changeset_action);
@@ -378,9 +378,9 @@ int of_overlay_create(struct device_node *tree)}/* apply the changeset */-err=of_changeset_apply(&ov->cset);+err=__of_changeset_apply(&ov->cset);if(err){-pr_err("%s: of_changeset_apply() failed for tree@%s\n",+pr_err("%s: __of_changeset_apply() failed for tree@%s\n",__func__,tree->full_name);gotoerr_revert_overlay;}
@@ -508,7 +508,7 @@ int of_overlay_destroy(int id)list_del(&ov->node);-of_changeset_revert(&ov->cset);+__of_changeset_revert(&ov->cset);of_free_overlay_info(ov);idr_remove(&ov_idr,id);of_changeset_destroy(&ov->cset);
@@ -539,7 +539,7 @@ int of_overlay_destroy_all(void)/* the tail of list is guaranteed to be safe to remove */list_for_each_entry_safe_reverse(ov,ovn,&ov_list,node){list_del(&ov->node);-of_changeset_revert(&ov->cset);+__of_changeset_revert(&ov->cset);of_free_overlay_info(ov);idr_remove(&ov_idr,ov->id);kfree(ov);
@@ -1001,7 +1001,9 @@ extern int of_reconfig_get_state_change(unsigned long action,externvoidof_changeset_init(structof_changeset*ocs);externvoidof_changeset_destroy(structof_changeset*ocs);+externint__of_changeset_apply(structof_changeset*ocs);externintof_changeset_apply(structof_changeset*ocs);+externint__of_changeset_revert(structof_changeset*ocs);
These should go in of_private.h.
extern int of_changeset_revert(struct of_changeset *ocs);
extern int of_changeset_action(struct of_changeset *ocs,
unsigned long action, struct device_node *np,
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Rob Herring <hidden> Date: 2015-08-06 14:09:11
On Wed, Aug 5, 2015 at 11:11 PM, Gavin Shan [off-list ref] wrote:
unflatten_dt_node() is called recursively to unflatten FDT nodes
with the assumption that FDT blob has only one root node, which
isn't true when the FDT blob represents device sub-tree. This
improves the function to supporting device sub-tree that have
multiple nodes in the first level:
* Rename original unflatten_dt_node() to __unflatten_dt_node().
* Wrapper unflatten_dt_node() calls __unflatten_dt_node() with
adjusted current node depth to 1 to avoid underflow.
Signed-off-by: Gavin Shan <redacted>
@@ -408,7 +434,8 @@ static void __unflatten_device_tree(const void *blob,/* First pass, scan for size */start=0;-size=(unsignedlong)unflatten_dt_node(blob,NULL,&start,NULL,NULL,0,true);+size=(unsignedlong)unflatten_dt_node(blob,NULL,&start,+NULL,NULL,true);size=ALIGN(size,4);pr_debug(" size is %lx, allocating...\n",size);
@@ -423,7 +450,7 @@ static void __unflatten_device_tree(const void *blob,/* Second pass, do actual unflattening */start=0;-unflatten_dt_node(blob,mem,&start,NULL,mynodes,0,false);+unflatten_dt_node(blob,mem,&start,NULL,mynodes,false);if(be32_to_cpup(mem+size)!=0xdeadbeef)pr_warning("End of tree marker overwritten: %08x\n",be32_to_cpup(mem+size));--
On Thu, Aug 06, 2015 at 08:48:10AM -0500, Rob Herring wrote:
On Wed, Aug 5, 2015 at 11:11 PM, Gavin Shan [off-list ref] wrote:
Thanks, Rob. All your comments will be covered in next revision.
Thanks,
Gavin
quoted
The PowerNV PCI hotplug driver is going to use the OF changeset
to manage the changed device sub-tree, which requires those OF
changeset functions are exported.
Signed-off-by: Gavin Shan <redacted>
---
drivers/of/dynamic.c | 65 ++++++++++++++++++++++++++++++++++++---------------
drivers/of/overlay.c | 8 +++----
drivers/of/unittest.c | 4 ++--
include/linux/of.h | 2 ++
4 files changed, 54 insertions(+), 25 deletions(-)
@@ -662,20 +663,9 @@ void of_changeset_destroy(struct of_changeset *ocs) list_for_each_entry_safe_reverse(ce, cen, &ocs->entries, node) __of_changeset_entry_destroy(ce); }+EXPORT_SYMBOL(of_changeset_destroy);-/**- * of_changeset_apply - Applies a changeset- *- * @ocs: changeset pointer- *- * Applies a changeset to the live tree.- * Any side-effects of live tree state changes are applied here on- * sucess, like creation/destruction of devices and side-effects- * like creation of sysfs properties and directories.- * Returns 0 on success, a negative error value in case of an error.- * On error the partially applied effects are reverted.- */-int of_changeset_apply(struct of_changeset *ocs)+int __of_changeset_apply(struct of_changeset *ocs) { struct of_changeset_entry *ce; int ret;
@@ -704,17 +694,30 @@ int of_changeset_apply(struct of_changeset *ocs) } /**- * of_changeset_revert - Reverts an applied changeset+ * of_changeset_apply - Applies a changeset * * @ocs: changeset pointer *- * Reverts a changeset returning the state of the tree to what it- * was before the application.- * Any side-effects like creation/destruction of devices and- * removal of sysfs properties and directories are applied.+ * Applies a changeset to the live tree.+ * Any side-effects of live tree state changes are applied here on+ * sucess, like creation/destruction of devices and side-effects
s/sucess/success/
quoted
+ * like creation of sysfs properties and directories.
* Returns 0 on success, a negative error value in case of an error.
+ * On error the partially applied effects are reverted.
*/
-int of_changeset_revert(struct of_changeset *ocs)
+int of_changeset_apply(struct of_changeset *ocs)
+{
+ int ret;
+
+ mutex_lock(&of_mutex);
+ ret = __of_changeset_apply(ocs);
+ mutex_unlock(&of_mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL(of_changeset_apply);
+
+int __of_changeset_revert(struct of_changeset *ocs)
{
struct of_changeset_entry *ce;
int ret;
@@ -742,6 +745,29 @@ int of_changeset_revert(struct of_changeset *ocs) } /**+ * of_changeset_revert - Reverts an applied changeset+ *+ * @ocs: changeset pointer+ *+ * Reverts a changeset returning the state of the tree to what it+ * was before the application.+ * Any side-effects like creation/destruction of devices and+ * removal of sysfs properties and directories are applied.+ * Returns 0 on success, a negative error value in case of an error.+ */+int of_changeset_revert(struct of_changeset *ocs)+{+ int ret;++ mutex_lock(&of_mutex);+ ret = __of_changeset_revert(ocs);+ mutex_unlock(&of_mutex);++ return ret;+}+EXPORT_SYMBOL(of_changeset_revert);++/** * of_changeset_action - Perform a changeset action * * @ocs: changeset pointer
@@ -779,3 +805,4 @@ int of_changeset_action(struct of_changeset *ocs, unsigned long action, list_add_tail(&ce->node, &ocs->entries); return 0; }+EXPORT_SYMBOL(of_changeset_action);
@@ -378,9 +378,9 @@ int of_overlay_create(struct device_node *tree)}/* apply the changeset */-err=of_changeset_apply(&ov->cset);+err=__of_changeset_apply(&ov->cset);if(err){-pr_err("%s: of_changeset_apply() failed for tree@%s\n",+pr_err("%s: __of_changeset_apply() failed for tree@%s\n",__func__,tree->full_name);gotoerr_revert_overlay;}
@@ -508,7 +508,7 @@ int of_overlay_destroy(int id)list_del(&ov->node);-of_changeset_revert(&ov->cset);+__of_changeset_revert(&ov->cset);of_free_overlay_info(ov);idr_remove(&ov_idr,id);of_changeset_destroy(&ov->cset);
@@ -539,7 +539,7 @@ int of_overlay_destroy_all(void)/* the tail of list is guaranteed to be safe to remove */list_for_each_entry_safe_reverse(ov,ovn,&ov_list,node){list_del(&ov->node);-of_changeset_revert(&ov->cset);+__of_changeset_revert(&ov->cset);of_free_overlay_info(ov);idr_remove(&ov_idr,ov->id);kfree(ov);
@@ -1001,7 +1001,9 @@ extern int of_reconfig_get_state_change(unsigned long action,externvoidof_changeset_init(structof_changeset*ocs);externvoidof_changeset_destroy(structof_changeset*ocs);+externint__of_changeset_apply(structof_changeset*ocs);externintof_changeset_apply(structof_changeset*ocs);+externint__of_changeset_revert(structof_changeset*ocs);
These should go in of_private.h.
quoted
extern int of_changeset_revert(struct of_changeset *ocs);
extern int of_changeset_action(struct of_changeset *ocs,
unsigned long action, struct device_node *np,
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
The series of patches intend to support PCI slot for PowerPC PowerNV platform,
which is running on top of skiboot firmware. The patchset requires corresponding
changes from skiboot firmware, which is sent to skiboot@lists.ozlabs.org
for review. The PCI slots are exposed by skiboot with device node properties,
and kernel utilizes those properties to populated PCI slots accordingly.
This does not apply on top of any actual trees I have - torvalds/master,
powerpc/master, powerpc/next.
The problem patches are (at least):
powerpc/powernv: Enable M64 on P7IOC
powerpc/powernv: Release PEs dynamically
What did you base them on (sha1)? It is always worth mentioning.
--
Alexey
The patch enables M64 window on P7IOC, which has been enabled on
PHB3. Different from PHB3 where 16 M64 BARs are supported and each
of them can be owned by one particular PE# exclusively or divided
evenly to 256 segments, each P7IOC PHB has 16 M64 BARs and each
of them are divided into 8 segments.
Is this a limitation of POWER7 chip or it is from IODA1?
quoted hunk
So each P7IOC PHB can support
128 M64 segments only. Also, P7IOC has M64DT, which helps mapping
one particular M64 segment# to arbitrary PE#. PHB3 doesn't have
M64DT, indicating that one M64 segment can only be pinned to the
fixed PE#. In order to have similar logic to support M64 for PHB3
and P7IOC, we just provide 128 M64 (16 BARs) segments and fixed
mapping between PE# and M64 segment# on P7IOC. In turn, we just
need different phb->init_m64() hooks for P7IOC and PHB3 to support
M64.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 116 ++++++++++++++++++++++++++----
1 file changed, 104 insertions(+), 12 deletions(-)
@@ -172,6 +172,69 @@ static void pnv_ioda_free_pe(struct pnv_phb *phb, int pe)clear_bit(pe,phb->ioda.pe_alloc);}+staticintpnv_ioda1_init_m64(structpnv_phb*phb)+{+structresource*r;+intseg;++/* There are as many M64 segments as the maximum number+*ofPEs,whichis128.+*/+for(seg=0;seg<phb->ioda.total_pe;seg+=8){
This "8" is used a lot across the patch, please make it a macro
(PNV_PHB_P7IOC_SEGNUM or PNV_PHB_IODA1_SEGNUM or whatever you think it is)
with a short comment why it is "8". Or a pnv_phb member.
+ unsigned long base;
+ int64_t rc;
+
+ base = phb->ioda.m64_base + seg * phb->ioda.m64_segsize;
+ rc = opal_pci_set_phb_mem_window(phb->opal_id,
+ OPAL_M64_WINDOW_TYPE,
+ seg / 8,
+ base,
+ 0, /* unused */
+ 8 * phb->ioda.m64_segsize);
+ if (rc != OPAL_SUCCESS) {
+ pr_warn(" Error %lld setting M64 PHB#%d-BAR#%d\n",
+ rc, phb->hose->global_number, seg / 8);
+ goto fail;
+ }
+
+ rc = opal_pci_phb_mmio_enable(phb->opal_id,
+ OPAL_M64_WINDOW_TYPE,
+ seg / 8,
+ OPAL_ENABLE_M64_SPLIT);
+ if (rc != OPAL_SUCCESS) {
+ pr_warn(" Error %lld enabling M64 PHB#%d-BAR#%d\n",
+ rc, phb->hose->global_number, seg / 8);
+ goto fail;
+ }
+ }
+
+ /* Strip off the segment used by the reserved PE, which
What is this reserved PE on P7IOC? "Strip off" means "exclude" here?
+ * is expected to be 0 or last supported PE#. The PHB's
+ * first memory window traces the 32-bits MMIO range
s/traces/filters/ ? Or I did not understand this comment...
+ * while the second one traces the 64-bits prefetchable
+ * MMIO range that the PHB supports.
32/64 ranges comment seems irrelevant here.
quoted hunk
+ */
+ r = &phb->hose->mem_resources[1];
+ if (phb->ioda.reserved_pe == 0)
+ r->start += phb->ioda.m64_segsize;
+ else if (phb->ioda.reserved_pe == (phb->ioda.total_pe - 1))
+ r->end -= phb->ioda.m64_segsize;
+ else
+ pr_warn(" Cannot strip M64 segment for reserved PE#%d\n",
+ phb->ioda.reserved_pe);
+
+ return 0;
+
+fail:
+ for ( ; seg >= 0; seg -= 8)
+ opal_pci_phb_mmio_enable(phb->opal_id,
+ OPAL_M64_WINDOW_TYPE,
+ seg / 8,
+ OPAL_DISABLE_M64);
+
+ return -EIO;
+}
+
/* The default M64 BAR is shared by all PEs */
static int pnv_ioda2_init_m64(struct pnv_phb *phb)
{
@@ -293,7 +356,7 @@ static int pnv_ioda2_pick_m64_pe(struct pci_bus *bus, bool all) } /* Figure out reserved PE numbers by the PE */- pnv_ioda2_reserve_m64_pe(bus, pe_alloc, all);+ pnv_ioda_reserve_m64_pe(bus, pe_alloc, all); /* * the current bus might not own M64 window and that's all
@@ -324,6 +387,26 @@ static int pnv_ioda2_pick_m64_pe(struct pci_bus *bus, bool all) pe->master = master_pe; list_add_tail(&pe->list, &master_pe->slaves); }++ /* P7IOC supports M64DT, which helps mapping M64 segment+ * to one particular PE#. However, PHB3 has fixed mapping+ * between M64 segment and PE#. In order to have same logic+ * for P7IOC and PHB3, we enforce fixed mapping between M64+ * segment and PE# on P7IOC.+ */+ if (phb->type == PNV_PHB_IODA1) {+ int64_t rc;++ rc = opal_pci_map_pe_mmio_window(phb->opal_id,+ pe->pe_number,+ OPAL_M64_WINDOW_TYPE,+ pe->pe_number / 8,+ pe->pe_number % 8);+ if (rc != OPAL_SUCCESS)+ pr_warn("%s: Error %lld mapping M64 for PHB#%d-PE#%d\n",+ __func__, rc, phb->hose->global_number,+ pe->pe_number);+ } } kfree(pe_alloc);
@@ -338,8 +421,8 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb) const u32 *r; u64 pci_addr;- /* FIXME: Support M64 for P7IOC */- if (phb->type != PNV_PHB_IODA2) {+ if (phb->type != PNV_PHB_IODA1 &&+ phb->type != PNV_PHB_IODA2) { pr_info(" Not support M64 window\n"); return;
You are adding P7IOC support so at least "fixme" should go. Also,
pnv_ioda_parse_m64_window() is only called from pnv_pci_init_ioda_phb()
which is called only with PNV_PHB_IODA1 and PNV_PHB_IODA2 (no other value
is passed there a type) so the check above will never succeed, just remove it.
quoted hunk
}
@@ -372,9 +455,18 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb) /* Use last M64 BAR to cover M64 window */ phb->ioda.m64_bar_idx = 15;- phb->init_m64 = pnv_ioda2_init_m64;- phb->reserve_m64_pe = pnv_ioda2_reserve_m64_pe;- phb->pick_m64_pe = pnv_ioda2_pick_m64_pe;+ phb->reserve_m64_pe = pnv_ioda_reserve_m64_pe;+ phb->pick_m64_pe = pnv_ioda_pick_m64_pe;+ switch (phb->type) {+ case PNV_PHB_IODA1:+ phb->init_m64 = pnv_ioda1_init_m64;+ break;+ case PNV_PHB_IODA2:+ phb->init_m64 = pnv_ioda2_init_m64;+ break;+ default:+ pr_debug(" M64 not supported\n");+ } } static void pnv_ioda_freeze_pe(struct pnv_phb *phb, int pe_no)
The patch is adding 6 bitmaps, three to PE and three to PHB, to track
The patch is also removing 2 arrays (io_segmap and m32_segmap), what is
that all about? Also, there was no m64_segmap, now there is, needs an
explanation may be.
the consumed by one particular PE, which can be released once the PE
is destroyed during PCI unplugging time. Also, we're using fixed
quantity of bits to trace the used IO, M32 and M64 segments by PEs
in one particular PHB.
Out of curiosity - have you considered having just 3 arrays, in PHB,
storing PE numbers, and ditching PE's arrays? Does PE itself need to know
what PEs it is using? Not sure about this master/slave PEs though.
It would be easier to read patches if this one was right before
[PATCH v6 23/42] powerpc/powernv: Release PEs dynamically
@@ -3175,19 +3185,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 */
This comment came with if(IODA1) below, since you are removing the
condition below, makes sense to remove the comment as well or move it where
people will look for it (arch/powerpc/platforms/powernv/pci.h ?)
@@ -49,6 +49,15 @@ struct pnv_ioda_pe {/* PE number */unsignedintpe_number;+/* IO/M32/M64 segments consumed by the PE. Each PE can+*haveoneM64segmentatmost,butM64segmentsconsumed+*byslavePEswillbecontributedtothemasterPE.One+*PEcanownmultipleIOandM32segments.
A PE can have multiple IO and M32 segments but just one M64 segment? Is
this correct for IODA1 or IODA2 or both? Is this a limitation of this
implementation or it comes from P7IOC/PHB3 hardware?
+ */
+ unsigned long io_segmap[8];
+ unsigned long m32_segmap[8];
+ unsigned long m64_segmap[8];
Magic constant "8", 64bit*8 = 512 PEs - where did this come from?
Anyway,
#define PNV_IODA_MAX_PE_NUM 512
unsigned long io_segmap[PNV_IODA_MAX_PE_NUM/BITS_PER_LONG]
quoted hunk
+
/* "Weight" assigned to the PE for the sake of DMA resource
* allocations
*/
@@ -145,15 +154,16 @@ struct pnv_phb { unsigned int io_segsize; unsigned int io_pci_base;+ /* IO, M32, M64 segment maps */+ unsigned long io_segmap[8];+ unsigned long m32_segmap[8];+ unsigned long m64_segmap[8];+ /* PE allocation */ struct mutex pe_alloc_mutex; unsigned long *pe_alloc; struct pnv_ioda_pe *pe_array;- /* M32 & IO segment maps */- unsigned int *m32_segmap;- unsigned int *io_segmap;- /* IRQ chip */ int irq_chip_init; struct irq_chip irq_chip;
On Mon, Aug 10, 2015 at 04:05:40PM +1000, Alexey Kardashevskiy wrote:
On 08/06/2015 02:11 PM, Gavin Shan wrote:
quoted
The series of patches intend to support PCI slot for PowerPC PowerNV platform,
which is running on top of skiboot firmware. The patchset requires corresponding
changes from skiboot firmware, which is sent to skiboot@lists.ozlabs.org
for review. The PCI slots are exposed by skiboot with device node properties,
and kernel utilizes those properties to populated PCI slots accordingly.
This does not apply on top of any actual trees I have - torvalds/master,
powerpc/master, powerpc/next.
The problem patches are (at least):
powerpc/powernv: Enable M64 on P7IOC
powerpc/powernv: Release PEs dynamically
What did you base them on (sha1)? It is always worth mentioning.
The patchset bases on powerpc/next + below patches that will be merged
prior to this patchset, I think. I tried to avoid conflicts as much as
I can do:
e14f70b powerpc/powernv: compound PE for VFs <<< EEH Support for VF - END
42f59ac powerpc/eeh: Support error recovery for VF PE
9c1c221 powerpc/powernv: Support PCI config restore for VFs
8ac2231 powerpc/powernv: Support EEH reset for VF PE
a636ce5 powerpc/eeh: Create PE for VFs
a4e56fc powerpc/powernv: EEH device for VF
2f02884 powerpc/eeh: Cache only BARs, not windows or IOV BARs
1888e95 powerpc/pci: Remove VFs prior to PF
0dab41d powerpc/pci: Cache VF index in pci_dn
fdc2d8a PCI: Add pcibios_bus_add_device() weak function
2bcc609 PCI/IOV: Rename and export virtfn_add/virtfn_remove <<< EEH Support for VF - START
efde611 powerpc/eeh: Disable automatically blocked PCI config
All above patches can be found from linux-ppc mail archive.
Thanks,
Gavin
For P7IOC, the whole available DMA32 space, which is below the
MEM32 space, is divided evenly into 256MB segments. The number
of continuous segments assigned to one particular PE depends on
the PE's DMA weight that is calculated based on the type of each
PCI devices contained in the PE, and PHB's DMA weight which is
accumulative DMA weight of PEs contained in the PHB. It means
that the PHB's DMA weight calculation depends on existing PEs,
which works perfectly now, but not hotplug friendly. As the
whole available DMA32 space can be assigned to one PE on PHB3,
so we don't have the issue on PHB3.
The patch calculates PHB's DMA weight based on the PCI devices
contained in the PHB dynamically so that it's hotplug friendly.
It does not look like the patch changed anything about when to calculate
weights, it was and is pnv_ioda_setup_dma().
What the patch seems to be doing is changing weights by multiplying them by
phb->ioda.tce32_count but it is unclear why you do this.
@@ -927,6 +927,9 @@ static void pnv_ioda_link_pe_by_weight(struct pnv_phb *phb,staticunsignedintpnv_ioda_dma_weight(structpci_dev*dev){+structpci_controller*hose=pci_bus_to_host(dev->bus);+structpnv_phb*phb=hose->private_data;+/* This is quite simplistic. The "base" weight of a device*is10.0meansnoDMAistobeaccountedforit.*/
@@ -939,14 +942,34 @@ static unsigned int pnv_ioda_dma_weight(struct pci_dev *dev)if(dev->class==PCI_CLASS_SERIAL_USB_UHCI||dev->class==PCI_CLASS_SERIAL_USB_OHCI||dev->class==PCI_CLASS_SERIAL_USB_EHCI)-return3;+return3*phb->ioda.tce32_count;/* Increase the weight of RAID (includes Obsidian) */if((dev->class>>8)==PCI_CLASS_STORAGE_RAID)-return15;+return15*phb->ioda.tce32_count;/* Default */-return10;+return10*phb->ioda.tce32_count;+}++staticint__pnv_ioda_phb_dma_weight(structpci_dev*pdev,void*data)+{+unsignedint*dma_weight=data;++*dma_weight+=pnv_ioda_dma_weight(pdev);+return0;+}++staticunsignedintpnv_ioda_phb_dma_weight(structpnv_phb*phb)+{+unsignedintdma_weight=0;++if(!phb->hose->bus)+return0;++pci_walk_bus(phb->hose->bus,+__pnv_ioda_phb_dma_weight,&dma_weight);+returndma_weight;}#ifdef CONFIG_PCI_IOV
@@ -1097,14 +1120,6 @@ static void pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all)/* Put PE to the list */list_add_tail(&pe->list,&phb->ioda.pe_list);-/* Account for one DMA PE if at least one DMA capable device exist-*belowthebridge-*/-if(pe->dma_weight!=0){-phb->ioda.dma_weight+=pe->dma_weight;-phb->ioda.dma_pe_count++;-}-/* Link the PE */pnv_ioda_link_pe_by_weight(phb,pe);}
@@ -2431,24 +2446,13 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,staticvoidpnv_ioda_setup_dma(structpnv_phb*phb){structpci_controller*hose=phb->hose;-unsignedintresidual,remaining,segs,tw,base;structpnv_ioda_pe*pe;+unsignedintdma_weight;-/* If we have more PE# than segments available, hand out one-*perPEuntilwerunoutandlettherestfail.Ifnot,-*thenweassignatleastonesegmentperPE,plusmorebased-*ontheamountofdevicesunderthatPE-*/-if(phb->ioda.dma_pe_count>phb->ioda.tce32_count)-residual=0;-else-residual=phb->ioda.tce32_count--phb->ioda.dma_pe_count;--pr_info("PCI: Domain %04x has %ld available 32-bit DMA segments\n",-hose->global_number,phb->ioda.tce32_count);-pr_info("PCI: %d PE# for a total weight of %d\n",-phb->ioda.dma_pe_count,phb->ioda.dma_weight);+/* Calculate the PHB's DMA weight */+dma_weight=pnv_ioda_phb_dma_weight(phb);+pr_info("PCI%04x has %ld DMA32 segments, total weight %d\n",+hose->global_number,phb->ioda.tce32_count,dma_weight);pnv_pci_ioda_setup_opal_tce_kill(phb);
@@ -183,12 +183,6 @@ struct pnv_phb {/* 32-bit TCE tables allocation */unsignedlongtce32_count;-/* Total "weight" for the sake of DMA resources-*allocation-*/-unsignedintdma_weight;-unsignedintdma_pe_count;-/* Sorted list of used PE's, sorted at*bootforresourceallocationpurposes*/
The patch cleans up DMA32 in pci-ioda.c. It shouldn't introduce
behavioural changes:
* Rename various fields in "struct pnv_phb" and "struct pnv_ioda_pe"
as 32-bits DMA should be related to "DMA", not "TCE".
s/dma_weight/dma32_weight/ is ok (does not add much though_ but the rest is
not. The "tce32_" fields are still TCEs (translation entries) while DMA is
a process initiated by a device which does not know about how exactly DMA
addresses are translated later. Since we are on the host side and we
actually manage TCE tables here, I suggest keeping the "tce32_" prefix for
TCE tables and memory they use.
* Removed struct pnv_ioda_pe::tce32_segcount.
That's confusing - I had to walk through patches to find out where you
stopped using it. It would be simpler if you put this particular change to
[PATCH v6 02/42] powerpc/powernv: Drop pnv_ioda_setup_dev_PE()
where you remove dead code.
@@ -1936,7 +1936,7 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,/* XXX FIXME: Allocate multi-level tables on PHB3 *//* We shouldn't already have a 32-bit DMA associated */-if(WARN_ON(pe->tce32_seg>=0))+if(WARN_ON(pe->dma32_seg>=0))return;tbl=pnv_pci_table_alloc(phb->hose->node);
@@ -1945,7 +1945,7 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,pnv_pci_link_table_and_group(phb->hose->node,0,tbl,&pe->table_group);/* Grab a 32-bit TCE table */-pe->tce32_seg=base;+pe->dma32_seg=base;pe_info(pe," Setting up 32-bit TCE table at %08x..%08x\n",(base<<28),((base+segs)<<28)-1);
@@ -2006,8 +2006,8 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,return;fail:/* XXX Failure: Try to fallback to 64-bit only ? */-if(pe->tce32_seg>=0)-pe->tce32_seg=-1;+if(pe->dma32_seg>=0)+pe->dma32_seg=-1;if(tce_mem)__free_pages(tce_mem,get_order(TCE32_TABLE_SIZE*segs));if(tbl){
@@ -2405,7 +2405,7 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,int64_trc;/* We shouldn't already have a 32-bit DMA associated */-if(WARN_ON(pe->tce32_seg>=0))+if(WARN_ON(pe->dma32_seg>=0))return;/* TVE #1 is selected by PCI address bit 59 */
@@ -2415,7 +2415,7 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,pe->pe_number);/* The PE will reserve all possible 32-bits space */-pe->tce32_seg=0;+pe->dma32_seg=0;pe_info(pe,"Setting up 32-bit TCE table at 0..%08x\n",phb->ioda.m32_pci_base);
@@ -3211,7 +3211,7 @@ static void __init pnv_pci_init_ioda_phb(struct device_node *np,mutex_init(&phb->ioda.pe_list_mutex);/* Calculate how many 32-bit TCE segments we have */-phb->ioda.tce32_count=phb->ioda.m32_pci_base>>28;+phb->ioda.dma32_segcount=phb->ioda.m32_pci_base>>28;#if 0 /* We should really do that ... */rc=opal_pci_set_phb_mem_window(opal->phb_id,
@@ -61,11 +61,10 @@ struct pnv_ioda_pe {/* "Weight" assigned to the PE for the sake of DMA resource*allocations*/-unsignedintdma_weight;+unsignedintdma32_weight;/* "Base" iommu table, ie, 4K TCEs, 32-bit DMA */-inttce32_seg;-inttce32_segcount;+intdma32_seg;structiommu_table_grouptable_group;/* 64-bit TCE bypass region */
@@ -181,7 +180,7 @@ struct pnv_phb {unsignedcharpe_rmap[0x10000];/* 32-bit TCE tables allocation */-unsignedlongtce32_count;+unsignedlongdma32_segcount;/* Sorted list of used PE's, sorted at*bootforresourceallocationpurposes
For P7IOC, the whole available DMA32 space, which is below the
MEM32 space, is divided evenly into 256MB segments. The number
of continuous segments assigned to one particular PE depends on
the PE's DMA weight that is calculated based on the type of each
PCI devices contained in the PE, and PHB's DMA weight which is
accumulative DMA weight of PEs contained in the PHB. It means
that the PHB's DMA weight calculation depends on existing PEs,
which works perfectly now, but not hotplug friendly. As the
whole available DMA32 space can be assigned to one PE on PHB3,
so we don't have the issue on PHB3.
The patch calculates PHB's DMA weight based on the PCI devices
contained in the PHB dynamically so that it's hotplug friendly.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 88 +++++++++++++++----------------
arch/powerpc/platforms/powernv/pci.h | 6 ---
2 files changed, 43 insertions(+), 51 deletions(-)
@@ -927,6 +927,9 @@ static void pnv_ioda_link_pe_by_weight(struct pnv_phb *phb,staticunsignedintpnv_ioda_dma_weight(structpci_dev*dev){+structpci_controller*hose=pci_bus_to_host(dev->bus);+structpnv_phb*phb=hose->private_data;+/* This is quite simplistic. The "base" weight of a device*is10.0meansnoDMAistobeaccountedforit.*/
@@ -939,14 +942,34 @@ static unsigned int pnv_ioda_dma_weight(struct pci_dev *dev)if(dev->class==PCI_CLASS_SERIAL_USB_UHCI||dev->class==PCI_CLASS_SERIAL_USB_OHCI||dev->class==PCI_CLASS_SERIAL_USB_EHCI)-return3;+return3*phb->ioda.tce32_count;/* Increase the weight of RAID (includes Obsidian) */if((dev->class>>8)==PCI_CLASS_STORAGE_RAID)-return15;+return15*phb->ioda.tce32_count;/* Default */-return10;+return10*phb->ioda.tce32_count;+}++staticint__pnv_ioda_phb_dma_weight(structpci_dev*pdev,void*data)+{+unsignedint*dma_weight=data;++*dma_weight+=pnv_ioda_dma_weight(pdev);+return0;+}++staticunsignedintpnv_ioda_phb_dma_weight(structpnv_phb*phb)+{+unsignedintdma_weight=0;++if(!phb->hose->bus)+return0;++pci_walk_bus(phb->hose->bus,+__pnv_ioda_phb_dma_weight,&dma_weight);+returndma_weight;}#ifdef CONFIG_PCI_IOV
@@ -1097,14 +1120,6 @@ static void pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all)/* Put PE to the list */list_add_tail(&pe->list,&phb->ioda.pe_list);-/* Account for one DMA PE if at least one DMA capable device exist-*belowthebridge-*/-if(pe->dma_weight!=0){-phb->ioda.dma_weight+=pe->dma_weight;-phb->ioda.dma_pe_count++;-}-/* Link the PE */pnv_ioda_link_pe_by_weight(phb,pe);}
@@ -2431,24 +2446,13 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,staticvoidpnv_ioda_setup_dma(structpnv_phb*phb){structpci_controller*hose=phb->hose;-unsignedintresidual,remaining,segs,tw,base;structpnv_ioda_pe*pe;+unsignedintdma_weight;-/* If we have more PE# than segments available, hand out one-*perPEuntilwerunoutandlettherestfail.Ifnot,-*thenweassignatleastonesegmentperPE,plusmorebased-*ontheamountofdevicesunderthatPE-*/-if(phb->ioda.dma_pe_count>phb->ioda.tce32_count)-residual=0;-else-residual=phb->ioda.tce32_count--phb->ioda.dma_pe_count;--pr_info("PCI: Domain %04x has %ld available 32-bit DMA segments\n",-hose->global_number,phb->ioda.tce32_count);-pr_info("PCI: %d PE# for a total weight of %d\n",-phb->ioda.dma_pe_count,phb->ioda.dma_weight);+/* Calculate the PHB's DMA weight */+dma_weight=pnv_ioda_phb_dma_weight(phb);+pr_info("PCI%04x has %ld DMA32 segments, total weight %d\n",+hose->global_number,phb->ioda.tce32_count,dma_weight);pnv_pci_ioda_setup_opal_tce_kill(phb);
This is not right. @base here is a local variable in the scope,
pnv_pci_ioda_setup_dma_pe() will always be called with base==0.
Sorry for commenting the same patch twice.
@@ -183,12 +183,6 @@ struct pnv_phb {/* 32-bit TCE tables allocation */unsignedlongtce32_count;-/* Total "weight" for the sake of DMA resources-*allocation-*/-unsignedintdma_weight;-unsignedintdma_pe_count;-/* Sorted list of used PE's, sorted at*bootforresourceallocationpurposes*/
The original implementation of pnv_ioda_setup_dma() iterates the
list of PEs and configures the DMA32 space for them one by one.
The function was designed to be called during PHB fixup time.
When configuring PE's DMA32 space in pcibios_setup_bridge(), in
order to support PCI hotplug, we have to have the function PE
oriented.
This renames pnv_ioda_setup_dma() to pnv_ioda1_setup_dma() and
adds one more argument "struct pnv_ioda_pe *pe" to it. The caller,
pnv_pci_ioda_setup_DMA(), gets PE from the list and passes to it
or pnv_pci_ioda2_setup_dma_pe(). The patch shouldn't cause behavioral
changes.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 75 +++++++++++++++----------------
1 file changed, 36 insertions(+), 39 deletions(-)
@@ -2443,52 +2443,29 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,pnv_ioda_setup_bus_dma(pe,pe->pbus);}-staticvoidpnv_ioda_setup_dma(structpnv_phb*phb)+staticunsignedintpnv_ioda1_setup_dma(structpnv_phb*phb,+structpnv_ioda_pe*pe,+unsignedintbase){structpci_controller*hose=phb->hose;-structpnv_ioda_pe*pe;-unsignedintdma_weight;+unsignedintdma_weight,segs;/* Calculate the PHB's DMA weight */dma_weight=pnv_ioda_phb_dma_weight(phb);pr_info("PCI%04x has %ld DMA32 segments, total weight %d\n",hose->global_number,phb->ioda.dma32_segcount,dma_weight);-pnv_pci_ioda_setup_opal_tce_kill(phb);--/* Walk our PE list and configure their DMA segments, hand them-*outonebasesegmentplusanyresidualsegmentsbasedon-*weight-*/-list_for_each_entry(pe,&phb->ioda.pe_dma_list,dma_link){-if(!pe->dma32_weight)-continue;--/*-*ForIODA2compliantPHB3,weneedn'tcareabouttheweight.-*Theallavailable32-bitsDMAspacewillbeassignedto-*thespecificPE.-*/-if(phb->type==PNV_PHB_IODA1){-unsignedintsegs,base=0;--if(pe->dma32_weight<-dma_weight/phb->ioda.dma32_segcount)-segs=1;-else-segs=(pe->dma32_weight*-phb->ioda.dma32_segcount)/dma_weight;--pe_info(pe,"DMA32 weight %d, assigned %d segments\n",-pe->dma32_weight,segs);-pnv_pci_ioda_setup_dma_pe(phb,pe,base,segs);+if(pe->dma32_weight<+dma_weight/phb->ioda.dma32_segcount)
This @base handling seems never be tested between 8..11 as "[PATCH v6
11/42] powerpc/powernv: Trace DMA32 segments consumed by PE"
removes it and I suspect you only tested the final version. Which is ok for
the final result but not ok for bisectability.
Looks like 8/42, 9/42, 10/42, 11/42 need to be rearranged or merged to
remove this multiple @base touching.
+ break;
+ case PNV_PHB_IODA2:
+ pnv_pci_ioda2_setup_dma_pe(phb, pe);
+ break;
+ default:
+ pr_warn("%s: No DMA for PHB type %d\n",
+ __func__, phb->type);
+ }
+ }
/* Mark the PHB initialization done */
- phb = hose->private_data;
phb->initialized = 1;
}
}
On P7IOC, the whole DMA32 space is divided evenly to 256MB segments.
Each PE can consume one or multiple DMA32 segments. Current code
doesn't trace the available DMA32 segments and those consumed by
one particular PE. It's conflicting with PCI hotplug.
The patch introduces one bitmap to PHB to trace the available
DMA32 segments for allocation, more fields to "struct pnv_ioda_pe"
to trace the consumed DMA32 segments by the PE, which is going to
be released when the PE is destroyed at PCI unplugging time.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 40 +++++++++++++++++++++++--------
arch/powerpc/platforms/powernv/pci.h | 4 +++-
2 files changed, 33 insertions(+), 11 deletions(-)
Several functions used to configure PE take pe_number to indentify
PE instance. As the pe_number is included in PE instance after it
is reserved or allocated. It's convienent for those functions to
return PE instance which includes the required pe_number.
This is a description for the half of the patch but this patch also adds a
return value to functions which did not have it before and I am not sure
you need all of them to return something. It would be cleaner if you added
"return" when/where you really need it, not just because it seems that it
may be convenient later.
@@ -132,12 +132,12 @@ static inline bool pnv_pci_is_mem_pref_64(unsigned long flags)(IORESOURCE_MEM_64|IORESOURCE_PREFETCH));}-staticvoidpnv_ioda_reserve_pe(structpnv_phb*phb,intpe_no)+staticstructpnv_ioda_pe*pnv_ioda_reserve_pe(structpnv_phb*phb,intpe_no){if(!(pe_no>=0&&pe_no<phb->ioda.total_pe)){pr_warn("%s: Invalid PE %d on PHB#%x\n",__func__,pe_no,phb->hose->global_number);-return;+returnNULL;}if(test_and_set_bit(pe_no,phb->ioda.pe_alloc))
@@ -344,7 +346,7 @@ static int pnv_ioda_pick_m64_pe(struct pci_bus *bus, bool all)/* Root bus shouldn't use M64 */if(pci_is_root_bus(bus))-returnIODA_INVALID_PE;+returnNULL;/* Allocate bitmap */size=_ALIGN_UP(phb->ioda.total_pe/8,sizeof(unsignedlong));
@@ -352,7 +354,7 @@ static int pnv_ioda_pick_m64_pe(struct pci_bus *bus, bool all)if(!pe_alloc){pr_warn("%s: Out of memory !\n",__func__);-returnIODA_INVALID_PE;+returnNULL;}/* Figure out reserved PE numbers by the PE */
@@ -1069,28 +1071,26 @@ static void pnv_ioda_setup_same_PE(struct pci_bus *bus, struct pnv_ioda_pe *pe)*subordinatePCIdevicesandbuses.ThesecondtypeofPEisnormally*orgiriatedbyPCIe-to-PCIbridgeorPLXswitchdownstreamports.*/-staticvoidpnv_ioda_setup_bus_PE(structpci_bus*bus,boolall)+staticstructpnv_ioda_pe*pnv_ioda_setup_bus_PE(structpci_bus*bus,boolall){structpci_controller*hose=pci_bus_to_host(bus);structpnv_phb*phb=hose->private_data;-structpnv_ioda_pe*pe;-intpe_num=IODA_INVALID_PE;+structpnv_ioda_pe*pe=NULL;/* Check if PE is determined by M64 */if(phb->pick_m64_pe)-pe_num=phb->pick_m64_pe(bus,all);+pe=phb->pick_m64_pe(bus,all);/* The PE number isn't pinned by M64 */-if(pe_num==IODA_INVALID_PE)-pe_num=pnv_ioda_alloc_pe(phb);+if(!pe)+pe=pnv_ioda_alloc_pe(phb);-if(pe_num==IODA_INVALID_PE){-pr_warning("%s: Not enough PE# available for PCI bus %04x:%02x\n",+if(!pe){+pr_warning("%s: No enough PE# for PCI bus %04x:%02x\n",__func__,pci_domain_nr(bus),bus->number);-return;+returnNULL;}-pe=&phb->ioda.pe_array[pe_num];pe->flags|=(all?PNV_IODA_PE_BUS_ALL:PNV_IODA_PE_BUS);pe->pbus=bus;pe->pdev=NULL;
@@ -1101,17 +1101,16 @@ static void pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all)if(all)pe_info(pe,"Secondary bus %d..%d associated with PE#%d\n",-bus->busn_res.start,bus->busn_res.end,pe_num);+bus->busn_res.start,bus->busn_res.end,pe->pe_number);elsepe_info(pe,"Secondary bus %d associated with PE#%d\n",-bus->busn_res.start,pe_num);+bus->busn_res.start,pe->pe_number);if(pnv_ioda_configure_pe(phb,pe)){/* XXX What do we do here ? */-if(pe_num)-pnv_ioda_free_pe(phb,pe_num);+pnv_ioda_free_pe(phb,pe->pe_number);pe->pbus=NULL;-return;+returnNULL;}/* Associate it with all child devices */
@@ -1122,6 +1121,8 @@ static void pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all)/* Link the PE */pnv_ioda_link_pe_by_weight(phb,pe);++returnpe;}staticvoidpnv_ioda_setup_PEs(structpci_bus*bus)
From: Frank Rowand <hidden> Date: 2015-08-10 22:42:13
On 8/5/2015 9:11 PM, Gavin Shan wrote:
quoted hunk
This introduces one more argument to of_fdt_unflatten_tree()
to specify the root node for the FDT blob, which is going to be
unflattened. In the result, the function can be used to unflatten
FDT blob, which represents device sub-tree in PowerNV hotplug
driver.
Signed-off-by: Gavin Shan <redacted>
---
drivers/of/fdt.c | 13 ++++++++-----
drivers/of/unittest.c | 2 +-
include/linux/of_fdt.h | 1 +
3 files changed, 10 insertions(+), 6 deletions(-)
@@ -907,7 +907,7 @@ static int __init unittest_data_add(void)"not running tests\n",__func__);return-ENOMEM;}-of_fdt_unflatten_tree(unittest_data,&unittest_data_node);+of_fdt_unflatten_tree(unittest_data,NULL,&unittest_data_node);if(!unittest_data_node){pr_warn("%s: No tree to attach; not running tests\n",__func__);return-ENODATA;
From: Frank Rowand <hidden> Date: 2015-08-10 22:42:32
On 8/5/2015 9:11 PM, Gavin Shan wrote:
quoted hunk
This changes of_fdt_unflatten_tree() so that it returns the allocated
memory chunk for unflattened device-tree, which can be released once
it's obsoleted.
Signed-off-by: Gavin Shan <redacted>
---
drivers/of/fdt.c | 11 ++++++-----
include/linux/of_fdt.h | 2 +-
2 files changed, 7 insertions(+), 6 deletions(-)
@@ -473,11 +474,11 @@ static void *kernel_tree_alloc(u64 size, u64 align) * pointers of the nodes so the normal device-tree walking functions * can be used. */-void of_fdt_unflatten_tree(const unsigned long *blob,+void *of_fdt_unflatten_tree(const unsigned long *blob,
Please add a description of the return value to the documentation header.
On Mon, Aug 10, 2015 at 04:30:09PM +1000, Alexey Kardashevskiy wrote:
On 08/06/2015 02:11 PM, Gavin Shan wrote:
quoted
The patch enables M64 window on P7IOC, which has been enabled on
PHB3. Different from PHB3 where 16 M64 BARs are supported and each
of them can be owned by one particular PE# exclusively or divided
evenly to 256 segments, each P7IOC PHB has 16 M64 BARs and each
of them are divided into 8 segments.
Is this a limitation of POWER7 chip or it is from IODA1?
From IODA1.
quoted
So each P7IOC PHB can support
128 M64 segments only. Also, P7IOC has M64DT, which helps mapping
one particular M64 segment# to arbitrary PE#. PHB3 doesn't have
M64DT, indicating that one M64 segment can only be pinned to the
fixed PE#. In order to have similar logic to support M64 for PHB3
and P7IOC, we just provide 128 M64 (16 BARs) segments and fixed
mapping between PE# and M64 segment# on P7IOC. In turn, we just
need different phb->init_m64() hooks for P7IOC and PHB3 to support
M64.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 116 ++++++++++++++++++++++++++----
1 file changed, 104 insertions(+), 12 deletions(-)
@@ -172,6 +172,69 @@ static void pnv_ioda_free_pe(struct pnv_phb *phb, int pe)clear_bit(pe,phb->ioda.pe_alloc);}+staticintpnv_ioda1_init_m64(structpnv_phb*phb)+{+structresource*r;+intseg;++/* There are as many M64 segments as the maximum number+*ofPEs,whichis128.+*/+for(seg=0;seg<phb->ioda.total_pe;seg+=8){
This "8" is used a lot across the patch, please make it a macro
(PNV_PHB_P7IOC_SEGNUM or PNV_PHB_IODA1_SEGNUM or whatever you think it is)
with a short comment why it is "8". Or a pnv_phb member.
I would like to use "8". When having a macro, you have to check
the definition of the macro to get the real value of that. However,
it makes sense to add more comments explaining why it's 8 here.
quoted
+ unsigned long base;
+ int64_t rc;
+
+ base = phb->ioda.m64_base + seg * phb->ioda.m64_segsize;
+ rc = opal_pci_set_phb_mem_window(phb->opal_id,
+ OPAL_M64_WINDOW_TYPE,
+ seg / 8,
+ base,
+ 0, /* unused */
+ 8 * phb->ioda.m64_segsize);
+ if (rc != OPAL_SUCCESS) {
+ pr_warn(" Error %lld setting M64 PHB#%d-BAR#%d\n",
+ rc, phb->hose->global_number, seg / 8);
+ goto fail;
+ }
+
+ rc = opal_pci_phb_mmio_enable(phb->opal_id,
+ OPAL_M64_WINDOW_TYPE,
+ seg / 8,
+ OPAL_ENABLE_M64_SPLIT);
+ if (rc != OPAL_SUCCESS) {
+ pr_warn(" Error %lld enabling M64 PHB#%d-BAR#%d\n",
+ rc, phb->hose->global_number, seg / 8);
+ goto fail;
+ }
+ }
+
+ /* Strip off the segment used by the reserved PE, which
What is this reserved PE on P7IOC? "Strip off" means "exclude" here?
127 that was exported from skiboot. "Strip off" means "exclude".
quoted
+ * is expected to be 0 or last supported PE#. The PHB's
+ * first memory window traces the 32-bits MMIO range
s/traces/filters/ ? Or I did not understand this comment...
It seems you didn't understand it: there are two memory windows
in every PHB. The first one is tracing M32 resource and the
second one is tracing M64 resource.
quoted
+ * while the second one traces the 64-bits prefetchable
+ * MMIO range that the PHB supports.
32/64 ranges comment seems irrelevant here.
Maybe it's not so relevant, but still. We're stripping off the
M64 segment from the 2nd resource (as above), not first one.
quoted
+ */
+ r = &phb->hose->mem_resources[1];
+ if (phb->ioda.reserved_pe == 0)
+ r->start += phb->ioda.m64_segsize;
+ else if (phb->ioda.reserved_pe == (phb->ioda.total_pe - 1))
+ r->end -= phb->ioda.m64_segsize;
+ else
+ pr_warn(" Cannot strip M64 segment for reserved PE#%d\n",
+ phb->ioda.reserved_pe);
+
+ return 0;
+
+fail:
+ for ( ; seg >= 0; seg -= 8)
+ opal_pci_phb_mmio_enable(phb->opal_id,
+ OPAL_M64_WINDOW_TYPE,
+ seg / 8,
+ OPAL_DISABLE_M64);
+
+ return -EIO;
+}
+
/* The default M64 BAR is shared by all PEs */
static int pnv_ioda2_init_m64(struct pnv_phb *phb)
{
@@ -293,7 +356,7 @@ static int pnv_ioda2_pick_m64_pe(struct pci_bus *bus, bool all) } /* Figure out reserved PE numbers by the PE */- pnv_ioda2_reserve_m64_pe(bus, pe_alloc, all);+ pnv_ioda_reserve_m64_pe(bus, pe_alloc, all); /* * the current bus might not own M64 window and that's all
@@ -324,6 +387,26 @@ static int pnv_ioda2_pick_m64_pe(struct pci_bus *bus, bool all) pe->master = master_pe; list_add_tail(&pe->list, &master_pe->slaves); }++ /* P7IOC supports M64DT, which helps mapping M64 segment+ * to one particular PE#. However, PHB3 has fixed mapping+ * between M64 segment and PE#. In order to have same logic+ * for P7IOC and PHB3, we enforce fixed mapping between M64+ * segment and PE# on P7IOC.+ */+ if (phb->type == PNV_PHB_IODA1) {+ int64_t rc;++ rc = opal_pci_map_pe_mmio_window(phb->opal_id,+ pe->pe_number,+ OPAL_M64_WINDOW_TYPE,+ pe->pe_number / 8,+ pe->pe_number % 8);+ if (rc != OPAL_SUCCESS)+ pr_warn("%s: Error %lld mapping M64 for PHB#%d-PE#%d\n",+ __func__, rc, phb->hose->global_number,+ pe->pe_number);+ } } kfree(pe_alloc);
@@ -338,8 +421,8 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb) const u32 *r; u64 pci_addr;- /* FIXME: Support M64 for P7IOC */- if (phb->type != PNV_PHB_IODA2) {+ if (phb->type != PNV_PHB_IODA1 &&+ phb->type != PNV_PHB_IODA2) { pr_info(" Not support M64 window\n"); return;
You are adding P7IOC support so at least "fixme" should go. Also,
pnv_ioda_parse_m64_window() is only called from pnv_pci_init_ioda_phb() which
is called only with PNV_PHB_IODA1 and PNV_PHB_IODA2 (no other value is passed
there a type) so the check above will never succeed, just remove it.
The "fixme" is removed, isn't it?
As I explained last time, there will have another new type PHB and the function
will be called on the new type of PHB. The code has been there and it's not
in upstream yet. So it's reasonable to keep it, instead of removing it.
quoted
}
@@ -372,9 +455,18 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb) /* Use last M64 BAR to cover M64 window */ phb->ioda.m64_bar_idx = 15;- phb->init_m64 = pnv_ioda2_init_m64;- phb->reserve_m64_pe = pnv_ioda2_reserve_m64_pe;- phb->pick_m64_pe = pnv_ioda2_pick_m64_pe;+ phb->reserve_m64_pe = pnv_ioda_reserve_m64_pe;+ phb->pick_m64_pe = pnv_ioda_pick_m64_pe;+ switch (phb->type) {+ case PNV_PHB_IODA1:+ phb->init_m64 = pnv_ioda1_init_m64;+ break;+ case PNV_PHB_IODA2:+ phb->init_m64 = pnv_ioda2_init_m64;+ break;+ default:+ pr_debug(" M64 not supported\n");+ } } static void pnv_ioda_freeze_pe(struct pnv_phb *phb, int pe_no)
On Mon, Aug 10, 2015 at 05:16:40PM +1000, Alexey Kardashevskiy wrote:
On 08/06/2015 02:11 PM, Gavin Shan wrote:
quoted
The patch is adding 6 bitmaps, three to PE and three to PHB, to track
The patch is also removing 2 arrays (io_segmap and m32_segmap), what is that
all about? Also, there was no m64_segmap, now there is, needs an explanation
may be.
Originally, the bitmaps (io_segmap and m32_segmap) are allocated dynamically.
Now, they have fixed sizes - 512 bits.
The subject "powerpc/powernv: Track IO/M32/M64 segments from PE" indicates
why m64_segmap is added.
quoted
the consumed by one particular PE, which can be released once the PE
is destroyed during PCI unplugging time. Also, we're using fixed
quantity of bits to trace the used IO, M32 and M64 segments by PEs
in one particular PHB.
Out of curiosity - have you considered having just 3 arrays, in PHB, storing
PE numbers, and ditching PE's arrays? Does PE itself need to know what PEs it
is using? Not sure about this master/slave PEs though.
I don't follow your suggestion. Can you rephrase and explain it a bit more?
It would be easier to read patches if this one was right before
[PATCH v6 23/42] powerpc/powernv: Release PEs dynamically
I'll try to reoder the patch, but not expect too much...
@@ -3175,19 +3185,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 */
This comment came with if(IODA1) below, since you are removing the condition
below, makes sense to remove the comment as well or move it where people will
look for it (arch/powerpc/platforms/powernv/pci.h ?)
@@ -49,6 +49,15 @@ struct pnv_ioda_pe {/* PE number */unsignedintpe_number;+/* IO/M32/M64 segments consumed by the PE. Each PE can+*haveoneM64segmentatmost,butM64segmentsconsumed+*byslavePEswillbecontributedtothemasterPE.One+*PEcanownmultipleIOandM32segments.
A PE can have multiple IO and M32 segments but just one M64 segment? Is this
correct for IODA1 or IODA2 or both? Is this a limitation of this
implementation or it comes from P7IOC/PHB3 hardware?
It's correct for IO and M32. However, on IODA1 or IODA2, one PE can have
multiple M64 segments as well.
quoted
+ */
+ unsigned long io_segmap[8];
+ unsigned long m32_segmap[8];
+ unsigned long m64_segmap[8];
Magic constant "8", 64bit*8 = 512 PEs - where did this come from?
Anyway,
#define PNV_IODA_MAX_PE_NUM 512
unsigned long io_segmap[PNV_IODA_MAX_PE_NUM/BITS_PER_LONG]
I prefer "8", not macro for 3 reasons:
- The macro won't be used in the code.
- The total segment number of specific resource is variable
on IODA1 and IODA2. I just choosed the max value with margin.
- PNV_IODA_MAX_PE_NUM, indicating max PE number, isn't 512 on
IODA1 or IODA2.
quoted
+
/* "Weight" assigned to the PE for the sake of DMA resource
* allocations
*/
@@ -145,15 +154,16 @@ struct pnv_phb { unsigned int io_segsize; unsigned int io_pci_base;+ /* IO, M32, M64 segment maps */+ unsigned long io_segmap[8];+ unsigned long m32_segmap[8];+ unsigned long m64_segmap[8];+ /* PE allocation */ struct mutex pe_alloc_mutex; unsigned long *pe_alloc; struct pnv_ioda_pe *pe_array;- /* M32 & IO segment maps */- unsigned int *m32_segmap;- unsigned int *io_segmap;- /* IRQ chip */ int irq_chip_init; struct irq_chip irq_chip;
On Mon, Aug 10, 2015 at 06:07:27PM +1000, Alexey Kardashevskiy wrote:
On 08/06/2015 02:11 PM, Gavin Shan wrote:
quoted
The patch cleans up DMA32 in pci-ioda.c. It shouldn't introduce
behavioural changes:
* Rename various fields in "struct pnv_phb" and "struct pnv_ioda_pe"
as 32-bits DMA should be related to "DMA", not "TCE".
s/dma_weight/dma32_weight/ is ok (does not add much though_ but the rest is
not. The "tce32_" fields are still TCEs (translation entries) while DMA is a
process initiated by a device which does not know about how exactly DMA
addresses are translated later. Since we are on the host side and we actually
manage TCE tables here, I suggest keeping the "tce32_" prefix for TCE tables
and memory they use.
Ok. Will change accordingly.
quoted
* Removed struct pnv_ioda_pe::tce32_segcount.
That's confusing - I had to walk through patches to find out where you
stopped using it. It would be simpler if you put this particular change to
[PATCH v6 02/42] powerpc/powernv: Drop pnv_ioda_setup_dev_PE()
where you remove dead code.
@@ -1936,7 +1936,7 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,/* XXX FIXME: Allocate multi-level tables on PHB3 *//* We shouldn't already have a 32-bit DMA associated */-if(WARN_ON(pe->tce32_seg>=0))+if(WARN_ON(pe->dma32_seg>=0))return;tbl=pnv_pci_table_alloc(phb->hose->node);
@@ -1945,7 +1945,7 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,pnv_pci_link_table_and_group(phb->hose->node,0,tbl,&pe->table_group);/* Grab a 32-bit TCE table */-pe->tce32_seg=base;+pe->dma32_seg=base;pe_info(pe," Setting up 32-bit TCE table at %08x..%08x\n",(base<<28),((base+segs)<<28)-1);
@@ -2006,8 +2006,8 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,return;fail:/* XXX Failure: Try to fallback to 64-bit only ? */-if(pe->tce32_seg>=0)-pe->tce32_seg=-1;+if(pe->dma32_seg>=0)+pe->dma32_seg=-1;if(tce_mem)__free_pages(tce_mem,get_order(TCE32_TABLE_SIZE*segs));if(tbl){
@@ -2405,7 +2405,7 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,int64_trc;/* We shouldn't already have a 32-bit DMA associated */-if(WARN_ON(pe->tce32_seg>=0))+if(WARN_ON(pe->dma32_seg>=0))return;/* TVE #1 is selected by PCI address bit 59 */
@@ -2415,7 +2415,7 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,pe->pe_number);/* The PE will reserve all possible 32-bits space */-pe->tce32_seg=0;+pe->dma32_seg=0;pe_info(pe,"Setting up 32-bit TCE table at 0..%08x\n",phb->ioda.m32_pci_base);
@@ -3211,7 +3211,7 @@ static void __init pnv_pci_init_ioda_phb(struct device_node *np,mutex_init(&phb->ioda.pe_list_mutex);/* Calculate how many 32-bit TCE segments we have */-phb->ioda.tce32_count=phb->ioda.m32_pci_base>>28;+phb->ioda.dma32_segcount=phb->ioda.m32_pci_base>>28;#if 0 /* We should really do that ... */rc=opal_pci_set_phb_mem_window(opal->phb_id,
@@ -61,11 +61,10 @@ struct pnv_ioda_pe {/* "Weight" assigned to the PE for the sake of DMA resource*allocations*/-unsignedintdma_weight;+unsignedintdma32_weight;/* "Base" iommu table, ie, 4K TCEs, 32-bit DMA */-inttce32_seg;-inttce32_segcount;+intdma32_seg;structiommu_table_grouptable_group;/* 64-bit TCE bypass region */
@@ -181,7 +180,7 @@ struct pnv_phb {unsignedcharpe_rmap[0x10000];/* 32-bit TCE tables allocation */-unsignedlongtce32_count;+unsignedlongdma32_segcount;/* Sorted list of used PE's, sorted at*bootforresourceallocationpurposes
On Mon, Aug 10, 2015 at 07:31:11PM +1000, Alexey Kardashevskiy wrote:
On 08/06/2015 02:11 PM, Gavin Shan wrote:
quoted
The original implementation of pnv_ioda_setup_dma() iterates the
list of PEs and configures the DMA32 space for them one by one.
The function was designed to be called during PHB fixup time.
When configuring PE's DMA32 space in pcibios_setup_bridge(), in
order to support PCI hotplug, we have to have the function PE
oriented.
This renames pnv_ioda_setup_dma() to pnv_ioda1_setup_dma() and
adds one more argument "struct pnv_ioda_pe *pe" to it. The caller,
pnv_pci_ioda_setup_DMA(), gets PE from the list and passes to it
or pnv_pci_ioda2_setup_dma_pe(). The patch shouldn't cause behavioral
changes.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 75 +++++++++++++++----------------
1 file changed, 36 insertions(+), 39 deletions(-)
@@ -2443,52 +2443,29 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,pnv_ioda_setup_bus_dma(pe,pe->pbus);}-staticvoidpnv_ioda_setup_dma(structpnv_phb*phb)+staticunsignedintpnv_ioda1_setup_dma(structpnv_phb*phb,+structpnv_ioda_pe*pe,+unsignedintbase){structpci_controller*hose=phb->hose;-structpnv_ioda_pe*pe;-unsignedintdma_weight;+unsignedintdma_weight,segs;/* Calculate the PHB's DMA weight */dma_weight=pnv_ioda_phb_dma_weight(phb);pr_info("PCI%04x has %ld DMA32 segments, total weight %d\n",hose->global_number,phb->ioda.dma32_segcount,dma_weight);-pnv_pci_ioda_setup_opal_tce_kill(phb);--/* Walk our PE list and configure their DMA segments, hand them-*outonebasesegmentplusanyresidualsegmentsbasedon-*weight-*/-list_for_each_entry(pe,&phb->ioda.pe_dma_list,dma_link){-if(!pe->dma32_weight)-continue;--/*-*ForIODA2compliantPHB3,weneedn'tcareabouttheweight.-*Theallavailable32-bitsDMAspacewillbeassignedto-*thespecificPE.-*/-if(phb->type==PNV_PHB_IODA1){-unsignedintsegs,base=0;--if(pe->dma32_weight<-dma_weight/phb->ioda.dma32_segcount)-segs=1;-else-segs=(pe->dma32_weight*-phb->ioda.dma32_segcount)/dma_weight;--pe_info(pe,"DMA32 weight %d, assigned %d segments\n",-pe->dma32_weight,segs);-pnv_pci_ioda_setup_dma_pe(phb,pe,base,segs);+if(pe->dma32_weight<+dma_weight/phb->ioda.dma32_segcount)
Why not to merge pnv_ioda1_setup_dma() to pnv_pci_ioda_setup_dma_pe()?
There're two reasons:
- They're separate logically. One is calculating number of DMA32 segments required.
Another one is allocate TCE32 tables and configure devices with them.
- In PCI hotplug path, I need pnv_ioda1_setup_dma() which has "pe" as parameter.
This @base handling seems never be tested between 8..11 as "[PATCH v6 11/42]
powerpc/powernv: Trace DMA32 segments consumed by PE"
removes it and I suspect you only tested the final version. Which is ok for
the final result but not ok for bisectability.
Looks like 8/42, 9/42, 10/42, 11/42 need to be rearranged or merged to remove
this multiple @base touching.
Why ?
quoted
+ break;
+ case PNV_PHB_IODA2:
+ pnv_pci_ioda2_setup_dma_pe(phb, pe);
+ break;
+ default:
+ pr_warn("%s: No DMA for PHB type %d\n",
+ __func__, phb->type);
+ }
+ }
/* Mark the PHB initialization done */
- phb = hose->private_data;
phb->initialized = 1;
}
}
On Mon, Aug 10, 2015 at 07:43:48PM +1000, Alexey Kardashevskiy wrote:
On 08/06/2015 02:11 PM, Gavin Shan wrote:
quoted
On P7IOC, the whole DMA32 space is divided evenly to 256MB segments.
Each PE can consume one or multiple DMA32 segments. Current code
doesn't trace the available DMA32 segments and those consumed by
one particular PE. It's conflicting with PCI hotplug.
The patch introduces one bitmap to PHB to trace the available
DMA32 segments for allocation, more fields to "struct pnv_ioda_pe"
to trace the consumed DMA32 segments by the PE, which is going to
be released when the PE is destroyed at PCI unplugging time.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 40 +++++++++++++++++++++++--------
arch/powerpc/platforms/powernv/pci.h | 4 +++-
2 files changed, 33 insertions(+), 11 deletions(-)
On Mon, Aug 10, 2015 at 08:02:20PM +1000, Alexey Kardashevskiy wrote:
On 08/06/2015 02:11 PM, Gavin Shan wrote:
quoted
Several functions used to configure PE take pe_number to indentify
PE instance. As the pe_number is included in PE instance after it
is reserved or allocated. It's convienent for those functions to
return PE instance which includes the required pe_number.
This is a description for the half of the patch but this patch also adds a
return value to functions which did not have it before and I am not sure you
need all of them to return something. It would be cleaner if you added
"return" when/where you really need it, not just because it seems that it may
be convenient later.
Fair enough. I'll change the commit log accordingly.
@@ -132,12 +132,12 @@ static inline bool pnv_pci_is_mem_pref_64(unsigned long flags)(IORESOURCE_MEM_64|IORESOURCE_PREFETCH));}-staticvoidpnv_ioda_reserve_pe(structpnv_phb*phb,intpe_no)+staticstructpnv_ioda_pe*pnv_ioda_reserve_pe(structpnv_phb*phb,intpe_no){if(!(pe_no>=0&&pe_no<phb->ioda.total_pe)){pr_warn("%s: Invalid PE %d on PHB#%x\n",__func__,pe_no,phb->hose->global_number);-return;+returnNULL;}if(test_and_set_bit(pe_no,phb->ioda.pe_alloc))
@@ -344,7 +346,7 @@ static int pnv_ioda_pick_m64_pe(struct pci_bus *bus, bool all)/* Root bus shouldn't use M64 */if(pci_is_root_bus(bus))-returnIODA_INVALID_PE;+returnNULL;/* Allocate bitmap */size=_ALIGN_UP(phb->ioda.total_pe/8,sizeof(unsignedlong));
@@ -352,7 +354,7 @@ static int pnv_ioda_pick_m64_pe(struct pci_bus *bus, bool all)if(!pe_alloc){pr_warn("%s: Out of memory !\n",__func__);-returnIODA_INVALID_PE;+returnNULL;}/* Figure out reserved PE numbers by the PE */
@@ -1069,28 +1071,26 @@ static void pnv_ioda_setup_same_PE(struct pci_bus *bus, struct pnv_ioda_pe *pe)*subordinatePCIdevicesandbuses.ThesecondtypeofPEisnormally*orgiriatedbyPCIe-to-PCIbridgeorPLXswitchdownstreamports.*/-staticvoidpnv_ioda_setup_bus_PE(structpci_bus*bus,boolall)+staticstructpnv_ioda_pe*pnv_ioda_setup_bus_PE(structpci_bus*bus,boolall){structpci_controller*hose=pci_bus_to_host(bus);structpnv_phb*phb=hose->private_data;-structpnv_ioda_pe*pe;-intpe_num=IODA_INVALID_PE;+structpnv_ioda_pe*pe=NULL;/* Check if PE is determined by M64 */if(phb->pick_m64_pe)-pe_num=phb->pick_m64_pe(bus,all);+pe=phb->pick_m64_pe(bus,all);/* The PE number isn't pinned by M64 */-if(pe_num==IODA_INVALID_PE)-pe_num=pnv_ioda_alloc_pe(phb);+if(!pe)+pe=pnv_ioda_alloc_pe(phb);-if(pe_num==IODA_INVALID_PE){-pr_warning("%s: Not enough PE# available for PCI bus %04x:%02x\n",+if(!pe){+pr_warning("%s: No enough PE# for PCI bus %04x:%02x\n",__func__,pci_domain_nr(bus),bus->number);-return;+returnNULL;}-pe=&phb->ioda.pe_array[pe_num];pe->flags|=(all?PNV_IODA_PE_BUS_ALL:PNV_IODA_PE_BUS);pe->pbus=bus;pe->pdev=NULL;
@@ -1101,17 +1101,16 @@ static void pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all)if(all)pe_info(pe,"Secondary bus %d..%d associated with PE#%d\n",-bus->busn_res.start,bus->busn_res.end,pe_num);+bus->busn_res.start,bus->busn_res.end,pe->pe_number);elsepe_info(pe,"Secondary bus %d associated with PE#%d\n",-bus->busn_res.start,pe_num);+bus->busn_res.start,pe->pe_number);if(pnv_ioda_configure_pe(phb,pe)){/* XXX What do we do here ? */-if(pe_num)-pnv_ioda_free_pe(phb,pe_num);+pnv_ioda_free_pe(phb,pe->pe_number);pe->pbus=NULL;-return;+returnNULL;}/* Associate it with all child devices */
@@ -1122,6 +1121,8 @@ static void pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all)/* Link the PE */pnv_ioda_link_pe_by_weight(phb,pe);++returnpe;}staticvoidpnv_ioda_setup_PEs(structpci_bus*bus)
On Tue, Aug 11, 2015 at 12:39:02AM +1000, Alexey Kardashevskiy wrote:
On 08/06/2015 02:11 PM, Gavin Shan wrote:
quoted
The available PE#, represented by a bitmap in the PHB, is allocated
in ascending order.
Available PE# is available exactly because it is not allocated ;)
Yeah, will correct it.
quoted
It conflicts with the fact that M64 segments are
assigned in same order. In order to avoid the conflict, the patch
allocates PE# in descending order.
What kind of conflict?
On PHB3, the M64 segment is assigned to one PE whose PE number is
determined. M64 segment are allocated in ascending order. It's why
I would like to allocate PE# in deascending order.
On Mon, Aug 10, 2015 at 03:42:32PM -0700, Frank Rowand wrote:
On 8/5/2015 9:11 PM, Gavin Shan wrote:
Frank, thanks for your comments. All of them will be included
in next revision.
Thanks,
Gavin
quoted
This changes of_fdt_unflatten_tree() so that it returns the allocated
memory chunk for unflattened device-tree, which can be released once
it's obsoleted.
Signed-off-by: Gavin Shan <redacted>
---
drivers/of/fdt.c | 11 ++++++-----
include/linux/of_fdt.h | 2 +-
2 files changed, 7 insertions(+), 6 deletions(-)
@@ -473,11 +474,11 @@ static void *kernel_tree_alloc(u64 size, u64 align) * pointers of the nodes so the normal device-tree walking functions * can be used. */-void of_fdt_unflatten_tree(const unsigned long *blob,+void *of_fdt_unflatten_tree(const unsigned long *blob,
Please add a description of the return value to the documentation header.
On Mon, Aug 10, 2015 at 03:42:13PM -0700, Frank Rowand wrote:
On 8/5/2015 9:11 PM, Gavin Shan wrote:
quoted
This introduces one more argument to of_fdt_unflatten_tree()
to specify the root node for the FDT blob, which is going to be
unflattened. In the result, the function can be used to unflatten
FDT blob, which represents device sub-tree in PowerNV hotplug
driver.
Signed-off-by: Gavin Shan <redacted>
---
drivers/of/fdt.c | 13 ++++++++-----
drivers/of/unittest.c | 2 +-
include/linux/of_fdt.h | 1 +
3 files changed, 10 insertions(+), 6 deletions(-)
@@ -907,7 +907,7 @@ static int __init unittest_data_add(void)"not running tests\n",__func__);return-ENOMEM;}-of_fdt_unflatten_tree(unittest_data,&unittest_data_node);+of_fdt_unflatten_tree(unittest_data,NULL,&unittest_data_node);if(!unittest_data_node){pr_warn("%s: No tree to attach; not running tests\n",__func__);return-ENODATA;
On Mon, Aug 10, 2015 at 04:30:09PM +1000, Alexey Kardashevskiy wrote:
quoted
On 08/06/2015 02:11 PM, Gavin Shan wrote:
quoted
The patch enables M64 window on P7IOC, which has been enabled on
PHB3. Different from PHB3 where 16 M64 BARs are supported and each
of them can be owned by one particular PE# exclusively or divided
evenly to 256 segments, each P7IOC PHB has 16 M64 BARs and each
of them are divided into 8 segments.
Is this a limitation of POWER7 chip or it is from IODA1?
From IODA1.
quoted
quoted
So each P7IOC PHB can support
128 M64 segments only. Also, P7IOC has M64DT, which helps mapping
one particular M64 segment# to arbitrary PE#. PHB3 doesn't have
M64DT, indicating that one M64 segment can only be pinned to the
fixed PE#. In order to have similar logic to support M64 for PHB3
and P7IOC, we just provide 128 M64 (16 BARs) segments and fixed
mapping between PE# and M64 segment# on P7IOC. In turn, we just
need different phb->init_m64() hooks for P7IOC and PHB3 to support
M64.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 116 ++++++++++++++++++++++++++----
1 file changed, 104 insertions(+), 12 deletions(-)
@@ -172,6 +172,69 @@ static void pnv_ioda_free_pe(struct pnv_phb *phb, int pe)clear_bit(pe,phb->ioda.pe_alloc);}+staticintpnv_ioda1_init_m64(structpnv_phb*phb)+{+structresource*r;+intseg;++/* There are as many M64 segments as the maximum number+*ofPEs,whichis128.+*/+for(seg=0;seg<phb->ioda.total_pe;seg+=8){
This "8" is used a lot across the patch, please make it a macro
(PNV_PHB_P7IOC_SEGNUM or PNV_PHB_IODA1_SEGNUM or whatever you think it is)
with a short comment why it is "8". Or a pnv_phb member.
I would like to use "8". When having a macro, you have to check
the definition of the macro to get the real value of that.
Give it a good name then.
However,
it makes sense to add more comments explaining why it's 8 here.
You cannot comment it everywhere and everywhere is exact place when you'll
have to comment it as I believe sometime it is segments-per-M64 and
sometime it is number of bits in a byte (or not? anyway, this is will
always distract unless you use macro for segments-per-M64).
quoted
quoted
+ unsigned long base;
+ int64_t rc;
+
+ base = phb->ioda.m64_base + seg * phb->ioda.m64_segsize;
+ rc = opal_pci_set_phb_mem_window(phb->opal_id,
+ OPAL_M64_WINDOW_TYPE,
+ seg / 8,
+ base,
+ 0, /* unused */
+ 8 * phb->ioda.m64_segsize);
+ if (rc != OPAL_SUCCESS) {
+ pr_warn(" Error %lld setting M64 PHB#%d-BAR#%d\n",
+ rc, phb->hose->global_number, seg / 8);
+ goto fail;
+ }
+
+ rc = opal_pci_phb_mmio_enable(phb->opal_id,
+ OPAL_M64_WINDOW_TYPE,
+ seg / 8,
+ OPAL_ENABLE_M64_SPLIT);
+ if (rc != OPAL_SUCCESS) {
+ pr_warn(" Error %lld enabling M64 PHB#%d-BAR#%d\n",
+ rc, phb->hose->global_number, seg / 8);
+ goto fail;
+ }
+ }
+
+ /* Strip off the segment used by the reserved PE, which
What is this reserved PE on P7IOC? "Strip off" means "exclude" here?
127 that was exported from skiboot. "Strip off" means "exclude".
I like "exclude" lot better.
quoted
quoted
+ * is expected to be 0 or last supported PE#. The PHB's
+ * first memory window traces the 32-bits MMIO range
s/traces/filters/ ? Or I did not understand this comment...
It seems you didn't understand it: there are two memory windows
in every PHB. The first one is tracing M32 resource and the
second one is tracing M64 resource.
Tracing means logging, pretty much. Is this what you mean here?
quoted
quoted
+ * while the second one traces the 64-bits prefetchable
+ * MMIO range that the PHB supports.
32/64 ranges comment seems irrelevant here.
Maybe it's not so relevant, but still.
Not relevant -> remove it. Put this text to the commit log.
We're stripping off the
M64 segment from the 2nd resource (as above), not first one.
2nd window (not _resource_), you mean?
quoted
quoted
+ */
+ r = &phb->hose->mem_resources[1];
+ if (phb->ioda.reserved_pe == 0)
+ r->start += phb->ioda.m64_segsize;
+ else if (phb->ioda.reserved_pe == (phb->ioda.total_pe - 1))
+ r->end -= phb->ioda.m64_segsize;
+ else
+ pr_warn(" Cannot strip M64 segment for reserved PE#%d\n",
+ phb->ioda.reserved_pe);
+
+ return 0;
+
+fail:
+ for ( ; seg >= 0; seg -= 8)
+ opal_pci_phb_mmio_enable(phb->opal_id,
+ OPAL_M64_WINDOW_TYPE,
+ seg / 8,
+ OPAL_DISABLE_M64);
+
+ return -EIO;
+}
+
/* The default M64 BAR is shared by all PEs */
static int pnv_ioda2_init_m64(struct pnv_phb *phb)
{
@@ -293,7 +356,7 @@ static int pnv_ioda2_pick_m64_pe(struct pci_bus *bus, bool all) } /* Figure out reserved PE numbers by the PE */- pnv_ioda2_reserve_m64_pe(bus, pe_alloc, all);+ pnv_ioda_reserve_m64_pe(bus, pe_alloc, all); /* * the current bus might not own M64 window and that's all
@@ -324,6 +387,26 @@ static int pnv_ioda2_pick_m64_pe(struct pci_bus *bus, bool all) pe->master = master_pe; list_add_tail(&pe->list, &master_pe->slaves); }++ /* P7IOC supports M64DT, which helps mapping M64 segment+ * to one particular PE#. However, PHB3 has fixed mapping+ * between M64 segment and PE#. In order to have same logic+ * for P7IOC and PHB3, we enforce fixed mapping between M64+ * segment and PE# on P7IOC.+ */+ if (phb->type == PNV_PHB_IODA1) {+ int64_t rc;++ rc = opal_pci_map_pe_mmio_window(phb->opal_id,+ pe->pe_number,+ OPAL_M64_WINDOW_TYPE,+ pe->pe_number / 8,+ pe->pe_number % 8);+ if (rc != OPAL_SUCCESS)+ pr_warn("%s: Error %lld mapping M64 for PHB#%d-PE#%d\n",+ __func__, rc, phb->hose->global_number,+ pe->pe_number);+ } } kfree(pe_alloc);
@@ -338,8 +421,8 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb) const u32 *r; u64 pci_addr;- /* FIXME: Support M64 for P7IOC */- if (phb->type != PNV_PHB_IODA2) {+ if (phb->type != PNV_PHB_IODA1 &&+ phb->type != PNV_PHB_IODA2) { pr_info(" Not support M64 window\n"); return;
You are adding P7IOC support so at least "fixme" should go. Also,
pnv_ioda_parse_m64_window() is only called from pnv_pci_init_ioda_phb() which
is called only with PNV_PHB_IODA1 and PNV_PHB_IODA2 (no other value is passed
there a type) so the check above will never succeed, just remove it.
The "fixme" is removed, isn't it?
Ah, my bad.
As I explained last time, there will have another new type PHB and the function
will be called on the new type of PHB.
Then a new patch adding new PHB should take care of this check too. This is
not something which can possibly happen on a real machine, we support one
of 2 (later - 3) PHBs and if a machine got something else, we won't get
that far anyway and we cannot gracefully fallback to some "generic PHB"
(like 440fx on x86) as we do not have one.
At least make it BUG_ON() to document it.
The code has been there and it's not
in upstream yet. So it's reasonable to keep it, instead of removing it.
No, not really.
quoted
quoted
}
@@ -372,9 +455,18 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb) /* Use last M64 BAR to cover M64 window */ phb->ioda.m64_bar_idx = 15;- phb->init_m64 = pnv_ioda2_init_m64;- phb->reserve_m64_pe = pnv_ioda2_reserve_m64_pe;- phb->pick_m64_pe = pnv_ioda2_pick_m64_pe;+ phb->reserve_m64_pe = pnv_ioda_reserve_m64_pe;+ phb->pick_m64_pe = pnv_ioda_pick_m64_pe;+ switch (phb->type) {+ case PNV_PHB_IODA1:+ phb->init_m64 = pnv_ioda1_init_m64;+ break;+ case PNV_PHB_IODA2:+ phb->init_m64 = pnv_ioda2_init_m64;+ break;+ default:+ pr_debug(" M64 not supported\n");+ } } static void pnv_ioda_freeze_pe(struct pnv_phb *phb, int pe_no)
On Mon, Aug 10, 2015 at 05:16:40PM +1000, Alexey Kardashevskiy wrote:
quoted
On 08/06/2015 02:11 PM, Gavin Shan wrote:
quoted
The patch is adding 6 bitmaps, three to PE and three to PHB, to track
The patch is also removing 2 arrays (io_segmap and m32_segmap), what is that
all about? Also, there was no m64_segmap, now there is, needs an explanation
may be.
Originally, the bitmaps (io_segmap and m32_segmap) are allocated dynamically.
Now, they have fixed sizes - 512 bits.
The subject "powerpc/powernv: Track IO/M32/M64 segments from PE" indicates
why m64_segmap is added.
But before this patch, you somehow managed to keep it working without a map
for M64, by the same time you needed map for IO and M32. It seems you are
making things consistent in this patch but it also feels like you do not
have to do so as M64 did not need a map before and I cannot see why it
needs one now.
quoted
quoted
the consumed by one particular PE, which can be released once the PE
is destroyed during PCI unplugging time. Also, we're using fixed
quantity of bits to trace the used IO, M32 and M64 segments by PEs
in one particular PHB.
Out of curiosity - have you considered having just 3 arrays, in PHB, storing
PE numbers, and ditching PE's arrays? Does PE itself need to know what PEs it
is using? Not sure about this master/slave PEs though.
I don't follow your suggestion. Can you rephrase and explain it a bit more?
Please explains in what situations you need same map in both PHB and PE and
how you are going to use them. For example, pe::m64_segmap and phb::m64_segmap.
I believe you need to know what segment is used by what PE and that's it
and having 2 bitmaps is overcomplicated hard to follow. Is there anything
else what I am missing?
quoted
It would be easier to read patches if this one was right before
[PATCH v6 23/42] powerpc/powernv: Release PEs dynamically
I'll try to reoder the patch, but not expect too much...
@@ -3175,19 +3185,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 */
This comment came with if(IODA1) below, since you are removing the condition
below, makes sense to remove the comment as well or move it where people will
look for it (arch/powerpc/platforms/powernv/pci.h ?)
@@ -49,6 +49,15 @@ struct pnv_ioda_pe {/* PE number */unsignedintpe_number;+/* IO/M32/M64 segments consumed by the PE. Each PE can+*haveoneM64segmentatmost,butM64segmentsconsumed+*byslavePEswillbecontributedtothemasterPE.One+*PEcanownmultipleIOandM32segments.
A PE can have multiple IO and M32 segments but just one M64 segment? Is this
correct for IODA1 or IODA2 or both? Is this a limitation of this
implementation or it comes from P7IOC/PHB3 hardware?
It's correct for IO and M32. However, on IODA1 or IODA2, one PE can have
multiple M64 segments as well.
But the comment says "Each PE can have one M64 segment at most". Which
statement is correct?
quoted
quoted
+ */
+ unsigned long io_segmap[8];
+ unsigned long m32_segmap[8];
+ unsigned long m64_segmap[8];
Magic constant "8", 64bit*8 = 512 PEs - where did this come from?
Anyway,
#define PNV_IODA_MAX_PE_NUM 512
unsigned long io_segmap[PNV_IODA_MAX_PE_NUM/BITS_PER_LONG]
I prefer "8", not macro for 3 reasons:
- The macro won't be used in the code.
You will use it 6 times in the header, if you give it a good name, people
won't have to guess if the meaning of all these "8"s is the same and you
won't have to comment every use of it in this header file (now you have).
Also, using BITS_PER_LONG tells the reader that this is a bitmask for sure.
- The total segment number of specific resource is variable
on IODA1 and IODA2. I just choosed the max value with margin.
- PNV_IODA_MAX_PE_NUM, indicating max PE number, isn't 512 on
IODA1 or IODA2.
Give it a better name.
quoted
quoted
+
/* "Weight" assigned to the PE for the sake of DMA resource
* allocations
*/
@@ -145,15 +154,16 @@ struct pnv_phb { unsigned int io_segsize; unsigned int io_pci_base;+ /* IO, M32, M64 segment maps */+ unsigned long io_segmap[8];+ unsigned long m32_segmap[8];+ unsigned long m64_segmap[8];+ /* PE allocation */ struct mutex pe_alloc_mutex; unsigned long *pe_alloc; struct pnv_ioda_pe *pe_array;- /* M32 & IO segment maps */- unsigned int *m32_segmap;- unsigned int *io_segmap;- /* IRQ chip */ int irq_chip_init; struct irq_chip irq_chip;
On Mon, Aug 10, 2015 at 07:31:11PM +1000, Alexey Kardashevskiy wrote:
quoted
On 08/06/2015 02:11 PM, Gavin Shan wrote:
quoted
The original implementation of pnv_ioda_setup_dma() iterates the
list of PEs and configures the DMA32 space for them one by one.
The function was designed to be called during PHB fixup time.
When configuring PE's DMA32 space in pcibios_setup_bridge(), in
order to support PCI hotplug, we have to have the function PE
oriented.
This renames pnv_ioda_setup_dma() to pnv_ioda1_setup_dma() and
adds one more argument "struct pnv_ioda_pe *pe" to it. The caller,
pnv_pci_ioda_setup_DMA(), gets PE from the list and passes to it
or pnv_pci_ioda2_setup_dma_pe(). The patch shouldn't cause behavioral
changes.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 75 +++++++++++++++----------------
1 file changed, 36 insertions(+), 39 deletions(-)
@@ -2443,52 +2443,29 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,pnv_ioda_setup_bus_dma(pe,pe->pbus);}-staticvoidpnv_ioda_setup_dma(structpnv_phb*phb)+staticunsignedintpnv_ioda1_setup_dma(structpnv_phb*phb,+structpnv_ioda_pe*pe,+unsignedintbase){structpci_controller*hose=phb->hose;-structpnv_ioda_pe*pe;-unsignedintdma_weight;+unsignedintdma_weight,segs;/* Calculate the PHB's DMA weight */dma_weight=pnv_ioda_phb_dma_weight(phb);pr_info("PCI%04x has %ld DMA32 segments, total weight %d\n",hose->global_number,phb->ioda.dma32_segcount,dma_weight);-pnv_pci_ioda_setup_opal_tce_kill(phb);--/* Walk our PE list and configure their DMA segments, hand them-*outonebasesegmentplusanyresidualsegmentsbasedon-*weight-*/-list_for_each_entry(pe,&phb->ioda.pe_dma_list,dma_link){-if(!pe->dma32_weight)-continue;--/*-*ForIODA2compliantPHB3,weneedn'tcareabouttheweight.-*Theallavailable32-bitsDMAspacewillbeassignedto-*thespecificPE.-*/-if(phb->type==PNV_PHB_IODA1){-unsignedintsegs,base=0;--if(pe->dma32_weight<-dma_weight/phb->ioda.dma32_segcount)-segs=1;-else-segs=(pe->dma32_weight*-phb->ioda.dma32_segcount)/dma_weight;--pe_info(pe,"DMA32 weight %d, assigned %d segments\n",-pe->dma32_weight,segs);-pnv_pci_ioda_setup_dma_pe(phb,pe,base,segs);+if(pe->dma32_weight<+dma_weight/phb->ioda.dma32_segcount)
Why not to merge pnv_ioda1_setup_dma() to pnv_pci_ioda_setup_dma_pe()?
There're two reasons:
- They're separate logically. One is calculating number of DMA32 segments required.
Another one is allocate TCE32 tables and configure devices with them.
- In PCI hotplug path, I need pnv_ioda1_setup_dma() which has "pe" as parameter.
And hotplug path does not care about dma weight why?
This @base handling seems never be tested between 8..11 as "[PATCH v6 11/42]
powerpc/powernv: Trace DMA32 segments consumed by PE"
removes it and I suspect you only tested the final version. Which is ok for
the final result but not ok for bisectability.
Looks like 8/42, 9/42, 10/42, 11/42 need to be rearranged or merged to remove
this multiple @base touching.
Why ?
You are touching this @base from 8/42 to 11/12 and in between it is very
broken, you only get it fixed (by removing) in 11/42. Read my comment for
8/42. After every single patch in any patchset the functionality should not
break but it does in this patchset.
quoted
quoted
+ break;
+ case PNV_PHB_IODA2:
+ pnv_pci_ioda2_setup_dma_pe(phb, pe);
+ break;
+ default:
+ pr_warn("%s: No DMA for PHB type %d\n",
+ __func__, phb->type);
+ }
+ }
/* Mark the PHB initialization done */
- phb = hose->private_data;
phb->initialized = 1;
}
}
On Tue, Aug 11, 2015 at 12:39:02AM +1000, Alexey Kardashevskiy wrote:
quoted
On 08/06/2015 02:11 PM, Gavin Shan wrote:
quoted
The available PE#, represented by a bitmap in the PHB, is allocated
in ascending order.
Available PE# is available exactly because it is not allocated ;)
Yeah, will correct it.
quoted
quoted
It conflicts with the fact that M64 segments are
assigned in same order. In order to avoid the conflict, the patch
allocates PE# in descending order.
What kind of conflict?
On PHB3, the M64 segment is assigned to one PE whose PE number is
determined. M64 segment are allocated in ascending order. It's why
I would like to allocate PE# in deascending order.
From previous lessons, I thought M64 segment number is PE# number as well :-/
Seems this is not the case, so what does store this seg#<->PE# mapping in PHB?
This adds the refcount to PE, which represents number of PCI
devices contained in the PE. When last device leaves from the
PE, the PE together with its consumed resources (IO, DMA, PELTM,
PELTV) are released, to support PCI hotplug.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 233 +++++++++++++++++++++++++++---
arch/powerpc/platforms/powernv/pci.h | 3 +
2 files changed, 217 insertions(+), 19 deletions(-)
This is BUG_ON() indeed because we cannot possibly get that far with
unsupported PHB type, it would have crashed earlier.
+ }
+}
+
+static void pnv_ioda_release_pe_one_seg(struct pnv_ioda_pe *pe, int win)
+{
+ struct pnv_phb *phb = pe->phb;
+ unsigned long *segmap = NULL;
+ unsigned long *pe_segmap = NULL;
+ int segno, limit, mod = 0;
+
+ switch (win) {
+ case OPAL_IO_WINDOW_TYPE:
+ segmap = phb->ioda.io_segmap;
+ pe_segmap = pe->io_segmap;
+ break;
+ case OPAL_M32_WINDOW_TYPE:
+ segmap = phb->ioda.m32_segmap;
+ pe_segmap = pe->m32_segmap;
+ break;
+ case OPAL_M64_WINDOW_TYPE:
+ if (phb->type != PNV_PHB_IODA1)
+ return;
+ segmap = phb->ioda.m64_segmap;
+ pe_segmap = pe->m64_segmap;
You seem to keep phb->ioda.m64_segmap update but you never actually read
it, you only read pe->m64_segmap. Is that correct or I am missing something
here?
The name of pnv_ioda_free_pe() suggests it should work for non-SRIOV case
too but you put it under #ifdef IOV, is that correct? Is so, rename it please.
@@ -25,11 +25,14 @@ enum pnv_phb_model {#define PNV_IODA_PE_SLAVE (1 << 4) /* Slave PE in compound case */#define PNV_IODA_PE_VF (1 << 5) /* PE for one VF */+#define PNV_INVALID_SEGMENT (-1)+/* Data associated with a PE, including IOMMU tracking etc.. */structpnv_phb;structpnv_ioda_pe{unsignedlongflags;structpnv_phb*phb;+intdevice_count;/* A PE can be associated with a single device or an*entirebus(&children).Intheformercase,pdev
On Tue, Aug 11, 2015 at 12:06:26PM +1000, Alexey Kardashevskiy wrote:
On 08/11/2015 09:45 AM, Gavin Shan wrote:
quoted
On Mon, Aug 10, 2015 at 04:30:09PM +1000, Alexey Kardashevskiy wrote:
quoted
On 08/06/2015 02:11 PM, Gavin Shan wrote:
quoted
The patch enables M64 window on P7IOC, which has been enabled on
PHB3. Different from PHB3 where 16 M64 BARs are supported and each
of them can be owned by one particular PE# exclusively or divided
evenly to 256 segments, each P7IOC PHB has 16 M64 BARs and each
of them are divided into 8 segments.
Is this a limitation of POWER7 chip or it is from IODA1?
From IODA1.
quoted
quoted
So each P7IOC PHB can support
128 M64 segments only. Also, P7IOC has M64DT, which helps mapping
one particular M64 segment# to arbitrary PE#. PHB3 doesn't have
M64DT, indicating that one M64 segment can only be pinned to the
fixed PE#. In order to have similar logic to support M64 for PHB3
and P7IOC, we just provide 128 M64 (16 BARs) segments and fixed
mapping between PE# and M64 segment# on P7IOC. In turn, we just
need different phb->init_m64() hooks for P7IOC and PHB3 to support
M64.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 116 ++++++++++++++++++++++++++----
1 file changed, 104 insertions(+), 12 deletions(-)
@@ -172,6 +172,69 @@ static void pnv_ioda_free_pe(struct pnv_phb *phb, int pe)clear_bit(pe,phb->ioda.pe_alloc);}+staticintpnv_ioda1_init_m64(structpnv_phb*phb)+{+structresource*r;+intseg;++/* There are as many M64 segments as the maximum number+*ofPEs,whichis128.+*/+for(seg=0;seg<phb->ioda.total_pe;seg+=8){
This "8" is used a lot across the patch, please make it a macro
(PNV_PHB_P7IOC_SEGNUM or PNV_PHB_IODA1_SEGNUM or whatever you think it is)
with a short comment why it is "8". Or a pnv_phb member.
I would like to use "8". When having a macro, you have to check
the definition of the macro to get the real value of that.
Give it a good name then.
quoted
However,
it makes sense to add more comments explaining why it's 8 here.
You cannot comment it everywhere and everywhere is exact place when you'll
have to comment it as I believe sometime it is segments-per-M64 and sometime
it is number of bits in a byte (or not? anyway, this is will always distract
unless you use macro for segments-per-M64).
Ok. I will use PNV_PHB_IODA1_SEGNUM then.
quoted
quoted
quoted
+ unsigned long base;
+ int64_t rc;
+
+ base = phb->ioda.m64_base + seg * phb->ioda.m64_segsize;
+ rc = opal_pci_set_phb_mem_window(phb->opal_id,
+ OPAL_M64_WINDOW_TYPE,
+ seg / 8,
+ base,
+ 0, /* unused */
+ 8 * phb->ioda.m64_segsize);
+ if (rc != OPAL_SUCCESS) {
+ pr_warn(" Error %lld setting M64 PHB#%d-BAR#%d\n",
+ rc, phb->hose->global_number, seg / 8);
+ goto fail;
+ }
+
+ rc = opal_pci_phb_mmio_enable(phb->opal_id,
+ OPAL_M64_WINDOW_TYPE,
+ seg / 8,
+ OPAL_ENABLE_M64_SPLIT);
+ if (rc != OPAL_SUCCESS) {
+ pr_warn(" Error %lld enabling M64 PHB#%d-BAR#%d\n",
+ rc, phb->hose->global_number, seg / 8);
+ goto fail;
+ }
+ }
+
+ /* Strip off the segment used by the reserved PE, which
What is this reserved PE on P7IOC? "Strip off" means "exclude" here?
127 that was exported from skiboot. "Strip off" means "exclude".
I like "exclude" lot better.
Ok. Will use it.
quoted
quoted
quoted
+ * is expected to be 0 or last supported PE#. The PHB's
+ * first memory window traces the 32-bits MMIO range
s/traces/filters/ ? Or I did not understand this comment...
It seems you didn't understand it: there are two memory windows
in every PHB. The first one is tracing M32 resource and the
second one is tracing M64 resource.
Tracing means logging, pretty much. Is this what you mean here?
No, it means "recording", not "logging". So it would be appropriate
to replace it with "track"?
quoted
quoted
quoted
+ * while the second one traces the 64-bits prefetchable
+ * MMIO range that the PHB supports.
32/64 ranges comment seems irrelevant here.
Maybe it's not so relevant, but still.
Not relevant -> remove it. Put this text to the commit log.
Ok.
quoted
We're stripping off the
M64 segment from the 2nd resource (as above), not first one.
2nd window (not _resource_), you mean?
I mean struct pci_controller::mem_resources[1].
quoted
quoted
quoted
+ */
+ r = &phb->hose->mem_resources[1];
+ if (phb->ioda.reserved_pe == 0)
+ r->start += phb->ioda.m64_segsize;
+ else if (phb->ioda.reserved_pe == (phb->ioda.total_pe - 1))
+ r->end -= phb->ioda.m64_segsize;
+ else
+ pr_warn(" Cannot strip M64 segment for reserved PE#%d\n",
+ phb->ioda.reserved_pe);
+
+ return 0;
+
+fail:
+ for ( ; seg >= 0; seg -= 8)
+ opal_pci_phb_mmio_enable(phb->opal_id,
+ OPAL_M64_WINDOW_TYPE,
+ seg / 8,
+ OPAL_DISABLE_M64);
+
+ return -EIO;
+}
+
/* The default M64 BAR is shared by all PEs */
static int pnv_ioda2_init_m64(struct pnv_phb *phb)
{
@@ -293,7 +356,7 @@ static int pnv_ioda2_pick_m64_pe(struct pci_bus *bus, bool all) } /* Figure out reserved PE numbers by the PE */- pnv_ioda2_reserve_m64_pe(bus, pe_alloc, all);+ pnv_ioda_reserve_m64_pe(bus, pe_alloc, all); /* * the current bus might not own M64 window and that's all
@@ -324,6 +387,26 @@ static int pnv_ioda2_pick_m64_pe(struct pci_bus *bus, bool all) pe->master = master_pe; list_add_tail(&pe->list, &master_pe->slaves); }++ /* P7IOC supports M64DT, which helps mapping M64 segment+ * to one particular PE#. However, PHB3 has fixed mapping+ * between M64 segment and PE#. In order to have same logic+ * for P7IOC and PHB3, we enforce fixed mapping between M64+ * segment and PE# on P7IOC.+ */+ if (phb->type == PNV_PHB_IODA1) {+ int64_t rc;++ rc = opal_pci_map_pe_mmio_window(phb->opal_id,+ pe->pe_number,+ OPAL_M64_WINDOW_TYPE,+ pe->pe_number / 8,+ pe->pe_number % 8);+ if (rc != OPAL_SUCCESS)+ pr_warn("%s: Error %lld mapping M64 for PHB#%d-PE#%d\n",+ __func__, rc, phb->hose->global_number,+ pe->pe_number);+ } } kfree(pe_alloc);
@@ -338,8 +421,8 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb) const u32 *r; u64 pci_addr;- /* FIXME: Support M64 for P7IOC */- if (phb->type != PNV_PHB_IODA2) {+ if (phb->type != PNV_PHB_IODA1 &&+ phb->type != PNV_PHB_IODA2) { pr_info(" Not support M64 window\n"); return;
You are adding P7IOC support so at least "fixme" should go. Also,
pnv_ioda_parse_m64_window() is only called from pnv_pci_init_ioda_phb() which
is called only with PNV_PHB_IODA1 and PNV_PHB_IODA2 (no other value is passed
there a type) so the check above will never succeed, just remove it.
The "fixme" is removed, isn't it?
Ah, my bad.
quoted
As I explained last time, there will have another new type PHB and the function
will be called on the new type of PHB.
Then a new patch adding new PHB should take care of this check too. This is
not something which can possibly happen on a real machine, we support one of
2 (later - 3) PHBs and if a machine got something else, we won't get that far
anyway and we cannot gracefully fallback to some "generic PHB" (like 440fx on
x86) as we do not have one.
At least make it BUG_ON() to document it.
ok. I'll change accordingly.
quoted
The code has been there and it's not
in upstream yet. So it's reasonable to keep it, instead of removing it.
No, not really.
quoted
quoted
quoted
}
@@ -372,9 +455,18 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb) /* Use last M64 BAR to cover M64 window */ phb->ioda.m64_bar_idx = 15;- phb->init_m64 = pnv_ioda2_init_m64;- phb->reserve_m64_pe = pnv_ioda2_reserve_m64_pe;- phb->pick_m64_pe = pnv_ioda2_pick_m64_pe;+ phb->reserve_m64_pe = pnv_ioda_reserve_m64_pe;+ phb->pick_m64_pe = pnv_ioda_pick_m64_pe;+ switch (phb->type) {+ case PNV_PHB_IODA1:+ phb->init_m64 = pnv_ioda1_init_m64;+ break;+ case PNV_PHB_IODA2:+ phb->init_m64 = pnv_ioda2_init_m64;+ break;+ default:+ pr_debug(" M64 not supported\n");+ } } static void pnv_ioda_freeze_pe(struct pnv_phb *phb, int pe_no)
On Tue, Aug 11, 2015 at 12:23:42PM +1000, Alexey Kardashevskiy wrote:
On 08/11/2015 10:03 AM, Gavin Shan wrote:
quoted
On Mon, Aug 10, 2015 at 05:16:40PM +1000, Alexey Kardashevskiy wrote:
quoted
On 08/06/2015 02:11 PM, Gavin Shan wrote:
quoted
The patch is adding 6 bitmaps, three to PE and three to PHB, to track
The patch is also removing 2 arrays (io_segmap and m32_segmap), what is that
all about? Also, there was no m64_segmap, now there is, needs an explanation
may be.
Originally, the bitmaps (io_segmap and m32_segmap) are allocated dynamically.
Now, they have fixed sizes - 512 bits.
The subject "powerpc/powernv: Track IO/M32/M64 segments from PE" indicates
why m64_segmap is added.
But before this patch, you somehow managed to keep it working without a map
for M64, by the same time you needed map for IO and M32. It seems you are
making things consistent in this patch but it also feels like you do not have
to do so as M64 did not need a map before and I cannot see why it needs one
now.
The M64 map is used by [PATCH v6 23/42] powerpc/powernv: Release PEs dynamically
where the M64 segments consumed by one particular PE will be released.
quoted
quoted
quoted
the consumed by one particular PE, which can be released once the PE
is destroyed during PCI unplugging time. Also, we're using fixed
quantity of bits to trace the used IO, M32 and M64 segments by PEs
in one particular PHB.
Out of curiosity - have you considered having just 3 arrays, in PHB, storing
PE numbers, and ditching PE's arrays? Does PE itself need to know what PEs it
is using? Not sure about this master/slave PEs though.
I don't follow your suggestion. Can you rephrase and explain it a bit more?
Please explains in what situations you need same map in both PHB and PE and
how you are going to use them. For example, pe::m64_segmap and
phb::m64_segmap.
I believe you need to know what segment is used by what PE and that's it and
having 2 bitmaps is overcomplicated hard to follow. Is there anything else
what I am missing?
The situation is same to all (IO, M32 and M64) segment maps. Taking m64_segmap
as an example, it will be used when creating or destroying the PE who consumes
M64 segments. phb::m64_segmap is recording the M64 segment usage in PHB's domain.
It's used to check same M64 segment won't be used for towice. pe::m64_segmap tracks
the M64 segments consumed by the PE.
quoted
quoted
It would be easier to read patches if this one was right before
[PATCH v6 23/42] powerpc/powernv: Release PEs dynamically
I'll try to reoder the patch, but not expect too much...
@@ -3175,19 +3185,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 */
This comment came with if(IODA1) below, since you are removing the condition
below, makes sense to remove the comment as well or move it where people will
look for it (arch/powerpc/platforms/powernv/pci.h ?)
@@ -49,6 +49,15 @@ struct pnv_ioda_pe {/* PE number */unsignedintpe_number;+/* IO/M32/M64 segments consumed by the PE. Each PE can+*haveoneM64segmentatmost,butM64segmentsconsumed+*byslavePEswillbecontributedtothemasterPE.One+*PEcanownmultipleIOandM32segments.
A PE can have multiple IO and M32 segments but just one M64 segment? Is this
correct for IODA1 or IODA2 or both? Is this a limitation of this
implementation or it comes from P7IOC/PHB3 hardware?
It's correct for IO and M32. However, on IODA1 or IODA2, one PE can have
multiple M64 segments as well.
But the comment says "Each PE can have one M64 segment at most". Which
statement is correct?
The comment is correct regarding PHB's 15th M64 BAR: Each PE can have one
M64 segment at post. It's from hardware limitation. However, once one PE
consumes multiple M64 segments. all those M64 segments will be tracked in
"master" PE and it's determined by software implementation.
quoted
quoted
quoted
+ */
+ unsigned long io_segmap[8];
+ unsigned long m32_segmap[8];
+ unsigned long m64_segmap[8];
Magic constant "8", 64bit*8 = 512 PEs - where did this come from?
Anyway,
#define PNV_IODA_MAX_PE_NUM 512
unsigned long io_segmap[PNV_IODA_MAX_PE_NUM/BITS_PER_LONG]
I prefer "8", not macro for 3 reasons:
- The macro won't be used in the code.
You will use it 6 times in the header, if you give it a good name, people
won't have to guess if the meaning of all these "8"s is the same and you
won't have to comment every use of it in this header file (now you have).
Also, using BITS_PER_LONG tells the reader that this is a bitmask for sure.
quoted
- The total segment number of specific resource is variable
on IODA1 and IODA2. I just choosed the max value with margin.
- PNV_IODA_MAX_PE_NUM, indicating max PE number, isn't 512 on
IODA1 or IODA2.
Give it a better name.
Ok. It it has to be a macro, then it's as below:
#define PNV_IODA_MAX_SEG_NUM 512
quoted
quoted
quoted
+
/* "Weight" assigned to the PE for the sake of DMA resource
* allocations
*/
@@ -145,15 +154,16 @@ struct pnv_phb { unsigned int io_segsize; unsigned int io_pci_base;+ /* IO, M32, M64 segment maps */+ unsigned long io_segmap[8];+ unsigned long m32_segmap[8];+ unsigned long m64_segmap[8];+ /* PE allocation */ struct mutex pe_alloc_mutex; unsigned long *pe_alloc; struct pnv_ioda_pe *pe_array;- /* M32 & IO segment maps */- unsigned int *m32_segmap;- unsigned int *io_segmap;- /* IRQ chip */ int irq_chip_init; struct irq_chip irq_chip;
On Tue, Aug 11, 2015 at 12:23:42PM +1000, Alexey Kardashevskiy wrote:
quoted
On 08/11/2015 10:03 AM, Gavin Shan wrote:
quoted
On Mon, Aug 10, 2015 at 05:16:40PM +1000, Alexey Kardashevskiy wrote:
quoted
On 08/06/2015 02:11 PM, Gavin Shan wrote:
quoted
The patch is adding 6 bitmaps, three to PE and three to PHB, to track
The patch is also removing 2 arrays (io_segmap and m32_segmap), what is that
all about? Also, there was no m64_segmap, now there is, needs an explanation
may be.
Originally, the bitmaps (io_segmap and m32_segmap) are allocated dynamically.
Now, they have fixed sizes - 512 bits.
The subject "powerpc/powernv: Track IO/M32/M64 segments from PE" indicates
why m64_segmap is added.
But before this patch, you somehow managed to keep it working without a map
for M64, by the same time you needed map for IO and M32. It seems you are
making things consistent in this patch but it also feels like you do not have
to do so as M64 did not need a map before and I cannot see why it needs one
now.
The M64 map is used by [PATCH v6 23/42] powerpc/powernv: Release PEs dynamically
where the M64 segments consumed by one particular PE will be released.
Then add it where it is really started being used. It is really hard to
review a patch which is actually spread between patches. Do not count that
reviewers will just trust you.
quoted
quoted
quoted
quoted
the consumed by one particular PE, which can be released once the PE
is destroyed during PCI unplugging time. Also, we're using fixed
quantity of bits to trace the used IO, M32 and M64 segments by PEs
in one particular PHB.
Out of curiosity - have you considered having just 3 arrays, in PHB, storing
PE numbers, and ditching PE's arrays? Does PE itself need to know what PEs it
is using? Not sure about this master/slave PEs though.
I don't follow your suggestion. Can you rephrase and explain it a bit more?
Please explains in what situations you need same map in both PHB and PE and
how you are going to use them. For example, pe::m64_segmap and
phb::m64_segmap.
I believe you need to know what segment is used by what PE and that's it and
having 2 bitmaps is overcomplicated hard to follow. Is there anything else
what I am missing?
The situation is same to all (IO, M32 and M64) segment maps. Taking m64_segmap
as an example, it will be used when creating or destroying the PE who consumes
M64 segments. phb::m64_segmap is recording the M64 segment usage in PHB's domain.
It's used to check same M64 segment won't be used for towice. pe::m64_segmap tracks
the M64 segments consumed by the PE.
You could have a single map in PHB, key would be a segment number and value
would be PE number. No need to have a map in PE. At all. No need to
initialize bitmaps, etc.
quoted
quoted
quoted
It would be easier to read patches if this one was right before
[PATCH v6 23/42] powerpc/powernv: Release PEs dynamically
I'll try to reoder the patch, but not expect too much...
@@ -3175,19 +3185,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 */
This comment came with if(IODA1) below, since you are removing the condition
below, makes sense to remove the comment as well or move it where people will
look for it (arch/powerpc/platforms/powernv/pci.h ?)
@@ -49,6 +49,15 @@ struct pnv_ioda_pe {/* PE number */unsignedintpe_number;+/* IO/M32/M64 segments consumed by the PE. Each PE can+*haveoneM64segmentatmost,butM64segmentsconsumed+*byslavePEswillbecontributedtothemasterPE.One+*PEcanownmultipleIOandM32segments.
A PE can have multiple IO and M32 segments but just one M64 segment? Is this
correct for IODA1 or IODA2 or both? Is this a limitation of this
implementation or it comes from P7IOC/PHB3 hardware?
It's correct for IO and M32. However, on IODA1 or IODA2, one PE can have
multiple M64 segments as well.
But the comment says "Each PE can have one M64 segment at most". Which
statement is correct?
The comment is correct regarding PHB's 15th M64 BAR: Each PE can have one
M64 segment at post. It's from hardware limitation. However, once one PE
consumes multiple M64 segments. all those M64 segments will be tracked in
"master" PE and it's determined by software implementation.
quoted
quoted
quoted
quoted
+ */
+ unsigned long io_segmap[8];
+ unsigned long m32_segmap[8];
+ unsigned long m64_segmap[8];
Magic constant "8", 64bit*8 = 512 PEs - where did this come from?
Anyway,
#define PNV_IODA_MAX_PE_NUM 512
unsigned long io_segmap[PNV_IODA_MAX_PE_NUM/BITS_PER_LONG]
I prefer "8", not macro for 3 reasons:
- The macro won't be used in the code.
You will use it 6 times in the header, if you give it a good name, people
won't have to guess if the meaning of all these "8"s is the same and you
won't have to comment every use of it in this header file (now you have).
Also, using BITS_PER_LONG tells the reader that this is a bitmask for sure.
quoted
- The total segment number of specific resource is variable
on IODA1 and IODA2. I just choosed the max value with margin.
- PNV_IODA_MAX_PE_NUM, indicating max PE number, isn't 512 on
IODA1 or IODA2.
Give it a better name.
Ok. It it has to be a macro, then it's as below:
#define PNV_IODA_MAX_SEG_NUM 512
Thanks mate :)
quoted
quoted
quoted
quoted
+
/* "Weight" assigned to the PE for the sake of DMA resource
* allocations
*/
@@ -145,15 +154,16 @@ struct pnv_phb { unsigned int io_segsize; unsigned int io_pci_base;+ /* IO, M32, M64 segment maps */+ unsigned long io_segmap[8];+ unsigned long m32_segmap[8];+ unsigned long m64_segmap[8];+ /* PE allocation */ struct mutex pe_alloc_mutex; unsigned long *pe_alloc; struct pnv_ioda_pe *pe_array;- /* M32 & IO segment maps */- unsigned int *m32_segmap;- unsigned int *io_segmap;- /* IRQ chip */ int irq_chip_init; struct irq_chip irq_chip;
On Wed, Aug 12, 2015 at 09:05:09PM +1000, Alexey Kardashevskiy wrote:
On 08/12/2015 08:45 PM, Gavin Shan wrote:
quoted
On Tue, Aug 11, 2015 at 12:23:42PM +1000, Alexey Kardashevskiy wrote:
quoted
On 08/11/2015 10:03 AM, Gavin Shan wrote:
quoted
On Mon, Aug 10, 2015 at 05:16:40PM +1000, Alexey Kardashevskiy wrote:
quoted
On 08/06/2015 02:11 PM, Gavin Shan wrote:
quoted
The patch is adding 6 bitmaps, three to PE and three to PHB, to track
The patch is also removing 2 arrays (io_segmap and m32_segmap), what is that
all about? Also, there was no m64_segmap, now there is, needs an explanation
may be.
Originally, the bitmaps (io_segmap and m32_segmap) are allocated dynamically.
Now, they have fixed sizes - 512 bits.
The subject "powerpc/powernv: Track IO/M32/M64 segments from PE" indicates
why m64_segmap is added.
But before this patch, you somehow managed to keep it working without a map
for M64, by the same time you needed map for IO and M32. It seems you are
making things consistent in this patch but it also feels like you do not have
to do so as M64 did not need a map before and I cannot see why it needs one
now.
The M64 map is used by [PATCH v6 23/42] powerpc/powernv: Release PEs dynamically
where the M64 segments consumed by one particular PE will be released.
Then add it where it is really started being used. It is really hard to
review a patch which is actually spread between patches. Do not count that
reviewers will just trust you.
Ok. I'll try.
quoted
quoted
quoted
quoted
quoted
the consumed by one particular PE, which can be released once the PE
is destroyed during PCI unplugging time. Also, we're using fixed
quantity of bits to trace the used IO, M32 and M64 segments by PEs
in one particular PHB.
Out of curiosity - have you considered having just 3 arrays, in PHB, storing
PE numbers, and ditching PE's arrays? Does PE itself need to know what PEs it
is using? Not sure about this master/slave PEs though.
I don't follow your suggestion. Can you rephrase and explain it a bit more?
Please explains in what situations you need same map in both PHB and PE and
how you are going to use them. For example, pe::m64_segmap and
phb::m64_segmap.
I believe you need to know what segment is used by what PE and that's it and
having 2 bitmaps is overcomplicated hard to follow. Is there anything else
what I am missing?
The situation is same to all (IO, M32 and M64) segment maps. Taking m64_segmap
as an example, it will be used when creating or destroying the PE who consumes
M64 segments. phb::m64_segmap is recording the M64 segment usage in PHB's domain.
It's used to check same M64 segment won't be used for towice. pe::m64_segmap tracks
the M64 segments consumed by the PE.
You could have a single map in PHB, key would be a segment number and value
would be PE number. No need to have a map in PE. At all. No need to
initialize bitmaps, etc.
So it would be arrays for various segmant maps if I understood your suggestion
as below. Please confirm:
#define PNV_IODA_MAX_SEG_NUM 512
int struct pnv_phb::io_segmap[PNV_IODA_MAX_SEG_NUM];
m32_segmap[PNV_IODA_MAX_SEG_NUM];
m64_segmap[PNV_IODA_MAX_SEG_NUM];
- Initially, they are initialize to IODA_INVALID_PE;
- When one segment is assigned to one PE, the corresponding entry
of the array is set to PE number.
- When one segment is relased, the corresponding entry of the array
is set to IODA_INVALID_PE;
quoted
quoted
quoted
quoted
It would be easier to read patches if this one was right before
[PATCH v6 23/42] powerpc/powernv: Release PEs dynamically
I'll try to reoder the patch, but not expect too much...
@@ -3175,19 +3185,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 */
This comment came with if(IODA1) below, since you are removing the condition
below, makes sense to remove the comment as well or move it where people will
look for it (arch/powerpc/platforms/powernv/pci.h ?)
@@ -49,6 +49,15 @@ struct pnv_ioda_pe {/* PE number */unsignedintpe_number;+/* IO/M32/M64 segments consumed by the PE. Each PE can+*haveoneM64segmentatmost,butM64segmentsconsumed+*byslavePEswillbecontributedtothemasterPE.One+*PEcanownmultipleIOandM32segments.
A PE can have multiple IO and M32 segments but just one M64 segment? Is this
correct for IODA1 or IODA2 or both? Is this a limitation of this
implementation or it comes from P7IOC/PHB3 hardware?
It's correct for IO and M32. However, on IODA1 or IODA2, one PE can have
multiple M64 segments as well.
But the comment says "Each PE can have one M64 segment at most". Which
statement is correct?
The comment is correct regarding PHB's 15th M64 BAR: Each PE can have one
M64 segment at post. It's from hardware limitation. However, once one PE
consumes multiple M64 segments. all those M64 segments will be tracked in
"master" PE and it's determined by software implementation.
quoted
quoted
quoted
quoted
+ */
+ unsigned long io_segmap[8];
+ unsigned long m32_segmap[8];
+ unsigned long m64_segmap[8];
Magic constant "8", 64bit*8 = 512 PEs - where did this come from?
Anyway,
#define PNV_IODA_MAX_PE_NUM 512
unsigned long io_segmap[PNV_IODA_MAX_PE_NUM/BITS_PER_LONG]
I prefer "8", not macro for 3 reasons:
- The macro won't be used in the code.
You will use it 6 times in the header, if you give it a good name, people
won't have to guess if the meaning of all these "8"s is the same and you
won't have to comment every use of it in this header file (now you have).
Also, using BITS_PER_LONG tells the reader that this is a bitmask for sure.
quoted
- The total segment number of specific resource is variable
on IODA1 and IODA2. I just choosed the max value with margin.
- PNV_IODA_MAX_PE_NUM, indicating max PE number, isn't 512 on
IODA1 or IODA2.
Give it a better name.
Ok. It it has to be a macro, then it's as below:
#define PNV_IODA_MAX_SEG_NUM 512
Thanks mate :)
quoted
quoted
quoted
quoted
quoted
+
/* "Weight" assigned to the PE for the sake of DMA resource
* allocations
*/
@@ -145,15 +154,16 @@ struct pnv_phb { unsigned int io_segsize; unsigned int io_pci_base;+ /* IO, M32, M64 segment maps */+ unsigned long io_segmap[8];+ unsigned long m32_segmap[8];+ unsigned long m64_segmap[8];+ /* PE allocation */ struct mutex pe_alloc_mutex; unsigned long *pe_alloc; struct pnv_ioda_pe *pe_array;- /* M32 & IO segment maps */- unsigned int *m32_segmap;- unsigned int *io_segmap;- /* IRQ chip */ int irq_chip_init; struct irq_chip irq_chip;
On Wed, Aug 12, 2015 at 09:05:09PM +1000, Alexey Kardashevskiy wrote:
quoted
On 08/12/2015 08:45 PM, Gavin Shan wrote:
quoted
On Tue, Aug 11, 2015 at 12:23:42PM +1000, Alexey Kardashevskiy wrote:
quoted
On 08/11/2015 10:03 AM, Gavin Shan wrote:
quoted
On Mon, Aug 10, 2015 at 05:16:40PM +1000, Alexey Kardashevskiy wrote:
quoted
On 08/06/2015 02:11 PM, Gavin Shan wrote:
quoted
The patch is adding 6 bitmaps, three to PE and three to PHB, to track
The patch is also removing 2 arrays (io_segmap and m32_segmap), what is that
all about? Also, there was no m64_segmap, now there is, needs an explanation
may be.
Originally, the bitmaps (io_segmap and m32_segmap) are allocated dynamically.
Now, they have fixed sizes - 512 bits.
The subject "powerpc/powernv: Track IO/M32/M64 segments from PE" indicates
why m64_segmap is added.
But before this patch, you somehow managed to keep it working without a map
for M64, by the same time you needed map for IO and M32. It seems you are
making things consistent in this patch but it also feels like you do not have
to do so as M64 did not need a map before and I cannot see why it needs one
now.
The M64 map is used by [PATCH v6 23/42] powerpc/powernv: Release PEs dynamically
where the M64 segments consumed by one particular PE will be released.
Then add it where it is really started being used. It is really hard to
review a patch which is actually spread between patches. Do not count that
reviewers will just trust you.
Ok. I'll try.
quoted
quoted
quoted
quoted
quoted
quoted
the consumed by one particular PE, which can be released once the PE
is destroyed during PCI unplugging time. Also, we're using fixed
quantity of bits to trace the used IO, M32 and M64 segments by PEs
in one particular PHB.
Out of curiosity - have you considered having just 3 arrays, in PHB, storing
PE numbers, and ditching PE's arrays? Does PE itself need to know what PEs it
is using? Not sure about this master/slave PEs though.
I don't follow your suggestion. Can you rephrase and explain it a bit more?
Please explains in what situations you need same map in both PHB and PE and
how you are going to use them. For example, pe::m64_segmap and
phb::m64_segmap.
I believe you need to know what segment is used by what PE and that's it and
having 2 bitmaps is overcomplicated hard to follow. Is there anything else
what I am missing?
The situation is same to all (IO, M32 and M64) segment maps. Taking m64_segmap
as an example, it will be used when creating or destroying the PE who consumes
M64 segments. phb::m64_segmap is recording the M64 segment usage in PHB's domain.
It's used to check same M64 segment won't be used for towice. pe::m64_segmap tracks
the M64 segments consumed by the PE.
You could have a single map in PHB, key would be a segment number and value
would be PE number. No need to have a map in PE. At all. No need to
initialize bitmaps, etc.
So it would be arrays for various segmant maps if I understood your suggestion
as below. Please confirm:
#define PNV_IODA_MAX_SEG_NUM 512
int struct pnv_phb::io_segmap[PNV_IODA_MAX_SEG_NUM];
m32_segmap[PNV_IODA_MAX_SEG_NUM];
m64_segmap[PNV_IODA_MAX_SEG_NUM];
- Initially, they are initialize to IODA_INVALID_PE;
- When one segment is assigned to one PE, the corresponding entry
of the array is set to PE number.
- When one segment is relased, the corresponding entry of the array
is set to IODA_INVALID_PE;
No, not arrays, I meant DEFINE_HASHTABLE(), hash_add(), etc from
include/linux/hashtable.h.
http://kernelnewbies.org/FAQ/Hashtables is a good place to start :)
--
Alexey
On Wed, Aug 12, 2015 at 10:57:33PM +1000, Alexey Kardashevskiy wrote:
On 08/12/2015 09:20 PM, Gavin Shan wrote:
quoted
On Wed, Aug 12, 2015 at 09:05:09PM +1000, Alexey Kardashevskiy wrote:
quoted
On 08/12/2015 08:45 PM, Gavin Shan wrote:
quoted
On Tue, Aug 11, 2015 at 12:23:42PM +1000, Alexey Kardashevskiy wrote:
quoted
On 08/11/2015 10:03 AM, Gavin Shan wrote:
quoted
On Mon, Aug 10, 2015 at 05:16:40PM +1000, Alexey Kardashevskiy wrote:
quoted
On 08/06/2015 02:11 PM, Gavin Shan wrote:
quoted
The patch is adding 6 bitmaps, three to PE and three to PHB, to track
The patch is also removing 2 arrays (io_segmap and m32_segmap), what is that
all about? Also, there was no m64_segmap, now there is, needs an explanation
may be.
Originally, the bitmaps (io_segmap and m32_segmap) are allocated dynamically.
Now, they have fixed sizes - 512 bits.
The subject "powerpc/powernv: Track IO/M32/M64 segments from PE" indicates
why m64_segmap is added.
But before this patch, you somehow managed to keep it working without a map
for M64, by the same time you needed map for IO and M32. It seems you are
making things consistent in this patch but it also feels like you do not have
to do so as M64 did not need a map before and I cannot see why it needs one
now.
The M64 map is used by [PATCH v6 23/42] powerpc/powernv: Release PEs dynamically
where the M64 segments consumed by one particular PE will be released.
Then add it where it is really started being used. It is really hard to
review a patch which is actually spread between patches. Do not count that
reviewers will just trust you.
Ok. I'll try.
quoted
quoted
quoted
quoted
quoted
quoted
the consumed by one particular PE, which can be released once the PE
is destroyed during PCI unplugging time. Also, we're using fixed
quantity of bits to trace the used IO, M32 and M64 segments by PEs
in one particular PHB.
Out of curiosity - have you considered having just 3 arrays, in PHB, storing
PE numbers, and ditching PE's arrays? Does PE itself need to know what PEs it
is using? Not sure about this master/slave PEs though.
I don't follow your suggestion. Can you rephrase and explain it a bit more?
Please explains in what situations you need same map in both PHB and PE and
how you are going to use them. For example, pe::m64_segmap and
phb::m64_segmap.
I believe you need to know what segment is used by what PE and that's it and
having 2 bitmaps is overcomplicated hard to follow. Is there anything else
what I am missing?
The situation is same to all (IO, M32 and M64) segment maps. Taking m64_segmap
as an example, it will be used when creating or destroying the PE who consumes
M64 segments. phb::m64_segmap is recording the M64 segment usage in PHB's domain.
It's used to check same M64 segment won't be used for towice. pe::m64_segmap tracks
the M64 segments consumed by the PE.
You could have a single map in PHB, key would be a segment number and value
would be PE number. No need to have a map in PE. At all. No need to
initialize bitmaps, etc.
So it would be arrays for various segmant maps if I understood your suggestion
as below. Please confirm:
#define PNV_IODA_MAX_SEG_NUM 512
int struct pnv_phb::io_segmap[PNV_IODA_MAX_SEG_NUM];
m32_segmap[PNV_IODA_MAX_SEG_NUM];
m64_segmap[PNV_IODA_MAX_SEG_NUM];
- Initially, they are initialize to IODA_INVALID_PE;
- When one segment is assigned to one PE, the corresponding entry
of the array is set to PE number.
- When one segment is relased, the corresponding entry of the array
is set to IODA_INVALID_PE;
No, not arrays, I meant DEFINE_HASHTABLE(), hash_add(), etc from
include/linux/hashtable.h.
http://kernelnewbies.org/FAQ/Hashtables is a good place to start :)
Are you sure it needs hashtable to represent the simple data struct?
I really don't understand the benefits, could you provide more details
about the benefits?
With hashtable, every bucket will include multiple items with conflicting
hash key, each of which would be represented by data struct as below. The
data struct uses 24 bytes memory and not efficient enough from this aspect.
When one more segment consued, instance of "struct pnv_ioda_segment" is
allocated and put into the conflicting list of the target bucket. At later
point, the instance is removed from the list and released when the segment
is detached from the PE. It's more complex than it should be.
struct pnv_ioda_segment {
int pe_number;
int seg_number;
struct hlist_node node;
};
Thanks,
Gavin
On Mon, Aug 10, 2015 at 07:21:12PM +1000, Alexey Kardashevskiy wrote:
On 08/06/2015 02:11 PM, Gavin Shan wrote:
quoted
For P7IOC, the whole available DMA32 space, which is below the
MEM32 space, is divided evenly into 256MB segments. The number
of continuous segments assigned to one particular PE depends on
the PE's DMA weight that is calculated based on the type of each
PCI devices contained in the PE, and PHB's DMA weight which is
accumulative DMA weight of PEs contained in the PHB. It means
that the PHB's DMA weight calculation depends on existing PEs,
which works perfectly now, but not hotplug friendly. As the
whole available DMA32 space can be assigned to one PE on PHB3,
so we don't have the issue on PHB3.
The patch calculates PHB's DMA weight based on the PCI devices
contained in the PHB dynamically so that it's hotplug friendly.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 88 +++++++++++++++----------------
arch/powerpc/platforms/powernv/pci.h | 6 ---
2 files changed, 43 insertions(+), 51 deletions(-)
@@ -927,6 +927,9 @@ static void pnv_ioda_link_pe_by_weight(struct pnv_phb *phb,staticunsignedintpnv_ioda_dma_weight(structpci_dev*dev){+structpci_controller*hose=pci_bus_to_host(dev->bus);+structpnv_phb*phb=hose->private_data;+/* This is quite simplistic. The "base" weight of a device*is10.0meansnoDMAistobeaccountedforit.*/
@@ -939,14 +942,34 @@ static unsigned int pnv_ioda_dma_weight(struct pci_dev *dev)if(dev->class==PCI_CLASS_SERIAL_USB_UHCI||dev->class==PCI_CLASS_SERIAL_USB_OHCI||dev->class==PCI_CLASS_SERIAL_USB_EHCI)-return3;+return3*phb->ioda.tce32_count;/* Increase the weight of RAID (includes Obsidian) */if((dev->class>>8)==PCI_CLASS_STORAGE_RAID)-return15;+return15*phb->ioda.tce32_count;/* Default */-return10;+return10*phb->ioda.tce32_count;+}++staticint__pnv_ioda_phb_dma_weight(structpci_dev*pdev,void*data)+{+unsignedint*dma_weight=data;++*dma_weight+=pnv_ioda_dma_weight(pdev);+return0;+}++staticunsignedintpnv_ioda_phb_dma_weight(structpnv_phb*phb)+{+unsignedintdma_weight=0;++if(!phb->hose->bus)+return0;++pci_walk_bus(phb->hose->bus,+__pnv_ioda_phb_dma_weight,&dma_weight);+returndma_weight;}#ifdef CONFIG_PCI_IOV
@@ -1097,14 +1120,6 @@ static void pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all)/* Put PE to the list */list_add_tail(&pe->list,&phb->ioda.pe_list);-/* Account for one DMA PE if at least one DMA capable device exist-*belowthebridge-*/-if(pe->dma_weight!=0){-phb->ioda.dma_weight+=pe->dma_weight;-phb->ioda.dma_pe_count++;-}-/* Link the PE */pnv_ioda_link_pe_by_weight(phb,pe);}
@@ -2431,24 +2446,13 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,staticvoidpnv_ioda_setup_dma(structpnv_phb*phb){structpci_controller*hose=phb->hose;-unsignedintresidual,remaining,segs,tw,base;structpnv_ioda_pe*pe;+unsignedintdma_weight;-/* If we have more PE# than segments available, hand out one-*perPEuntilwerunoutandlettherestfail.Ifnot,-*thenweassignatleastonesegmentperPE,plusmorebased-*ontheamountofdevicesunderthatPE-*/-if(phb->ioda.dma_pe_count>phb->ioda.tce32_count)-residual=0;-else-residual=phb->ioda.tce32_count--phb->ioda.dma_pe_count;--pr_info("PCI: Domain %04x has %ld available 32-bit DMA segments\n",-hose->global_number,phb->ioda.tce32_count);-pr_info("PCI: %d PE# for a total weight of %d\n",-phb->ioda.dma_pe_count,phb->ioda.dma_weight);+/* Calculate the PHB's DMA weight */+dma_weight=pnv_ioda_phb_dma_weight(phb);+pr_info("PCI%04x has %ld DMA32 segments, total weight %d\n",+hose->global_number,phb->ioda.tce32_count,dma_weight);pnv_pci_ioda_setup_opal_tce_kill(phb);
This is not right. @base here is a local variable in the scope,
pnv_pci_ioda_setup_dma_pe() will always be called with base==0.
Sorry for commenting the same patch twice.
That's ok to comment for twice on same patch. But I don't see
how it's wrong. The function (pnv_ioda_setup_dma()) is called
as below and it iterate all PEs in the PHB's DMA32 list. That
means the function is affects PHB, not every PE yet. It's out
of problem with "base=0".
pnv_pci_ioda_fixup
pnv_pci_ioda_setup_DMA
pnv_ioda_setup_dma
@@ -183,12 +183,6 @@ struct pnv_phb {/* 32-bit TCE tables allocation */unsignedlongtce32_count;-/* Total "weight" for the sake of DMA resources-*allocation-*/-unsignedintdma_weight;-unsignedintdma_pe_count;-/* Sorted list of used PE's, sorted at*bootforresourceallocationpurposes*/
On Tue, Aug 11, 2015 at 12:39:02PM +1000, Alexey Kardashevskiy wrote:
On 08/11/2015 10:29 AM, Gavin Shan wrote:
quoted
On Mon, Aug 10, 2015 at 07:31:11PM +1000, Alexey Kardashevskiy wrote:
quoted
On 08/06/2015 02:11 PM, Gavin Shan wrote:
quoted
The original implementation of pnv_ioda_setup_dma() iterates the
list of PEs and configures the DMA32 space for them one by one.
The function was designed to be called during PHB fixup time.
When configuring PE's DMA32 space in pcibios_setup_bridge(), in
order to support PCI hotplug, we have to have the function PE
oriented.
This renames pnv_ioda_setup_dma() to pnv_ioda1_setup_dma() and
adds one more argument "struct pnv_ioda_pe *pe" to it. The caller,
pnv_pci_ioda_setup_DMA(), gets PE from the list and passes to it
or pnv_pci_ioda2_setup_dma_pe(). The patch shouldn't cause behavioral
changes.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 75 +++++++++++++++----------------
1 file changed, 36 insertions(+), 39 deletions(-)
@@ -2443,52 +2443,29 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,pnv_ioda_setup_bus_dma(pe,pe->pbus);}-staticvoidpnv_ioda_setup_dma(structpnv_phb*phb)+staticunsignedintpnv_ioda1_setup_dma(structpnv_phb*phb,+structpnv_ioda_pe*pe,+unsignedintbase){structpci_controller*hose=phb->hose;-structpnv_ioda_pe*pe;-unsignedintdma_weight;+unsignedintdma_weight,segs;/* Calculate the PHB's DMA weight */dma_weight=pnv_ioda_phb_dma_weight(phb);pr_info("PCI%04x has %ld DMA32 segments, total weight %d\n",hose->global_number,phb->ioda.dma32_segcount,dma_weight);-pnv_pci_ioda_setup_opal_tce_kill(phb);--/* Walk our PE list and configure their DMA segments, hand them-*outonebasesegmentplusanyresidualsegmentsbasedon-*weight-*/-list_for_each_entry(pe,&phb->ioda.pe_dma_list,dma_link){-if(!pe->dma32_weight)-continue;--/*-*ForIODA2compliantPHB3,weneedn'tcareabouttheweight.-*Theallavailable32-bitsDMAspacewillbeassignedto-*thespecificPE.-*/-if(phb->type==PNV_PHB_IODA1){-unsignedintsegs,base=0;--if(pe->dma32_weight<-dma_weight/phb->ioda.dma32_segcount)-segs=1;-else-segs=(pe->dma32_weight*-phb->ioda.dma32_segcount)/dma_weight;--pe_info(pe,"DMA32 weight %d, assigned %d segments\n",-pe->dma32_weight,segs);-pnv_pci_ioda_setup_dma_pe(phb,pe,base,segs);+if(pe->dma32_weight<+dma_weight/phb->ioda.dma32_segcount)
Why not to merge pnv_ioda1_setup_dma() to pnv_pci_ioda_setup_dma_pe()?
There're two reasons:
- They're separate logically. One is calculating number of DMA32 segments required.
Another one is allocate TCE32 tables and configure devices with them.
- In PCI hotplug path, I need pnv_ioda1_setup_dma() which has "pe" as parameter.
And hotplug path does not care about dma weight why?
PHB3 doesn't care about DMA weight, but P7IOC needs.
This @base handling seems never be tested between 8..11 as "[PATCH v6 11/42]
powerpc/powernv: Trace DMA32 segments consumed by PE"
removes it and I suspect you only tested the final version. Which is ok for
the final result but not ok for bisectability.
Looks like 8/42, 9/42, 10/42, 11/42 need to be rearranged or merged to remove
this multiple @base touching.
Why ?
You are touching this @base from 8/42 to 11/12 and in between it is very
broken, you only get it fixed (by removing) in 11/42. Read my comment for
8/42. After every single patch in any patchset the functionality should not
break but it does in this patchset.
Please refer the reply to PATCH[8/42] then.
quoted
quoted
quoted
+ break;
+ case PNV_PHB_IODA2:
+ pnv_pci_ioda2_setup_dma_pe(phb, pe);
+ break;
+ default:
+ pr_warn("%s: No DMA for PHB type %d\n",
+ __func__, phb->type);
+ }
+ }
/* Mark the PHB initialization done */
- phb = hose->private_data;
phb->initialized = 1;
}
}
On Mon, Aug 10, 2015 at 07:43:48PM +1000, Alexey Kardashevskiy wrote:
On 08/06/2015 02:11 PM, Gavin Shan wrote:
quoted
On P7IOC, the whole DMA32 space is divided evenly to 256MB segments.
Each PE can consume one or multiple DMA32 segments. Current code
doesn't trace the available DMA32 segments and those consumed by
one particular PE. It's conflicting with PCI hotplug.
The patch introduces one bitmap to PHB to trace the available
DMA32 segments for allocation, more fields to "struct pnv_ioda_pe"
to trace the consumed DMA32 segments by the PE, which is going to
be released when the PE is destroyed at PCI unplugging time.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 40 +++++++++++++++++++++++--------
arch/powerpc/platforms/powernv/pci.h | 4 +++-
2 files changed, 33 insertions(+), 11 deletions(-)