Re: [PATCH v7 02/14] cxl: Add Get Supported Features command for kernel usage

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

On 2/25/25 2:01 PM, Davidlohr Bueso wrote:
> On Thu, 20 Feb 2025, Dave Jiang wrote:
> 
>> CXL spec r3.2 8.2.9.6.1 Get Supported Features (Opcode 0500h)
>> The command retrieve the list of supported device-specific features
>> (identified by UUID) and general information about each Feature.
>>
>> The driver will retrieve the Feature entries in order to make checks and
>> provide information for the Get Feature and Set Feature command. One of
>> the main piece of information retrieved are the effects a Set Feature
>> command would have for a particular feature. The retrieved Feature
>> entries are stored in the cxl_mailbox context.
>>
>> The setup of Features is initiated via devm_cxl_setup_features() during the
>> pci probe function before the cxl_memdev is enumerated.
>>
>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
>> Reviewed-by: Li Ming <ming.li@zohomail.com>
>> Tested-by: Shiju Jose <shiju.jose@huawei.com>
> 
> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
> 
> ... with a comment below.
> 
>> +config CXL_FEATURES
>> +    bool "CXL: Features"
>> +    depends on CXL_PCI
>> +    help
>> +      Enable support for CXL Features. A CXL device that includes a mailbox
>> +      supports commands that allows listing, getting, and setting of
>> +      optionally defined features such as memory sparing or post package
>> +      sparing. Vendors may define custom features for the device.
>> +
>> +      If unsure say 'n'
> 
> When would a user want to disable this (admittedly this might become clearer further
> into the series)? But it seems this should not be tucked behind a kconfig, much less
> default to off.

At this point probably when the user want a locked down kernel without fwctl support. Dan wanted the entire Features support behind a kconfig for now.
https://lore.kernel.org/linux-cxl/67a2b6d345b61_2d2c2943c@dwillia2-xfh.jf.intel.com.notmuch/

 

> 
> Thanks,
> Davidlohr