[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <384d6c26-33f2-4699-986e-a8c71d225f49@amd.com>
Date: Mon, 7 Apr 2025 15:13:30 -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 3/7] x86/resctrl: Detect io_alloc feature
Hi Reinette,
On 3/21/25 17:51, Reinette Chatre wrote:
> Hi Babu,
>
> On 1/30/25 1:20 PM, Babu Moger wrote:
>> "io_alloc" feature 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, feature reduces
>> the demands on DRAM bandwidth and reduces latency to the processor
>> consuming the I/O data.
>
> This provides good context but the changelog does not mention what this patch
> does.
>
> An idea to get started (please improve):
> Data from I/O devices can be inserted directly into L3 cache. This
> reduces demands on DRAM bandwidth and reduces latency to the processor
> consuming the I/O data.
>
> Introduce cache resource property "io_alloc_capable" that an
> architecture can set if a portion of the L3 cache can be allocated
> for I/O traffic. Set this property on x86 systems that support SDCIAE.
Looks good. Thanks
>
>>
>> Signed-off-by: Babu Moger <babu.moger@....com>
>> ---
>> v3: Rewrote commit log. Changed the text to bit generic than the AMD specific.
>> Renamed the rdt_get_sdciae_alloc_cfg() to rdt_set_io_alloc_capable().
>> Removed leftover comment from v2.
>>
>> v2: Changed sdciae_capable to io_alloc_capable to make it generic feature.
>> Also moved the io_alloc_capable in struct resctrl_cache.
>> ---
>> arch/x86/kernel/cpu/resctrl/core.c | 7 +++++++
>> include/linux/resctrl.h | 3 +++
>> 2 files changed, 10 insertions(+)
>>
>> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
>> index c2450cd52511..1ebdb2dcc009 100644
>> --- a/arch/x86/kernel/cpu/resctrl/core.c
>> +++ b/arch/x86/kernel/cpu/resctrl/core.c
>> @@ -306,6 +306,11 @@ static void rdt_get_cdp_config(int level)
>> rdt_resources_all[level].r_resctrl.cdp_capable = true;
>> }
>>
>> +static void rdt_set_io_alloc_capable(struct rdt_resource *r)
>> +{
>> + r->cache.io_alloc_capable = true;
>> +}
>> +
>> static void rdt_get_cdp_l3_config(void)
>> {
>> rdt_get_cdp_config(RDT_RESOURCE_L3);
>> @@ -931,6 +936,8 @@ static __init bool get_rdt_alloc_resources(void)
>> rdt_get_cache_alloc_cfg(1, r);
>> if (rdt_cpu_has(X86_FEATURE_CDP_L3))
>> rdt_get_cdp_l3_config();
>> + if (rdt_cpu_has(X86_FEATURE_SDCIAE))
>> + rdt_set_io_alloc_capable(r);
>> ret = true;
>> }
>> if (rdt_cpu_has(X86_FEATURE_CAT_L2)) {
>> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
>> index d94abba1c716..dbe6461f3fbc 100644
>> --- a/include/linux/resctrl.h
>> +++ b/include/linux/resctrl.h
>> @@ -129,6 +129,8 @@ struct rdt_mon_domain {
>> * @arch_has_sparse_bitmasks: True if a bitmask like f00f is valid.
>> * @arch_has_per_cpu_cfg: True if QOS_CFG register for this cache
>> * level has CPU scope.
>> + * @io_alloc_capable: True if portion of the L3 cache can be allocated
>> + * for I/O traffic.
>
> Enforcing that this should be for L3 cache is confusing. On a system with
> L3 and L2 cache resources each resource will be described by the properties in
> struct resctrl_cache for particular resource. We do not want to set "io_alloc_capable"
> to true in the L2's struct if the L3 cache supports this feature.
>
> This can just be: "True if portion of the cache can be allocated for I/O traffic"
Sure.
>
>
>> */
>> struct resctrl_cache {
>> unsigned int cbm_len;
>> @@ -136,6 +138,7 @@ struct resctrl_cache {
>> unsigned int shareable_bits;
>> bool arch_has_sparse_bitmasks;
>> bool arch_has_per_cpu_cfg;
>> + bool io_alloc_capable;
>> };
>>
>> /**
>
> Reinette
>
--
Thanks
Babu Moger
Powered by blists - more mailing lists