(reviewed late in the evening, apologies if I've missed something)
On 11/07/16 21:50, Ian Munsie wrote:
From: Ian Munsie <imun...@au1.ibm.com>
This adds support for the peer model of the cxl kernel api to the
PowerNV PHB, in which physical function 0 represents the cxl function on
the card (an XSL in the case of the CX4), which other physical functions
will use for memory access and interrupt services. It is referred to as
the peer model as these functions are peers of one another, as opposed
to the Virtual PHB model which forms a hierarchy.
This patch exports APIs to enable the peer 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 <imun...@au1.ibm.com>
Some comments below - with those addressed:
Reviewed-by: Andrew Donnellan <andrew.donnel...@au1.ibm.com>
---
V1->V2:
- Add an explanation of the peer model to the commit message,
and a comment above the pnv_cxl_enable_device_hook function.
The comments are good!
+/*
+ * Sets flags and switches the controller ops to enable the cxl kernel api.
+ * Original the cxl kernel API operated on a virtual PHB, but certain cards
Originally
+ * such as the Mellanox CX4 use a peer model instead and for these cards the
+ * cxl kernel api will operate on the real PHB.
+ */
+int pnv_cxl_enable_phb_kernel_api(struct pci_controller *hose, bool enable)
+{
+ struct pnv_phb *phb = hose->private_data;
+ struct module *cxl_module;
+
+ if (!enable) {
+ /*
+ * Once cxl mode is enabled on the PHB, there is currently no
+ * known safe method to disable it again, and trying risks a
+ * checkstop. If we can find a way to safely disable cxl mode
+ * in the future we can revisit this, but for now the only sane
+ * thing to do is to refuse to disable cxl mode:
+ */
+ return -EPERM;
+ }
+
+ /*
+ * Hold a reference to the cxl module since several PHB operations now
+ * depend on it, and it would be insane to allow it to be removed so
+ * long as we are in this mode (and since we can't safely disable this
+ * mode once enabled...).
+ */
+ 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;
+
+ return 0;
+}
+EXPORT_SYMBOL(pnv_cxl_enable_phb_kernel_api);
+
+bool pnv_pci_on_cxl_phb(struct pci_dev *dev)
+{
+ struct pci_controller *hose = pci_bus_to_host(dev->bus);
+ struct pnv_phb *phb = hose->private_data;
+
+ return !!(phb->flags & PNV_PHB_FLAG_CXL);
+}
+EXPORT_SYMBOL(pnv_pci_on_cxl_phb);
Should these two exports be _GPL as well?
-static void pnv_pci_release_device(struct pci_dev *pdev)
+void pnv_pci_release_device(struct pci_dev *pdev)
Why is this being unstatic-ified? I don't see us introducing any new
uses of it.
{
struct pci_controller *hose = pci_bus_to_host(pdev->bus);
struct pnv_phb *phb = hose->private_data;
@@ -3423,7 +3423,7 @@ static void pnv_pci_ioda_shutdown(struct pci_controller
*hose)
OPAL_ASSERT_RESET);
}
-static const struct pci_controller_ops pnv_pci_ioda_controller_ops = {
+const struct pci_controller_ops pnv_pci_ioda_controller_ops = {
It looks like we don't currently use this - is this being unstatic-ised
in view of allowing a switch back to regular mode in future?
extern struct pci_ops pnv_pci_ops;
@@ -218,6 +222,8 @@ extern int pnv_setup_msi_irqs(struct pci_dev *pdev, int
nvec, int type);
extern void pnv_teardown_msi_irqs(struct pci_dev *pdev);
extern struct pnv_ioda_pe *pnv_ioda_get_pe(struct pci_dev *dev);
extern void pnv_set_msi_irq_chip(struct pnv_phb *phb, unsigned int virq);
+extern bool pnv_pci_enable_device_hook(struct pci_dev *dev);
+extern void pnv_pci_release_device(struct pci_dev *pdev);
extern void pe_level_printk(const struct pnv_ioda_pe *pe, const char *level,
const char *fmt, ...);
@@ -238,4 +244,14 @@ extern long pnv_npu_unset_window(struct pnv_ioda_pe *npe,
int num);
extern void pnv_npu_take_ownership(struct pnv_ioda_pe *npe);
extern void pnv_npu_release_ownership(struct pnv_ioda_pe *npe);
+
+/* cxl functions */
+extern bool pnv_cxl_enable_device_hook(struct pci_dev *dev);
+extern void pnv_cxl_disable_device(struct pci_dev *dev);
+
+
+/* phb ops (cxl switches these when enabling the kernel api on the phb) */
+extern const struct pci_controller_ops pnv_cxl_cx4_ioda_controller_ops;
+extern const struct pci_controller_ops pnv_pci_ioda_controller_ops;
See applicable comments about static.
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnel...@au1.ibm.com IBM Australia Limited
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
