RE: [PATCH v11 12/22] cxl: Add callback to parse the DSMAS subtables from CDAT

[Dan Williams <dan.j.williams@intel.com>]

Dave Jiang wrote:
> Provide a callback function to the CDAT parser in order to parse the Device
> Scoped Memory Affinity Structure (DSMAS). Each DSMAS structure contains the
> DPA range and its associated attributes in each entry. See the CDAT
> specification for details. The device handle and the DPA range is saved and
> to be associated with the DSLBIS locality data when the DSLBIS entries are
> parsed. The list is a local list. When the total path performance data is
> calculated and storred this list can be discarded.
> 
> Coherent Device Attribute Table 1.03 2.1 Device Scoped memory Affinity
> Structure (DSMAS)
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/Kconfig       |    1 +
>  drivers/cxl/core/Makefile |    1 +
>  drivers/cxl/core/cdat.c   |   58 +++++++++++++++++++++++++++++++++++++++++++++
>  drivers/cxl/cxl.h         |   23 ++++++++++++++++++
>  drivers/cxl/port.c        |   12 +++++++++
>  5 files changed, 95 insertions(+)
>  create mode 100644 drivers/cxl/core/cdat.c
> 
> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> index 8ea1d340e438..a1ae35bd1afc 100644
> --- a/drivers/cxl/Kconfig
> +++ b/drivers/cxl/Kconfig
> @@ -5,6 +5,7 @@ menuconfig CXL_BUS
>  	select FW_LOADER
>  	select FW_UPLOAD
>  	select PCI_DOE
> +	select FIRMWARE_TABLE
>  	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/core/Makefile b/drivers/cxl/core/Makefile
> index 1f66b5d4d935..f4eabfdd50ce 100644
> --- a/drivers/cxl/core/Makefile
> +++ b/drivers/cxl/core/Makefile
> @@ -13,5 +13,6 @@ cxl_core-y += mbox.o
>  cxl_core-y += pci.o
>  cxl_core-y += hdm.o
>  cxl_core-y += pmu.o
> +cxl_core-$(CONFIG_FIRMWARE_TABLE) += cdat.o

CXL_BUS selects FIRMWARE_TABLE, this can just be cxl_core-y.

...and whenever drivers/cxl/core/Makefile changes,
tools/testing/cxl/Kbuild needs updating too.

>  cxl_core-$(CONFIG_TRACING) += trace.o
>  cxl_core-$(CONFIG_CXL_REGION) += region.o
> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
> new file mode 100644
> index 000000000000..a858fd1b5744
> --- /dev/null
> +++ b/drivers/cxl/core/cdat.c
> @@ -0,0 +1,58 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright(c) 2023 Intel Corporation. All rights reserved. */
> +#include <linux/acpi.h>
> +#include <linux/fw_table.h>
> +#include "cxlpci.h"
> +#include "cxl.h"
> +
> +static int cdat_dsmas_handler(union acpi_subtable_headers *header, void *arg,
> +			      const unsigned long end)
> +{
> +	struct acpi_cdat_header *hdr = &header->cdat;
> +	struct acpi_cdat_dsmas *dsmas;
> +	int size = sizeof(*hdr) + sizeof(*dsmas);
> +	struct list_head *dsmas_list = arg;
> +	struct dsmas_entry *dent;
> +	u16 len;
> +
> +	len = le16_to_cpu((__force __le16)hdr->length);
> +	if (len != size || (unsigned long)hdr + len > end) {
> +		pr_warn("Malformed DSMAS table length: (%u:%u)\n", size, len);
> +		return -EINVAL;
> +	}
> +
> +	/* Skip common header */
> +	dsmas = (struct acpi_cdat_dsmas *)(hdr + 1);
> +
> +	dent = kzalloc(sizeof(*dent), GFP_KERNEL);
> +	if (!dent)
> +		return -ENOMEM;
> +
> +	dent->handle = dsmas->dsmad_handle;
> +	dent->dpa_range.start = le64_to_cpu((__force __le64)dsmas->dpa_base_address);
> +	dent->dpa_range.end = le64_to_cpu((__force __le64)dsmas->dpa_base_address) +
> +			      le64_to_cpu((__force __le64)dsmas->dpa_length) - 1;
> +	list_add_tail(&dent->list, dsmas_list);
> +
> +	return 0;
> +}
> +
> +int cxl_cdat_endpoint_process(struct cxl_port *port, struct list_head *list)
> +{
> +	return cdat_table_parse(ACPI_CDAT_TYPE_DSMAS, cdat_dsmas_handler,
> +				list, port->cdat.table);
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_cdat_endpoint_process, CXL);
> +
> +void cxl_cdat_dsmas_list_destroy(struct list_head *dsmas_list)
> +{
> +	struct dsmas_entry *dentry, *n;
> +
> +	list_for_each_entry_safe(dentry, n, dsmas_list, list) {
> +		list_del(&dentry->list);
> +		kfree(dentry);
> +	}
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_cdat_dsmas_list_destroy, CXL);
> +
> +MODULE_IMPORT_NS(CXL);
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index eb7924648cb0..be2fd8dcb37b 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -7,6 +7,7 @@
>  #include <linux/libnvdimm.h>
>  #include <linux/bitfield.h>
>  #include <linux/bitops.h>
> +#include <linux/list.h>
>  #include <linux/log2.h>
>  #include <linux/io.h>
>  
> @@ -824,6 +825,28 @@ static inline struct cxl_dax_region *to_cxl_dax_region(struct device *dev)
>  }
>  #endif
>  
> +/* CDAT related bits */
> +struct dsmas_entry {
> +	struct list_head list;
> +	struct range dpa_range;
> +	u8 handle;
> +};
> +
> +#ifdef CONFIG_FIRMWARE_TABLE
> +int cxl_cdat_endpoint_process(struct cxl_port *port, struct list_head *list);
> +void cxl_cdat_dsmas_list_destroy(struct list_head *dsmas_list);
> +#else
> +static inline int cxl_cdat_endpoint_process(struct cxl_port *port,
> +					    struct list_head *list)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static inline void cxl_cdat_dsmas_list_destroy(struct list_head *dsmas_list)
> +{
> +}
> +#endif

