Re: [PATCH v5 03/16] x86/resctrl: Add PLZA configuration definitions and data structures

[Babu Moger <babu.moger@amd.com>]

Hi Reinette,

On 9/16/26 00:16, Reinette Chatre wrote:
> Hi Babu,
> 
> On 8/26/26 12:32 PM, Babu Moger wrote:
>> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
>> index e3cfa0c10e92..2f4c60487a5d 100644
>> --- a/arch/x86/kernel/cpu/resctrl/internal.h
>> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
>> @@ -222,6 +222,53 @@ union l3_qos_abmc_cfg {
>>   	unsigned long full;
>>   };
>>   
>> +/*
> 
> I understand that this is not specifically kernel-doc (since no /**)
> but it clearly follows the pattern and also needs to match the
> style used in this file. Considering this, please use custom that
> describes members first before the long description? With members
> described first the long description can then use @ when referring to
> the members.

Sure.

> 
>> + * Privilege Level Zero Association (PLZA) is configured via
>> + * MSR_IA32_PQR_PLZA_ASSOC. This MSR must be programmed on every CPU in
>> + * a QoS domain. Each domain supports a single PLZA CLOSID and/or RMID
>> + * association, and all fields except PLZA_EN must be programmed
>> + * identically across the domain to ensure consistent behavior.
>> + *
>> + * RMID and CLOSID selection are controlled independently. When PLZA is
>> + * enabled (plza_en = 1), CPL0 uses CLOSID and RMID values from
>> + * MSR_IA32_PQR_PLZA_ASSOC depending on the corresponding enable bits.
>> + * When PLZA is disabled (plza_en = 0), the CLOSID and RMID are always
>> + * taken from MSR_IA32_PQR_ASSOC regardless of privilege level.
>> + *
>> + *   - rmid_en = 1: use the RMID programmed in this MSR.
> 
> "this MSR" -> "MSR_IA32_PQR_PLZA_ASSOC" or keeping text similar to the
> rmid_en = 0 scenario may make it easier to follow what the bit accomplishes:
> 	- @rmid_en = 1: use the RMID from MSR_IA32_PQR_PLZA_ASSOC if
> 			@plza_en = 1

Sure.

> 
>> + *   - rmid_en = 0: use the RMID from MSR_IA32_PQR_ASSOC.
>> + *   - closid_en = 1: use the CLOSID programmed in this MSR.
> 
> same comment as for rmid_en bit:
> 	- @closid_en = 1: use the CLOSID from MSR_IA32_PQR_PLZA_ASSOC
> 			  if @plza_en = 1
> 

Sure.

>> + *   - closid_en = 0: use the CLOSID from MSR_IA32_PQR_ASSOC.
>> + *
>> + * This allows PLZA to override only RMID, only CLOSID, or both. If both
>> + * rmid_en and closid_en are clear, the effective behavior is the same as
>> + * when PLZA is disabled, with RMID and CLOSID derived from
> 
> "derived from" -> "used from"?
> 

Sure.

Thanks
Babu