Re: [PATCH 1/7] x86/cpufeatures: Add support for L3 Smart Data Cache Injection Allocation Enforcement

["Moger, Babu" <bmoger@amd.com>]

Hi Reinette,

On 9/13/2024 3:44 PM, Reinette Chatre wrote:
> Hi Babu,
> 
> On 8/16/24 9:16 AM, Babu Moger wrote:
>> Smart Data Cache Injection (SDCI) is a mechanism that enables direct
>> insertion of data from I/O devices into the L3 cache. By directly caching
>> data from I/O devices rather than first storing the I/O data in DRAM,
>> SDCI reduces demands on DRAM bandwidth and reduces latency to the 
>> processor
>> consuming the I/O data.
>>
>> The SDCIAE (SDCI Allocation Enforcement) PQE feature allows system 
>> software
>> to limit the portion of the L3 cache used for SDCI.
>>
>> When enabled, SDCIAE forces all SDCI lines to be placed into the L3 cache
>> partitions identified by the highest-supported L3_MASK_n register where n
>> maximum supported CLOSID.
> 
> "where n maximum supported CLOSID" -> "where n is the maximum supported 
> CLOSID" ?
> 
Sure.

>>
>> Add CPUID feature bit that can be used to configure SDCIAE.
>>
>> The feature details are documented in APM listed below [1].
>> [1] AMD64 Architecture Programmer's Manual Volume 2: System Programming
>> Publication # 24593 Revision 3.41 section 19.4.7 L3 Smart Data Cache
>> Injection Allocation Enforcement (SDCIAE)
>>
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>>   arch/x86/include/asm/cpufeatures.h | 1 +
>>   arch/x86/kernel/cpu/cpuid-deps.c   | 1 +
>>   arch/x86/kernel/cpu/scattered.c    | 1 +
>>   3 files changed, 3 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/cpufeatures.h 
>> b/arch/x86/include/asm/cpufeatures.h
>> index dd4682857c12..5ca39431d423 100644
>> --- a/arch/x86/include/asm/cpufeatures.h
>> +++ b/arch/x86/include/asm/cpufeatures.h
>> @@ -473,6 +473,7 @@
>>   #define X86_FEATURE_CLEAR_BHB_HW    (21*32+ 3) /* BHI_DIS_S HW 
>> control enabled */
>>   #define X86_FEATURE_CLEAR_BHB_LOOP_ON_VMEXIT (21*32+ 4) /* Clear 
>> branch history at vmexit using SW loop */
>>   #define X86_FEATURE_FAST_CPPC        (21*32 + 5) /* AMD Fast CPPC */
>> +#define X86_FEATURE_SDCIAE        (21*32 + 6) /* "" L3 Smart Data 
>> Cache Injection Allocation Enforcement */
>>   /*
>>    * BUG word(s)
>> diff --git a/arch/x86/kernel/cpu/cpuid-deps.c 
>> b/arch/x86/kernel/cpu/cpuid-deps.c
>> index b7d9f530ae16..1ef42cc4cc75 100644
>> --- a/arch/x86/kernel/cpu/cpuid-deps.c
>> +++ b/arch/x86/kernel/cpu/cpuid-deps.c
>> @@ -70,6 +70,7 @@ static const struct cpuid_dep cpuid_deps[] = {
>>       { X86_FEATURE_CQM_MBM_LOCAL,        X86_FEATURE_CQM_LLC   },
>>       { X86_FEATURE_BMEC,            X86_FEATURE_CQM_MBM_TOTAL   },
>>       { X86_FEATURE_BMEC,            X86_FEATURE_CQM_MBM_LOCAL   },
>> +    { X86_FEATURE_SDCIAE,            X86_FEATURE_RDT_A     },
> 
> The need for this dependency is not clear to me. If there was a dependency
> then I would have expected it to be X86_FEATURE_CAT_L3 but we have not
> previously needed to do this. For example, X86_FEATURE_CDP_L3 does not 
> depend
> on X86_FEATURE_CAT_L3 and in turn X86_FEATURE_CAT_L3 does not depend on
> X86_FEATURE_RDT_A. Could you please elaborate why this is needed?

SDCIAE is allocation feature. So, I added X86_FEATURE_RDT_A.
Yea, It may be appropriate to add dependency on X86_FEATURE_CAT_L3. 
Because it is CAT_L3 related feature. I can change that.

I don't know the history why we didn't have dependency on CDP and CAT_L3.

Thanks
- Babu Moger