Message-ID: <251c8fe1-603f-4993-a822-afb35b49cdfa@amd.com>
Date: Mon, 7 Apr 2025 15:12:50 -0500
From: "Moger, Babu" <babu.moger@....com>
To: Reinette Chatre <reinette.chatre@...el.com>, tglx@...utronix.de,
 mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com
Cc: x86@...nel.org, hpa@...or.com, akpm@...ux-foundation.org,
 paulmck@...nel.org, thuth@...hat.com, rostedt@...dmis.org,
 xiongwei.song@...driver.com, pawan.kumar.gupta@...ux.intel.com,
 jpoimboe@...nel.org, daniel.sneddon@...ux.intel.com,
 thomas.lendacky@....com, perry.yuan@....com, sandipan.das@....com,
 kai.huang@...el.com, seanjc@...gle.com, xin3.li@...el.com,
 ebiggers@...gle.com, andrew.cooper3@...rix.com, mario.limonciello@....com,
 tan.shaopeng@...itsu.com, james.morse@....com, tony.luck@...el.com,
 peternewman@...gle.com, linux-doc@...r.kernel.org,
 linux-kernel@...r.kernel.org, eranian@...gle.com, corbet@....net
Subject: Re: [PATCH v3 0/7] Support L3 Smart Data Cache Injection Allocation
 Enforcement (SDCIAE)

Hi Reinette,

Thanks for the feedback. Sorry for the delay.

On 3/21/25 17:50, Reinette Chatre wrote:
> Hi Babu,
> 
> On 1/30/25 1:20 PM, Babu Moger wrote:
>>
>> This series adds the support for L3 Smart Data Cache Injection Allocation
>> Enforcement (SDCIAE) to resctrl infrastructure. It is refered as io_alloc
> 
> "refered as" -> "referred to as"?

sure.

> 
>> in resctrl subsystem.
>>
> 
> ...
> 
>>
>> # Linux Implementation
>>
>> Feature adds following interface files when the resctrl "io_alloc" feature is
>> supported:
> 
> Please note that, for example when L3 supports io_alloc but L2 does not, the
> implementation also creates the files for L2 (since it sets flags for all
> cache resources). "io_alloc" may just be present for resource that does not
> actually support "io_alloc". (more below)
> 
>>
>> /sys/fs/resctrl/info/L3/io_alloc: Reports the feature status. Feature can be
>> 				  enabled/disabled by writing to the interface.
>>
>> /sys/fs/resctrl/info/L3/io_alloc_cbm: Capacity Bit Masks (CBMs) available to SDCI
>> 				      supported IO devices. CBM can be configured
> 
> (please use imperative tone throughout)

ok.

> 
>> 				      by writing to the interface in the following
>> 				      format::
> 
> ("in the following format" is not followed by a format example as expected)

Sure.
> 
>> # Examples
>>
>> a. Check if io_alloc feature is available
>> 	#mount -t resctrl resctrl /sys/fs/resctrl/
>>
>> 	# cat /sys/fs/resctrl/info/L3/io_alloc
>> 	0
> 
> Considering that io_alloc will be present for resources that may not support
> io_alloc I think it may be helpful if it instead display "enabled/disabled/not supported".
> User space can still interact with the file with typical boolean variants recognized
> by kstrtobool()

Sounds good.

> 
>>
>> b. Enable the io_alloc feature. 
>>
>> 	# echo 1 > /sys/fs/resctrl/info/L3/io_alloc 
>>
>> c. Check the CBM values for the io_alloc feature.
>>
>> 	# cat /sys/fs/resctrl/info/L3/io_alloc_cbm 
>> 	L3:0=ffff;1=ffff
> 
> Is it expected that more resources may support IO alloc?

No. There is no plan for that.

> In above output the resource name is redundant, but if
> more resources could support IO allocation then it may
> be more intuitive to move io_alloc_cbm one level up in hierarchy
> and keep resource name in output in order to potentially
> manage multiple resources. I do not see impact on feature
> in current design if files are kept within resource directories,
> but in that case the name seems redundant.

Yea. Will remove "L3".  Just keep 0=ffff;1=ffff

> 
>>
>> d. Change the CBM value for the domain 1:
>> 	# echo L3:1=FF > /sys/fs/resctrl/info/L3/io_alloc_cbm
>>
>> 	# cat /sys/fs/resctrl/info/L3/io_alloc_cbm 
>> 	L3:0=ffff;1=00ff
>>
>> d. Disable io_alloc feature and exit.
>>
>> 	# echo 0 > /sys/fs/resctrl/info/L3/io_alloc 
>> 	#umount /sys/fs/resctrl/
>>
> 
> AMD also supports what is exposed to user space as "shareable_bits". According
> to APM:
> 	Depending on the implementation, some portions of the L3 Cache may be
> 	shared by other system functions or used for some other purpose not
> 	under the control of the PQOS feature set. The L3 Cache Allocation
> 	Sharing Mask returned by CPUID Fn0000_0010_EBX_x1[L3ShareAllocMask] is a
> 	bitmask that represents portions of the L3 that may be shared by those
> 	functions.

Here is the complete text.

The L3 Cache allocation sharing mask (L3ShareAllocMask) returned in EBX by
CPUID Fn0000_0010 with ECX=1 is a bit vector which represents portions of
the cache which may be shared with other system entities or used for some
other purpose not under the control of the QOS feature set. When software
sets a bit in one of the L3_MASK_n registers at the same bit positions a
bit in the L3ShareAllocMask, processors executing with the corresponding
COS will competitively share that portion of the cache with the other
function. If this mask is all 0’s, then there is no other entity in the
system competing with the processors for use of the L3 cache.

The "L3ShareAllocMask" is always reported as 0 on AMD systems.

> Could you please include what (if any) the relationship is between the CBM
> discoverable via Fn0000_0010_EBX_x1[L3ShareAllocMask] and the CBM of 
> "highest-supported L3_MASK_n register" when SDCIAE is enabled?

No. There is no relationship in here.

> 
> On the resctrl interface side the documentation currently states:
> 
> 	"shareable_bits":                                                               
> 		Bitmask of shareable resource with other executing              
> 		entities (e.g. I/O). User can use this when                     
> 		setting up exclusive cache partitions. Note that                
> 		some platforms support devices that have their                  
> 		own settings for cache use which can over-ride                  
> 		these bits.                             
> 
> Even though this was originally used to expose the content of
> Fn0000_0010_EBX_x1[L3ShareAllocMask] the intent of the content does
> seem to also apply to the "io_alloc" CBM also.

It says "shared by other system functions or used for some other purpose
not under the control of the PQOS feature set".

"io_alloc" is PQOS feature set. I feel it should not affect "shareable_bits".

> 
> I also expect "bit_usage" to appropriately reflect the io_alloc mask.
>>>From what I can tell SDCIAE does not enforce isolation between hardware and
> software use of these cache allocations so it does seem to me that the
> existing legends apply. 
> 
> Reinette
> 
> 

-- 
Thanks
Babu Moger

Powered by blists - more mailing lists