Re: [PATCH v9 04/27] cxl/pci: add check for validating capabilities

[Alejandro Lucero Palau <alucerop@amd.com>]

On 1/18/25 01:40, Dan Williams wrote:
> alejandro.lucero-palau@ wrote:
>> From: Alejandro Lucero <alucerop@amd.com>
>>
>> During CXL device initialization supported capabilities by the device
>> are discovered. Type3 and Type2 devices have different mandatory
>> capabilities and a Type2 expects a specific set including optional
>> capabilities.
>>
>> Add a function for checking expected capabilities against those found
>> during initialization and allow those mandatory/expected capabilities to
>> be a subset of the capabilities found.
>>
>> Rely on this function for validating capabilities instead of when CXL
>> regs are probed.
>>
>> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
>> Reviewed-by: Zhi Wang <zhiw@nvidia.com>
>> ---
>>   drivers/cxl/core/pci.c  | 16 ++++++++++++++++
>>   drivers/cxl/core/regs.c |  9 ---------
>>   drivers/cxl/pci.c       | 24 ++++++++++++++++++++++++
>>   include/cxl/cxl.h       |  3 +++
>>   4 files changed, 43 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
>> index ec57caf5b2d7..57318cdc368a 100644
>> --- a/drivers/cxl/core/pci.c
>> +++ b/drivers/cxl/core/pci.c
>> @@ -8,6 +8,7 @@
>>   #include <linux/pci.h>
>>   #include <linux/pci-doe.h>
>>   #include <linux/aer.h>
>> +#include <cxl/cxl.h>
>>   #include <cxlpci.h>
>>   #include <cxlmem.h>
>>   #include <cxl.h>
>> @@ -1055,3 +1056,18 @@ int cxl_pci_get_bandwidth(struct pci_dev *pdev, struct access_coordinate *c)
>>   
>>   	return 0;
>>   }
>> +
>> +bool cxl_pci_check_caps(struct cxl_dev_state *cxlds, unsigned long *expected_caps,
>> +			unsigned long *current_caps)
>> +{
>> +
>> +	if (current_caps)
>> +		bitmap_copy(current_caps, cxlds->capabilities, CXL_MAX_CAPS);
>> +
>> +	dev_dbg(cxlds->dev, "Checking cxlds caps 0x%pb vs expected caps 0x%pb\n",
>> +		cxlds->capabilities, expected_caps);
>> +
>> +	/* Checking a minimum of mandatory/expected capabilities */
>> +	return bitmap_subset(expected_caps, cxlds->capabilities, CXL_MAX_CAPS);
>> +}
>> +EXPORT_SYMBOL_NS_GPL(cxl_pci_check_caps, "CXL");
> cxl_setup_regs() is already exported from the core. Just make the caller
> of cxl_setup_regs() responsible for checking the valid bits per its
> constraints rather a new mechanism.


I prefer to keep the regs setup separated from the checks, and I think 
your suggestion involves a higher impact on the current code.

Note this is the API for accel drivers and by design what the accel 
driver can do with cxl structs is restricted. The patchset adds a new 
function in patch 6 for regs setup  by accel drivers.