Re: [PATCH v12 07/23] cxl: support dpa initialization without a mailbox

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

On Mon, 31 Mar 2025 15:45:39 +0100
alejandro.lucero-palau@amd.com wrote:

> From: Alejandro Lucero <alucerop@amd.com>
> 
> Type3 relies on mailbox CXL_MBOX_OP_IDENTIFY command for initializing
> memdev state params which end up being used for dma initialization.
> 
> Allow a Type2 driver to initialize dpa simply by giving the size of its
> volatile and/or non-volatile hardware partitions.
> 
> Export cxl_dpa_setup as well for initializing those added dpa partitions
> with the proper resources.
> 
> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
> ---
>  drivers/cxl/core/mbox.c | 17 ++++++++++++++---
>  drivers/cxl/cxlmem.h    | 13 -------------
>  include/cxl/cxl.h       | 14 ++++++++++++++
>  3 files changed, 28 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index ab994d459f46..e4610e778723 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -1284,6 +1284,18 @@ static void add_part(struct cxl_dpa_info *info, u64 start, u64 size, enum cxl_pa
>  	info->nr_partitions++;
>  }
>  
> +void cxl_mem_dpa_init(struct cxl_dpa_info *info, u64 volatile_bytes,
> +		      u64 persistent_bytes)
> +{
> +	if (!info->size)
> +		info->size = volatile_bytes + persistent_bytes;
> +
> +	add_part(info, 0, volatile_bytes, CXL_PARTMODE_RAM);
> +	add_part(info, volatile_bytes, persistent_bytes,
> +		 CXL_PARTMODE_PMEM);
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_mem_dpa_init, "CXL");
> +
>  int cxl_mem_dpa_fetch(struct cxl_memdev_state *mds, struct cxl_dpa_info *info)
>  {
>  	struct cxl_dev_state *cxlds = &mds->cxlds;
> @@ -1298,9 +1310,8 @@ int cxl_mem_dpa_fetch(struct cxl_memdev_state *mds, struct cxl_dpa_info *info)
>  	info->size = mds->total_bytes;
>  
>  	if (mds->partition_align_bytes == 0) {
> -		add_part(info, 0, mds->volatile_only_bytes, CXL_PARTMODE_RAM);
> -		add_part(info, mds->volatile_only_bytes,
> -			 mds->persistent_only_bytes, CXL_PARTMODE_PMEM);
> +		cxl_mem_dpa_init(info, mds->volatile_only_bytes,
> +				 mds->persistent_only_bytes);

Why use this here but not a few lines later where the variant with
active_*_bytes are used?

>  		return 0;
>  	}
>  
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index e7cd31b9f107..e47f51025efd 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -98,19 +98,6 @@ int devm_cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
>  			 resource_size_t base, resource_size_t len,
>  			 resource_size_t skipped);
>  
> -#define CXL_NR_PARTITIONS_MAX 2
> -
> -struct cxl_dpa_info {
> -	u64 size;
> -	struct cxl_dpa_part_info {
> -		struct range range;
> -		enum cxl_partition_mode mode;
> -	} part[CXL_NR_PARTITIONS_MAX];
> -	int nr_partitions;
> -};
> -
> -int cxl_dpa_setup(struct cxl_dev_state *cxlds, const struct cxl_dpa_info *info);
> -
>  static inline struct cxl_ep *cxl_ep_load(struct cxl_port *port,
>  					 struct cxl_memdev *cxlmd)
>  {
> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
> index a3cbf3a620e4..74f03773baed 100644
> --- a/include/cxl/cxl.h
> +++ b/include/cxl/cxl.h
> @@ -213,6 +213,17 @@ struct cxl_dev_state {
>  #endif
>  };
>  
> +#define CXL_NR_PARTITIONS_MAX 2
> +
> +struct cxl_dpa_info {
> +	u64 size;
> +	struct cxl_dpa_part_info {
> +		struct range range;
> +		enum cxl_partition_mode mode;
> +	} part[CXL_NR_PARTITIONS_MAX];
> +	int nr_partitions;
> +};
> +
>  struct cxl_dev_state *_cxl_dev_state_create(struct device *dev,
>  					    enum cxl_devtype type, u64 serial,
>  					    u16 dvsec, size_t size,
> @@ -231,4 +242,7 @@ struct pci_dev;
>  struct cxl_memdev_state;
>  int cxl_pci_accel_setup_regs(struct pci_dev *pdev, struct cxl_dev_state *cxlmds,
>  			     unsigned long *caps);
> +void cxl_mem_dpa_init(struct cxl_dpa_info *info, u64 volatile_bytes,
> +		      u64 persistent_bytes);
> +int cxl_dpa_setup(struct cxl_dev_state *cxlds, const struct cxl_dpa_info *info);
>  #endif