The pdn (struct pci_dn) instances are allocated from memblock or
bootmem when creating PCI controller (hoses) in setup_arch(). PCI
hotplug, which will be supported by proceeding patches, releases
PCI device nodes and their corresponding pdn on unplugging event.
The memory chunks for pdn instances allocated from memblock or
bootmem are hard to reused after being released.
This delays creating pdn by pci_devs_phb_init() from setup_arch()
to core_initcall() so that they are allocated from slab. The memory
consumed by pdn can be released to system without problem during
PCI unplugging time. It indicates that pci_dn is unavailable in
setup_arch() and the the fixup on pdn (like AGP's) can't be carried
out that time. We have to do that in ppc_md.pcibios_root_bridge_prepare()
on maple/pasemi/powermac platforms where/when the pdn is available.
At the mean while, the EEH device is created when pdn is populated,
meaning pdn and EEH device have same life cycle. In turn, we needn't
call eeh_dev_init() to create EEH device explicitly.
Signed-off-by: Gavin Shan <redacted>
Uff. It would not hurt to mention that pcibios_root_bridge_prepare is
called from subsys_initcall() which is executed after core_initcall() so
the code flow does not change.
Have you checked if there is anything in between
core_initcall(pci_devs_phb_init) and subsys_initcall(pcibios_init) which
might need device tree nodes? For example, subsys_initcall(pcibios_init)
calls (eventually) pnv_pci_ioda_fixup(), if we are unlucky and
pcibios_init() (and therefore pnv_pci_ioda_fixup() or what pseries/others
do) is called before pcibios_init() - won't we crash or something?
--
Alexey
In hotplug case, function pci_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 one more check to skip scanning PCI bus that doesn't have
any subordinate devices from device-tree, in order to avoid kernel
crash.
Signed-off-by: Gavin Shan <redacted>
On the PCI plugging event, PCI slot's subordinate devices are
scanned and their (IO and MMIO) resources are assigned. Platform
dependent resources (PE#, IO/MMIO/DMA windows) are allocated or
created on updating windows of the slot's upstream bridge.
This updates the windows of the hot plugged slot's upstream bridge
in pcibios_finish_adding_to_bus() so that the platform resources
(PE#, IO/MMIO/DMA segments) are allocated or created accordingly.
Signed-off-by: Gavin Shan <redacted>
To my very limited knowledge of the common PCI code, looks good.
Reviewed-by: Alexey Kardashevskiy <redacted>
This drops unnecessary nested if statements in pnv_eeh_reset() to
improve the code readability. After the changes, the unused local
variable "ret" is dropped as well. No logical changes introduced.
Signed-off-by: Gavin Shan <redacted>
The function pnv_pci_reset_secondary_bus() is called like below.
It's impossible for call the function on root bus. So it's safe
to remove the root bus case in the function. No functional changes
introduced.
pci_parent_bus_reset() / pci_bus_reset() / pci_try_reset_bus()
pci_reset_bridge_secondary_bus()
pcibios_reset_secondary_bus()
pnv_pci_reset_secondary_bus()
Signed-off-by: Gavin Shan <redacted>
Reviewed-by: Daniel Axtens <redacted>
The skiboot firmware might provide the PCI slot reset capability
which is identified by property "ibm,reset-by-firmware" on the
PCI slot associated device node.
This checks the property. If it exists, the reset request is routed
to firmware. Otherwise, the reset is done by kernel as before.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/eeh-powernv.c | 41 +++++++++++++++++++++++++++-
1 file changed, 40 insertions(+), 1 deletion(-)
The device tree will change dynamically in PowerNV PCI hotplug
driver. This enables CONFIG_OF_DYNAMIC to support that.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/Kconfig | 1 +
1 file changed, 1 insertion(+)
On Wed, Apr 13, 2016 at 04:45:39PM +1000, Alexey Kardashevskiy wrote:
On 02/17/2016 02:43 PM, Gavin Shan wrote:
quoted
The original implementation of pnv_ioda_setup_pe_seg() configures
IO and M32 segments by separate logics, which can be merged by
by caching @segmap, @seg_size, @win in advance. This shouldn't
cause any behavioural changes.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 62 ++++++++++++++-----------------
1 file changed, 28 insertions(+), 34 deletions(-)
@@ -2958,23 +2960,9 @@ static void pnv_ioda_setup_pe_seg(struct pci_controller *hose,if(res->flags&IORESOURCE_IO){region.start=res->start-phb->ioda.io_pci_base;region.end=res->end-phb->ioda.io_pci_base;-index=region.start/phb->ioda.io_segsize;--while(index<phb->ioda.total_pe_num&&-region.start<=region.end){-phb->ioda.io_segmap[index]=pe->pe_number;-rc=opal_pci_map_pe_mmio_window(phb->opal_id,-pe->pe_number,OPAL_IO_WINDOW_TYPE,0,index);-if(rc!=OPAL_SUCCESS){-pr_err("%s: OPAL error %d when mapping IO "-"segment #%d to PE#%d\n",-__func__,rc,index,pe->pe_number);-break;-}--region.start+=phb->ioda.io_segsize;-index++;-}+segsize=phb->ioda.io_segsize;+segmap=phb->ioda.io_segmap;+win=OPAL_IO_WINDOW_TYPE;}elseif((res->flags&IORESOURCE_MEM)&&!pnv_pci_is_mem_pref_64(res->flags)){region.start=res->start-
@@ -2983,23 +2971,29 @@ static void pnv_ioda_setup_pe_seg(struct pci_controller *hose,region.end=res->end-hose->mem_offset[0]-phb->ioda.m32_pci_base;-index=region.start/phb->ioda.m32_segsize;--while(index<phb->ioda.total_pe_num&&-region.start<=region.end){-phb->ioda.m32_segmap[index]=pe->pe_number;-rc=opal_pci_map_pe_mmio_window(phb->opal_id,-pe->pe_number,OPAL_M32_WINDOW_TYPE,0,index);-if(rc!=OPAL_SUCCESS){-pr_err("%s: OPAL error %d when mapping M32 "-"segment#%d to PE#%d",-__func__,rc,index,pe->pe_number);-break;-}+segsize=phb->ioda.m32_segsize;+segmap=phb->ioda.m32_segmap;+win=OPAL_M32_WINDOW_TYPE;+}else{+continue;+}-region.start+=phb->ioda.m32_segsize;-index++;+index=region.start/segsize;+while(index<phb->ioda.total_pe_num&&+region.start<=region.end){+segmap[index]=pe->pe_number;+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;
Please move this loop to a helper and stop caching segsize/segmap/win; this
will make the code easier to read and the next patch will look much cleaner
as it will not have to move this exact loop.
Thanks. It's good idea and I'll change the code accordingly in next revision.
On Wed, Apr 13, 2016 at 05:09:45PM +1000, Alexey Kardashevskiy wrote:
On 02/17/2016 02:43 PM, Gavin Shan wrote:
quoted
When unplugging PCI devices, their parent PEs might be offline.
The consumed M64 resource by the PEs should be released at that
time. As we track M32 segment consumption, this introduces an
array to the PHB to track the mapping between M64 segment and
PE number.
Signed-off-by: Gavin Shan <redacted>
Reviewed-by: Alexey Kardashevskiy <redacted>
but it would not hurt to mention in the commit log why M64 segment is not
tracked/setup by the existing (at this point, at least)
pnv_ioda_setup_one_res().
Right, I'll add something for it to the commit log in next revision, thanks!
@@ -3332,6 +3333,8 @@ 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_num/8,sizeof(unsignedlong));+m64map_off=size;+size+=phb->ioda.total_pe_num*sizeof(phb->ioda.m64_segmap[0]);m32map_off=size;size+=phb->ioda.total_pe_num*sizeof(phb->ioda.m32_segmap[0]);if(phb->type==PNV_PHB_IODA1){
On Wed, Apr 13, 2016 at 06:59:40PM +1000, Alexey Kardashevskiy wrote:
On 02/17/2016 02:43 PM, Gavin Shan wrote:
quoted
PEs are put into PHB DMA32 list (phb->ioda.pe_dma_list) according
to their DMA32 weight. The PEs on the list are iterated to setup
their TCE32 tables at system booting time. The list is used for
once and there is for keep having it.
"there is no need to keep it" may be?
Sorry, I should have fixed it in early revision. Will fix it
up in next revision.
quoted
This moves the logic calculating DMA32 weight of PHB and PE to
pnv_ioda_setup_dma() to drop PHB's DMA32 list. Also, every PE
traces the consumed DMA32 segment by @tce32_seg and @tce32_segcount
are useless and they're removed.
Signed-off-by: Gavin Shan <redacted>
Reviewed-by: Alexey Kardashevskiy <redacted>
with few comments below...
@@ -886,44 +886,6 @@ out:return0;}-staticvoidpnv_ioda_link_pe_by_weight(structpnv_phb*phb,-structpnv_ioda_pe*pe)-{-structpnv_ioda_pe*lpe;--list_for_each_entry(lpe,&phb->ioda.pe_dma_list,dma_link){-if(lpe->dma_weight<pe->dma_weight){-list_add_tail(&pe->dma_link,&lpe->dma_link);-return;-}-}-list_add_tail(&pe->dma_link,&phb->ioda.pe_dma_list);-}--staticunsignedintpnv_ioda_dma_weight(structpci_dev*dev)-{-/* This is quite simplistic. The "base" weight of a device-*is10.0meansnoDMAistobeaccountedforit.-*/--/* If it's a bridge, no DMA */-if(dev->hdr_type!=PCI_HEADER_TYPE_NORMAL)-return0;--/* Reduce the weight of slow USB controllers */-if(dev->class==PCI_CLASS_SERIAL_USB_UHCI||-dev->class==PCI_CLASS_SERIAL_USB_OHCI||-dev->class==PCI_CLASS_SERIAL_USB_EHCI)-return3;--/* Increase the weight of RAID (includes Obsidian) */-if((dev->class>>8)==PCI_CLASS_STORAGE_RAID)-return15;--/* Default */-return10;-}-#ifdef CONFIG_PCI_IOVstaticintpnv_pci_vf_resource_shift(structpci_dev*dev,intoffset){
@@ -1044,16 +1005,6 @@ static struct pnv_ioda_pe *pnv_ioda_setup_dev_PE(struct pci_dev *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;}
@@ -1108,10 +1058,8 @@ static void pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all)pe->flags|=(all?PNV_IODA_PE_BUS_ALL:PNV_IODA_PE_BUS);pe->pbus=bus;pe->pdev=NULL;-pe->tce32_seg=-1;pe->mve_number=-1;pe->rid=bus->busn_res.start<<8;-pe->dma_weight=0;if(all)pe_info(pe,"Secondary bus %d..%d associated with PE#%d\n",
@@ -1133,17 +1081,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);}staticstructpnv_ioda_pe*pnv_ioda_setup_npu_PE(structpci_dev*npu_pdev)
@@ -1184,7 +1121,6 @@ static struct pnv_ioda_pe *pnv_ioda_setup_npu_PE(struct pci_dev *npu_pdev)rid=npu_pdev->bus->number<<8|npu_pdn->devfn;npu_pdn->pcidev=npu_pdev;npu_pdn->pe_number=pe_num;-pe->dma_weight+=pnv_ioda_dma_weight(npu_pdev);phb->ioda.pe_rmap[rid]=pe->pe_number;/* Map the PE to this link */
@@ -2023,6 +1958,54 @@ static struct iommu_table_ops pnv_ioda2_iommu_ops = {.free=pnv_ioda2_table_free,};+staticintpnv_pci_ioda_dev_dma_weight(structpci_dev*dev,void*data)+{+unsignedint*weight=(unsignedint*)data;++/* This is quite simplistic. The "base" weight of a device+*is10.0meansnoDMAistobeaccountedforit.+*/+if(dev->hdr_type!=PCI_HEADER_TYPE_NORMAL)+return0;++if(dev->class==PCI_CLASS_SERIAL_USB_UHCI||+dev->class==PCI_CLASS_SERIAL_USB_OHCI||+dev->class==PCI_CLASS_SERIAL_USB_EHCI)+*weight+=3;+elseif((dev->class>>8)==PCI_CLASS_STORAGE_RAID)+*weight+=15;+else+*weight+=10;++return0;+}++staticunsignedintpnv_pci_ioda_pe_dma_weight(structpnv_ioda_pe*pe)+{+unsignedintweight=0;++if((pe->flags&PNV_IODA_PE_DEV)&&pe->pdev){+pnv_pci_ioda_dev_dma_weight(pe->pdev,&weight);+}elseif((pe->flags&PNV_IODA_PE_BUS)&&pe->pbus){+structpci_dev*pdev;++list_for_each_entry(pdev,&pe->pbus->devices,bus_list)+pnv_pci_ioda_dev_dma_weight(pdev,&weight);+}elseif((pe->flags&PNV_IODA_PE_BUS_ALL)&&pe->pbus){+pci_walk_bus(pe->pbus,pnv_pci_ioda_dev_dma_weight,&weight);+}++returnweight;+}++staticunsignedintpnv_pci_ioda_total_dma_weight(structpnv_phb*phb)
s/pnv_pci_ioda_total_dma_weight/pnv_pci_ioda1_phb_dma_weight/ ? "total" does
not say much. Or just merge it into pnv_pci_ioda1_setup_dma_pe() as it is
useless for anything but IODA1.
Nice suggestion. I will merge it to pnv_pci_ioda1_setup_dma_pe().
@@ -2039,17 +2022,12 @@ static void pnv_pci_ioda1_setup_dma_pe(struct pnv_phb *phb, /* XXX FIXME: Provide 64-bit DMA facilities & non-4K TCE tables etc.. */ /* 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))- return;- tbl = pnv_pci_table_alloc(phb->hose->node); iommu_register_group(&pe->table_group, phb->hose->global_number, pe->pe_number); 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_info(pe, " Setting up 32-bit TCE table at %08x..%08x\n", base * PNV_IODA1_DMA32_SEGSIZE, (base + segs) * PNV_IODA1_DMA32_SEGSIZE - 1);
@@ -2116,8 +2094,6 @@ static void pnv_pci_ioda1_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 (tce_mem) __free_pages(tce_mem, get_order(tce32_segsz * segs)); if (tbl) {
@@ -2528,10 +2504,6 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb, { int64_t rc;- /* We shouldn't already have a 32-bit DMA associated */- if (WARN_ON(pe->tce32_seg >= 0))- return;- /* TVE #1 is selected by PCI address bit 59 */ pe->tce_bypass_base = 1ull << 59;
@@ -2539,7 +2511,6 @@ 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_info(pe, "Setting up 32-bit TCE table at 0..%08x\n", phb->ioda.m32_pci_base);
@@ -2555,11 +2526,8 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb, #endif rc = pnv_pci_ioda2_setup_default_config(pe);- if (rc) {- if (pe->tce32_seg >= 0)- pe->tce32_seg = -1;+ if (rc) return;- } if (pe->flags & PNV_IODA_PE_DEV) iommu_add_device(&pe->pdev->dev);
@@ -2570,24 +2538,32 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb, static void pnv_ioda_setup_dma(struct pnv_phb *phb) { struct pci_controller *hose = phb->hose;- unsigned int residual, remaining, segs, tw, base;+ unsigned int weight, total_weight, dma_pe_count;+ unsigned int residual, remaining, segs, base; struct pnv_ioda_pe *pe;+ total_weight = pnv_pci_ioda_total_dma_weight(phb);+ dma_pe_count = 0;+ list_for_each_entry(pe, &phb->ioda.pe_list, list) {+ weight = pnv_pci_ioda_pe_dma_weight(pe);+ if (weight > 0)+ dma_pe_count++;+ }+ /* If we have more PE# than segments available, hand out one * per PE until we run out and let the rest fail. If not, * then we assign at least one segment per PE, plus more based * on the amount of devices under that PE */- if (phb->ioda.dma_pe_count > phb->ioda.tce32_count)+ if (dma_pe_count > phb->ioda.tce32_count) residual = 0; else- residual = phb->ioda.tce32_count -- phb->ioda.dma_pe_count;+ residual = phb->ioda.tce32_count - 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);+ dma_pe_count, total_weight); pnv_pci_ioda_setup_opal_tce_kill(phb);
@@ -53,14 +53,7 @@ struct pnv_ioda_pe {/* PE number */unsignedintpe_number;-/* "Weight" assigned to the PE for the sake of DMA resource-*allocations-*/-unsignedintdma_weight;-/* "Base" iommu table, ie, 4K TCEs, 32-bit DMA */-inttce32_seg;-inttce32_segcount;structiommu_table_grouptable_group;/* 64-bit TCE bypass region */
@@ -78,7 +71,6 @@ struct pnv_ioda_pe {structlist_headslaves;/* Link in list of PE#s */-structlist_headdma_link;structlist_headlist;};
@@ -173,17 +165,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-*/-structlist_headpe_dma_list;-/* TCE cache invalidate registers (physical and*remapped)*/
On Tue, Apr 19, 2016 at 12:02:23PM +1000, Alexey Kardashevskiy wrote:
On 02/17/2016 02:44 PM, Gavin Shan wrote:
quoted
Each PHB maintains an array helping to translate 2-bytes Request
ID (RID) to PE# with the assumption that PE# takes one byte, meaning
that we can't have more than 256 PEs. However, pci_dn->pe_number
already had 4-bytes for the PE#.
This extends the PE# capacity for every PHB. After that, the PE number
is represented by 4-bytes value. Then we can reuse IODA_INVALID_PE to
check the PE# in phb->pe_rmap[] is valid or not.
This should be merged into "[PATCH v8 21/45] powerpc/powernv: Create PEs at
PCI hot plugging time" as it does not make sense alone (this patch does the
initialization but only 3 patches apart this default value is analyzed ->
hard to review).
On Tue, Apr 19, 2016 at 01:07:59PM +1000, Alexey Kardashevskiy wrote:
On 02/17/2016 02:44 PM, Gavin Shan wrote:
quoted
PE number for one particular PE can be allocated dynamically or
reserved according to the consumed M64 (64-bits prefetchable)
segments of the PE. The M64 resources, and hence their segments
and PE number are assigned/reserved in ascending order. The PE
numbers are allocated dynamically in ascending order as well.
It's not a problem as the PE numbers are reserved and then
allocated all at once in fine order. However, it will introduce
conflicts when PCI hotplug is supported: the PE number to be
reserved for newly added PE might have been assigned.
To resolve above conflicts, this forces the PE number to be
allocated dynamically in reverse order. With this patch applied,
the PE numbers are reserved in ascending order, but allocated
dynamically in reverse order.
The patch is probably is ok, the commit log is not - I do not follow it. Some
PEs are reserved (for what? why does the absolute PE number matter? put it in
the commit log), that means that the corresponding bits in pe_alloc[] should
be set so when you will be allocating PEs for a just plugged device, you
won't pick them and you will pick free ones, and the order should not matter.
I would think that "reservation" happens once at the boot time so you set
"used" bits for the reserved PEs then and after that the dynamic allocator
will skip them.
I will enhance the commit log in next revision, perhaps just pick part of
below words: On PHB3, there are 16 M64 BARs in hardware. The last one is
split ovenly into 256 segments. Each segment can be associated/assigned
to fixed PE# (segment#x <-> PE#x) which is how the hardware was designed.
If one plugged PE has M64 (64-bits prefetchable memory) resources, its
PE# is equal to the segment#. Otherwise, the PE# is allocated dynamically
if the PE doesn't contain M64 resource.
The M64 resources are assigned from low to high end, meaning the reserved
PE# (according to the M64 segments) are grown from low to high end. It's
most likely to get a dynamically allocated PE# which should be reserved
because of M64 segment. It's the conflicts the patch tries to resolve.
The PE# reservation doesn't happen once at boot time because it's
unknow how many PEs and how much M64 resources will be hot added.
On Tue, Apr 19, 2016 at 02:16:42PM +1000, Alexey Kardashevskiy wrote:
On 02/17/2016 02:44 PM, Gavin Shan wrote:
quoted
Currently, the PEs and their associated resources are assigned
in ppc_md.pcibios_fixup() except those used by SRIOV VFs.
But this new code does not affect IOV and VF's PEs will still be created
somewhere else rather than pnv_pci_setup_bridge()?
Correct. VF PEs cannot be created in pnv_pci_setup_bridge() as the PF's
IOV capability isn't enabled at that point.
quoted
The
function is called for once after PCI probing and resources
assignment is completed. So it isn't hotplug friendly.
This creates PEs dynamically by ppc_md.pcibios_setup_bridge(), which
is called on the event during system bootup and PCI hotplug: updating
PCI bridge's windows after resource assignment/reassignment are done.
For partial hotplug case, where not all PCI devices belonging to the
PE are unplugged and plugged again, we just need unbinding/binding
the affected PCI devices with the corresponding PE without creating
new one.
As there is no upstream bridge for root bus that needs to be covered
by PE, we have to create PE for root bus in ppc_md.pcibios_setup_bridge()
before any other PEs can be created, as PE for root bus is the ancestor
to anyone else.
We did not need a root bus PE before? What is the other PE reserved for?
Comments only say "reserved"...
No, A PE for root bus is needed before. other PEs can be for the PCI bus
originated from root port and the subordinate domains.
quoted
Also, the windows of root port or the upstream port of PCIe switch behind
root port are extended to be PHB's apertures to accommodate the additional
resources needed by newly plugged devices based on the fact: hotpluggable
slot is behind root port or downstream port of the PCIe switch behind
root port. The extension for those PCI brdiges' windows is done in
ppc_md.pcibios_setup_bridge() as well.
This patch seems to be doing way too many things, hard to follow.
Could you please split the patch into smaller chunks? For example (you can do
it totally different):
- move pnv_pci_ioda_setup_opal_tce_kill()
- move PE creation from pnv_pci_ioda_fixup() to pnv_pci_setup_bridge();
- add pnv_pci_fixup_bridge_resources()
- add an extra reserved PE for the root bus (and all this magic with
root_pe_idx/root_pe_populated)
- ...
I'll evaluate it later. It's always nice to have small patches. Thanks
for the comments.
On Tue, Apr 19, 2016 at 02:28:51PM +1000, Alexey Kardashevskiy wrote:
On 02/17/2016 02:44 PM, Gavin Shan wrote:
quoted
pnv_pci_ioda_table_free_pages() can be reused to release the IODA1
TCE table when releasing IODA1 PE in subsequent patches.
This renames the following functions to support releasing IODA1 TCE
table: pnv_pci_ioda2_table_free_pages() to pnv_pci_ioda_table_free_pages(),
pnv_pci_ioda2_table_do_free_pages() to pnv_pci_ioda_table_do_free_pages().
No logical changes introduced.
I can only see renaming here but it seems (from
IODA_architecture_04-14-2008.pdf) that IODA1 does not support multi-level TCE
tables in the way IODA2 does.
Note that the change was proposed by you in last round. Yes, TVE on P7IOC
doesn't support multiple levels of TCE tables. In this case, we will always
have "tbl->it_indirect_levels" to 1, right?
@@ -263,10 +263,10 @@ static inline struct eeh_dev *pdn_to_eeh_dev(struct pci_dn *pdn)externstructpci_bus*pcibios_find_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);
pci_lala_pci_lala() ("pci" is used twice) looks weird, if the prefix is
"pci", what other device types can they handle?...
May be pcihp_add_devices(), pcihp_remove_devices() as these as defined in
pci-hotplug.c?
I assume you're talking about drivers/pci/hotplug/pci_hotplug_core.c.
pci_hotplug_core.c uses pci_hp_ prefix rather than pcihp_. I will
rename them to pci_hp_*() in next revision.
gwshan@gwshan:~/sandbox/linux$ find . -name pci-hotplug.c
./arch/powerpc/kernel/pci-hotplug.c
gwshan@gwshan:~/sandbox/linux$ grep pci*hp arch/powerpc/kernel/pci-hotplug.c
quoted
/** Discover new pci devices under this bus, and add them */
-extern void pcibios_add_pci_devices(struct pci_bus *bus);
+extern void pci_add_pci_devices(struct pci_bus *bus);
extern void isa_bridge_find_early(struct pci_controller *hose);
@@ -38,20 +38,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);
On Tue, Apr 19, 2016 at 03:48:26PM +1000, Alexey Kardashevskiy wrote:
On 02/17/2016 02:44 PM, Gavin Shan wrote:
quoted
This implements and exports pci_remove_device_node_info(). It's
used to remove the pdn (struct pci_dn) for the indicated device
node. The function is going to be used by PowerNV PCI hotplug
driver.
Signed-off-by: Gavin Shan <redacted>
Kind of strange that there is no such helper for pseries, is there?
I don't find one actually. If you find one, pls let me know, thanks!
On Tue, Apr 19, 2016 at 06:19:20PM +1000, Alexey Kardashevskiy wrote:
On 02/17/2016 02:44 PM, Gavin Shan wrote:
quoted
The pdn (struct pci_dn) instances are allocated from memblock or
bootmem when creating PCI controller (hoses) in setup_arch(). PCI
hotplug, which will be supported by proceeding patches, releases
PCI device nodes and their corresponding pdn on unplugging event.
The memory chunks for pdn instances allocated from memblock or
bootmem are hard to reused after being released.
This delays creating pdn by pci_devs_phb_init() from setup_arch()
to core_initcall() so that they are allocated from slab. The memory
consumed by pdn can be released to system without problem during
PCI unplugging time. It indicates that pci_dn is unavailable in
setup_arch() and the the fixup on pdn (like AGP's) can't be carried
out that time. We have to do that in ppc_md.pcibios_root_bridge_prepare()
on maple/pasemi/powermac platforms where/when the pdn is available.
At the mean while, the EEH device is created when pdn is populated,
meaning pdn and EEH device have same life cycle. In turn, we needn't
call eeh_dev_init() to create EEH device explicitly.
Signed-off-by: Gavin Shan <redacted>
Uff. It would not hurt to mention that pcibios_root_bridge_prepare is called
from subsys_initcall() which is executed after core_initcall() so the code
flow does not change.
Yes, will do in next revision.
Have you checked if there is anything in between
core_initcall(pci_devs_phb_init) and subsys_initcall(pcibios_init) which
might need device tree nodes? For example, subsys_initcall(pcibios_init)
calls (eventually) pnv_pci_ioda_fixup(), if we are unlucky and pcibios_init()
(and therefore pnv_pci_ioda_fixup() or what pseries/others do) is called
before pcibios_init() - won't we crash or something?
I don't catch what you were asking. device-tree nodes (struct device_node)
are always there. This patch doesn't affect them. Perhaps you were talking
about pdn (PCI_DN). If it's the case, this patch delays creating pdn from
setup_arch() to core_initcall(pci_devs_phb_init). I don't see anything need
pdn between setup_arch() and core_initcall().
The changes introduced to powermac/pasemi platforms are: move fixing the child
pdns of the specifiec PHB's pdn from setup_arch() to subsys_initcall(pcibios_init).
I don't see anything between them needs the fixed pdns.
I don't understand how pcibios_init() is called before pcibios_init() in your
context. Sorry for my bad English. Perhaps you're asking the the called sequence
on core_initcall() and subsys_init()? If so, they're defined like below:
#define core_initcall(fn) __define_initcall(fn, 1)
#define subsys_initcall(fn) __define_initcall(fn, 4)
On Tue, Apr 19, 2016 at 07:34:55PM +1000, Alexey Kardashevskiy wrote:
On 02/17/2016 02:44 PM, Gavin Shan wrote:
quoted
The skiboot firmware might provide the PCI slot reset capability
which is identified by property "ibm,reset-by-firmware" on the
PCI slot associated device node.
This checks the property. If it exists, the reset request is routed
to firmware. Otherwise, the reset is done by kernel as before.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/eeh-powernv.c | 41 +++++++++++++++++++++++++++-
1 file changed, 40 insertions(+), 1 deletion(-)
@@ -789,7 +789,7 @@ static int pnv_eeh_root_reset(struct pci_controller *hose, int option)returnret;}-staticintpnv_eeh_bridge_reset(structpci_dev*dev,intoption)+staticint__pnv_eeh_bridge_reset(structpci_dev*dev,intoption){structpci_dn*pdn=pci_get_pdn_by_devfn(dev->bus,dev->devfn);structeeh_dev*edev=pdn_to_eeh_dev(pdn);
@@ -840,6 +840,45 @@ 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);
What is this 1<<60 for?
As you replied in other threads, it's worthy to have some macros for this
piece of business. This bit indicates the ID of the slot behind a switch
port. If this bit is cleared, the ID represents a PHB slot.
quoted
+ uint8_t scope;
+ int64_t rc;
+
+ /*
+ * If the firmware can't handle it, we will issue hot reset
+ * on the secondary bus despite the requested reset type.
+ */
+ 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) {
+ case EEH_RESET_HOT:
+ scope = OPAL_RESET_PCI_HOT;
+ break;
+ case EEH_RESET_FUNDAMENTAL:
+ scope = OPAL_RESET_PCI_FUNDAMENTAL;
+ break;
+ case EEH_RESET_DEACTIVATE:
+ return 0;
+ default:
+ dev_warn(&pdev->dev, "%s: Unsupported reset %d\n",
+ __func__, option);
Can the userspace trigger this case (via VFIO-EEH) and flood dmesg?
It depends on how you defined message flooding actually. It's abnormal
path caused by program internal error, not external users.
On Tue, Apr 19, 2016 at 07:42:01PM +1000, Alexey Kardashevskiy wrote:
On 02/17/2016 02:44 PM, Gavin Shan wrote:
quoted
The device tree will change dynamically in PowerNV PCI hotplug
driver. This enables CONFIG_OF_DYNAMIC to support that.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/Kconfig | 1 +
1 file changed, 1 insertion(+)
On Wed, Feb 17, 2016 at 08:30:42AM -0600, Rob Herring wrote:
On Tue, Feb 16, 2016 at 9:44 PM, Gavin Shan [off-list ref] wrote:
quoted
The function unflatten_dt_node() is called recursively to unflatten
device nodes and properties in the FDT blob. It looks complicated
and hard to be understood.
This splits the function into 3 functions: populate_properties(),
populate_node() and unflatten_dt_node(). populate_properties(),
which is called by populate_node(), creates properties for the
indicated device node. The later one creates the device nodes
from FDT blob. populate_node() gets the offset in FDT blob for
next device nodes and then calls populate_node(). No logical
changes introduced.
Signed-off-by: Gavin Shan <redacted>
---
drivers/of/fdt.c | 249 ++++++++++++++++++++++++++++++++-----------------------
1 file changed, 147 insertions(+), 102 deletions(-)
One nit, otherwise:
Acked-by: Rob Herring <robh@kernel.org>
[...]
quoted
+ /* And we process the "ibm,phandle" property
+ * used in pSeries dynamic device tree
+ * stuff
+ */
+ if (!strcmp(pname, "ibm,phandle"))
+ np->phandle = be32_to_cpup(val);
+
+ pp->name = (char *)pname;
+ pp->length = sz;
+ pp->value = (__be32 *)val;
This cast should not be needed.
Rob, very sorry to response so lately. I will fix it up in next revision.
On Fri, Apr 15, 2016 at 11:10:21AM -0500, Rob Herring wrote:
On Wed, Apr 13, 2016 at 8:30 PM, Gavin Shan [off-list ref] wrote:
quoted
On Thu, Apr 14, 2016 at 09:57:32AM +1000, Alistair Popple wrote:
quoted
Hi Gavin,
<snip>
quoted
quoted
Why exactly cannot EEH reset changes go to a smaller separate patchset
(before hotplug)?
As I explained before, the patchset's order is: PCI generic part,
PowerNV PCI related, EEH related, device-tree part and hotplug driver.
The EEH reset change is included in PATCH[37/45]. There is no point
to reorder the patches.
I don't understand all of the dependencies but if possible splitting the
series up into a set of smaller self-contained patch series makes things
easier to review and may make it easier for you to get this functionality
reviewed and accepted into upstream.
Thanks, Alistair. I will move those cleanup/refactor related patches
to form a separate series which is expected to be merged first. That
will helps the reviewers to focus on the patches with complicated
changes as you suggested. Alexey, please let me know if that way is
you like to see or not.
As I said last cycle, I'll happily take the DT refactoring patches
separately, but you have to tell me if you want me to apply them and
it has to be well before the merge window.
Thanks, Rob. I hope to post next revision (v9) soon and the device-tree
related cleanup patches should be ready for next merge window in it.
On Tue, Apr 19, 2016 at 02:16:42PM +1000, Alexey Kardashevskiy wrote:
quoted
On 02/17/2016 02:44 PM, Gavin Shan wrote:
quoted
Currently, the PEs and their associated resources are assigned
in ppc_md.pcibios_fixup() except those used by SRIOV VFs.
But this new code does not affect IOV and VF's PEs will still be created
somewhere else rather than pnv_pci_setup_bridge()?
Correct. VF PEs cannot be created in pnv_pci_setup_bridge() as the PF's
IOV capability isn't enabled at that point.
quoted
quoted
The
function is called for once after PCI probing and resources
assignment is completed. So it isn't hotplug friendly.
This creates PEs dynamically by ppc_md.pcibios_setup_bridge(), which
is called on the event during system bootup and PCI hotplug: updating
PCI bridge's windows after resource assignment/reassignment are done.
For partial hotplug case, where not all PCI devices belonging to the
PE are unplugged and plugged again, we just need unbinding/binding
the affected PCI devices with the corresponding PE without creating
new one.
As there is no upstream bridge for root bus that needs to be covered
by PE, we have to create PE for root bus in ppc_md.pcibios_setup_bridge()
before any other PEs can be created, as PE for root bus is the ancestor
to anyone else.
We did not need a root bus PE before? What is the other PE reserved for?
Comments only say "reserved"...
No, A PE for root bus is needed before.
Ok. We needed a PE for the root bus and we need it now. What changed? Why
do you reserve another PE?
other PEs can be for the PCI bus
originated from root port and the subordinate domains.
quoted
quoted
Also, the windows of root port or the upstream port of PCIe switch behind
root port are extended to be PHB's apertures to accommodate the additional
resources needed by newly plugged devices based on the fact: hotpluggable
slot is behind root port or downstream port of the PCIe switch behind
root port. The extension for those PCI brdiges' windows is done in
ppc_md.pcibios_setup_bridge() as well.
This patch seems to be doing way too many things, hard to follow.
Could you please split the patch into smaller chunks? For example (you can do
it totally different):
- move pnv_pci_ioda_setup_opal_tce_kill()
- move PE creation from pnv_pci_ioda_fixup() to pnv_pci_setup_bridge();
- add pnv_pci_fixup_bridge_resources()
- add an extra reserved PE for the root bus (and all this magic with
root_pe_idx/root_pe_populated)
- ...
I'll evaluate it later. It's always nice to have small patches. Thanks
for the comments.
quoted
--
Alexey
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 19, 2016 at 02:28:51PM +1000, Alexey Kardashevskiy wrote:
quoted
On 02/17/2016 02:44 PM, Gavin Shan wrote:
quoted
pnv_pci_ioda_table_free_pages() can be reused to release the IODA1
TCE table when releasing IODA1 PE in subsequent patches.
This renames the following functions to support releasing IODA1 TCE
table: pnv_pci_ioda2_table_free_pages() to pnv_pci_ioda_table_free_pages(),
pnv_pci_ioda2_table_do_free_pages() to pnv_pci_ioda_table_do_free_pages().
No logical changes introduced.
I can only see renaming here but it seems (from
IODA_architecture_04-14-2008.pdf) that IODA1 does not support multi-level TCE
tables in the way IODA2 does.
Note that the change was proposed by you in last round.
Hm. I do not recall proposing exactly that :-/
Yes, TVE on P7IOC
doesn't support multiple levels of TCE tables.
I thought it supports 2 levels.
In this case, we will always
have "tbl->it_indirect_levels" to 1, right?
Nope, it will be 0. But it is still ugly to use release function but not to
use its allocating counterpart which is pnv_pci_ioda2_table_alloc_pages().
I suggest having pnv_pci_ioda1_table_free_pages() which will be just a
single free_pages() call. If you need some ioda*-common code to free a
table, then define pnv_ioda1_iommu_ops::free().
@@ -263,10 +263,10 @@ static inline struct eeh_dev *pdn_to_eeh_dev(struct pci_dn *pdn)externstructpci_bus*pcibios_find_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);
pci_lala_pci_lala() ("pci" is used twice) looks weird, if the prefix is
"pci", what other device types can they handle?...
May be pcihp_add_devices(), pcihp_remove_devices() as these as defined in
pci-hotplug.c?
I assume you're talking about drivers/pci/hotplug/pci_hotplug_core.c.
No, the helpers you are renaming are in pci-hotplug.c which uses "pci_" as
a prefix even though the file is supposed to be about hotplug.
pci_hotplug_core.c uses pci_hp_ prefix rather than pcihp_. I will
rename them to pci_hp_*() in next revision.
@@ -38,20 +38,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);
@@ -116,7 +116,7 @@ int rpaphp_enable_slot(struct slot *slot)}if(list_empty(&bus->devices))-pcibios_add_pci_devices(bus);+pci_add_pci_devices(bus);if(!list_empty(&bus->devices)){info->adapter_status=CONFIGURED;
--
Alexey
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 20, 2016 at 01:00:38PM +1000, Alexey Kardashevskiy wrote:
On 04/20/2016 11:12 AM, Gavin Shan wrote:
quoted
On Tue, Apr 19, 2016 at 02:16:42PM +1000, Alexey Kardashevskiy wrote:
quoted
On 02/17/2016 02:44 PM, Gavin Shan wrote:
quoted
Currently, the PEs and their associated resources are assigned
in ppc_md.pcibios_fixup() except those used by SRIOV VFs.
But this new code does not affect IOV and VF's PEs will still be created
somewhere else rather than pnv_pci_setup_bridge()?
Correct. VF PEs cannot be created in pnv_pci_setup_bridge() as the PF's
IOV capability isn't enabled at that point.
quoted
quoted
The
function is called for once after PCI probing and resources
assignment is completed. So it isn't hotplug friendly.
This creates PEs dynamically by ppc_md.pcibios_setup_bridge(), which
is called on the event during system bootup and PCI hotplug: updating
PCI bridge's windows after resource assignment/reassignment are done.
For partial hotplug case, where not all PCI devices belonging to the
PE are unplugged and plugged again, we just need unbinding/binding
the affected PCI devices with the corresponding PE without creating
new one.
As there is no upstream bridge for root bus that needs to be covered
by PE, we have to create PE for root bus in ppc_md.pcibios_setup_bridge()
before any other PEs can be created, as PE for root bus is the ancestor
to anyone else.
We did not need a root bus PE before? What is the other PE reserved for?
Comments only say "reserved"...
No, A PE for root bus is needed before.
Ok. We needed a PE for the root bus and we need it now. What changed? Why do
you reserve another PE?
Originally, all PEs (include the one for root bus) were created at PHB fixup time
in pnv_pci_ioda_fixup(). With this patch, all PEs are created in pnv_pci_setup_bridge().
pnv_pci_setup_bridge() is called for every PCI buses other than root bus. It means
pnv_pci_setup_bridge() isn't called for root bus. So we have to create PE for root
bus before the left PEs are created there. The PE# for root bus is reserved in advance
and used in pnv_pci_setup_bridge() at that point.
quoted
other PEs can be for the PCI bus
quoted
originated from root port and the subordinate domains.
quoted
quoted
Also, the windows of root port or the upstream port of PCIe switch behind
root port are extended to be PHB's apertures to accommodate the additional
resources needed by newly plugged devices based on the fact: hotpluggable
slot is behind root port or downstream port of the PCIe switch behind
root port. The extension for those PCI brdiges' windows is done in
ppc_md.pcibios_setup_bridge() as well.
This patch seems to be doing way too many things, hard to follow.
Could you please split the patch into smaller chunks? For example (you can do
it totally different):
- move pnv_pci_ioda_setup_opal_tce_kill()
- move PE creation from pnv_pci_ioda_fixup() to pnv_pci_setup_bridge();
- add pnv_pci_fixup_bridge_resources()
- add an extra reserved PE for the root bus (and all this magic with
root_pe_idx/root_pe_populated)
- ...
I'll evaluate it later. It's always nice to have small patches. Thanks
for the comments.
quoted
--
Alexey
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 19, 2016 at 06:19:20PM +1000, Alexey Kardashevskiy wrote:
quoted
On 02/17/2016 02:44 PM, Gavin Shan wrote:
quoted
The pdn (struct pci_dn) instances are allocated from memblock or
bootmem when creating PCI controller (hoses) in setup_arch(). PCI
hotplug, which will be supported by proceeding patches, releases
PCI device nodes and their corresponding pdn on unplugging event.
The memory chunks for pdn instances allocated from memblock or
bootmem are hard to reused after being released.
This delays creating pdn by pci_devs_phb_init() from setup_arch()
to core_initcall() so that they are allocated from slab. The memory
consumed by pdn can be released to system without problem during
PCI unplugging time. It indicates that pci_dn is unavailable in
setup_arch() and the the fixup on pdn (like AGP's) can't be carried
out that time. We have to do that in ppc_md.pcibios_root_bridge_prepare()
on maple/pasemi/powermac platforms where/when the pdn is available.
At the mean while, the EEH device is created when pdn is populated,
meaning pdn and EEH device have same life cycle. In turn, we needn't
call eeh_dev_init() to create EEH device explicitly.
Signed-off-by: Gavin Shan <redacted>
Uff. It would not hurt to mention that pcibios_root_bridge_prepare is called
from subsys_initcall() which is executed after core_initcall() so the code
flow does not change.
Yes, will do in next revision.
quoted
Have you checked if there is anything in between
core_initcall(pci_devs_phb_init) and subsys_initcall(pcibios_init) which
might need device tree nodes? For example, subsys_initcall(pcibios_init)
calls (eventually) pnv_pci_ioda_fixup(), if we are unlucky and pcibios_init()
(and therefore pnv_pci_ioda_fixup() or what pseries/others do) is called
before pcibios_init() - won't we crash or something?
I don't catch what you were asking. device-tree nodes (struct device_node)
are always there. This patch doesn't affect them. Perhaps you were talking
about pdn (PCI_DN). If it's the case, this patch delays creating pdn from
setup_arch() to core_initcall(pci_devs_phb_init).
While thinking of explaining what I wanted to ask, I found my answer :)
pcibios_init() calls ppc_md.pcibios_root_bridge_prepare() first, then
ppc_md.pcibios_fixup() so we are fine here with ordering.
I don't see anything need pdn between setup_arch() and core_initcall().
The changes introduced to powermac/pasemi platforms are: move fixing the child
pdns of the specifiec PHB's pdn from setup_arch() to subsys_initcall(pcibios_init).
I don't see anything between them needs the fixed pdns.
I don't understand how pcibios_init() is called before pcibios_init() in your
pcibios_init() is used twice in the sentence above :)
Anyway,
Reviewed-by: Alexey Kardashevskiy <redacted>
context. Sorry for my bad English. Perhaps you're asking the the called sequence
on core_initcall() and subsys_init()? If so, they're defined like below:
#define core_initcall(fn) __define_initcall(fn, 1)
#define subsys_initcall(fn) __define_initcall(fn, 4)
>
quoted
--
Alexey
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 19, 2016 at 07:34:55PM +1000, Alexey Kardashevskiy wrote:
quoted
On 02/17/2016 02:44 PM, Gavin Shan wrote:
quoted
The skiboot firmware might provide the PCI slot reset capability
which is identified by property "ibm,reset-by-firmware" on the
PCI slot associated device node.
This checks the property. If it exists, the reset request is routed
to firmware. Otherwise, the reset is done by kernel as before.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/eeh-powernv.c | 41 +++++++++++++++++++++++++++-
1 file changed, 40 insertions(+), 1 deletion(-)
@@ -789,7 +789,7 @@ static int pnv_eeh_root_reset(struct pci_controller *hose, int option)returnret;}-staticintpnv_eeh_bridge_reset(structpci_dev*dev,intoption)+staticint__pnv_eeh_bridge_reset(structpci_dev*dev,intoption){structpci_dn*pdn=pci_get_pdn_by_devfn(dev->bus,dev->devfn);structeeh_dev*edev=pdn_to_eeh_dev(pdn);
@@ -840,6 +840,45 @@ 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);
What is this 1<<60 for?
As you replied in other threads, it's worthy to have some macros for this
piece of business. This bit indicates the ID of the slot behind a switch
port. If this bit is cleared, the ID represents a PHB slot.
quoted
quoted
+ uint8_t scope;
+ int64_t rc;
+
+ /*
+ * If the firmware can't handle it, we will issue hot reset
+ * on the secondary bus despite the requested reset type.
+ */
+ 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) {
+ case EEH_RESET_HOT:
+ scope = OPAL_RESET_PCI_HOT;
+ break;
+ case EEH_RESET_FUNDAMENTAL:
+ scope = OPAL_RESET_PCI_FUNDAMENTAL;
+ break;
+ case EEH_RESET_DEACTIVATE:
+ return 0;
+ default:
+ dev_warn(&pdev->dev, "%s: Unsupported reset %d\n",
+ __func__, option);
Can the userspace trigger this case (via VFIO-EEH) and flood dmesg?
It depends on how you defined message flooding actually. It's abnormal
path caused by program internal error, not external users.
Can QEMU be changed to do something special (cause reset with a wrong
option) via VFIO/EEH interface in a loop to make this message appear? Or
the call with a wrong option will never reach this point?
On Wed, Feb 17, 2016 at 08:30:42AM -0600, Rob Herring wrote:
On Tue, Feb 16, 2016 at 9:44 PM, Gavin Shan [off-list ref] wrote:
quoted
The function unflatten_dt_node() is called recursively to unflatten
device nodes and properties in the FDT blob. It looks complicated
and hard to be understood.
This splits the function into 3 functions: populate_properties(),
populate_node() and unflatten_dt_node(). populate_properties(),
which is called by populate_node(), creates properties for the
indicated device node. The later one creates the device nodes
from FDT blob. populate_node() gets the offset in FDT blob for
next device nodes and then calls populate_node(). No logical
changes introduced.
Signed-off-by: Gavin Shan <redacted>
---
drivers/of/fdt.c | 249 ++++++++++++++++++++++++++++++++-----------------------
1 file changed, 147 insertions(+), 102 deletions(-)
One nit, otherwise:
Acked-by: Rob Herring <robh@kernel.org>
[...]
quoted
+ /* And we process the "ibm,phandle" property
+ * used in pSeries dynamic device tree
+ * stuff
+ */
+ if (!strcmp(pname, "ibm,phandle"))
+ np->phandle = be32_to_cpup(val);
+
+ pp->name = (char *)pname;
+ pp->length = sz;
+ pp->value = (__be32 *)val;
This cast should not be needed.
It's needed. Otherwise, we will have warning. So I will keep it. I just
went through this one for next revision and sorry for late response.
drivers/of/fdt.c:225:14: warning: assignment discards ‘const’ qualifier from pointer target type
pp->value = val;
^
Thanks,
Gavin