Re: [PATCH] cxl: Remove core/acpi.c and cxl core dependency on ACPI

[Dave Jiang <dave.jiang@intel.com>]

On 6/9/25 10:03 AM, Fabio M. De Francesco wrote:
> Hi Dave,
> 
> On Thursday, June 5, 2025 2:11:47 AM Central European Summer Time Dave Jiang wrote:
>> It was a mistake to introduce core/acpi.c and putting ACPI dependency on
>> cxl_core when adding the extended linear cache support. Add a callback
>> in the cxl_root_decoder to retrieve the extended linear cache size from
>> ACPI via the cxl_acpi driver.
>>
>> In order to deal with cxl_test, a device parameter had to be introduced
>> to the hmat_get_extended_linear_cache_size() call in order to help with
>> the mock wrapped function from ACPI. Even though the 'struct device' is
>> not used by the actual hmat_get_extended_linear_cache_size() function.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>>  drivers/acpi/numa/hmat.c      |  4 +++-
>>  drivers/cxl/acpi.c            | 15 ++++++++++++++-
>>  drivers/cxl/core/Makefile     |  1 -
>>  drivers/cxl/core/acpi.c       | 11 -----------
>>  drivers/cxl/core/core.h       |  2 --
>>  drivers/cxl/core/port.c       |  5 ++++-
>>  drivers/cxl/core/region.c     |  6 +++++-
>>  drivers/cxl/cxl.h             | 12 +++++++++++-
>>  include/linux/acpi.h          |  6 ++++--
>>  tools/testing/cxl/Kbuild      |  2 +-
>>  tools/testing/cxl/test/cxl.c  | 20 ++++++++++++++++++++
>>  tools/testing/cxl/test/mock.c | 23 +++++++++++++++++++++++
>>  tools/testing/cxl/test/mock.h |  4 ++++
>>  13 files changed, 89 insertions(+), 22 deletions(-)
>>  delete mode 100644 drivers/cxl/core/acpi.c
>>
>> diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
>> index 9d9052258e92..bc8441dc7fe2 100644
>> --- a/drivers/acpi/numa/hmat.c
>> +++ b/drivers/acpi/numa/hmat.c
>> @@ -110,6 +110,7 @@ static struct memory_target *find_mem_target(unsigned int mem_pxm)
>>  
>>  /**
>>   * hmat_get_extended_linear_cache_size - Retrieve the extended linear cache size
>> + * @dev: device for debug output
> 
> Isn't dev used to find a port check if it is a mock one?
> BTW, should the test part go to a separate patch? 

I'm dropping the dev parameter. So this change should go away.

> 
>>   * @backing_res: resource from the backing media
>>   * @nid: node id for the memory region
>>   * @cache_size: (Output) size of extended linear cache.
>> @@ -117,7 +118,8 @@ static struct memory_target *find_mem_target(unsigned int mem_pxm)
>>   * Return: 0 on success. Errno on failure.
>>   *
>>   */
>> -int hmat_get_extended_linear_cache_size(struct resource *backing_res, int nid,
>> +int hmat_get_extended_linear_cache_size(struct device *dev,
>> +					struct resource *backing_res, int nid,
>>  					resource_size_t *cache_size)
>>  {
>>  	unsigned int pxm = node_to_pxm(nid);
>> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
>> index cb14829bb9be..7076d471ada5 100644
>> --- a/drivers/cxl/acpi.c
>> +++ b/drivers/cxl/acpi.c
>> @@ -337,6 +337,19 @@ static int add_or_reset_cxl_resource(struct resource *parent, struct resource *r
>>  	return rc;
>>  }
>>  
>> +static int cxl_acpi_get_extended_linear_cache_size(struct cxl_root_decoder *cxlrd,
>> +						   struct resource *backing_res,
>> +						   int nid,
>> +						   resource_size_t *size)
>> +{
>> +	return hmat_get_extended_linear_cache_size(&cxlrd->cxlsd.cxld.dev,
>> +						   backing_res, nid, size);
>> +}
>> +
>> +static const struct cxl_rcd_ops acpi_rcd_ops = {
> 
> What does _rcd_ stand for?

Not sure why I named it rcd. Going to change it to rd for root decoder.
> 
>> +	.get_extended_linear_cache_size = cxl_acpi_get_extended_linear_cache_size,
>> +};
>> +
> 
> [skip]
> 
>>  };
>>  
>> +static int mock_hmat_get_extended_linear_cache_size(struct device *dev,
>> +						    struct resource *backing_res,
>> +						    int nid, resource_size_t *size)
>> +{
>> +	struct cxl_decoder *cxld;
>> +	struct cxl_port *port;
>> +
>> +	if (!is_root_decoder(dev))
>> +		return -EINVAL;
> 
> Is there any real chance that dev is not a root decoder? Is that check necessary?

dev getting dropped.

> 
> Thanks,
> 
> Fabio
> 
> 
> Thanks,
> 
> Fabio
> 
> 
>