[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z61tsoz3_MGrjvjG@aschofie-mobl2.lan>
Date: Wed, 12 Feb 2025 19:57:38 -0800
From: Alison Schofield <alison.schofield@...el.com>
To: alucerop@....com
Cc: linux-cxl@...r.kernel.org, netdev@...r.kernel.org,
dan.j.williams@...el.com, edward.cree@....com, davem@...emloft.net,
kuba@...nel.org, pabeni@...hat.com, edumazet@...gle.com,
dave.jiang@...el.com
Subject: Re: [PATCH v10 01/26] cxl: make memdev creation type agnostic
On Wed, Feb 05, 2025 at 03:19:25PM +0000, alucerop@....com wrote:
> From: Alejandro Lucero <alucerop@....com>
>
> In preparation for Type2 support, change memdev creation making
> type based on argument.
>
> Integrate initialization of dvsec and serial fields in the related
> cxl_dev_state within same function creating the memdev.
>
> Move the code from mbox file to memdev file.
>
> Add new header files with type2 required definitions for memdev
> state creation.
>
> Signed-off-by: Alejandro Lucero <alucerop@....com>
> ---
> drivers/cxl/core/mbox.c | 20 --------------------
> drivers/cxl/core/memdev.c | 23 +++++++++++++++++++++++
> drivers/cxl/cxlmem.h | 18 +++---------------
> drivers/cxl/cxlpci.h | 17 +----------------
> drivers/cxl/pci.c | 16 +++++++++-------
> include/cxl/cxl.h | 26 ++++++++++++++++++++++++++
> include/cxl/pci.h | 23 +++++++++++++++++++++++
> 7 files changed, 85 insertions(+), 58 deletions(-)
> create mode 100644 include/cxl/cxl.h
> create mode 100644 include/cxl/pci.h
>
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 4d22bb731177..96155b8af535 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -1435,26 +1435,6 @@ int cxl_mailbox_init(struct cxl_mailbox *cxl_mbox, struct device *host)
> }
> EXPORT_SYMBOL_NS_GPL(cxl_mailbox_init, "CXL");
>
> -struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev)
> -{
> - struct cxl_memdev_state *mds;
> -
> - mds = devm_kzalloc(dev, sizeof(*mds), GFP_KERNEL);
> - if (!mds) {
> - dev_err(dev, "No memory available\n");
> - return ERR_PTR(-ENOMEM);
> - }
> -
> - mutex_init(&mds->event.log_lock);
> - mds->cxlds.dev = dev;
> - mds->cxlds.reg_map.host = dev;
> - mds->cxlds.reg_map.resource = CXL_RESOURCE_NONE;
> - mds->cxlds.type = CXL_DEVTYPE_CLASSMEM;
> -
> - return mds;
> -}
> -EXPORT_SYMBOL_NS_GPL(cxl_memdev_state_create, "CXL");
> -
> void __init cxl_mbox_init(void)
> {
> struct dentry *mbox_debugfs;
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 63c6c681125d..456d505f1bc8 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -632,6 +632,29 @@ static void detach_memdev(struct work_struct *work)
>
> static struct lock_class_key cxl_memdev_key;
>
> +struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev, u64 serial,
> + u16 dvsec, enum cxl_devtype type)
> +{
> + struct cxl_memdev_state *mds;
> +
> + mds = devm_kzalloc(dev, sizeof(*mds), GFP_KERNEL);
> + if (!mds) {
> + dev_err(dev, "No memory available\n");
> + return ERR_PTR(-ENOMEM);
> + }
I know you are only the 'mover' of the above code, but can
you drop the dev_err message. OOM messages from the core are
typically enough.
> +
> + mutex_init(&mds->event.log_lock);
> + mds->cxlds.dev = dev;
> + mds->cxlds.reg_map.host = dev;
> + mds->cxlds.reg_map.resource = CXL_RESOURCE_NONE;
> + mds->cxlds.cxl_dvsec = dvsec;
> + mds->cxlds.serial = serial;
> + mds->cxlds.type = type;
> +
> + return mds;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_memdev_state_create, "CXL");
> +
> static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
> const struct file_operations *fops)
> {
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 536cbe521d16..62a459078ec3 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -7,6 +7,7 @@
> #include <linux/cdev.h>
> #include <linux/uuid.h>
> #include <linux/node.h>
> +#include <cxl/cxl.h>
> #include <cxl/event.h>
> #include <cxl/mailbox.h>
> #include "cxl.h"
> @@ -393,20 +394,6 @@ struct cxl_security_state {
> struct kernfs_node *sanitize_node;
> };
>
> -/*
> - * enum cxl_devtype - delineate type-2 from a generic type-3 device
> - * @CXL_DEVTYPE_DEVMEM - Vendor specific CXL Type-2 device implementing HDM-D or
> - * HDM-DB, no requirement that this device implements a
> - * mailbox, or other memory-device-standard manageability
> - * flows.
> - * @CXL_DEVTYPE_CLASSMEM - Common class definition of a CXL Type-3 device with
> - * HDM-H and class-mandatory memory device registers
> - */
> -enum cxl_devtype {
> - CXL_DEVTYPE_DEVMEM,
> - CXL_DEVTYPE_CLASSMEM,
> -};
> -
> /**
> * struct cxl_dpa_perf - DPA performance property entry
> * @dpa_range: range for DPA address
> @@ -856,7 +843,8 @@ int cxl_dev_state_identify(struct cxl_memdev_state *mds);
> int cxl_await_media_ready(struct cxl_dev_state *cxlds);
> int cxl_enumerate_cmds(struct cxl_memdev_state *mds);
> int cxl_mem_dpa_fetch(struct cxl_memdev_state *mds, struct cxl_dpa_info *info);
> -struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev);
> +struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev, u64 serial,
> + u16 dvsec, enum cxl_devtype type);
> void set_exclusive_cxl_commands(struct cxl_memdev_state *mds,
> unsigned long *cmds);
> void clear_exclusive_cxl_commands(struct cxl_memdev_state *mds,
> diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
> index 54e219b0049e..9fcf5387e388 100644
> --- a/drivers/cxl/cxlpci.h
> +++ b/drivers/cxl/cxlpci.h
> @@ -3,6 +3,7 @@
> #ifndef __CXL_PCI_H__
> #define __CXL_PCI_H__
> #include <linux/pci.h>
> +#include <cxl/pci.h>
> #include "cxl.h"
>
> #define CXL_MEMORY_PROGIF 0x10
> @@ -14,22 +15,6 @@
> */
> #define PCI_DVSEC_HEADER1_LENGTH_MASK GENMASK(31, 20)
>
> -/* CXL 2.0 8.1.3: PCIe DVSEC for CXL Device */
> -#define CXL_DVSEC_PCIE_DEVICE 0
> -#define CXL_DVSEC_CAP_OFFSET 0xA
> -#define CXL_DVSEC_MEM_CAPABLE BIT(2)
> -#define CXL_DVSEC_HDM_COUNT_MASK GENMASK(5, 4)
> -#define CXL_DVSEC_CTRL_OFFSET 0xC
> -#define CXL_DVSEC_MEM_ENABLE BIT(2)
> -#define CXL_DVSEC_RANGE_SIZE_HIGH(i) (0x18 + (i * 0x10))
> -#define CXL_DVSEC_RANGE_SIZE_LOW(i) (0x1C + (i * 0x10))
> -#define CXL_DVSEC_MEM_INFO_VALID BIT(0)
> -#define CXL_DVSEC_MEM_ACTIVE BIT(1)
> -#define CXL_DVSEC_MEM_SIZE_LOW_MASK GENMASK(31, 28)
> -#define CXL_DVSEC_RANGE_BASE_HIGH(i) (0x20 + (i * 0x10))
> -#define CXL_DVSEC_RANGE_BASE_LOW(i) (0x24 + (i * 0x10))
> -#define CXL_DVSEC_MEM_BASE_LOW_MASK GENMASK(31, 28)
> -
> #define CXL_DVSEC_RANGE_MAX 2
>
> /* CXL 2.0 8.1.4: Non-CXL Function Map DVSEC */
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index b2c943a4de0a..bd69dc07f387 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -911,6 +911,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> int rc, pmu_count;
> unsigned int i;
> bool irq_avail;
> + u16 dvsec;
>
> /*
> * Double check the anonymous union trickery in struct cxl_regs
> @@ -924,19 +925,20 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> return rc;
> pci_set_master(pdev);
>
> - mds = cxl_memdev_state_create(&pdev->dev);
> + dvsec = pci_find_dvsec_capability(pdev, PCI_VENDOR_ID_CXL,
> + CXL_DVSEC_PCIE_DEVICE);
> + if (!dvsec)
> + dev_warn(&pdev->dev,
> + "Device DVSEC not present, skip CXL.mem init\n");
> +
> + mds = cxl_memdev_state_create(&pdev->dev, pci_get_dsn(pdev), dvsec,
> + CXL_DEVTYPE_CLASSMEM);
> if (IS_ERR(mds))
> return PTR_ERR(mds);
> cxlds = &mds->cxlds;
> pci_set_drvdata(pdev, cxlds);
>
> cxlds->rcd = is_cxl_restricted(pdev);
> - cxlds->serial = pci_get_dsn(pdev);
> - cxlds->cxl_dvsec = pci_find_dvsec_capability(
> - pdev, PCI_VENDOR_ID_CXL, CXL_DVSEC_PCIE_DEVICE);
> - if (!cxlds->cxl_dvsec)
> - dev_warn(&pdev->dev,
> - "Device DVSEC not present, skip CXL.mem init\n");
>
> rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map);
> if (rc)
> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
> new file mode 100644
> index 000000000000..722782b868ac
> --- /dev/null
> +++ b/include/cxl/cxl.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright(c) 2025 Advanced Micro Devices, Inc. */
> +
> +#ifndef __CXL_H
> +#define __CXL_H
> +
> +#include <linux/types.h>
> +/*
> + * enum cxl_devtype - delineate type-2 from a generic type-3 device
> + * @CXL_DEVTYPE_DEVMEM - Vendor specific CXL Type-2 device implementing HDM-D or
> + * HDM-DB, no requirement that this device implements a
> + * mailbox, or other memory-device-standard manageability
> + * flows.
> + * @CXL_DEVTYPE_CLASSMEM - Common class definition of a CXL Type-3 device with
> + * HDM-H and class-mandatory memory device registers
> + */
> +enum cxl_devtype {
> + CXL_DEVTYPE_DEVMEM,
> + CXL_DEVTYPE_CLASSMEM,
> +};
> +
> +struct device;
> +struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev, u64 serial,
> + u16 dvsec, enum cxl_devtype type);
> +
> +#endif
> diff --git a/include/cxl/pci.h b/include/cxl/pci.h
> new file mode 100644
> index 000000000000..ad63560caa2c
> --- /dev/null
> +++ b/include/cxl/pci.h
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/* Copyright(c) 2020 Intel Corporation. All rights reserved. */
> +
> +#ifndef __CXL_ACCEL_PCI_H
> +#define __CXL_ACCEL_PCI_H
> +
> +/* CXL 2.0 8.1.3: PCIe DVSEC for CXL Device */
> +#define CXL_DVSEC_PCIE_DEVICE 0
> +#define CXL_DVSEC_CAP_OFFSET 0xA
> +#define CXL_DVSEC_MEM_CAPABLE BIT(2)
> +#define CXL_DVSEC_HDM_COUNT_MASK GENMASK(5, 4)
> +#define CXL_DVSEC_CTRL_OFFSET 0xC
> +#define CXL_DVSEC_MEM_ENABLE BIT(2)
> +#define CXL_DVSEC_RANGE_SIZE_HIGH(i) (0x18 + ((i) * 0x10))
> +#define CXL_DVSEC_RANGE_SIZE_LOW(i) (0x1C + ((i) * 0x10))
> +#define CXL_DVSEC_MEM_INFO_VALID BIT(0)
> +#define CXL_DVSEC_MEM_ACTIVE BIT(1)
> +#define CXL_DVSEC_MEM_SIZE_LOW_MASK GENMASK(31, 28)
> +#define CXL_DVSEC_RANGE_BASE_HIGH(i) (0x20 + ((i) * 0x10))
> +#define CXL_DVSEC_RANGE_BASE_LOW(i) (0x24 + ((i) * 0x10))
> +#define CXL_DVSEC_MEM_BASE_LOW_MASK GENMASK(31, 28)
> +
> +#endif
> --
> 2.17.1
>
>
Powered by blists - more mailing lists