Re: [PATCH] cxl: identify CXL3.0 cache/mem register capability IDs

[Ben Cheatham <benjamin.cheatham@amd.com>]

On 10/20/22 1:54 PM, Alison Schofield wrote:
> On Thu, Oct 20, 2022 at 12:52:39PM -0500, Ben Cheatham wrote:
>> Add code to identify the non extended cache mem CXL register capabilities
>> based on capability ID.
>>
>> As of now, nothing is done with this information except logging that the
>> capabilities were found. In the case of the NULL capability this is
>> probably all that should be done since the spec specifies software should
>> skip this capability. The other capabilities are added so that if any
>> additional setup/info is required, like for the HDM decoder capability, the
>> stub is already there.
> Hi Ben,
>
> What is the user visible impact of making this change.
> Perhaps it would be better reviewed alongside it's use case.
>
> Alison

Hi Alison,

As this patch is written it's mainly for debugging/logging, but there is 
some work going on inside AMD that could use the RAS ID case for getting 
info about/initializing RAS capabilities. Originally the patch was just 
for logging skipped NULL capabilities, but I figured I may as well add 
cases for the other capability IDs in case someone wanted to use them. 
That being said, I would be fine submitting this code inside another 
patch where it's being used at a later date. Thanks for the comments.

Ben

>> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
>> ---
>>   drivers/cxl/core/regs.c | 42 ++++++++++++++++++++++++++++++++++++++---
>>   drivers/cxl/cxl.h       | 13 ++++++++++++-
>>   2 files changed, 51 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
>> index 39a129c57d40..616d926de1e6 100644
>> --- a/drivers/cxl/core/regs.c
>> +++ b/drivers/cxl/core/regs.c
>> @@ -27,8 +27,8 @@
>>    * @base: Mapping containing the HDM Decoder Capability Header
>>    * @map: Map object describing the register block information found
>>    *
>> - * See CXL 2.0 8.2.4 Component Register Layout and Definition
>> - * See CXL 2.0 8.2.5.5 CXL Device Register Interface
>> + * See CXL 3.0 8.2.3 Component Register Layout and Definition
>> + * See CXL 3.0 8.2.8 CXL Device Register Interface
>>    *
>>    * Probe for component register information and return it in map object.
>>    */
>> @@ -42,7 +42,7 @@ void cxl_probe_component_regs(struct device *dev, void
>> __iomem *base,
>>
>>       /*
>>        * CXL.cache and CXL.mem registers are at offset 0x1000 as defined in
>> -     * CXL 2.0 8.2.4 Table 141.
>> +     * CXL 3.0 8.2.3 Table 8-21.
>>        */
>>       base += CXL_CM_OFFSET;
>>
>> @@ -71,6 +71,26 @@ void cxl_probe_component_regs(struct device *dev, void
>> __iomem *base,
>>           register_block = base + offset;
>>
>>           switch (cap_id) {
>> +        case CXL_CM_CAP_CAP_ID_NULL:
>> +            dev_dbg(dev, "found NULL capability (0x%x)\n",
>> +                offset);
>> +            break;
>> +        case CXL_CM_CAP_CAP_ID_CXL:
>> +            dev_dbg(dev, "found CXL capability (0x%x)\n",
>> +                offset);
>> +            break;
>> +        case CXL_CM_CAP_CAP_ID_RAS:
>> +            dev_dbg(dev, "found RAS capability (0x%x)\n",
>> +                offset);
>> +            break;
>> +        case CXL_CM_CAP_CAP_ID_SECURITY:
>> +            dev_dbg(dev, "found Security capability (0x%x)\n",
>> +                offset);
>> +            break;
>> +        case CXL_CM_CAP_CAP_ID_LINK:
>> +            dev_dbg(dev, "found Link capability (0x%x)\n",
>> +                offset);
>> +            break;
>>           case CXL_CM_CAP_CAP_ID_HDM:
>>               dev_dbg(dev, "found HDM decoder capability (0x%x)\n",
>>                   offset);
>> @@ -84,6 +104,22 @@ void cxl_probe_component_regs(struct device *dev, void
>> __iomem *base,
>>               map->hdm_decoder.offset = CXL_CM_OFFSET + offset;
>>               map->hdm_decoder.size = length;
>>               break;
>> +        case CXL_CM_CAP_CAP_ID_EXT_SEC:
>> +            dev_dbg(dev, "found Extended Security capability (0x%x)\n",
>> +                offset);
>> +            break;
>> +        case CXL_CM_CAP_CAP_ID_IDE:
>> +            dev_dbg(dev, "found IDE capability (0x%x)\n",
>> +                offset);
>> +            break;
>> +        case CXL_CM_CAP_CAP_ID_SNOOP:
>> +            dev_dbg(dev, "found Snoop Filter capability (0x%x)\n",
>> +                offset);
>> +            break;
>> +        case CXL_CM_CAP_CAP_ID_TIMEOUT:
>> +            dev_dbg(dev, "found Timeout and Isolation capability (0x%x)\n",
>> +                offset);
>> +            break;
>>           default:
>>               dev_dbg(dev, "Unknown CM cap ID: %d (0x%x)\n", cap_id,
>>                   offset);
>> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
>> index f680450f0b16..762e5243162a 100644
>> --- a/drivers/cxl/cxl.h
>> +++ b/drivers/cxl/cxl.h
>> @@ -33,9 +33,20 @@
>>   #define   CXL_CM_CAP_HDR_ARRAY_SIZE_MASK GENMASK(31, 24)
>>   #define CXL_CM_CAP_PTR_MASK GENMASK(31, 20)
>>
>> -#define   CXL_CM_CAP_CAP_ID_HDM 0x5
>>   #define   CXL_CM_CAP_CAP_HDM_VERSION 1
>>
>> +/* CXL 3.0 8.2.4 Table 8-22 CXL_Capability_ID Assignment*/
>> +#define CXL_CM_CAP_CAP_ID_NULL 0x0
>> +#define CXL_CM_CAP_CAP_ID_CXL 0x1
>> +#define CXL_CM_CAP_CAP_ID_RAS 0x2
>> +#define CXL_CM_CAP_CAP_ID_SECURITY 0x3
>> +#define CXL_CM_CAP_CAP_ID_LINK 0x4
>> +#define CXL_CM_CAP_CAP_ID_HDM 0x5
>> +#define CXL_CM_CAP_CAP_ID_EXT_SEC 0x6
>> +#define CXL_CM_CAP_CAP_ID_IDE 0x7
>> +#define CXL_CM_CAP_CAP_ID_SNOOP 0x8
>> +#define CXL_CM_CAP_CAP_ID_TIMEOUT 0x9
>> +
>>   /* HDM decoders CXL 2.0 8.2.5.12 CXL HDM Decoder Capability Structure */
>>   #define CXL_HDM_DECODER_CAP_OFFSET 0x0
>>   #define   CXL_HDM_DECODER_COUNT_MASK GENMASK(3, 0)
>> -- 
>> 2.34.1