Re: [PATCH 3/3] cxl: Support creating HDM-DB regions

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

On Mon, 11 Aug 2025 18:02:28 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:

> A single Type 3 device can expose different parts of its memory with different
> coherency semantics. For example, some memory ranges within a Type 3 device
> could be configured as HDM-H, while other memory ranges on the same device
> could be configured as HDM-DB. This allows for flexible memory configuration
> within a single device. As such, coherency models are defined per memory region.
> 
> For accelerators (type2), it is expected for accelerator drivers to manage the
> HDM-D[B] region creation. For type3, relevant sysfs tunables are provided to
> the user.
> 
> Other than the HDM decoder supported coherency models, the main dependency
> to create these regions is for the device state to be BI-ready (cxlds->bi),
> for which already committed HDM decoders with the BI bit set detected during
> enumeration is currently not supported because endpoint and port enumerations
> are independent.
> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
>  Documentation/ABI/testing/sysfs-bus-cxl | 41 ++++++++++++++----
>  drivers/cxl/acpi.c                      |  2 +
>  drivers/cxl/core/core.h                 |  4 ++
>  drivers/cxl/core/hdm.c                  | 56 +++++++++++++++++++++---
>  drivers/cxl/core/port.c                 | 55 +++++++++++++++++++++++-
>  drivers/cxl/core/region.c               | 57 +++++++++++++++++++++----
>  drivers/cxl/cxl.h                       |  7 ++-
>  drivers/cxl/cxlmem.h                    |  1 +
>  include/acpi/actbl1.h                   |  1 +
>  9 files changed, 199 insertions(+), 25 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 6b4e8c7a963d..7d9e3db736c3 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 6361365c5ce9..0a9532fa4ec1 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -51,6 +51,7 @@ extern const struct nvdimm_security_ops *cxl_security_ops;
>  #define   CXL_HDM_DECODER_INTERLEAVE_14_12 BIT(9)
>  #define   CXL_HDM_DECODER_INTERLEAVE_3_6_12_WAY BIT(11)
>  #define   CXL_HDM_DECODER_INTERLEAVE_16_WAY BIT(12)
> +#define   CXL_HDM_DECODER_SUPPORTED_COHERENCY_MASK GENMASK(22, 21)

Given the values are a bit non obvious (particularly what 0 means :),
I'd add defines for them alongside the mask.

You use the value 1 later which should have been a define.


>  #define CXL_HDM_DECODER_CTRL_OFFSET 0x4
>  #define   CXL_HDM_DECODER_ENABLE BIT(1)
>  #define CXL_HDM_DECODER0_BASE_LOW_OFFSET(i) (0x20 * (i) + 0x10)
> @@ -65,6 +66,7 @@ extern const struct nvdimm_security_ops *cxl_security_ops;
>  #define   CXL_HDM_DECODER0_CTRL_COMMITTED BIT(10)
>  #define   CXL_HDM_DECODER0_CTRL_COMMIT_ERROR BIT(11)
>  #define   CXL_HDM_DECODER0_CTRL_HOSTONLY BIT(12)