From: Ian Munsie <hidden> Date: 2016-07-04 13:22:41
This series adds support for the Mellanox CX4 network adapter operating in cxl
mode to the cxl driver and the PowerNV PHB code. The Mellanox developers will
submit a separate patch series that makes use of this in the mlx5 driver.
The CX4 card can operate in either pci mode, or cxl mode. In cxl mode, memory
accesses from the card go through the XSL (Translation Service Layer,
essentially a stripped down version of the Power Service Layer), allowing it to
transparently access unpinned memory with the cxl driver handling faulting in
pages as necessary, etc. Most of the support for the XSL is already upstream,
though this series does include a bug fix to enable bus mastering for this
(patch 3).
Patch 2 in this series provides an API which the mlx5 driver can query to check
if it is in a cxl capable slot. The card will come up in pci mode, and the mlx5
driver can choose to switch it to cxl mode, wherein it will reappear with an
additional physical function representing the XSL that the cxl driver will bind
to. Patches 12-14 add support for switching the card's mode, including using
the PCI hotplug support to re-enumerate the device tree and re-probind the
card.
Unlike previous users of the cxl kernel API where we used a virtual PHB and
exposed PCI devices under it, the Mellanox CX4 uses a peer model where cxl
binds to one of the physical functions of the card and the mlx5_core driver
binds to the other networking physical functions. Patches 6 and 7 add support
for using the cxl kernel API with the real PHB to enable this peer model.
Patches 4 and 5 are prepatory patches exposing some APIs that the PHB will need
to call.
While in cxl mode, interrupts from the CX4 are a little unusual - they are
neither pci interrupts, nor cxl interrutps, but rather a hybrid of the two. The
interrupts are passed from the networking hardware to the XSL using a custom
format in the MSIX table, and from there are treated as cxl interrupts. These
are configured mostly transparently using the standard msix APIs - the PHB
handles allocating and configuring the cxl interrupts, associating them with
the default context, and the mlx5 driver handles filling out the MSIX table
with their custom format (not included in this series). See patch 10.
Additionally, the CX4 has a hard limitation of the number of interrupts that
can be associated with a given context, so to overcome this patches 8 and 9
expose an API to allow the mlx5 driver to inform us of the limit, and the
interrupt allocation code in patch 10 will allocate additional contexts to
associate these with.
Patch 1 is a prepatory cleanup patch to reorganise cxl code in arch/powerpc
into a separate file.
Patch 11 is a workaround for a hardware limitation in the CX4 where a context
with PE=0 cannot be used.
Note that patch 2 depends on "cxl: Ignore CAPI adapters misplaced in switched
slot" by Philippe Bergheaud:
http://patchwork.ozlabs.org/patch/642920/
Additionally, the following stand-alone patches related to the CX4 are also
pending on the mainling list, but are *not* dependencies of this series:
- cxl: Fix bug where AFU disable operation had no effect
- cxl: Workaround XSL bug that does not clear the RA bit after a reset
- cxl: Fix NULL pointer dereference on kernel contexts with no AFU interrupts
The entire series is bisectable.
From: Ian Munsie <hidden> Date: 2016-07-04 13:22:44
From: Ian Munsie <redacted>
The support for using the Mellanox CX4 in cxl mode will require
additions to the PHB code. In preparation for this, move the existing
cxl code out of pci-ioda.c into a separate pci-cxl.c file to keep things
more organised.
Signed-off-by: Ian Munsie <redacted>
---
arch/powerpc/platforms/powernv/Makefile | 1 +
arch/powerpc/platforms/powernv/pci-cxl.c | 163 ++++++++++++++++++++++++++++++
arch/powerpc/platforms/powernv/pci-ioda.c | 159 +----------------------------
arch/powerpc/platforms/powernv/pci.h | 6 ++
4 files changed, 173 insertions(+), 156 deletions(-)
create mode 100644 arch/powerpc/platforms/powernv/pci-cxl.c
@@ -0,0 +1,163 @@+/*+*Copyright2015IBMCorp.+*+*Thisprogramisfreesoftware;youcanredistributeitand/or+*modifyitunderthetermsoftheGNUGeneralPublicLicense+*aspublishedbytheFreeSoftwareFoundation;eitherversion+*2oftheLicense,or(atyouroption)anylaterversion.+*/++#include<asm/pnv-pci.h>+#include<asm/opal.h>++#include"pci.h"++structdevice_node*pnv_pci_get_phb_node(structpci_dev*dev)+{+structpci_controller*hose=pci_bus_to_host(dev->bus);++returnof_node_get(hose->dn);+}+EXPORT_SYMBOL(pnv_pci_get_phb_node);++intpnv_phb_to_cxl_mode(structpci_dev*dev,uint64_tmode)+{+structpci_controller*hose=pci_bus_to_host(dev->bus);+structpnv_phb*phb=hose->private_data;+structpnv_ioda_pe*pe;+intrc;++pe=pnv_ioda_get_pe(dev);+if(!pe)+return-ENODEV;++pe_info(pe,"Switching PHB to CXL\n");++rc=opal_pci_set_phb_cxl_mode(phb->opal_id,mode,pe->pe_number);+if(rc==OPAL_UNSUPPORTED)+dev_err(&dev->dev,"Required cxl mode not supported by firmware - update skiboot\n");+elseif(rc)+dev_err(&dev->dev,"opal_pci_set_phb_cxl_mode failed: %i\n",rc);++returnrc;+}+EXPORT_SYMBOL(pnv_phb_to_cxl_mode);++/* Find PHB for cxl dev and allocate MSI hwirqs?+*ReturnstheabsolutehardwareIRQnumber+*/+intpnv_cxl_alloc_hwirqs(structpci_dev*dev,intnum)+{+structpci_controller*hose=pci_bus_to_host(dev->bus);+structpnv_phb*phb=hose->private_data;+inthwirq=msi_bitmap_alloc_hwirqs(&phb->msi_bmp,num);++if(hwirq<0){+dev_warn(&dev->dev,"Failed to find a free MSI\n");+return-ENOSPC;+}++returnphb->msi_base+hwirq;+}+EXPORT_SYMBOL(pnv_cxl_alloc_hwirqs);++voidpnv_cxl_release_hwirqs(structpci_dev*dev,inthwirq,intnum)+{+structpci_controller*hose=pci_bus_to_host(dev->bus);+structpnv_phb*phb=hose->private_data;++msi_bitmap_free_hwirqs(&phb->msi_bmp,hwirq-phb->msi_base,num);+}+EXPORT_SYMBOL(pnv_cxl_release_hwirqs);++voidpnv_cxl_release_hwirq_ranges(structcxl_irq_ranges*irqs,+structpci_dev*dev)+{+structpci_controller*hose=pci_bus_to_host(dev->bus);+structpnv_phb*phb=hose->private_data;+inti,hwirq;++for(i=1;i<CXL_IRQ_RANGES;i++){+if(!irqs->range[i])+continue;+pr_devel("cxl release irq range 0x%x: offset: 0x%lx limit: %ld\n",+i,irqs->offset[i],+irqs->range[i]);+hwirq=irqs->offset[i]-phb->msi_base;+msi_bitmap_free_hwirqs(&phb->msi_bmp,hwirq,+irqs->range[i]);+}+}+EXPORT_SYMBOL(pnv_cxl_release_hwirq_ranges);++intpnv_cxl_alloc_hwirq_ranges(structcxl_irq_ranges*irqs,+structpci_dev*dev,intnum)+{+structpci_controller*hose=pci_bus_to_host(dev->bus);+structpnv_phb*phb=hose->private_data;+inti,hwirq,try;++memset(irqs,0,sizeof(structcxl_irq_ranges));++/* 0 is reserved for the multiplexed PSL DSI interrupt */+for(i=1;i<CXL_IRQ_RANGES&#i++){+try=num;+while(try){+hwirq=msi_bitmap_alloc_hwirqs(&phb->msi_bmp,try);+if(hwirq>=0)+break;+try/=2;+}+if(!try)+gotofail;++irqs->offset[i]=phb->msi_base+hwirq;+irqs->range[i]=try;+pr_devel("cxl alloc irq range 0x%x: offset: 0x%lx limit: %li\n",+i,irqs->offset[i],irqs->range[i]);+num-=try;+}+if(num)+gotofail;++return0;+fail:+pnv_cxl_release_hwirq_ranges(irqs,dev);+return-ENOSPC;+}+EXPORT_SYMBOL(pnv_cxl_alloc_hwirq_ranges);++intpnv_cxl_get_irq_count(structpci_dev*dev)+{+structpci_controller*hose=pci_bus_to_host(dev->bus);+structpnv_phb*phb=hose->private_data;++returnphb->msi_bmp.irq_count;+}+EXPORT_SYMBOL(pnv_cxl_get_irq_count);++intpnv_cxl_ioda_msi_setup(structpci_dev*dev,unsignedinthwirq,+unsignedintvirq)+{+structpci_controller*hose=pci_bus_to_host(dev->bus);+structpnv_phb*phb=hose->private_data;+unsignedintxive_num=hwirq-phb->msi_base;+structpnv_ioda_pe*pe;+intrc;++if(!(pe=pnv_ioda_get_pe(dev)))+return-ENODEV;++/* Assign XIVE to PE */+rc=opal_pci_set_xive_pe(phb->opal_id,pe->pe_number,xive_num);+if(rc){+pe_warn(pe,"%s: OPAL error %d setting msi_base 0x%x "+"hwirq 0x%x XIVE 0x%x PE\n",+pci_name(dev),rc,phb->msi_base,hwirq,xive_num);+return-EIO;+}+pnv_set_msi_irq_chip(phb,virq);++return0;+}+EXPORT_SYMBOL(pnv_cxl_ioda_msi_setup);
From: Ian Munsie <hidden> Date: 2016-07-04 13:22:50
From: Ian Munsie <redacted>
Devices that use CAPP DMA mode (such as the Mellanox CX4) require bus
master to be enabled in order for the CAPI traffic to flow. This should
be harmless to enable for other cxl devices, so unconditionally enable
it in the adapter init flow.
Signed-off-by: Ian Munsie <redacted>
---
drivers/misc/cxl/pci.c | 3 +++
1 file changed, 3 insertions(+)
From: Ian Munsie <hidden> Date: 2016-07-04 13:22:52
From: Ian Munsie <redacted>
The cxl kernel API has a concept of a default context associated with
each PCI device under the virtual PHB. The Mellanox CX4 will also use
the cxl kernel API, but it does not use a virtual PHB - rather, the AFU
appears as a physical function as a peer to the networking functions.
In order to allow the kernel API to work with those networking
functions, we will need to associate a default context with them as
well. To this end, refactor the corresponding code to do this in vphb.c
and export it so that it can be called from the PHB code.
Signed-off-by: Ian Munsie <redacted>
---
drivers/misc/cxl/base.c | 35 +++++++++++++++++++++++++++++++++++
drivers/misc/cxl/cxl.h | 6 ++++++
drivers/misc/cxl/main.c | 2 ++
drivers/misc/cxl/vphb.c | 37 +++++++++++++++++++++++--------------
include/misc/cxl-base.h | 6 ++++++
5 files changed, 72 insertions(+), 14 deletions(-)
From: Ian Munsie <hidden> Date: 2016-07-04 13:22:53
From: Ian Munsie <redacted>
This adds support for the peer model of the cxl kernel api to the
PowerNV PHB, and exports APIs to enable the mode, check if a PCI device
is attached to a PHB in this mode, and to set and get the peer AFU for
this mode.
The cxl driver will enable this mode for supported cards by calling
pnv_cxl_enable_phb_kernel_api(). This will set a flag in the PHB to note
that this mode is enabled, and switch out it's controller_ops for the
cxl version.
The cxl version of the controller_ops struct implements it's own
versions of the enable_device_hook and release_device to handle
refcounting on the peer AFU and to allocate a default context for the
device.
Once enabled, the cxl kernel API may not be disabled on a PHB. Currently
there is no safe way to disable cxl mode short of a reboot, so until
that changes there is no reason to support the disable path.
Signed-off-by: Ian Munsie <redacted>
---
arch/powerpc/include/asm/pnv-pci.h | 7 ++
arch/powerpc/platforms/powernv/pci-cxl.c | 112 ++++++++++++++++++++++++++++++
arch/powerpc/platforms/powernv/pci-ioda.c | 22 +++++-
arch/powerpc/platforms/powernv/pci.h | 16 +++++
4 files changed, 154 insertions(+), 3 deletions(-)
@@ -38,6 +38,13 @@ int pnv_cxl_alloc_hwirq_ranges(struct cxl_irq_ranges *irqs,structpci_dev*dev,intnum);voidpnv_cxl_release_hwirq_ranges(structcxl_irq_ranges*irqs,structpci_dev*dev);++/* Support for the cxl kernel api on the real PHB (instead of vPHB) */+intpnv_cxl_enable_phb_kernel_api(structpci_controller*hose,boolenable);+boolpnv_pci_on_cxl_phb(structpci_dev*dev);+structcxl_afu*pnv_cxl_phb_to_afu(structpci_controller*hose);+voidpnv_cxl_phb_set_peer_afu(structpci_dev*dev,structcxl_afu*afu);+#endif#endif
@@ -161,3 +164,112 @@ int pnv_cxl_ioda_msi_setup(struct pci_dev *dev, unsigned int hwirq,return0;}EXPORT_SYMBOL(pnv_cxl_ioda_msi_setup);++/*+*Setsflagsandswitchesthecontrolleropstoenablethecxlkernelapi.+*OriginalthecxlkernelAPIoperatedonavirtualPHB,butcertaincards+*suchastheMellanoxCX4useapeermodelinsteadandforthesecardsthe+*cxlkernelapiwilloperateontherealPHB.+*/+intpnv_cxl_enable_phb_kernel_api(structpci_controller*hose,boolenable)+{+structpnv_phb*phb=hose->private_data;+structmodule*cxl_module;++if(!enable){+/*+*OncecxlmodeisenabledonthePHB,thereiscurrentlyno+*knownsafemethodtodisableitagain,andtryingrisksa+*checkstop.Ifwecanfindawaytosafelydisablecxlmode+*inthefuturewecanrevisitthis,butfornowtheonlysane+*thingtodoistorefusetodisablecxlmode:+*/+return-EPERM;+}++/*+*HoldareferencetothecxlmodulesinceseveralPHBoperationsnow+*dependonit,anditwouldbeinsanetoallowittoberemovedso+*longasweareinthismode(andsincewecan'tsafelydisablethis+*modeonceenabled...).+*/+mutex_lock(&module_mutex);+cxl_module=find_module("cxl");+if(cxl_module)+__module_get(cxl_module);+mutex_unlock(&module_mutex);+if(!cxl_module)+return-ENODEV;++phb->flags|=PNV_PHB_FLAG_CXL;+hose->controller_ops=pnv_cxl_cx4_ioda_controller_ops;++return0;+}+EXPORT_SYMBOL(pnv_cxl_enable_phb_kernel_api);++boolpnv_pci_on_cxl_phb(structpci_dev*dev)+{+structpci_controller*hose=pci_bus_to_host(dev->bus);+structpnv_phb*phb=hose->private_data;++return!!(phb->flags&PNV_PHB_FLAG_CXL);+}+EXPORT_SYMBOL(pnv_pci_on_cxl_phb);++structcxl_afu*pnv_cxl_phb_to_afu(structpci_controller*hose)+{+structpnv_phb*phb=hose->private_data;++return(structcxl_afu*)phb->cxl_afu;+}+EXPORT_SYMBOL_GPL(pnv_cxl_phb_to_afu);++voidpnv_cxl_phb_set_peer_afu(structpci_dev*dev,structcxl_afu*afu)+{+structpci_controller*hose=pci_bus_to_host(dev->bus);+structpnv_phb*phb=hose->private_data;++phb->cxl_afu=afu;+}+EXPORT_SYMBOL_GPL(pnv_cxl_phb_set_peer_afu);++boolpnv_cxl_enable_device_hook(structpci_dev*dev)+{+structpci_controller*hose=pci_bus_to_host(dev->bus);+structpnv_phb*phb=hose->private_data;+structcxl_afu*afu=phb->cxl_afu;++if(!pnv_pci_enable_device_hook(dev))+returnfalse;++/* No special handling for cxl function: */+if(PCI_FUNC(dev->devfn)==0)+returntrue;++if(!afu){+dev_WARN(&dev->dev,"Attempted to enable function > 0 on CXL PHB without a peer AFU\n");+returnfalse;+}++dev_info(&dev->dev,"Enabling function on CXL enabled PHB with peer AFU\n");++/* Make sure the peer AFU can't go away while this device is active */+cxl_afu_get(afu);++returncxl_pci_associate_default_context(dev,afu);+}++voidpnv_cxl_disable_device(structpci_dev*dev)+{+structpci_controller*hose=pci_bus_to_host(dev->bus);+structpnv_phb*phb=hose->private_data;+structcxl_afu*afu=phb->cxl_afu;++/* No special handling for cxl function: */+if(PCI_FUNC(dev->devfn)==0)+return;++cxl_pci_disable_device(dev);+cxl_afu_put(afu);+}
@@ -218,6 +222,8 @@ extern int pnv_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type);externvoidpnv_teardown_msi_irqs(structpci_dev*pdev);externstructpnv_ioda_pe*pnv_ioda_get_pe(structpci_dev*dev);externvoidpnv_set_msi_irq_chip(structpnv_phb*phb,unsignedintvirq);+externboolpnv_pci_enable_device_hook(structpci_dev*dev);+externvoidpnv_pci_release_device(structpci_dev*pdev);externvoidpe_level_printk(conststructpnv_ioda_pe*pe,constchar*level,constchar*fmt,...);
@@ -238,4 +244,14 @@ extern long pnv_npu_unset_window(struct pnv_ioda_pe *npe, int num);externvoidpnv_npu_take_ownership(structpnv_ioda_pe*npe);externvoidpnv_npu_release_ownership(structpnv_ioda_pe*npe);++/* cxl functions */+externboolpnv_cxl_enable_device_hook(structpci_dev*dev);+externvoidpnv_cxl_disable_device(structpci_dev*dev);+++/* phb ops (cxl switches these when enabling the kernel api on the phb) */+externconststructpci_controller_opspnv_cxl_cx4_ioda_controller_ops;+externconststructpci_controller_opspnv_pci_ioda_controller_ops;+#endif /* __POWERNV_PCI_H */
From: Ian Munsie <hidden> Date: 2016-07-04 13:22:54
From: Ian Munsie <redacted>
This hooks up support for using the kernel API with a real PHB. After
the AFU initialisation has completed it calls into the PHB code to pass
it the AFU that will be used by other peer physical functions on the
adapter.
The cxl_pci_to_afu API is extended to work with peer PCI devices,
retrieving the peer AFU from the PHB. This API may also now return an
error if it is called on a PCI device that is not associated with either
a cxl vPHB or a peer PCI device to an AFU, and this error is propagated
down.
Signed-off-by: Ian Munsie <redacted>
---
drivers/misc/cxl/api.c | 5 +++++
drivers/misc/cxl/pci.c | 6 ++++++
drivers/misc/cxl/vphb.c | 16 ++++++++++++++--
3 files changed, 25 insertions(+), 2 deletions(-)
@@ -1502,6 +1502,9 @@ static int cxl_probe(struct pci_dev *dev, const struct pci_device_id *id)dev_err(&dev->dev,"AFU %i failed to start: %i\n",slice,rc);}+if(pnv_pci_on_cxl_phb(dev)&&adapter->slices>=1)+pnv_cxl_phb_set_peer_afu(dev,adapter->afu[0]);+return0;}
@@ -1572,6 +1575,9 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,*/for(i=0;i<adapter->slices;i++){afu=adapter->afu[i];+/* Only participate in EEH if we are on a virtual PHB */+if(afu->phb==NULL)+returnPCI_ERS_RESULT_NONE;cxl_vphb_error_detected(afu,state);}returnPCI_ERS_RESULT_DISCONNECT;
From: Ian Munsie <hidden> Date: 2016-07-04 13:22:55
From: Ian Munsie <redacted>
These APIs will be used by the Mellanox CX4 support. While they function
standalone to configure existing behaviour, their primary purpose is to
allow the Mellanox driver to inform the cxl driver of a hardware
limitation, which will be used in a future patch.
Signed-off-by: Ian Munsie <redacted>
---
drivers/misc/cxl/api.c | 27 +++++++++++++++++++++++++++
include/misc/cxl.h | 10 ++++++++++
2 files changed, 37 insertions(+)
@@ -447,3 +447,30 @@ ssize_t cxl_read_adapter_vpd(struct pci_dev *dev, void *buf, size_t count)returncxl_ops->read_adapter_vpd(afu->adapter,buf,count);}EXPORT_SYMBOL_GPL(cxl_read_adapter_vpd);++intcxl_set_max_irqs_per_process(structpci_dev*dev,intirqs)+{+structcxl_afu*afu=cxl_pci_to_afu(dev);+if(IS_ERR(afu))+return-ENODEV;++if(irqs>afu->adapter->user_irqs)+return-EINVAL;++/* Limit user_irqs to prevent the user increasing this via sysfs */+afu->adapter->user_irqs=irqs;+afu->irqs_max=irqs;++return0;+}+EXPORT_SYMBOL_GPL(cxl_set_max_irqs_per_process);++intcxl_get_max_irqs_per_process(structpci_dev*dev)+{+structcxl_afu*afu=cxl_pci_to_afu(dev);+if(IS_ERR(afu))+return-ENODEV;++returnafu->irqs_max;+}+EXPORT_SYMBOL_GPL(cxl_get_max_irqs_per_process);
@@ -166,6 +166,16 @@ void cxl_psa_unmap(void __iomem *addr);/* Get the process element for this context */intcxl_process_element(structcxl_context*ctx);+/*+*Limitthenumberofinterruptsthatasinglecontextcanallocatevia+*cxl_start_work.Ifusingtheapiwitharealphb,thismaybeusedto+*requestthatadditionaldefaultcontextsbecreatedwhenallocating+*interruptsviapci_enable_msix_range.Thesewillbesettothesamerunning+*stateasthedefaultcontext,andifthatisrunningitwillreusethe+*parameterspreviouslypassedtocxl_start_contextforthedefaultcontext.+*/+intcxl_set_max_irqs_per_process(structpci_dev*dev,intirqs);+intcxl_get_max_irqs_per_process(structpci_dev*dev);/**Thesecallsallowdriverstocreatetheirownfiledescriptorsandmakethem
From: Ian Munsie <hidden> Date: 2016-07-04 13:22:56
From: Ian Munsie <redacted>
The Mellanox CX4 has a hardware limitation where only 4 bits of the
AFU interrupt number can be passed to the XSL when sending an interrupt,
limiting it to only 15 interrupts per context (AFU interrupt number 0 is
invalid).
In order to overcome this, we will allocate additional contexts linked
to the default context as extra address space for the extra interrupts -
this will be implemented in the next patch.
This patch adds the preliminary support to allow this, by way of adding
a linked list in the context structure that we use to keep track of the
contexts dedicated to interrupts, and an API to simultaneously iterate
over the related context structures, AFU interrupt numbers and hardware
interrupt numbers. The point of using a single API to iterate these is
to hide some of the details of the iteration from external code, and to
reduce the number of APIs that need to be exported via base.c to allow
built in code to call.
Signed-off-by: Ian Munsie <redacted>
---
drivers/misc/cxl/api.c | 15 +++++++++++++++
drivers/misc/cxl/base.c | 17 +++++++++++++++++
drivers/misc/cxl/context.c | 1 +
drivers/misc/cxl/cxl.h | 10 ++++++++++
drivers/misc/cxl/main.c | 1 +
include/misc/cxl.h | 9 +++++++++
6 files changed, 53 insertions(+)
@@ -178,6 +178,15 @@ int cxl_set_max_irqs_per_process(struct pci_dev *dev, int irqs);intcxl_get_max_irqs_per_process(structpci_dev*dev);/*+*Usetosimultaneouslyiterateoverhardwareinterruptnumbers,contextsand+*afuinterruptnumbersallocatedforthedeviceviapci_enable_msix_rangeand+*isausefulconviniencefunctionwhenworkingwithhardwarethathas+*limitationsonthenumberofinterruptsperprocess.*ctxand*afu_irq+*shouldbeNULLand0tostarttheiteration.+*/+intcxl_next_msi_hwirq(structpci_dev*pdev,structcxl_context**ctx,int*afu_irq);++/**Thesecallsallowdriverstocreatetheirownfiledescriptorsandmakethem*identicaltothecxlfiledescriptoruserAPI.Anexampleusecase:*
From: Ian Munsie <hidden> Date: 2016-07-04 13:23:00
From: Ian Munsie <redacted>
The CX4 card cannot cope with a context with PE=0 due to a hardware
limitation, resulting in:
[ 34.166577] command failed, status limits exceeded(0x8), syndrome 0x5a7939
[ 34.166580] mlx5_core 0000:01:00.1: Failed allocating uar, aborting
Since the kernel API allocates a default context very early during
device init that will almost certainly get Process Element ID 0 there is
no easy way for us to extend the API to allow the Mellanox to inform us
of this limitation ahead of time.
Instead, work around the issue by extending the XSL structure to include
a minimum PE to allocate. Although the bug is not in the XSL, it is the
easiest place to work around this limitation given that the CX4 is
currently the only card that uses an XSL.
Signed-off-by: Ian Munsie <redacted>
---
drivers/misc/cxl/context.c | 3 ++-
drivers/misc/cxl/cxl.h | 1 +
drivers/misc/cxl/pci.c | 1 +
3 files changed, 4 insertions(+), 1 deletion(-)
From: Ian Munsie <hidden> Date: 2016-07-04 13:23:02
From: Andrew Donnellan <redacted>
The cxl driver will use infrastructure from pnv_php to handle device tree
updates when switching bi-modal CAPI cards into CAPI mode.
To enable this, export pnv_php_find_slot() and
pnv_php_set_slot_power_state(), and add corresponding declarations, as well
as the definition of struct pnv_php_slot, to asm/pnv-pci.h.
Cc: Gavin Shan <redacted>
Cc: linux-pci@vger.kernel.org
Cc: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Andrew Donnellan <redacted>
Acked-by: Gavin Shan <redacted>
---
arch/powerpc/include/asm/pnv-pci.h | 28 ++++++++++++++++++++++++++++
drivers/pci/hotplug/Kconfig | 1 +
drivers/pci/hotplug/pnv_php.c | 32 +++++---------------------------
3 files changed, 34 insertions(+), 27 deletions(-)
From: Ian Munsie <hidden> Date: 2016-07-04 13:23:04
From: Andrew Donnellan <redacted>
When calling pnv_php_set_slot_power_state() with state ==
OPAL_PCI_SLOT_OFFLINE, remove devices from the device tree as if we're
dealing with OPAL_PCI_SLOT_POWER_OFF.
Cc: Gavin Shan <redacted>
Cc: linux-pci@vger.kernel.org
Cc: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Andrew Donnellan <redacted>
Acked-by: Gavin Shan <redacted>
---
drivers/pci/hotplug/pnv_php.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Ian Munsie <hidden> Date: 2016-07-04 13:23:07
From: Andrew Donnellan <redacted>
Add a new API, cxl_check_and_switch_mode() to allow for switching of
bi-modal CAPI cards, such as the Mellanox CX-4 network card.
When a driver requests to switch a card to CAPI mode, use PCI hotplug
infrastructure to remove all PCI devices underneath the slot. We then write
an updated mode control register to the CAPI VSEC, hot reset the card, and
reprobe the card.
As the card may present a different set of PCI devices after the mode
switch, use the infrastructure provided by the pnv_php driver and the OPAL
PCI slot management facilities to ensure that:
* the old devices are removed from both the OPAL and Linux device trees
* the new devices are probed by OPAL and added to the OPAL device tree
* the new devices are added to the Linux device tree and probed through
the regular PCI device probe path
As such, introduce a new option, CONFIG_CXL_BIMODAL, with a dependency on
the pnv_php driver.
Refactor existing code that touches the mode control register in the
regular single mode case into a new function, setup_cxl_protocol_area().
Co-authored-by: Ian Munsie [off-list ref]
Cc: Gavin Shan <redacted>
Signed-off-by: Andrew Donnellan <redacted>
Reviewed-by: Gavin Shan <redacted>
---
drivers/misc/cxl/Kconfig | 8 ++
drivers/misc/cxl/pci.c | 234 +++++++++++++++++++++++++++++++++++++++++++----
include/misc/cxl.h | 25 +++++
3 files changed, 249 insertions(+), 18 deletions(-)
@@ -614,36 +616,232 @@ static int setup_cxl_bars(struct pci_dev *dev)return0;}-/* pciex node: ibm,opal-m64-window = <0x3d058 0x0 0x3d058 0x0 0x8 0x0>; */-staticintswitch_card_to_cxl(structpci_dev*dev)-{+#ifdef CONFIG_CXL_BIMODAL++structcxl_switch_work{+structpci_dev*dev;+structwork_structwork;intvsec;+intmode;+};++staticvoidswitch_card_to_cxl(structwork_struct*work)+{+structcxl_switch_work*switch_work=+container_of(work,structcxl_switch_work,work);+structpci_dev*dev=switch_work->dev;+structpci_bus*bus=dev->bus;+structpci_controller*hose=pci_bus_to_host(bus);+structpci_dev*bridge;+structpnv_php_slot*php_slot;+unsignedintdevfn;u8val;intrc;-dev_info(&dev->dev,"switch card to CXL\n");+dev_info(&bus->dev,"cxl: Preparing for mode switch...\n");+bridge=list_first_entry_or_null(&hose->bus->devices,structpci_dev,+bus_list);+if(!bridge){+dev_WARN(&bus->dev,"cxl: Couldn't find root port!\n");+gotoerr_free_work;+}-if(!(vsec=find_cxl_vsec(dev))){-dev_err(&dev->dev,"ABORTING: CXL VSEC not found!\n");+php_slot=pnv_php_find_slot(pci_device_to_OF_node(bridge));+if(!php_slot){+dev_err(&bus->dev,"cxl: Failed to find slot hotplug "+"information. You may need to upgrade "+"skiboot. Aborting.\n");+pci_dev_put(dev);+gotoerr_free_work;+}++rc=CXL_READ_VSEC_MODE_CONTROL(dev,switch_work->vsec,&val);+if(rc){+dev_err(&bus->dev,"cxl: Failed to read CAPI mode control: %i\n",rc);+pci_dev_put(dev);+gotoerr_free_work;+}+devfn=dev->devfn;+pci_dev_put(dev);++dev_dbg(&bus->dev,"cxl: Removing PCI devices from kernel\n");+pci_lock_rescan_remove();+pci_hp_remove_devices(bridge->subordinate);+pci_unlock_rescan_remove();++/* Switch the CXL protocol on the card */+if(switch_work->mode==CXL_BIMODE_CXL){+dev_info(&bus->dev,"cxl: Switching card to CXL mode\n");+val&=~CXL_VSEC_PROTOCOL_MASK;+val|=CXL_VSEC_PROTOCOL_256TB|CXL_VSEC_PROTOCOL_ENABLE;+rc=pnv_cxl_enable_phb_kernel_api(hose,true);+if(rc){+dev_err(&bus->dev,"cxl: Failed to enable kernel API"+" on real PHB, aborting\n");+gotoerr_free_work;+}+}else{+dev_WARN(&bus->dev,"cxl: Switching card to PCI mode not supported!\n");+gotoerr_free_work;+}++rc=CXL_WRITE_VSEC_MODE_CONTROL_BUS(bus,devfn,switch_work->vsec,val);+if(rc){+dev_err(&bus->dev,"cxl: Failed to configure CXL protocol: %i\n",rc);+gotoerr_free_work;+}++/*+*TheCAIAspec(v1.1,Section10.6Bi-modalDeviceSupport)states+*wemustwait100msafterthismodeswitchbeforetouchingPCIeconfig+*space.+*/+msleep(100);++/*+*Hotresettocausethecardtocomebackincxlmode.A+*OPAL_RESET_PCI_LINKwouldbesufficient,butcurrentlylackssupport+*inskiboot,soweuseahotresetinstead.+*+*Wecallpci_set_pcie_reset_state()onthebridge,asaCAPIcardis+*guaranteedtositdirectlyundertherootport,andsettingthereset+*stateonadevicedirectlyundertherootportisequivalenttodoing+*itontherootportiself.+*/+dev_info(&bus->dev,"cxl: Configuration write complete, resetting card\n");+pci_set_pcie_reset_state(bridge,pcie_hot_reset);+pci_set_pcie_reset_state(bridge,pcie_deassert_reset);++dev_dbg(&bus->dev,"cxl: Offlining slot\n");+rc=pnv_php_set_slot_power_state(&php_slot->slot,OPAL_PCI_SLOT_OFFLINE);+if(rc){+dev_err(&bus->dev,"cxl: OPAL offlining call failed: %i\n",rc);+gotoerr_free_work;+}++dev_dbg(&bus->dev,"cxl: Onlining and probing slot\n");+rc=pnv_php_set_slot_power_state(&php_slot->slot,OPAL_PCI_SLOT_ONLINE);+if(rc){+dev_err(&bus->dev,"cxl: OPAL onlining call failed: %i\n",rc);+gotoerr_free_work;+}++pci_lock_rescan_remove();+pci_hp_add_devices(bridge->subordinate);+pci_unlock_rescan_remove();++dev_info(&bus->dev,"cxl: CAPI mode switch completed\n");+kfree(switch_work);+return;++err_free_work:+kfree(switch_work);+}++intcxl_check_and_switch_mode(structpci_dev*dev,intmode,intvsec)+{+structcxl_switch_work*work;+u8val;+intrc;++if(!cpu_has_feature(CPU_FTR_HVMODE))return-ENODEV;++if(!vsec){+vsec=find_cxl_vsec(dev);+if(!vsec){+dev_info(&dev->dev,"CXL VSEC not found\n");+return-ENODEV;+}}-if((rc=CXL_READ_VSEC_MODE_CONTROL(dev,vsec,&val))){-dev_err(&dev->dev,"failed to read current mode control: %i",rc);+rc=CXL_READ_VSEC_MODE_CONTROL(dev,vsec,&val);+if(rc){+dev_err(&dev->dev,"Failed to read current mode control: %i",rc);returnrc;}-val&=~CXL_VSEC_PROTOCOL_MASK;-val|=CXL_VSEC_PROTOCOL_256TB|CXL_VSEC_PROTOCOL_ENABLE;-if((rc=CXL_WRITE_VSEC_MODE_CONTROL(dev,vsec,val))){-dev_err(&dev->dev,"failed to enable CXL protocol: %i",rc);-returnrc;++if(mode==CXL_BIMODE_PCI){+if(!(val&CXL_VSEC_PROTOCOL_ENABLE)){+dev_info(&dev->dev,"Card is already in PCI mode\n");+return0;+}+/*+*TODO:Beforeit'ssafetoswitchthecardbacktoPCImode+*weneedtodisabletheCAPPandmakesureanycachelinesthe+*cardholdshavebeenflushedout.Needsskibootsupport.+*/+dev_WARN(&dev->dev,"CXL mode switch to PCI unsupported!\n");+return-EIO;}++if(val&CXL_VSEC_PROTOCOL_ENABLE){+dev_info(&dev->dev,"Card is already in CXL mode\n");+return0;+}++dev_info(&dev->dev,"Card is in PCI mode, scheduling kernel thread "+"to switch to CXL mode\n");++work=kmalloc(sizeof(structcxl_switch_work),GFP_KERNEL);+if(!work)+return-ENOMEM;++pci_dev_get(dev);+work->dev=dev;+work->vsec=vsec;+work->mode=mode;+INIT_WORK(&work->work,switch_card_to_cxl);++schedule_work(&work->work);+/*-*TheCAIAspec(v0.1211.6Bi-modalDeviceSupport)states-*wemustwait100msafterthismodeswitchbeforetouching-*PCIeconfigspace.+*Wereturnafailurenowtoabortthedriverinit.Oncethe+*linkhasbeencycledandthecardisincxlmodewewill+*comeback(possiblyusingthegenericcxldriver),but+*returnsuccessasthecardshouldthenbeincxlmode.+*+*TODO:WhatifthecardcomesbackinPCImodeevenafter+*theswitch?Don'twanttospinendlessly.*/-msleep(100);+return-EBUSY;+}+EXPORT_SYMBOL_GPL(cxl_check_and_switch_mode);++#endif /* CONFIG_CXL_BIMODAL */++staticintsetup_cxl_protocol_area(structpci_dev*dev)+{+u8val;+intrc;+intvsec=find_cxl_vsec(dev);++if(!vsec){+dev_info(&dev->dev,"CXL VSEC not found\n");+return-ENODEV;+}++rc=CXL_READ_VSEC_MODE_CONTROL(dev,vsec,&val);+if(rc){+dev_err(&dev->dev,"Failed to read current mode control: %i\n",rc);+returnrc;+}++if(!(val&CXL_VSEC_PROTOCOL_ENABLE)){+dev_err(&dev->dev,"Card not in CAPI mode!\n");+return-EIO;+}++/* Still configure the protocol area for single mode cards */+if((val&CXL_VSEC_PROTOCOL_MASK)!=CXL_VSEC_PROTOCOL_256TB){+val&=~CXL_VSEC_PROTOCOL_MASK;+val|=CXL_VSEC_PROTOCOL_256TB;+rc=CXL_WRITE_VSEC_MODE_CONTROL(dev,vsec,val);+if(rc){+dev_err(&dev->dev,"Failed to set CXL protocol area: %i\n",rc);+returnrc;+}+}return0;}
@@ -39,6 +39,31 @@boolcxl_slot_is_supported(structpci_dev*dev,intflags);+#define CXL_BIMODE_CXL 1+#define CXL_BIMODE_PCI 2++/*+*Checkthemodethatthegivenbi-modalCXLadapteriscurrentlyinand+*changeitifnecessary.ThisdoesnotapplytoAFUdrivers.+*+*Ifthemodematchestherequestedmodethisfunctionwillreturn0-ifthe+*driverwasexpectingthegenericCXLdrivertohaveboundtotheadapterand+*itgetsthisreturnvalueitshouldfailtheprobefunctiontogivetheCXL+*driverachancetoprobeit.+*+*Ifthemodedoesnotmatchitwillstartabackgroundtasktounplugthe+*devicefromLinuxandswitchitsmode,andwillreturn-EBUSY.Atthis+*pointthecallingdrivershouldmakesureithasreleasedthedeviceand+*failitsprobefunction.+*+*TheoffsetoftheCXLVSECcanbeprovidedtothisfunction.If0ispassed,+*thisfunctionwillsearchforaCXLVSECwithID0x1280andreturn-ENODEV+*ifitisnotfound.+*/+#ifdef CONFIG_CXL_BIMODAL+intcxl_check_and_switch_mode(structpci_dev*dev,intmode,intvsec);+#endif+/* Get the AFU associated with a pci_dev */structcxl_afu*cxl_pci_to_afu(structpci_dev*dev);
From: Ian Munsie <hidden> Date: 2016-07-04 13:23:17
From: Ian Munsie <redacted>
This extends the check that the adapter is in a CAPI capable slot so
that it may be called by external users in the kernel API. This will be
used by the upcoming Mellanox CX4 support, which needs to know ahead of
time if the card can be switched to cxl mode so that it can leave it in
PCI mode if it is not.
This API takes a parameter to check if CAPP DMA mode is supported, which
it currently only allows on P8NVL systems, since that mode currently has
issues accessing memory < 4GB on P8, and we cannot realistically avoid
that.
This API does not currently check if a CAPP unit is available (i.e. not
already assigned to another PHB) on P8. Doing so would be racy since it
is assigned on a first come first serve basis, and so long as CAPP DMA
mode is not supported on P8 we don't need this, since the only
anticipated user of this API requires CAPP DMA mode.
Cc: Philippe Bergheaud <redacted>
Signed-off-by: Ian Munsie <redacted>
---
drivers/misc/cxl/pci.c | 37 +++++++++++++++++++++++++++++++++++++
include/misc/cxl.h | 15 +++++++++++++++
2 files changed, 52 insertions(+)
@@ -24,6 +24,21 @@*genericPCIAPI.ThisAPIisagnostictotheactualAFU.*/+#define CXL_SLOT_FLAG_DMA 0x1++/*+*Checksifthegivencardisinacxlcapableslot.PassCXL_SLOT_FLAG_DMAif+*thecardrequiresCAPPDMAmodetoalsocheckifthesystemsupportsit.+*Thisisintendedtobeusedbybi-modaldevicestodetermineiftheycanuse+*cxlmodeoriftheyshouldcontinuerunninginPCImode.+*+*Notethatthisonlychecksiftheslotiscxlcapable-itdoesnot+*currentlycheckiftheCAPPiscurrentlyavailableforchipswhereitcanbe+*assignedtodifferentPHBsonafirstcomefirstservebasis(i.e.P8)+*/+boolcxl_slot_is_supported(structpci_dev*dev,intflags);++/* Get the AFU associated with a pci_dev */structcxl_afu*cxl_pci_to_afu(structpci_dev*dev);
From: Ian Munsie <hidden> Date: 2016-07-04 13:23:19
From: Ian Munsie <redacted>
The Mellanox CX4 uses a model where the AFU is one physical function of
the device, and is used by other peer physical functions of the same
device. This will require those other devices to grab a reference on the
AFU when they are initialised to make sure that it does not go away
during their lifetime.
Move the AFU refcount functions to base.c so they can be called from
the PHB code.
Signed-off-by: Ian Munsie <redacted>
---
drivers/misc/cxl/base.c | 13 +++++++++++++
drivers/misc/cxl/cxl.h | 12 ------------
include/misc/cxl-base.h | 4 ++++
3 files changed, 17 insertions(+), 12 deletions(-)
From: Ian Munsie <hidden> Date: 2016-07-04 13:23:30
From: Ian Munsie <redacted>
The Mellanox CX4 in cxl mode uses a hybrid interrupt model, where
interrupts are routed from the networking hardware to the XSL using the
MSIX table, and from there will be transformed back into an MSIX
interrupt using the cxl style interrupts (i.e. using IVTE entries and
ranges to map a PE and AFU interrupt number to an MSIX address).
We want to hide the implementation details of cxl interrupts as much as
possible. To this end, we use a special version of the MSI setup &
teardown routines in the PHB while in cxl mode to allocate the cxl
interrupts and configure the IVTE entries in the process element.
This function does not configure the MSIX table - the CX4 card uses a
custom format in that table and it would not be appropriate to fill that
out in generic code. The rest of the functionality is similar to the
"Full MSI-X mode" described in the CAIA, and this could be easily
extended to support other adapters that use that mode in the future.
The interrupts will be associated with the default context. If the
maximum number of interrupts per context has been limited (e.g. by the
mlx5 driver), it will automatically allocate additional kernel contexts
to associate extra interrupts as required. These contexts will be
started using the same WED that was used to start the default context.
Signed-off-by: Ian Munsie <redacted>
---
arch/powerpc/platforms/powernv/pci-cxl.c | 84 +++++++++++++++++++++++++++++++
arch/powerpc/platforms/powernv/pci-ioda.c | 4 ++
arch/powerpc/platforms/powernv/pci.h | 2 +
drivers/misc/cxl/api.c | 71 ++++++++++++++++++++++++++
drivers/misc/cxl/base.c | 31 ++++++++++++
drivers/misc/cxl/cxl.h | 4 ++
drivers/misc/cxl/main.c | 2 +
include/misc/cxl-base.h | 4 ++
8 files changed, 202 insertions(+)
@@ -248,6 +248,8 @@ extern void pnv_npu_release_ownership(struct pnv_ioda_pe *npe);/* cxl functions */externboolpnv_cxl_enable_device_hook(structpci_dev*dev);externvoidpnv_cxl_disable_device(structpci_dev*dev);+externintpnv_cxl_cx4_setup_msi_irqs(structpci_dev*pdev,intnvec,inttype);+externvoidpnv_cxl_cx4_teardown_msi_irqs(structpci_dev*pdev);/* phb ops (cxl switches these when enabling the kernel api on the phb) */
@@ -489,3 +490,73 @@ int cxl_get_max_irqs_per_process(struct pci_dev *dev)returnafu->irqs_max;}EXPORT_SYMBOL_GPL(cxl_get_max_irqs_per_process);++/*+*ThisisaspecialinterruptallocationroutinecalledfromthePHB'sMSI+*setupfunction.Whencapiinterruptsareallocatedinthismannertheymust+*stillbeassociatedwitharunningcontext,butsincetheMSIAPIshaveno+*waytospecifythisweusethedefaultcontextassociatedwiththedevice.+*+*TheMellanoxCX4hasahardwarelimitationthatrestrictsthemaximumAFU+*interruptnumber,soinordertoovercomethistheirdriverinformsusof+*therestrictionbysettingthemaximuminterruptspercontext,andwe+*allocateadditionalcontextsasnecessarysothatwecankeeptheAFU+*interruptnumberwithinthesupportedrange.+*/+int_cxl_cx4_setup_msi_irqs(structpci_dev*pdev,intnvec,inttype)+{+structcxl_context*ctx,*new_ctx,*default_ctx;+intremaining;+intrc;++ctx=default_ctx=cxl_get_context(pdev);+if(WARN_ON(!default_ctx))+return-ENODEV;++remaining=nvec;+while(remaining>0){+rc=cxl_allocate_afu_irqs(ctx,min(remaining,ctx->afu->irqs_max));+if(rc){+pr_warn("%s: Failed to find enough free MSIs\n",pci_name(pdev));+returnrc;+}+remaining-=ctx->afu->irqs_max;++if(ctx!=default_ctx&&default_ctx->status==STARTED){+WARN_ON(cxl_start_context(ctx,+be64_to_cpu(default_ctx->elem->common.wed),+NULL));+}++if(remaining>0){+new_ctx=cxl_dev_context_init(pdev);+if(!new_ctx){+pr_warn("%s: Failed to allocate enough contexts for MSIs\n",pci_name(pdev));+return-ENOSPC;+}+list_add(&new_ctx->extra_irq_contexts,&ctx->extra_irq_contexts);+ctx=new_ctx;+}+}++return0;+}+/* Exported via cxl_base */++void_cxl_cx4_teardown_msi_irqs(structpci_dev*pdev)+{+structcxl_context*ctx,*pos,*tmp;++ctx=cxl_get_context(pdev);+if(WARN_ON(!ctx))+return;++cxl_free_afu_irqs(ctx);+list_for_each_entry_safe(pos,tmp,&ctx->extra_irq_contexts,extra_irq_contexts){+cxl_stop_context(pos);+cxl_free_afu_irqs(pos);+list_del(&pos->extra_irq_contexts);+cxl_release_context(pos);+}+}+/* Exported via cxl_base */
On Mon, Jul 04, 2016 at 11:22:10PM +1000, Ian Munsie wrote:
From: Andrew Donnellan <redacted>
The cxl driver will use infrastructure from pnv_php to handle device tree
updates when switching bi-modal CAPI cards into CAPI mode.
To enable this, export pnv_php_find_slot() and
pnv_php_set_slot_power_state(), and add corresponding declarations, as well
as the definition of struct pnv_php_slot, to asm/pnv-pci.h.
Cc: Gavin Shan <redacted>
Cc: linux-pci@vger.kernel.org
Cc: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Andrew Donnellan <redacted>
Acked-by: Gavin Shan <redacted>
---
tristate "PowerPC PowerNV PCI Hotplug driver"
depends on PPC_POWERNV && EEH
select OF_DYNAMIC
+ select HOTPLUG_PCI_POWERNV_BASE
help
Say Y here if you run PowerPC PowerNV platform that supports
PCI Hotplug
Andrew/Ian, it seems HOTPLUG_PCI_POWERNV_BASE isn't defined and we needn't it.
Thanks,
Gavin
From: Andrew Donnellan <hidden> Date: 2016-07-05 01:08:35
On 05/07/16 10:03, Gavin Shan wrote:
Andrew/Ian, it seems HOTPLUG_PCI_POWERNV_BASE isn't defined and we needn't it.
Argh, thanks for picking that up! I removed that option and all its
occurrences in the code based on your earlier private feedback but
forgot to drop this.
Will fix in V2.
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
From: Andrew Donnellan <hidden> Date: 2016-07-05 02:10:40
On 04/07/16 23:22, Ian Munsie wrote:
From: Ian Munsie <redacted>
The Mellanox CX4 uses a model where the AFU is one physical function of
the device, and is used by other peer physical functions of the same
device. This will require those other devices to grab a reference on the
AFU when they are initialised to make sure that it does not go away
during their lifetime.
Move the AFU refcount functions to base.c so they can be called from
the PHB code.
Signed-off-by: Ian Munsie <redacted>
Reviewed-by: Andrew Donnellan <redacted>
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
From: Andrew Donnellan <hidden> Date: 2016-07-06 02:03:34
On 04/07/16 23:22, Ian Munsie wrote:
From: Ian Munsie <redacted>
This extends the check that the adapter is in a CAPI capable slot so
that it may be called by external users in the kernel API. This will be
used by the upcoming Mellanox CX4 support, which needs to know ahead of
time if the card can be switched to cxl mode so that it can leave it in
PCI mode if it is not.
This API takes a parameter to check if CAPP DMA mode is supported, which
it currently only allows on P8NVL systems, since that mode currently has
issues accessing memory < 4GB on P8, and we cannot realistically avoid
that.
This API does not currently check if a CAPP unit is available (i.e. not
already assigned to another PHB) on P8. Doing so would be racy since it
is assigned on a first come first serve basis, and so long as CAPP DMA
mode is not supported on P8 we don't need this, since the only
anticipated user of this API requires CAPP DMA mode.
Cc: Philippe Bergheaud <redacted>
Signed-off-by: Ian Munsie <redacted>
@@ -1426,6 +1426,43 @@ static int cxl_slot_is_switched(struct pci_dev *dev)return(depth>CXL_MAX_PCIEX_PARENT);}+boolcxl_slot_is_supported(structpci_dev*dev,intflags)+{+if(!cpu_has_feature(CPU_FTR_HVMODE))+returnfalse;++if((flags&CXL_SLOT_FLAG_DMA)&&(!pvr_version_is(PVR_POWER8NVL))){+/*+*CAPPDMAmodeistechnicallysupportedonregularP8,but+*willEEHifthecardattemptstoacccessmemory<4GB,which
access
+ * we cannot realistically avoid. We might be able to work
+ * around the issue, but until then return unsupported:
+ */
+ return false;
+ }
+
+ if (cxl_slot_is_switched(dev))
+ return false;
+
+ /*
+ * XXX: This gets a little tricky on regular P8 (not POWER8NVL) since
+ * the CAPP can be connected to PHB 0, 1 or 2 on a first come first
+ * served basis, which is racy to check from here. If we need to
+ * support this in future we might need to consider having this
+ * function effectively reserve it ahead of time.
+ *
+ * Currently, the only user of this API is the Mellanox CX4, which is
+ * only supported on P8NVL due to the above mentioned limitation of
+ * CAPP DMA mode and therefore does not need to worry about thi. If the
this
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
From: Andrew Donnellan <hidden> Date: 2016-07-06 03:44:56
On 04/07/16 23:21, Ian Munsie wrote:
From: Ian Munsie <redacted>
The support for using the Mellanox CX4 in cxl mode will require
additions to the PHB code. In preparation for this, move the existing
cxl code out of pci-ioda.c into a separate pci-cxl.c file to keep things
more organised.
Signed-off-by: Ian Munsie <redacted>
Reviewed-by: Andrew Donnellan <redacted>
quoted hunk
+++ b/arch/powerpc/platforms/powernv/pci-cxl.c
@@ -0,0 +1,163 @@+/*+*Copyright2015IBMCorp.
If you end up spinning a V2 of this, could probably put "2015, 2016" on
there.
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
From: Andrew Donnellan <hidden> Date: 2016-07-06 04:05:12
On 04/07/16 23:22, Ian Munsie wrote:
From: Ian Munsie <redacted>
Devices that use CAPP DMA mode (such as the Mellanox CX4) require bus
master to be enabled in order for the CAPI traffic to flow. This should
be harmless to enable for other cxl devices, so unconditionally enable
it in the adapter init flow.
Signed-off-by: Ian Munsie <redacted>
Reviewed-by: Andrew Donnellan <redacted>
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
From: Andrew Donnellan <hidden> Date: 2016-07-06 04:42:19
On 04/07/16 23:22, Ian Munsie wrote:
From: Ian Munsie <redacted>
The CX4 card cannot cope with a context with PE=0 due to a hardware
limitation, resulting in:
[ 34.166577] command failed, status limits exceeded(0x8), syndrome 0x5a7939
[ 34.166580] mlx5_core 0000:01:00.1: Failed allocating uar, aborting
Since the kernel API allocates a default context very early during
device init that will almost certainly get Process Element ID 0 there is
no easy way for us to extend the API to allow the Mellanox to inform us
of this limitation ahead of time.
Instead, work around the issue by extending the XSL structure to include
a minimum PE to allocate. Although the bug is not in the XSL, it is the
easiest place to work around this limitation given that the CX4 is
currently the only card that uses an XSL.
Signed-off-by: Ian Munsie <redacted>
Reviewed-by: Andrew Donnellan <redacted>
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
From: Ian Munsie <redacted>
The support for using the Mellanox CX4 in cxl mode will require
additions to the PHB code. In preparation for this, move the existing
cxl code out of pci-ioda.c into a separate pci-cxl.c file to keep things
more organised.
Signed-off-by: Ian Munsie <redacted>
From: Ian Munsie <redacted>
This extends the check that the adapter is in a CAPI capable slot so
that it may be called by external users in the kernel API. This will be
used by the upcoming Mellanox CX4 support, which needs to know ahead of
time if the card can be switched to cxl mode so that it can leave it in
PCI mode if it is not.
This API takes a parameter to check if CAPP DMA mode is supported, which
it currently only allows on P8NVL systems, since that mode currently has
issues accessing memory < 4GB on P8, and we cannot realistically avoid
that.
This API does not currently check if a CAPP unit is available (i.e. not
already assigned to another PHB) on P8. Doing so would be racy since it
is assigned on a first come first serve basis, and so long as CAPP DMA
mode is not supported on P8 we don't need this, since the only
anticipated user of this API requires CAPP DMA mode.
Is it me or that last sentence is more complicated than it should? :-)
Anyway, I get it.
And the rest looks ok.
Reviewed-by: Frederic Barrat <redacted>
From: Ian Munsie <redacted>
Devices that use CAPP DMA mode (such as the Mellanox CX4) require bus
master to be enabled in order for the CAPI traffic to flow. This should
be harmless to enable for other cxl devices, so unconditionally enable
it in the adapter init flow.
Signed-off-by: Ian Munsie <redacted>
From: Ian Munsie <redacted>
The Mellanox CX4 uses a model where the AFU is one physical function of
the device, and is used by other peer physical functions of the same
device. This will require those other devices to grab a reference on the
AFU when they are initialised to make sure that it does not go away
during their lifetime.
Move the AFU refcount functions to base.c so they can be called from
the PHB code.
Signed-off-by: Ian Munsie <redacted>
From: Ian Munsie <redacted>
The cxl kernel API has a concept of a default context associated with
each PCI device under the virtual PHB. The Mellanox CX4 will also use
the cxl kernel API, but it does not use a virtual PHB - rather, the AFU
appears as a physical function as a peer to the networking functions.
In order to allow the kernel API to work with those networking
functions, we will need to associate a default context with them as
well. To this end, refactor the corresponding code to do this in vphb.c
and export it so that it can be called from the PHB code.
Signed-off-by: Ian Munsie <redacted>
+ /* No special handling for cxl function: */
+ if (PCI_FUNC(dev->devfn) == 0)
+ return true;
I believe that is the first time we're getting a hint of the black magic
which is going to occur when the card is switched to cxl mode and the
appearance of a new pci function. I think a general comment explaining
it is needed somewhere. In this patch or a later one. Also "peer model"
is used several times in the commit messages, though it's not clear to
the novice what it really means.
At this point of the review, I was a bit overwhelmed by all the new
APIs, wondering how everything would end up working together. By the
last patch, it's understandable, but a few extra comments would help.
For the vPHB model, pretty much all the relevant code is in one file,
which helps grabbing the full picture. But here it's spread between the
phb platform code and the cxl driver.
Fred
From: Ian Munsie <redacted>
This hooks up support for using the kernel API with a real PHB. After
the AFU initialisation has completed it calls into the PHB code to pass
it the AFU that will be used by other peer physical functions on the
adapter.
The cxl_pci_to_afu API is extended to work with peer PCI devices,
retrieving the peer AFU from the PHB. This API may also now return an
error if it is called on a PCI device that is not associated with either
a cxl vPHB or a peer PCI device to an AFU, and this error is propagated
down.
Signed-off-by: Ian Munsie <redacted>
From: Ian Munsie <redacted>
These APIs will be used by the Mellanox CX4 support. While they function
standalone to configure existing behaviour, their primary purpose is to
allow the Mellanox driver to inform the cxl driver of a hardware
limitation, which will be used in a future patch.
Signed-off-by: Ian Munsie <redacted>
Any way to add a check that the "set max" API is called before the
interrupts are allocated?
Reviewed-by: Frederic Barrat <redacted>
@@ -1572,6 +1575,9 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev, */ for (i = 0; i < adapter->slices; i++) { afu = adapter->afu[i];+ /* Only participate in EEH if we are on a virtual PHB */+ if (afu->phb == NULL)+ return PCI_ERS_RESULT_NONE; cxl_vphb_error_detected(afu, state); }
Sorry, I had my notes out of order, something is bugging me here. Don't
we always define afu->phb, though for Mellanox (or if there's no config
record in the general case), we don't have any devices attached to it?
Which raises the question of the handling of slot_reset and resume
callbacks...
Fred
From: Ian Munsie <redacted>
The Mellanox CX4 has a hardware limitation where only 4 bits of the
AFU interrupt number can be passed to the XSL when sending an interrupt,
limiting it to only 15 interrupts per context (AFU interrupt number 0 is
invalid).
In order to overcome this, we will allocate additional contexts linked
to the default context as extra address space for the extra interrupts -
this will be implemented in the next patch.
This patch adds the preliminary support to allow this, by way of adding
a linked list in the context structure that we use to keep track of the
contexts dedicated to interrupts, and an API to simultaneously iterate
over the related context structures, AFU interrupt numbers and hardware
interrupt numbers. The point of using a single API to iterate these is
to hide some of the details of the iteration from external code, and to
reduce the number of APIs that need to be exported via base.c to allow
built in code to call.
Reviewed-by: Frederic Barrat <redacted>
Just one typo below
@@ -178,6 +178,15 @@ int cxl_set_max_irqs_per_process(struct pci_dev *dev, int irqs);intcxl_get_max_irqs_per_process(structpci_dev*dev);/*+*Usetosimultaneouslyiterateoverhardwareinterruptnumbers,contextsand+*afuinterruptnumbersallocatedforthedeviceviapci_enable_msix_rangeand+*isausefulconviniencefunctionwhenworkingwithhardwarethathas
From: Ian Munsie <redacted>
The Mellanox CX4 in cxl mode uses a hybrid interrupt model, where
interrupts are routed from the networking hardware to the XSL using the
MSIX table, and from there will be transformed back into an MSIX
interrupt using the cxl style interrupts (i.e. using IVTE entries and
ranges to map a PE and AFU interrupt number to an MSIX address).
We want to hide the implementation details of cxl interrupts as much as
possible. To this end, we use a special version of the MSI setup &
teardown routines in the PHB while in cxl mode to allocate the cxl
interrupts and configure the IVTE entries in the process element.
This function does not configure the MSIX table - the CX4 card uses a
custom format in that table and it would not be appropriate to fill that
out in generic code. The rest of the functionality is similar to the
"Full MSI-X mode" described in the CAIA, and this could be easily
extended to support other adapters that use that mode in the future.
The interrupts will be associated with the default context. If the
maximum number of interrupts per context has been limited (e.g. by the
mlx5 driver), it will automatically allocate additional kernel contexts
to associate extra interrupts as required. These contexts will be
started using the same WED that was used to start the default context.
Signed-off-by: Ian Munsie <redacted>
---
arch/powerpc/platforms/powernv/pci-cxl.c | 84 +++++++++++++++++++++++++++++++
arch/powerpc/platforms/powernv/pci-ioda.c | 4 ++
arch/powerpc/platforms/powernv/pci.h | 2 +
drivers/misc/cxl/api.c | 71 ++++++++++++++++++++++++++
drivers/misc/cxl/base.c | 31 ++++++++++++
drivers/misc/cxl/cxl.h | 4 ++
drivers/misc/cxl/main.c | 2 +
include/misc/cxl-base.h | 4 ++
8 files changed, 202 insertions(+)
From: Ian Munsie <redacted>
The CX4 card cannot cope with a context with PE=0 due to a hardware
limitation, resulting in:
[ 34.166577] command failed, status limits exceeded(0x8), syndrome 0x5a7939
[ 34.166580] mlx5_core 0000:01:00.1: Failed allocating uar, aborting
Since the kernel API allocates a default context very early during
device init that will almost certainly get Process Element ID 0 there is
no easy way for us to extend the API to allow the Mellanox to inform us
of this limitation ahead of time.
Instead, work around the issue by extending the XSL structure to include
a minimum PE to allocate. Although the bug is not in the XSL, it is the
easiest place to work around this limitation given that the CX4 is
currently the only card that uses an XSL.
Signed-off-by: Ian Munsie <redacted>
From: Andrew Donnellan <redacted>
Add a new API, cxl_check_and_switch_mode() to allow for switching of
bi-modal CAPI cards, such as the Mellanox CX-4 network card.
When a driver requests to switch a card to CAPI mode, use PCI hotplug
infrastructure to remove all PCI devices underneath the slot. We then write
an updated mode control register to the CAPI VSEC, hot reset the card, and
reprobe the card.
As the card may present a different set of PCI devices after the mode
switch, use the infrastructure provided by the pnv_php driver and the OPAL
PCI slot management facilities to ensure that:
* the old devices are removed from both the OPAL and Linux device trees
* the new devices are probed by OPAL and added to the OPAL device tree
* the new devices are added to the Linux device tree and probed through
the regular PCI device probe path
As such, introduce a new option, CONFIG_CXL_BIMODAL, with a dependency on
the pnv_php driver.
Refactor existing code that touches the mode control register in the
regular single mode case into a new function, setup_cxl_protocol_area().
Co-authored-by: Ian Munsie [off-list ref]
Cc: Gavin Shan <redacted>
Signed-off-by: Andrew Donnellan <redacted>
Reviewed-by: Gavin Shan <redacted>
---
drivers/misc/cxl/Kconfig | 8 ++
drivers/misc/cxl/pci.c | 234 +++++++++++++++++++++++++++++++++++++++++++----
include/misc/cxl.h | 25 +++++
3 files changed, 249 insertions(+), 18 deletions(-)
@@ -614,36 +616,232 @@ static int setup_cxl_bars(struct pci_dev *dev)return0;}-/* pciex node: ibm,opal-m64-window = <0x3d058 0x0 0x3d058 0x0 0x8 0x0>; */-staticintswitch_card_to_cxl(structpci_dev*dev)-{+#ifdef CONFIG_CXL_BIMODAL++structcxl_switch_work{+structpci_dev*dev;+structwork_structwork;intvsec;+intmode;+};++staticvoidswitch_card_to_cxl(structwork_struct*work)+{+structcxl_switch_work*switch_work=+container_of(work,structcxl_switch_work,work);+structpci_dev*dev=switch_work->dev;+structpci_bus*bus=dev->bus;+structpci_controller*hose=pci_bus_to_host(bus);+structpci_dev*bridge;+structpnv_php_slot*php_slot;+unsignedintdevfn;u8val;intrc;-dev_info(&dev->dev,"switch card to CXL\n");+dev_info(&bus->dev,"cxl: Preparing for mode switch...\n");+bridge=list_first_entry_or_null(&hose->bus->devices,structpci_dev,+bus_list);+if(!bridge){+dev_WARN(&bus->dev,"cxl: Couldn't find root port!\n");+gotoerr_free_work;+}-if(!(vsec=find_cxl_vsec(dev))){-dev_err(&dev->dev,"ABORTING: CXL VSEC not found!\n");+php_slot=pnv_php_find_slot(pci_device_to_OF_node(bridge));+if(!php_slot){+dev_err(&bus->dev,"cxl: Failed to find slot hotplug "+"information. You may need to upgrade "+"skiboot. Aborting.\n");+pci_dev_put(dev);+gotoerr_free_work;+}++rc=CXL_READ_VSEC_MODE_CONTROL(dev,switch_work->vsec,&val);+if(rc){+dev_err(&bus->dev,"cxl: Failed to read CAPI mode control: %i\n",rc);+pci_dev_put(dev);+gotoerr_free_work;+}+devfn=dev->devfn;+pci_dev_put(dev);
This is to balance the 'get' done in cxl_check_and_switch_mode(), right?
A comment wouldn't hurt. I think we're missing the 'put' on the first
error path above (!bridge).
I was half-expecting to see a new entry in the cxl_pci_tbl pci ID table
for the Mellanox entry, but no such thing. By what magic is cxl_probe()
called after the switch? Because of the device class?
Out of curiosity, could you tell me what the 3rd pci function looks like
(vendor ID, device ID, ....)?
Thanks!
Fred
This is to balance the 'get' done in cxl_check_and_switch_mode(), right?
A comment wouldn't hurt. I think we're missing the 'put' on the first
error path above (!bridge).
Yep, it's to balance the pci_dev_get() in cxl_check_and_switch_mode() -
you're right, a comment to that effect wouldn't hurt.
You're also right about the error path. Will fix in V2.
I was half-expecting to see a new entry in the cxl_pci_tbl pci ID table
for the Mellanox entry, but no such thing. By what magic is cxl_probe()
called after the switch? Because of the device class?
It matches against the class, as function 0 of the device after reset
comes up as a class 1200 processing accelerator.
Perhaps we should be a bit more explicit though...
Out of curiosity, could you tell me what the 3rd pci function looks like
(vendor ID, device ID, ....)?
Before:
root@io163:~# lspci -vnn
0000:00:00.0 PCI bridge [0604]: IBM Device [1014:03dc] (prog-if 00
[Normal decode])
Flags: fast devsel
Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
I/O behind bridge: 00000000-00000fff
Capabilities: [40] Power Management version 3
Capabilities: [48] Express Root Port (Slot-), MSI 00
Capabilities: [100] Advanced Error Reporting
Capabilities: [148] #19
0000:01:00.0 Infiniband controller [0207]: Mellanox Technologies MT27700
Family [ConnectX-4] [15b3:1013]
Subsystem: IBM Device [1014:04f4]
Flags: fast devsel, IRQ 502
Memory at 200000000000 (64-bit, prefetchable) [disabled] [size=32M]
Capabilities: [60] Express Endpoint, MSI 00
Capabilities: [48] Vital Product Data
Capabilities: [9c] MSI-X: Enable- Count=128 Masked-
Capabilities: [c0] Vendor Specific Information: Len=18 <?>
Capabilities: [40] Power Management version 3
Capabilities: [100] Device Serial Number ba-da-ce-55-de-ad-ca-fe
Capabilities: [160] Vendor Specific Information: ID=1280 Rev=0
Len=080 <?>
Capabilities: [240] #19
0000:01:00.1 Infiniband controller [0207]: Mellanox Technologies MT27700
Family [ConnectX-4] [15b3:1013]
Subsystem: IBM Device [1014:04f4]
Flags: fast devsel, IRQ 502
Memory at 200002000000 (64-bit, prefetchable) [disabled] [size=32M]
Capabilities: [60] Express Endpoint, MSI 00
Capabilities: [48] Vital Product Data
Capabilities: [9c] MSI-X: Enable- Count=128 Masked-
Capabilities: [40] Power Management version 3
Capabilities: [100] Device Serial Number ba-da-ce-55-de-ad-ca-fe
After:
root@io163:~# lspci -vnn
0000:00:00.0 PCI bridge [0604]: IBM Device [1014:03dc] (prog-if 00
[Normal decode])
Flags: bus master, fast devsel, latency 0
Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
I/O behind bridge: 00000000-00000fff
Capabilities: [40] Power Management version 3
Capabilities: [48] Express Root Port (Slot-), MSI 00
Capabilities: [100] Advanced Error Reporting
Capabilities: [148] #19
0000:01:00.0 Processing accelerators [1200]: Mellanox Technologies
MT27700 Family [ConnectX-4] [15b3:1013]
Subsystem: IBM Device [1014:04f4]
Physical Slot: Slot3
Flags: bus master, fast devsel, latency 0, IRQ 502
Memory at 200004000000 (64-bit, prefetchable) [size=128K]
Memory at 200004020000 (64-bit, prefetchable) [size=128K]
Memory at <ignored> (64-bit, prefetchable) [size=256T]
Capabilities: [60] Express Endpoint, MSI 00
Capabilities: [48] Vital Product Data
Capabilities: [9c] MSI-X: Enable- Count=128 Masked-
Capabilities: [100] Device Serial Number ba-da-ce-55-de-ad-ca-fe
Capabilities: [160] Vendor Specific Information: ID=1280 Rev=0
Len=080 <?>
Kernel driver in use: cxl-pci
0000:01:00.1 Infiniband controller [0207]: Mellanox Technologies MT27700
Family [ConnectX-4] [15b3:1013]
Subsystem: IBM Device [1014:04f4]
Physical Slot: Slot3
Flags: bus master, fast devsel, latency 0, IRQ 502
Memory at 200000000000 (64-bit, prefetchable) [size=32M]
Capabilities: [60] Express Endpoint, MSI 00
Capabilities: [48] Vital Product Data
Capabilities: [9c] MSI-X: Enable+ Count=128 Masked-
Capabilities: [40] Power Management version 3
Capabilities: [100] Device Serial Number ba-da-ce-55-de-ad-ca-fe
Kernel driver in use: mlx5_core
0000:01:00.2 Infiniband controller [0207]: Mellanox Technologies MT27700
Family [ConnectX-4] [15b3:1013]
Subsystem: IBM Device [1014:04f4]
Physical Slot: Slot3
Flags: bus master, fast devsel, latency 0, IRQ 502
Memory at 200002000000 (64-bit, prefetchable) [size=32M]
Capabilities: [60] Express Endpoint, MSI 00
Capabilities: [48] Vital Product Data
Capabilities: [9c] MSI-X: Enable+ Count=128 Masked-
Capabilities: [40] Power Management version 3
Capabilities: [100] Device Serial Number ba-da-ce-55-de-ad-ca-fe
Kernel driver in use: mlx5_core
Andrew
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
From: Ian Munsie <hidden> Date: 2016-07-07 06:01:29
Excerpts from Frederic Barrat's message of 2016-07-06 20:11:48 +0200:
Le 04/07/2016 15:22, Ian Munsie a écrit :
quoted
From: Ian Munsie <redacted>
These APIs will be used by the Mellanox CX4 support. While they function
standalone to configure existing behaviour, their primary purpose is to
allow the Mellanox driver to inform the cxl driver of a hardware
limitation, which will be used in a future patch.
Signed-off-by: Ian Munsie <redacted>
Any way to add a check that the "set max" API is called before the
interrupts are allocated?
I don't think there is any real need - if the set max API has not been
called then we use the maximum number of interrupts possible on the PHB,
which is the correct thing to do if we don't need the workaround. We
could try adding a WARN in the set max API if interrupts have previously
been allocated, but realistically - if a driver developer needs to use
this they already know it and will be testing for it.
Cheers,
-Ian
From: Ian Munsie <hidden> Date: 2016-07-07 06:27:28
Excerpts from andrew.donnellan's message of 2016-07-07 11:18:37 +1000:
quoted
This is to balance the 'get' done in cxl_check_and_switch_mode(), right?
A comment wouldn't hurt. I think we're missing the 'put' on the first
error path above (!bridge).
Yep, it's to balance the pci_dev_get() in cxl_check_and_switch_mode() -
you're right, a comment to that effect wouldn't hurt.
You're also right about the error path. Will fix in V2.
We could probably use a dedicated error label for all the error paths
before the pci_dev_put in the main function so we don't need it in every
error path.
quoted
I was half-expecting to see a new entry in the cxl_pci_tbl pci ID table
for the Mellanox entry, but no such thing. By what magic is cxl_probe()
called after the switch? Because of the device class?
It matches against the class, as function 0 of the device after reset
comes up as a class 1200 processing accelerator.
Perhaps we should be a bit more explicit though...
If we explicitly match the Vendor + Device ID we will also match the
networking functions, which we can't do, because before the mode switch
there *IS* a CAPI VSEC in one of the networking functions and our driver
would mistake it as a generic accelerator and try to initialise it. We
could add a comment to this effect to the PCI ID table.
Cheers,
-Ian
From: Ian Munsie <hidden> Date: 2016-07-07 06:28:42
Excerpts from Frederic Barrat's message of 2016-07-06 19:38:18 +0200:
quoted
+ /* No special handling for cxl function: */
+ if (PCI_FUNC(dev->devfn) == 0)
+ return true;
I believe that is the first time we're getting a hint of the black magic
which is going to occur when the card is switched to cxl mode and the
appearance of a new pci function. I think a general comment explaining
it is needed somewhere. In this patch or a later one. Also "peer model"
is used several times in the commit messages, though it's not clear to
the novice what it really means.
At this point of the review, I was a bit overwhelmed by all the new
APIs, wondering how everything would end up working together. By the
last patch, it's understandable, but a few extra comments would help.
For the vPHB model, pretty much all the relevant code is in one file,
which helps grabbing the full picture. But here it's spread between the
phb platform code and the cxl driver.
Fred
From: Ian Munsie <hidden> Date: 2016-07-07 06:33:32
Excerpts from Frederic Barrat's message of 2016-07-06 20:30:41 +0200:
quoted
@@ -1572,6 +1575,9 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev, */ for (i = 0; i < adapter->slices; i++) { afu = adapter->afu[i];+ /* Only participate in EEH if we are on a virtual PHB */+ if (afu->phb == NULL)+ return PCI_ERS_RESULT_NONE; cxl_vphb_error_detected(afu, state); }
Sorry, I had my notes out of order, something is bugging me here. Don't
we always define afu->phb, though for Mellanox (or if there's no config
record in the general case), we don't have any devices attached to it?
I think you're right. I'll change the vPHB code to skip it if there are
no configuration records.
Which raises the question of the handling of slot_reset and resume
callbacks...
We aren't going to support EEH (at least not yet) - the vPHB model makes
this (relatively) easy since we can notify the AFU drivers when we get
notified, but in the peer model it will be the real PHB notifying us and
the networking drivers. If we do end up supporting that, it will come
later.
Cheers,
-Ian
From: Andrew Donnellan <hidden> Date: 2016-07-07 06:44:58
On 07/07/16 16:26, Ian Munsie wrote:
We could probably use a dedicated error label for all the error paths
before the pci_dev_put in the main function so we don't need it in every
error path.
Yep, I've added that.
If we explicitly match the Vendor + Device ID we will also match the
networking functions, which we can't do, because before the mode switch
there *IS* a CAPI VSEC in one of the networking functions and our driver
would mistake it as a generic accelerator and try to initialise it. We
could add a comment to this effect to the PCI ID table.
We can match the vendor, device ID *and* class code - unfortunately
there isn't a macro for this, which makes it a little bit less
aesthetically pleasing, but I'm pretty sure this works.
I'm not entirely sure how I feel about our current strategy of matching
on all class 1200 devices (though if it weren't a CAPI device we'd bail
very quickly...) - my quick grepping tells me we're one of a very small
set of drivers in the kernel that uses PCI_DEVICE_CLASS.
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
From: Andrew Donnellan <hidden> Date: 2016-07-07 08:15:14
On 07/07/16 16:44, Andrew Donnellan wrote:
We can match the vendor, device ID *and* class code - unfortunately
there isn't a macro for this, which makes it a little bit less
aesthetically pleasing, but I'm pretty sure this works.
Something like the below, which works fine:
/*
* Matches a given PCI vendor ID and device ID, but only for class 12
* (processing accelerators). Useful for bi-modal cards, such as the
* Mellanox ConnectX-4, which keep the same vendor/device ID
* post-mode-switch.
*/
#define PCI_DEVICE_ACCEL(vend, dev) \
.vendor = (vend), .device = (dev), \
.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID, \
.class = 0x120000, .class_mask = 0xff0000
static const struct pci_device_id cxl_pci_tbl[] = {
/* FPGA devices */
{ PCI_DEVICE(PCI_VENDOR_ID_IBM, 0x0477), },
{ PCI_DEVICE(PCI_VENDOR_ID_IBM, 0x044b), },
{ PCI_DEVICE(PCI_VENDOR_ID_IBM, 0x04cf), },
{ PCI_DEVICE(PCI_VENDOR_ID_IBM, 0x0601), },
/* Mellanox ConnectX-4 */
{ PCI_DEVICE_ACCEL(PCI_VENDOR_ID_MELLANOX, 0x1013), },
{ }
};
MODULE_DEVICE_TABLE(pci, cxl_pci_tbl);
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
From: Ian Munsie <hidden> Date: 2016-07-11 09:19:31
Excerpts from andrew.donnellan's message of 2016-07-07 18:15:06 +1000:
On 07/07/16 16:44, Andrew Donnellan wrote:
quoted
We can match the vendor, device ID *and* class code - unfortunately
there isn't a macro for this, which makes it a little bit less
aesthetically pleasing, but I'm pretty sure this works.
Something like the below, which works fine:
I like this solution, but I'm not going to include it in v2 of this
series and would rather it be submitted separately. The reason being is
that this series will work as is, and I'd like to see this undergo some
regression testing separate to the cx4 work, and a bit of scrutiny from
the hardware team just in case we are missing any device IDs that would
no longer be matched(I'm not aware of any, but you never know).
Cheers,
-Ian
From: Andrew Donnellan <hidden> Date: 2016-07-12 01:20:52
On 11/07/16 19:19, Ian Munsie wrote:
I like this solution, but I'm not going to include it in v2 of this
series and would rather it be submitted separately. The reason being is
that this series will work as is, and I'd like to see this undergo some
regression testing separate to the cx4 work, and a bit of scrutiny from
the hardware team just in case we are missing any device IDs that would
no longer be matched(I'm not aware of any, but you never know).
Yep, I can send it separately.
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited