Re: [PATCH 01/20] cxl: Introduce cxl_get_hdm_reg_info()

[Dave Jiang]

On 3/11/26 1:34 PM, mhonap@xxxxxxxxxx wrote:
> From: Manish Honap <mhonap@xxxxxxxxxx>
> 
> CXL core has the information of what CXL register groups a device has.
> When initializing the device, the CXL core probes the register groups
> and saves the information. The probing sequence is quite complicated.
> 
> vfio-cxl requires the HDM register information to emulate the HDM decoder

/register/register block/

Otherwise it implies a single register.

> registers.
> 
> Introduce cxl_get_hdm_reg_info() for vfio-cxl to leverage the HDM
> register information in the CXL core. Thus, it doesn't need to implement

same here. "register block"

> its own probing sequence.
> 
> Co-developed-by: Zhi Wang <zhiw@xxxxxxxxxx>
> Signed-off-by: Zhi Wang <zhiw@xxxxxxxxxx>
> Signed-off-by: Manish Honap <mhonap@xxxxxxxxxx>
> ---
>  drivers/cxl/core/pci.c | 45 ++++++++++++++++++++++++++++++++++++++++++
>  include/cxl/cxl.h      |  3 +++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index ba2d393c540a..52ed0b4f5e78 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -449,6 +449,51 @@ int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm,
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_hdm_decode_init, "CXL");
>  
> +/**
> + * cxl_get_hdm_reg_info - Get HDM decoder register block location and count

cxl_get_hdm_info() to be consistent with CXL core naming convention.

> + * @cxlds: CXL device state (must have component regs enumerated)
> + * @count: Output: number of HDM decoders (from DVSEC cap). Only set when
> + *         the device has a valid HDM decoder capability.
> + * @offset: Output: byte offset of the HDM decoder register block within the
> + *          component register BAR. Only set when valid.
> + * @size: Output: size in bytes of the HDM decoder register block. Only set
> + *        when valid.
> + *
> + * Reads the CXL component register map and DVSEC capability to return the
> + * Host Managed Device Memory (HDM) decoder register block offset and size,
> + * and the number of HDM decoders. This function requires cxlds->cxl_dvsec
> + * to be non-zero.
> + *
> + * Return: 0 on success. A negative errno is returned when config read
> + * failure or when the decoder registers are not valid.
> + */
> +int cxl_get_hdm_reg_info(struct cxl_dev_state *cxlds, u32 *count,

u8 for count probably sufficient as Jonathan mentioned. Decoder cap is only 4 bits for count.

> +			 resource_size_t *offset, resource_size_t *size)
> +{
> +	struct pci_dev *pdev = to_pci_dev(cxlds->dev);
> +	struct cxl_component_reg_map *map =
> +		&cxlds->reg_map.component_map;
> +	int d = cxlds->cxl_dvsec;

Name it dvsec for easier correlation when reading the code.

> +	u16 cap;
> +	int rc;
> +
> +	/* HDM decoder registers not implemented */
> +	if (!map->hdm_decoder.valid || !d)
> +		return -ENODEV;
> +
> +	rc = pci_read_config_word(pdev,
> +				  d + PCI_DVSEC_CXL_CAP, &cap);
> +	if (rc)
> +		return rc;
> +
> +	*count = FIELD_GET(PCI_DVSEC_CXL_HDM_COUNT, cap);

This is not the correct place to find number of decoders. You should be looking at CXL HDM Decoder Capability Register (CXL r4.0 8.2.4.20.1) for the 'Decoder Count' field.

DJ

> +	*offset = map->hdm_decoder.offset;
> +	*size = map->hdm_decoder.size;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_get_hdm_reg_info, "CXL");
> +
>  #define CXL_DOE_TABLE_ACCESS_REQ_CODE		0x000000ff
>  #define   CXL_DOE_TABLE_ACCESS_REQ_CODE_READ	0
>  #define CXL_DOE_TABLE_ACCESS_TABLE_TYPE		0x0000ff00
> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
> index 50acbd13bcf8..8456177b523e 100644
> --- a/include/cxl/cxl.h
> +++ b/include/cxl/cxl.h
> @@ -284,4 +284,7 @@ int cxl_dpa_free(struct cxl_endpoint_decoder *cxled);
>  struct cxl_region *cxl_create_region(struct cxl_root_decoder *cxlrd,
>  				     struct cxl_endpoint_decoder **cxled,
>  				     int ways);
> +
> +int cxl_get_hdm_reg_info(struct cxl_dev_state *cxlds, u32 *count,
> +			 resource_size_t *offset, resource_size_t *size);
>  #endif /* __CXL_CXL_H__ */