Why is there conditional compilation if FIRMWARE_TABLE is mandatory?

Folded in the following:

diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
index f4eabfdd50ce..9259bcc6773c 100644
--- a/drivers/cxl/core/Makefile
+++ b/drivers/cxl/core/Makefile
@@ -13,6 +13,6 @@ cxl_core-y += mbox.o
 cxl_core-y += pci.o
 cxl_core-y += hdm.o
 cxl_core-y += pmu.o
-cxl_core-$(CONFIG_FIRMWARE_TABLE) += cdat.o
+cxl_core-y += cdat.o
 cxl_core-$(CONFIG_TRACING) += trace.o
 cxl_core-$(CONFIG_CXL_REGION) += region.o
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index be2fd8dcb37b..48fd7e0c5e9b 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -832,20 +832,8 @@ struct dsmas_entry {
 	u8 handle;
 };
 
-#ifdef CONFIG_FIRMWARE_TABLE
 int cxl_cdat_endpoint_process(struct cxl_port *port, struct list_head *list);
 void cxl_cdat_dsmas_list_destroy(struct list_head *dsmas_list);
-#else
-static inline int cxl_cdat_endpoint_process(struct cxl_port *port,
-					    struct list_head *list)
-{
-	return -EOPNOTSUPP;
-}
-
-static inline void cxl_cdat_dsmas_list_destroy(struct list_head *dsmas_list)
-{
-}
-#endif
 
 /*
  * Unit test builds overrides this to __weak, find the 'strong' version
diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
index 90f3c9802ffb..e3a59b0c5564 100644
--- a/tools/testing/cxl/Kbuild
+++ b/tools/testing/cxl/Kbuild
@@ -58,6 +58,7 @@ cxl_core-y += $(CXL_CORE_SRC)/mbox.o
 cxl_core-y += $(CXL_CORE_SRC)/pci.o
 cxl_core-y += $(CXL_CORE_SRC)/hdm.o
 cxl_core-y += $(CXL_CORE_SRC)/pmu.o
+cxl_core-y += $(CXL_CORE_SRC)/cdat.o
 cxl_core-$(CONFIG_TRACING) += $(CXL_CORE_SRC)/trace.o
 cxl_core-$(CONFIG_CXL_REGION) += $(CXL_CORE_SRC)/region.o
 cxl_core-y += config_check.o