| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 13 Jul 2022 20:30:18 -0700
From: Dan Williams <dan.j.williams@...el.com>
To: <ira.weiny@...el.com>, Dan Williams <dan.j.williams@...el.com>,
"Bjorn Helgaas" <bhelgaas@...gle.com>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>
CC: Ira Weiny <ira.weiny@...el.com>,
Davidlohr Bueso <dave@...olabs.net>,
Lukas Wunner <lukas@...ner.de>,
Alison Schofield <alison.schofield@...el.com>,
Vishal Verma <vishal.l.verma@...el.com>,
"Dave Jiang" <dave.jiang@...el.com>,
Ben Widawsky <bwidawsk@...nel.org>,
<linux-kernel@...r.kernel.org>, <linux-cxl@...r.kernel.org>,
<linux-pci@...r.kernel.org>
Subject: RE: [PATCH V13 4/9] cxl/pci: Create PCI DOE mailbox's for memory
devices
ira.weiny@ wrote:
> From: Ira Weiny <ira.weiny@...el.com>
>
> DOE mailbox objects will be needed for various mailbox communications
> with each memory device.
>
> Iterate each DOE mailbox capability and create PCI DOE mailbox objects
> as found.
>
> It is not anticipated that this is the final resting place for the
> iteration of the DOE devices. The support of switch ports will drive
> this code into the PCIe side. In this imagined architecture the CXL
> port driver would then query into the PCI device for the DOE mailbox
> array.
>
> For now creating the mailboxes in the CXL port is good enough for the
> endpoints. Later PCIe ports will need to support this to support switch
> ports more generically.
>
> Cc: Dan Williams <dan.j.williams@...el.com>
> Cc: Davidlohr Bueso <dave@...olabs.net>
> Cc: Lukas Wunner <lukas@...ner.de>
> Signed-off-by: Ira Weiny <ira.weiny@...el.com>
>
> ---
> Changes from V12:
> remove irq param from CXL
> Jonathan:
> remove xa local variable
> clarify MB creation as best effort
> But ensure pci_err() if they fail
> Check devm_add_action() return for failure
> Davidlohr and Jonathan:
> Return error ...
>
> Changes from V11:
> Drop review from: Ben Widawsky <bwidawsk@...nel.org>
> Remove irq code for now
> Adjust for pci_doe_get_int_msg_num()
> Adjust for pcim_doe_create_mb()
> (No longer need to handle the destroy.)
> Use xarray for DOE mailbox array
>
> Changes from V9:
> Bug fix: ensure DOE mailboxes are iterated before memdev add
> Ben Widawsky
> Set use_irq to false and just return on error.
> Don't return a value from devm_cxl_pci_create_doe()
> Skip allocating doe_mb array if there are no mailboxes
> Skip requesting irqs if none found.
> Ben/Jonathan Cameron
> s/num_irqs/max_irqs
>
> Changes from V8:
> Move PCI_DOE selection to CXL_BUS to support future patches
> which move queries into the port code.
> Remove Auxiliary device arch
> Squash the functionality of the auxiliary driver into this
> patch.
> Split out the irq handling a bit.
>
> Changes from V7:
> Minor code clean ups
> Rebased on cxl-pending
>
> Changes from V6:
> Move all the auxiliary device stuff to the CXL layer
>
> Changes from V5:
> Split the CXL specific stuff off from the PCI DOE create
> auxiliary device code.
> ---
> drivers/cxl/Kconfig | 1 +
> drivers/cxl/cxlmem.h | 3 +++
> drivers/cxl/pci.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 48 insertions(+)
>
> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> index f64e3984689f..7adaaf80b302 100644
> --- a/drivers/cxl/Kconfig
> +++ b/drivers/cxl/Kconfig
> @@ -2,6 +2,7 @@
> menuconfig CXL_BUS
> tristate "CXL (Compute Express Link) Devices Support"
> depends on PCI
> + select PCI_DOE
> help
> CXL is a bus that is electrically compatible with PCI Express, but
> layers three protocols on that signalling (CXL.io, CXL.cache, and
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 60d10ee1e7fc..360f282ef80c 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -191,6 +191,7 @@ struct cxl_endpoint_dvsec_info {
> * @component_reg_phys: register base of component registers
> * @info: Cached DVSEC information about the device.
> * @serial: PCIe Device Serial Number
> + * @doe_mbs: PCI DOE mailbox array
> * @mbox_send: @dev specific transport for transmitting mailbox commands
> *
> * See section 8.2.9.5.2 Capacity Configuration and Label Storage for
> @@ -224,6 +225,8 @@ struct cxl_dev_state {
> resource_size_t component_reg_phys;
> u64 serial;
>
> + struct xarray doe_mbs;
> +
> int (*mbox_send)(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd);
> };
>
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 5a0ae46d4989..6228c95fd142 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -8,6 +8,7 @@
> #include <linux/mutex.h>
> #include <linux/list.h>
> #include <linux/pci.h>
> +#include <linux/pci-doe.h>
> #include <linux/io.h>
> #include "cxlmem.h"
> #include "cxlpci.h"
> @@ -386,6 +387,43 @@ static int cxl_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type,
> return rc;
> }
>
> +static void cxl_pci_destroy_doe(void *mbs)
> +{
> + xa_destroy(mbs);
> +}
> +
> +static void devm_cxl_pci_create_doe(struct cxl_dev_state *cxlds)
> +{
> + struct device *dev = cxlds->dev;
> + struct pci_dev *pdev = to_pci_dev(dev);
> + u16 off = 0;
> +
> + /*
> + * Mailbox creation is best effort. Higher layers must determine if
> + * the lack of a mailbox for their protocol is a device failure or not.
> + */
> + pci_doe_for_each_off(pdev, off) {
> + struct pci_doe_mb *doe_mb;
> +
> + doe_mb = pcim_doe_create_mb(pdev, off);
> + if (IS_ERR(doe_mb)) {
> + pci_err(pdev,
> + "Failed to create MB object for MB @ %x\n",
> + off);
oh, the rest of cxl_pci driver is using dev_err() and dev_dbg(), not a
big deal, can be a follow-on cleanup to make it consistent.
> + continue;
> + }
> +
> + if (xa_insert(&cxlds->doe_mbs, off, doe_mb, GFP_KERNEL)) {
xarray all the things! Nice.
> + pci_err(pdev,
> + "xa_insert failed to insert MB @ %x\n",
> + off);
> + continue;
Lets make these error messages more actionable:
"Failed to setup DOE, CDAT data may not be available"
> + }
> +
> + pci_dbg(pdev, "Created DOE mailbox @%x\n", off);
> + }
> +}
> +
> static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> {
> struct cxl_register_map map;
> @@ -408,6 +446,10 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> if (IS_ERR(cxlds))
> return PTR_ERR(cxlds);
>
> + xa_init(&cxlds->doe_mbs);
> + if (devm_add_action(&pdev->dev, cxl_pci_destroy_doe, &cxlds->doe_mbs))
> + return -ENOMEM;
> +
This belongs inside devm_cxl_pci_create_doe().
> cxlds->serial = pci_get_dsn(pdev);
> cxlds->cxl_dvsec = pci_find_dvsec_capability(
> pdev, PCI_DVSEC_VENDOR_ID_CXL, CXL_DVSEC_PCIE_DEVICE);
> @@ -434,6 +476,8 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>
> cxlds->component_reg_phys = cxl_regmap_to_base(pdev, &map);
>
> + devm_cxl_pci_create_doe(cxlds);
> +
> rc = cxl_pci_setup_mailbox(cxlds);
> if (rc)
> return rc;
> --
> 2.35.3
>
Powered by blists - more mailing lists