Re: [PATCH 03/17] cxl/core: Add function for getting CXL cache info

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Tue, 11 Nov 2025 15:40:18 -0600
Ben Cheatham <Benjamin.Cheatham@amd.com> wrote:

> Add a function for getting common CXL.cache information. This
> information will be stored in the struct cxl_cache_state member
> (cstate) of struct cxl_dev_state for easy access by endpoint drivers.
> 
> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
Hi Ben,

I'm only having a first read through so probably only superficial feedback
at this point.

We have so many major series floating around I suspect this will be a cycle
or two anyway :(

Anyhow, comments inline.

Thanks,

Jonathan

> ---
>  drivers/cxl/core/pci.c | 53 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/cxl/cxl.h      | 13 +++++++++++
>  drivers/cxl/cxlcache.h |  7 ++++++
>  drivers/cxl/cxlpci.h   |  4 ++++
>  4 files changed, 77 insertions(+)
>  create mode 100644 drivers/cxl/cxlcache.h
> 
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 18825e1505d6..5b1cace8fc0f 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -7,6 +7,7 @@
>  #include <linux/pci.h>
>  #include <linux/pci-doe.h>
>  #include <linux/aer.h>
> +#include <cxlcache.h>
>  #include <cxlpci.h>
>  #include <cxlmem.h>
>  #include <cxl.h>
> @@ -1258,3 +1259,55 @@ int cxl_port_get_possible_dports(struct cxl_port *port)
>  
>  	return ctx.count;
>  }
> +EXPORT_SYMBOL_NS_GPL(cxl_port_get_possible_dports, "CXL");

Stray change?

> +
> +/**
> + * cxl_accel_read_cache_info - Get the CXL cache information of a CXL cache device
> + * @cxlds: CXL device state associated with cache device
> + *
> + * Returns 0 and populates the struct cxl_cache_state member of @cxlds on
> + * success, error otherwise.
> + */
> +int cxl_accel_read_cache_info(struct cxl_dev_state *cxlds)
> +{
> +	struct cxl_cache_state *cstate = &cxlds->cstate;
> +	struct pci_dev *pdev;
> +	int dvsec, rc;
> +	u16 cap, cap2;
> +
> +	if (!dev_is_pci(cxlds->dev))
> +		return -EINVAL;
> +	pdev = to_pci_dev(cxlds->dev);
> +
> +	dvsec = cxlds->cxl_dvsec;
> +
> +	rc = pci_read_config_word(pdev, dvsec + CXL_DVSEC_CAP_OFFSET, &cap);
> +	if (rc)
> +		return rc;
> +
> +	if (!(cap & CXL_DVSEC_CACHE_CAPABLE))
> +		return -ENXIO;
> +
> +	rc = pci_read_config_word(pdev, dvsec + CXL_DVSEC_CAP2_OFFSET, &cap2);
> +	if (rc)
> +		return rc;
> +
> +	/* CXL 3.2 8.1.3.7 DVSEC CXL Capability2 for encoding */

Given old versions of CXL spec tend to become hard to get, probably
shift to using 4.0 references for this series (this probably raced
with that being published!)

> +	switch (FIELD_GET(CXL_DVSEC_CACHE_UNIT_MASK, cap2)) {
> +	case 1:
> +		cstate->unit = 64 * SZ_1K;

SZ_64K

I'll comment on it in the sysfs abi patch but I'm not seeing unit
as necessarily something we need to keep around once we have
used it to interpret the size.  So probably local variable is fine.

> +		break;
> +	case 2:
> +		cstate->unit = SZ_1M;
> +		break;
> +	default:

I wonder if there are devices out there that simply don't tell us
(so case 0)?

> +		return -ENXIO;
> +	}
> +
> +	cstate->size = FIELD_GET(CXL_DVSEC_CACHE_SIZE_MASK, cap2) * cstate->unit;
> +	if (!cstate->size)
> +		return -ENXIO;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_accel_read_cache_info, "CXL");
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 1950cf3d5399..259d806fb3e3 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -759,6 +759,17 @@ struct cxl_dpa_info {
>  	int nr_partitions;
>  };
>  
> +
> +/**
> + * struct cxl_cache_state - State of a device's CXL cache
> + * @size: Size of cache in bytes
> + * @unit: Unit size of cache in bytes
> + */
> +struct cxl_cache_state {
> +	u64 size;
> +	u32 unit;
> +};