diff mbox series

[v8,05/13] x86/resctrl: Detect and configure Slow Memory Bandwidth Allocation

Message ID 166759202922.3281208.6231300030834095574.stgit@bmoger-ubuntu
State New
Headers
Series Support for AMD QoS new features |

Commit Message

Moger, Babu Nov. 4, 2022, 8 p.m. UTC 
  The QoS slow memory configuration details are available via
CPUID_Fn80000020_EDX_x02. Detect the available details and
initialize the rest to defaults.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kernel/cpu/resctrl/core.c        |   36 +++++++++++++++++++++++++++--
 arch/x86/kernel/cpu/resctrl/ctrlmondata.c |    2 +-
 arch/x86/kernel/cpu/resctrl/internal.h    |    1 +
 arch/x86/kernel/cpu/resctrl/rdtgroup.c    |    8 ++++--
 4 files changed, 41 insertions(+), 6 deletions(-)

Comments

Reinette Chatre Nov. 23, 2022, 12:12 a.m. UTC | #1 
Hi Babu,

On 11/4/2022 1:00 PM, Babu Moger wrote:
> The QoS slow memory configuration details are available via
> CPUID_Fn80000020_EDX_x02. Detect the available details and
> initialize the rest to defaults.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>  arch/x86/kernel/cpu/resctrl/core.c        |   36 +++++++++++++++++++++++++++--
>  arch/x86/kernel/cpu/resctrl/ctrlmondata.c |    2 +-
>  arch/x86/kernel/cpu/resctrl/internal.h    |    1 +
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c    |    8 ++++--
>  4 files changed, 41 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index e31c98e2fafc..6571d08e2b0d 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -162,6 +162,13 @@ bool is_mba_sc(struct rdt_resource *r)
>  	if (!r)
>  		return rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl.membw.mba_sc;
>  
> +	/*
> +	 * The software controller support is only applicable to MBA resource.
> +	 * Make sure to check for resource type again.
> +	 */

/again/d

Not all callers of is_mba_sc() check if it is called for an MBA resource.

> +	if (r->rid != RDT_RESOURCE_MBA)
> +		return false;
> +
>  	return r->membw.mba_sc;
>  }
>  
> @@ -225,9 +232,15 @@ static bool __rdt_get_mem_config_amd(struct rdt_resource *r)
>  	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
>  	union cpuid_0x10_3_eax eax;
>  	union cpuid_0x10_x_edx edx;
> -	u32 ebx, ecx;
> +	u32 ebx, ecx, subleaf;
>  
> -	cpuid_count(0x80000020, 1, &eax.full, &ebx, &ecx, &edx.full);
> +	/*
> +	 * Query CPUID_Fn80000020_EDX_x01 for MBA and
> +	 * CPUID_Fn80000020_EDX_x02 for SMBA
> +	 */
> +	subleaf = (r->rid == RDT_RESOURCE_SMBA) ? 2 :  1;
> +
> +	cpuid_count(0x80000020, subleaf, &eax.full, &ebx, &ecx, &edx.full);
>  	hw_res->num_closid = edx.split.cos_max + 1;
>  	r->default_ctrl = MAX_MBA_BW_AMD;
>  
> @@ -750,6 +763,19 @@ static __init bool get_mem_config(void)
>  	return false;
>  }
>  
> +static __init bool get_slow_mem_config(void)
> +{
> +	struct rdt_hw_resource *hw_res = &rdt_resources_all[RDT_RESOURCE_SMBA];
> +
> +	if (!rdt_cpu_has(X86_FEATURE_SMBA))
> +		return false;
> +
> +	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
> +		return __rdt_get_mem_config_amd(&hw_res->r_resctrl);
> +
> +	return false;
> +}
> +
>  static __init bool get_rdt_alloc_resources(void)
>  {
>  	struct rdt_resource *r;
> @@ -780,6 +806,9 @@ static __init bool get_rdt_alloc_resources(void)
>  	if (get_mem_config())
>  		ret = true;
>  
> +	if (get_slow_mem_config())
> +		ret = true;
> +
>  	return ret;
>  }
>  
> @@ -869,6 +898,9 @@ static __init void rdt_init_res_defs_amd(void)
>  		} else if (r->rid == RDT_RESOURCE_MBA) {
>  			hw_res->msr_base = MSR_IA32_MBA_BW_BASE;
>  			hw_res->msr_update = mba_wrmsr_amd;
> +		} else if (r->rid == RDT_RESOURCE_SMBA) {
> +			hw_res->msr_base = MSR_IA32_SMBA_BW_BASE;
> +			hw_res->msr_update = mba_wrmsr_amd;
>  		}
>  	}
>  }

I mentioned earlier that this can be moved to init of
rdt_resources_all[]. No strong preference, leaving here works
also.

Reinette
Moger, Babu Nov. 23, 2022, 3:17 p.m. UTC | #2 
[AMD Official Use Only - General]

Hi Reinette,

> -----Original Message-----
> From: Reinette Chatre <reinette.chatre@intel.com>
> Sent: Tuesday, November 22, 2022 6:13 PM
> To: Moger, Babu <Babu.Moger@amd.com>; corbet@lwn.net;
> tglx@linutronix.de; mingo@redhat.com; bp@alien8.de
> Cc: fenghua.yu@intel.com; dave.hansen@linux.intel.com; x86@kernel.org;
> hpa@zytor.com; paulmck@kernel.org; akpm@linux-foundation.org;
> quic_neeraju@quicinc.com; rdunlap@infradead.org;
> damien.lemoal@opensource.wdc.com; songmuchun@bytedance.com;
> peterz@infradead.org; jpoimboe@kernel.org; pbonzini@redhat.com;
> chang.seok.bae@intel.com; pawan.kumar.gupta@linux.intel.com;
> jmattson@google.com; daniel.sneddon@linux.intel.com; Das1, Sandipan
> <Sandipan.Das@amd.com>; tony.luck@intel.com; james.morse@arm.com;
> linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org;
> bagasdotme@gmail.com; eranian@google.com
> Subject: Re: [PATCH v8 05/13] x86/resctrl: Detect and configure Slow Memory
> Bandwidth Allocation
> 
> Hi Babu,
> 
> On 11/4/2022 1:00 PM, Babu Moger wrote:
> > The QoS slow memory configuration details are available via
> > CPUID_Fn80000020_EDX_x02. Detect the available details and initialize
> > the rest to defaults.
> >
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> >  arch/x86/kernel/cpu/resctrl/core.c        |   36
> +++++++++++++++++++++++++++--
> >  arch/x86/kernel/cpu/resctrl/ctrlmondata.c |    2 +-
> >  arch/x86/kernel/cpu/resctrl/internal.h    |    1 +
> >  arch/x86/kernel/cpu/resctrl/rdtgroup.c    |    8 ++++--
> >  4 files changed, 41 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/resctrl/core.c
> > b/arch/x86/kernel/cpu/resctrl/core.c
> > index e31c98e2fafc..6571d08e2b0d 100644
> > --- a/arch/x86/kernel/cpu/resctrl/core.c
> > +++ b/arch/x86/kernel/cpu/resctrl/core.c
> > @@ -162,6 +162,13 @@ bool is_mba_sc(struct rdt_resource *r)
> >  	if (!r)
> >  		return
> rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl.membw.mba_sc;
> >
> > +	/*
> > +	 * The software controller support is only applicable to MBA resource.
> > +	 * Make sure to check for resource type again.
> > +	 */
> 
> /again/d

Sure.

> 
> Not all callers of is_mba_sc() check if it is called for an MBA resource.
> 
> > +	if (r->rid != RDT_RESOURCE_MBA)
> > +		return false;
> > +
> >  	return r->membw.mba_sc;
> >  }
> >
> > @@ -225,9 +232,15 @@ static bool __rdt_get_mem_config_amd(struct
> rdt_resource *r)
> >  	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
> >  	union cpuid_0x10_3_eax eax;
> >  	union cpuid_0x10_x_edx edx;
> > -	u32 ebx, ecx;
> > +	u32 ebx, ecx, subleaf;
> >
> > -	cpuid_count(0x80000020, 1, &eax.full, &ebx, &ecx, &edx.full);
> > +	/*
> > +	 * Query CPUID_Fn80000020_EDX_x01 for MBA and
> > +	 * CPUID_Fn80000020_EDX_x02 for SMBA
> > +	 */
> > +	subleaf = (r->rid == RDT_RESOURCE_SMBA) ? 2 :  1;
> > +
> > +	cpuid_count(0x80000020, subleaf, &eax.full, &ebx, &ecx, &edx.full);
> >  	hw_res->num_closid = edx.split.cos_max + 1;
> >  	r->default_ctrl = MAX_MBA_BW_AMD;
> >
> > @@ -750,6 +763,19 @@ static __init bool get_mem_config(void)
> >  	return false;
> >  }
> >
> > +static __init bool get_slow_mem_config(void) {
> > +	struct rdt_hw_resource *hw_res =
> > +&rdt_resources_all[RDT_RESOURCE_SMBA];
> > +
> > +	if (!rdt_cpu_has(X86_FEATURE_SMBA))
> > +		return false;
> > +
> > +	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
> > +		return __rdt_get_mem_config_amd(&hw_res->r_resctrl);
> > +
> > +	return false;
> > +}
> > +
> >  static __init bool get_rdt_alloc_resources(void)  {
> >  	struct rdt_resource *r;
> > @@ -780,6 +806,9 @@ static __init bool get_rdt_alloc_resources(void)
> >  	if (get_mem_config())
> >  		ret = true;
> >
> > +	if (get_slow_mem_config())
> > +		ret = true;
> > +
> >  	return ret;
> >  }
> >
> > @@ -869,6 +898,9 @@ static __init void rdt_init_res_defs_amd(void)
> >  		} else if (r->rid == RDT_RESOURCE_MBA) {
> >  			hw_res->msr_base = MSR_IA32_MBA_BW_BASE;
> >  			hw_res->msr_update = mba_wrmsr_amd;
> > +		} else if (r->rid == RDT_RESOURCE_SMBA) {
> > +			hw_res->msr_base = MSR_IA32_SMBA_BW_BASE;
> > +			hw_res->msr_update = mba_wrmsr_amd;
> >  		}
> >  	}
> >  }
> 
> I mentioned earlier that this can be moved to init of rdt_resources_all[]. No
> strong preference, leaving here works also.

Sure. Will do.
Thanks
Babu
> 
> Reinette
Moger, Babu Nov. 30, 2022, 6:43 p.m. UTC | #3 
Hi Reinette,

On 11/22/22 18:12, Reinette Chatre wrote:
> Hi Babu,
>
> On 11/4/2022 1:00 PM, Babu Moger wrote:
>> The QoS slow memory configuration details are available via
>> CPUID_Fn80000020_EDX_x02. Detect the available details and
>> initialize the rest to defaults.
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>>  arch/x86/kernel/cpu/resctrl/core.c        |   36 +++++++++++++++++++++++++++--
>>  arch/x86/kernel/cpu/resctrl/ctrlmondata.c |    2 +-
>>  arch/x86/kernel/cpu/resctrl/internal.h    |    1 +
>>  arch/x86/kernel/cpu/resctrl/rdtgroup.c    |    8 ++++--
>>  4 files changed, 41 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
>> index e31c98e2fafc..6571d08e2b0d 100644
>> --- a/arch/x86/kernel/cpu/resctrl/core.c
>> +++ b/arch/x86/kernel/cpu/resctrl/core.c
>> @@ -162,6 +162,13 @@ bool is_mba_sc(struct rdt_resource *r)
>>  	if (!r)
>>  		return rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl.membw.mba_sc;
>>  
>> +	/*
>> +	 * The software controller support is only applicable to MBA resource.
>> +	 * Make sure to check for resource type again.
>> +	 */
> /again/d
>
> Not all callers of is_mba_sc() check if it is called for an MBA resource.
>
>> +	if (r->rid != RDT_RESOURCE_MBA)
>> +		return false;
>> +
>>  	return r->membw.mba_sc;
>>  }
>>  
>> @@ -225,9 +232,15 @@ static bool __rdt_get_mem_config_amd(struct rdt_resource *r)
>>  	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
>>  	union cpuid_0x10_3_eax eax;
>>  	union cpuid_0x10_x_edx edx;
>> -	u32 ebx, ecx;
>> +	u32 ebx, ecx, subleaf;
>>  
>> -	cpuid_count(0x80000020, 1, &eax.full, &ebx, &ecx, &edx.full);
>> +	/*
>> +	 * Query CPUID_Fn80000020_EDX_x01 for MBA and
>> +	 * CPUID_Fn80000020_EDX_x02 for SMBA
>> +	 */
>> +	subleaf = (r->rid == RDT_RESOURCE_SMBA) ? 2 :  1;
>> +
>> +	cpuid_count(0x80000020, subleaf, &eax.full, &ebx, &ecx, &edx.full);
>>  	hw_res->num_closid = edx.split.cos_max + 1;
>>  	r->default_ctrl = MAX_MBA_BW_AMD;
>>  
>> @@ -750,6 +763,19 @@ static __init bool get_mem_config(void)
>>  	return false;
>>  }
>>  
>> +static __init bool get_slow_mem_config(void)
>> +{
>> +	struct rdt_hw_resource *hw_res = &rdt_resources_all[RDT_RESOURCE_SMBA];
>> +
>> +	if (!rdt_cpu_has(X86_FEATURE_SMBA))
>> +		return false;
>> +
>> +	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
>> +		return __rdt_get_mem_config_amd(&hw_res->r_resctrl);
>> +
>> +	return false;
>> +}
>> +
>>  static __init bool get_rdt_alloc_resources(void)
>>  {
>>  	struct rdt_resource *r;
>> @@ -780,6 +806,9 @@ static __init bool get_rdt_alloc_resources(void)
>>  	if (get_mem_config())
>>  		ret = true;
>>  
>> +	if (get_slow_mem_config())
>> +		ret = true;
>> +
>>  	return ret;
>>  }
>>  
>> @@ -869,6 +898,9 @@ static __init void rdt_init_res_defs_amd(void)
>>  		} else if (r->rid == RDT_RESOURCE_MBA) {
>>  			hw_res->msr_base = MSR_IA32_MBA_BW_BASE;
>>  			hw_res->msr_update = mba_wrmsr_amd;
>> +		} else if (r->rid == RDT_RESOURCE_SMBA) {
>> +			hw_res->msr_base = MSR_IA32_SMBA_BW_BASE;
>> +			hw_res->msr_update = mba_wrmsr_amd;
>>  		}
>>  	}
>>  }
> I mentioned earlier that this can be moved to init of
> rdt_resources_all[]. No strong preference, leaving here works
> also.

I am little confused about this comment. Initialization of
rdt_resources_all in core.c is mostly generic initialization. The msr_base
and msr_update routines here are vendor specific. I would prefer to keep
this in

rdt_init_res_defs_amd.Is that ok?

Thanks
Babu
Reinette Chatre Nov. 30, 2022, 8:07 p.m. UTC | #4 
Hi Babu,

On 11/30/2022 10:43 AM, Moger, Babu wrote:
> On 11/22/22 18:12, Reinette Chatre wrote:
>> On 11/4/2022 1:00 PM, Babu Moger wrote:
>>> The QoS slow memory configuration details are available via
>>> CPUID_Fn80000020_EDX_x02. Detect the available details and
>>> initialize the rest to defaults.
>>>
>>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>>> ---
>>>  arch/x86/kernel/cpu/resctrl/core.c        |   36 +++++++++++++++++++++++++++--
>>>  arch/x86/kernel/cpu/resctrl/ctrlmondata.c |    2 +-
>>>  arch/x86/kernel/cpu/resctrl/internal.h    |    1 +
>>>  arch/x86/kernel/cpu/resctrl/rdtgroup.c    |    8 ++++--
>>>  4 files changed, 41 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
>>> index e31c98e2fafc..6571d08e2b0d 100644
>>> --- a/arch/x86/kernel/cpu/resctrl/core.c
>>> +++ b/arch/x86/kernel/cpu/resctrl/core.c
>>> @@ -162,6 +162,13 @@ bool is_mba_sc(struct rdt_resource *r)
>>>  	if (!r)
>>>  		return rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl.membw.mba_sc;
>>>  
>>> +	/*
>>> +	 * The software controller support is only applicable to MBA resource.
>>> +	 * Make sure to check for resource type again.
>>> +	 */
>> /again/d
>>
>> Not all callers of is_mba_sc() check if it is called for an MBA resource.
>>
>>> +	if (r->rid != RDT_RESOURCE_MBA)
>>> +		return false;
>>> +
>>>  	return r->membw.mba_sc;
>>>  }
>>>  
>>> @@ -225,9 +232,15 @@ static bool __rdt_get_mem_config_amd(struct rdt_resource *r)
>>>  	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
>>>  	union cpuid_0x10_3_eax eax;
>>>  	union cpuid_0x10_x_edx edx;
>>> -	u32 ebx, ecx;
>>> +	u32 ebx, ecx, subleaf;
>>>  
>>> -	cpuid_count(0x80000020, 1, &eax.full, &ebx, &ecx, &edx.full);
>>> +	/*
>>> +	 * Query CPUID_Fn80000020_EDX_x01 for MBA and
>>> +	 * CPUID_Fn80000020_EDX_x02 for SMBA
>>> +	 */
>>> +	subleaf = (r->rid == RDT_RESOURCE_SMBA) ? 2 :  1;
>>> +
>>> +	cpuid_count(0x80000020, subleaf, &eax.full, &ebx, &ecx, &edx.full);
>>>  	hw_res->num_closid = edx.split.cos_max + 1;
>>>  	r->default_ctrl = MAX_MBA_BW_AMD;
>>>  
>>> @@ -750,6 +763,19 @@ static __init bool get_mem_config(void)
>>>  	return false;
>>>  }
>>>  
>>> +static __init bool get_slow_mem_config(void)
>>> +{
>>> +	struct rdt_hw_resource *hw_res = &rdt_resources_all[RDT_RESOURCE_SMBA];
>>> +
>>> +	if (!rdt_cpu_has(X86_FEATURE_SMBA))
>>> +		return false;
>>> +
>>> +	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
>>> +		return __rdt_get_mem_config_amd(&hw_res->r_resctrl);
>>> +
>>> +	return false;
>>> +}
>>> +
>>>  static __init bool get_rdt_alloc_resources(void)
>>>  {
>>>  	struct rdt_resource *r;
>>> @@ -780,6 +806,9 @@ static __init bool get_rdt_alloc_resources(void)
>>>  	if (get_mem_config())
>>>  		ret = true;
>>>  
>>> +	if (get_slow_mem_config())
>>> +		ret = true;
>>> +
>>>  	return ret;
>>>  }
>>>  
>>> @@ -869,6 +898,9 @@ static __init void rdt_init_res_defs_amd(void)
>>>  		} else if (r->rid == RDT_RESOURCE_MBA) {
>>>  			hw_res->msr_base = MSR_IA32_MBA_BW_BASE;
>>>  			hw_res->msr_update = mba_wrmsr_amd;
>>> +		} else if (r->rid == RDT_RESOURCE_SMBA) {
>>> +			hw_res->msr_base = MSR_IA32_SMBA_BW_BASE;
>>> +			hw_res->msr_update = mba_wrmsr_amd;
>>>  		}
>>>  	}
>>>  }
>> I mentioned earlier that this can be moved to init of
>> rdt_resources_all[]. No strong preference, leaving here works
>> also.
> 
> I am little confused about this comment. Initialization of
> rdt_resources_all in core.c is mostly generic initialization. The msr_base
> and msr_update routines here are vendor specific. I would prefer to keep
> this in

This is a contradiction. Yes, rdt_resources_all[] initialization in core.c
is indeed generic initialization, so why is SMBA there? If this was really
generic initialization then the entire initialization of SMBA resource
should rather move to AMD specific code.

SMBA is an AMD only feature yet its resource initialization is fragmented
with one portion treated as generic and another portion treated as vendor
specific while it all is vendor specific.

The current fragmentation is not clear to me. Keeping the initialization
as you have in patch #2 is the simplest and that is what prompted me
to suggest the move to keep initialization together at that location.

> 
> rdt_init_res_defs_amd.Is that ok?

The generic vs non-generic initialization argument is not convincing to me. 
Could you please elaborate why you prefer it this way? I already mentioned
that I do not have a strong preference but I would like to understand what
the motivation for this split initialization is.

Reinette
Moger, Babu Nov. 30, 2022, 8:40 p.m. UTC | #5 
On 11/30/22 14:07, Reinette Chatre wrote:
> Hi Babu,
>
> On 11/30/2022 10:43 AM, Moger, Babu wrote:
>> On 11/22/22 18:12, Reinette Chatre wrote:
>>> On 11/4/2022 1:00 PM, Babu Moger wrote:
>>>> The QoS slow memory configuration details are available via
>>>> CPUID_Fn80000020_EDX_x02. Detect the available details and
>>>> initialize the rest to defaults.
>>>>
>>>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>>>> ---
>>>>  arch/x86/kernel/cpu/resctrl/core.c        |   36 +++++++++++++++++++++++++++--
>>>>  arch/x86/kernel/cpu/resctrl/ctrlmondata.c |    2 +-
>>>>  arch/x86/kernel/cpu/resctrl/internal.h    |    1 +
>>>>  arch/x86/kernel/cpu/resctrl/rdtgroup.c    |    8 ++++--
>>>>  4 files changed, 41 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
>>>> index e31c98e2fafc..6571d08e2b0d 100644
>>>> --- a/arch/x86/kernel/cpu/resctrl/core.c
>>>> +++ b/arch/x86/kernel/cpu/resctrl/core.c
>>>> @@ -162,6 +162,13 @@ bool is_mba_sc(struct rdt_resource *r)
>>>>  	if (!r)
>>>>  		return rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl.membw.mba_sc;
>>>>  
>>>> +	/*
>>>> +	 * The software controller support is only applicable to MBA resource.
>>>> +	 * Make sure to check for resource type again.
>>>> +	 */
>>> /again/d
>>>
>>> Not all callers of is_mba_sc() check if it is called for an MBA resource.
>>>
>>>> +	if (r->rid != RDT_RESOURCE_MBA)
>>>> +		return false;
>>>> +
>>>>  	return r->membw.mba_sc;
>>>>  }
>>>>  
>>>> @@ -225,9 +232,15 @@ static bool __rdt_get_mem_config_amd(struct rdt_resource *r)
>>>>  	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
>>>>  	union cpuid_0x10_3_eax eax;
>>>>  	union cpuid_0x10_x_edx edx;
>>>> -	u32 ebx, ecx;
>>>> +	u32 ebx, ecx, subleaf;
>>>>  
>>>> -	cpuid_count(0x80000020, 1, &eax.full, &ebx, &ecx, &edx.full);
>>>> +	/*
>>>> +	 * Query CPUID_Fn80000020_EDX_x01 for MBA and
>>>> +	 * CPUID_Fn80000020_EDX_x02 for SMBA
>>>> +	 */
>>>> +	subleaf = (r->rid == RDT_RESOURCE_SMBA) ? 2 :  1;
>>>> +
>>>> +	cpuid_count(0x80000020, subleaf, &eax.full, &ebx, &ecx, &edx.full);
>>>>  	hw_res->num_closid = edx.split.cos_max + 1;
>>>>  	r->default_ctrl = MAX_MBA_BW_AMD;
>>>>  
>>>> @@ -750,6 +763,19 @@ static __init bool get_mem_config(void)
>>>>  	return false;
>>>>  }
>>>>  
>>>> +static __init bool get_slow_mem_config(void)
>>>> +{
>>>> +	struct rdt_hw_resource *hw_res = &rdt_resources_all[RDT_RESOURCE_SMBA];
>>>> +
>>>> +	if (!rdt_cpu_has(X86_FEATURE_SMBA))
>>>> +		return false;
>>>> +
>>>> +	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
>>>> +		return __rdt_get_mem_config_amd(&hw_res->r_resctrl);
>>>> +
>>>> +	return false;
>>>> +}
>>>> +
>>>>  static __init bool get_rdt_alloc_resources(void)
>>>>  {
>>>>  	struct rdt_resource *r;
>>>> @@ -780,6 +806,9 @@ static __init bool get_rdt_alloc_resources(void)
>>>>  	if (get_mem_config())
>>>>  		ret = true;
>>>>  
>>>> +	if (get_slow_mem_config())
>>>> +		ret = true;
>>>> +
>>>>  	return ret;
>>>>  }
>>>>  
>>>> @@ -869,6 +898,9 @@ static __init void rdt_init_res_defs_amd(void)
>>>>  		} else if (r->rid == RDT_RESOURCE_MBA) {
>>>>  			hw_res->msr_base = MSR_IA32_MBA_BW_BASE;
>>>>  			hw_res->msr_update = mba_wrmsr_amd;
>>>> +		} else if (r->rid == RDT_RESOURCE_SMBA) {
>>>> +			hw_res->msr_base = MSR_IA32_SMBA_BW_BASE;
>>>> +			hw_res->msr_update = mba_wrmsr_amd;
>>>>  		}
>>>>  	}
>>>>  }
>>> I mentioned earlier that this can be moved to init of
>>> rdt_resources_all[]. No strong preference, leaving here works
>>> also.
>> I am little confused about this comment. Initialization of
>> rdt_resources_all in core.c is mostly generic initialization. The msr_base
>> and msr_update routines here are vendor specific. I would prefer to keep
>> this in
> This is a contradiction. Yes, rdt_resources_all[] initialization in core.c
> is indeed generic initialization, so why is SMBA there? If this was really
> generic initialization then the entire initialization of SMBA resource
> should rather move to AMD specific code.
>
> SMBA is an AMD only feature yet its resource initialization is fragmented
> with one portion treated as generic and another portion treated as vendor
> specific while it all is vendor specific.
>
> The current fragmentation is not clear to me. Keeping the initialization
> as you have in patch #2 is the simplest and that is what prompted me
> to suggest the move to keep initialization together at that location.
>
>> rdt_init_res_defs_amd.Is that ok?
> The generic vs non-generic initialization argument is not convincing to me. 
> Could you please elaborate why you prefer it this way? I already mentioned
> that I do not have a strong preference but I would like to understand what
> the motivation for this split initialization is.
>
I dont have any strong argument. I was thinking, in case Intel supports
this resource in the future then they only have to change
rdt_init_res_defs_intel.

Thanks

Babu
Reinette Chatre Dec. 1, 2022, 12:35 a.m. UTC | #6 
Hi Babu,

On 11/30/2022 12:40 PM, Moger, Babu wrote:
> On 11/30/22 14:07, Reinette Chatre wrote:
>> On 11/30/2022 10:43 AM, Moger, Babu wrote:
>>> On 11/22/22 18:12, Reinette Chatre wrote:
>>>> On 11/4/2022 1:00 PM, Babu Moger wrote:
>>>>> The QoS slow memory configuration details are available via
>>>>> CPUID_Fn80000020_EDX_x02. Detect the available details and
>>>>> initialize the rest to defaults.
>>>>>
>>>>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>>>>> ---
>>>>>  arch/x86/kernel/cpu/resctrl/core.c        |   36 +++++++++++++++++++++++++++--
>>>>>  arch/x86/kernel/cpu/resctrl/ctrlmondata.c |    2 +-
>>>>>  arch/x86/kernel/cpu/resctrl/internal.h    |    1 +
>>>>>  arch/x86/kernel/cpu/resctrl/rdtgroup.c    |    8 ++++--
>>>>>  4 files changed, 41 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
>>>>> index e31c98e2fafc..6571d08e2b0d 100644
>>>>> --- a/arch/x86/kernel/cpu/resctrl/core.c
>>>>> +++ b/arch/x86/kernel/cpu/resctrl/core.c
>>>>> @@ -162,6 +162,13 @@ bool is_mba_sc(struct rdt_resource *r)
>>>>>  	if (!r)
>>>>>  		return rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl.membw.mba_sc;
>>>>>  
>>>>> +	/*
>>>>> +	 * The software controller support is only applicable to MBA resource.
>>>>> +	 * Make sure to check for resource type again.
>>>>> +	 */
>>>> /again/d
>>>>
>>>> Not all callers of is_mba_sc() check if it is called for an MBA resource.
>>>>
>>>>> +	if (r->rid != RDT_RESOURCE_MBA)
>>>>> +		return false;
>>>>> +
>>>>>  	return r->membw.mba_sc;
>>>>>  }
>>>>>  
>>>>> @@ -225,9 +232,15 @@ static bool __rdt_get_mem_config_amd(struct rdt_resource *r)
>>>>>  	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
>>>>>  	union cpuid_0x10_3_eax eax;
>>>>>  	union cpuid_0x10_x_edx edx;
>>>>> -	u32 ebx, ecx;
>>>>> +	u32 ebx, ecx, subleaf;
>>>>>  
>>>>> -	cpuid_count(0x80000020, 1, &eax.full, &ebx, &ecx, &edx.full);
>>>>> +	/*
>>>>> +	 * Query CPUID_Fn80000020_EDX_x01 for MBA and
>>>>> +	 * CPUID_Fn80000020_EDX_x02 for SMBA
>>>>> +	 */
>>>>> +	subleaf = (r->rid == RDT_RESOURCE_SMBA) ? 2 :  1;
>>>>> +
>>>>> +	cpuid_count(0x80000020, subleaf, &eax.full, &ebx, &ecx, &edx.full);
>>>>>  	hw_res->num_closid = edx.split.cos_max + 1;
>>>>>  	r->default_ctrl = MAX_MBA_BW_AMD;
>>>>>  
>>>>> @@ -750,6 +763,19 @@ static __init bool get_mem_config(void)
>>>>>  	return false;
>>>>>  }
>>>>>  
>>>>> +static __init bool get_slow_mem_config(void)
>>>>> +{
>>>>> +	struct rdt_hw_resource *hw_res = &rdt_resources_all[RDT_RESOURCE_SMBA];
>>>>> +
>>>>> +	if (!rdt_cpu_has(X86_FEATURE_SMBA))
>>>>> +		return false;
>>>>> +
>>>>> +	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
>>>>> +		return __rdt_get_mem_config_amd(&hw_res->r_resctrl);
>>>>> +
>>>>> +	return false;
>>>>> +}
>>>>> +
>>>>>  static __init bool get_rdt_alloc_resources(void)
>>>>>  {
>>>>>  	struct rdt_resource *r;
>>>>> @@ -780,6 +806,9 @@ static __init bool get_rdt_alloc_resources(void)
>>>>>  	if (get_mem_config())
>>>>>  		ret = true;
>>>>>  
>>>>> +	if (get_slow_mem_config())
>>>>> +		ret = true;
>>>>> +
>>>>>  	return ret;
>>>>>  }
>>>>>  
>>>>> @@ -869,6 +898,9 @@ static __init void rdt_init_res_defs_amd(void)
>>>>>  		} else if (r->rid == RDT_RESOURCE_MBA) {
>>>>>  			hw_res->msr_base = MSR_IA32_MBA_BW_BASE;
>>>>>  			hw_res->msr_update = mba_wrmsr_amd;
>>>>> +		} else if (r->rid == RDT_RESOURCE_SMBA) {
>>>>> +			hw_res->msr_base = MSR_IA32_SMBA_BW_BASE;
>>>>> +			hw_res->msr_update = mba_wrmsr_amd;
>>>>>  		}
>>>>>  	}
>>>>>  }
>>>> I mentioned earlier that this can be moved to init of
>>>> rdt_resources_all[]. No strong preference, leaving here works
>>>> also.
>>> I am little confused about this comment. Initialization of
>>> rdt_resources_all in core.c is mostly generic initialization. The msr_base
>>> and msr_update routines here are vendor specific. I would prefer to keep
>>> this in
>> This is a contradiction. Yes, rdt_resources_all[] initialization in core.c
>> is indeed generic initialization, so why is SMBA there? If this was really
>> generic initialization then the entire initialization of SMBA resource
>> should rather move to AMD specific code.
>>
>> SMBA is an AMD only feature yet its resource initialization is fragmented
>> with one portion treated as generic and another portion treated as vendor
>> specific while it all is vendor specific.
>>
>> The current fragmentation is not clear to me. Keeping the initialization
>> as you have in patch #2 is the simplest and that is what prompted me
>> to suggest the move to keep initialization together at that location.
>>
>>> rdt_init_res_defs_amd.Is that ok?
>> The generic vs non-generic initialization argument is not convincing to me. 
>> Could you please elaborate why you prefer it this way? I already mentioned
>> that I do not have a strong preference but I would like to understand what
>> the motivation for this split initialization is.
>>
> I dont have any strong argument. I was thinking, in case Intel supports
> this resource in the future then they only have to change
> rdt_init_res_defs_intel.

I agree that this is not a strong argument. If this happens then Intel can split
the initialization also. This is also not the only bits that would need
changing since only __rdt_get_mem_config_amd() can initialize an SMBA
resource.

It does not sound like there is a clear winner. To answer your earlier question
more succinctly, yes, from my perspective you can keep the change to
rdt_init_res_defs_amd(). At least with this change things would be more
familiar between MBA and SMBA and it will be obvious that SMBA is not
supported by Intel.

Reinette
Moger, Babu Dec. 1, 2022, 1:56 p.m. UTC | #7 
[AMD Official Use Only - General]

Hi Reinette,

> -----Original Message-----
> From: Reinette Chatre <reinette.chatre@intel.com>
> Sent: Wednesday, November 30, 2022 6:36 PM
> To: Moger, Babu <Babu.Moger@amd.com>; corbet@lwn.net;
> tglx@linutronix.de; mingo@redhat.com; bp@alien8.de
> Cc: fenghua.yu@intel.com; dave.hansen@linux.intel.com; x86@kernel.org;
> hpa@zytor.com; paulmck@kernel.org; akpm@linux-foundation.org;
> quic_neeraju@quicinc.com; rdunlap@infradead.org;
> damien.lemoal@opensource.wdc.com; songmuchun@bytedance.com;
> peterz@infradead.org; jpoimboe@kernel.org; pbonzini@redhat.com;
> chang.seok.bae@intel.com; pawan.kumar.gupta@linux.intel.com;
> jmattson@google.com; daniel.sneddon@linux.intel.com; Das1, Sandipan
> <Sandipan.Das@amd.com>; tony.luck@intel.com; james.morse@arm.com;
> linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org;
> bagasdotme@gmail.com; eranian@google.com
> Subject: Re: [PATCH v8 05/13] x86/resctrl: Detect and configure Slow Memory
> Bandwidth Allocation
> 
> Hi Babu,
> 
> On 11/30/2022 12:40 PM, Moger, Babu wrote:
> > On 11/30/22 14:07, Reinette Chatre wrote:
> >> On 11/30/2022 10:43 AM, Moger, Babu wrote:
> >>> On 11/22/22 18:12, Reinette Chatre wrote:
> >>>> On 11/4/2022 1:00 PM, Babu Moger wrote:
> >>>>> The QoS slow memory configuration details are available via
> >>>>> CPUID_Fn80000020_EDX_x02. Detect the available details and
> >>>>> initialize the rest to defaults.
> >>>>>
> >>>>> Signed-off-by: Babu Moger <babu.moger@amd.com>
> >>>>> ---
> >>>>>  arch/x86/kernel/cpu/resctrl/core.c        |   36
> +++++++++++++++++++++++++++--
> >>>>>  arch/x86/kernel/cpu/resctrl/ctrlmondata.c |    2 +-
> >>>>>  arch/x86/kernel/cpu/resctrl/internal.h    |    1 +
> >>>>>  arch/x86/kernel/cpu/resctrl/rdtgroup.c    |    8 ++++--
> >>>>>  4 files changed, 41 insertions(+), 6 deletions(-)
> >>>>>
> >>>>> diff --git a/arch/x86/kernel/cpu/resctrl/core.c
> >>>>> b/arch/x86/kernel/cpu/resctrl/core.c
> >>>>> index e31c98e2fafc..6571d08e2b0d 100644
> >>>>> --- a/arch/x86/kernel/cpu/resctrl/core.c
> >>>>> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> >>>>> @@ -162,6 +162,13 @@ bool is_mba_sc(struct rdt_resource *r)
> >>>>>  	if (!r)
> >>>>>  		return
> >>>>> rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl.membw.mba_sc;
> >>>>>
> >>>>> +	/*
> >>>>> +	 * The software controller support is only applicable to MBA
> resource.
> >>>>> +	 * Make sure to check for resource type again.
> >>>>> +	 */
> >>>> /again/d
> >>>>
> >>>> Not all callers of is_mba_sc() check if it is called for an MBA resource.
> >>>>
> >>>>> +	if (r->rid != RDT_RESOURCE_MBA)
> >>>>> +		return false;
> >>>>> +
> >>>>>  	return r->membw.mba_sc;
> >>>>>  }
> >>>>>
> >>>>> @@ -225,9 +232,15 @@ static bool __rdt_get_mem_config_amd(struct
> rdt_resource *r)
> >>>>>  	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
> >>>>>  	union cpuid_0x10_3_eax eax;
> >>>>>  	union cpuid_0x10_x_edx edx;
> >>>>> -	u32 ebx, ecx;
> >>>>> +	u32 ebx, ecx, subleaf;
> >>>>>
> >>>>> -	cpuid_count(0x80000020, 1, &eax.full, &ebx, &ecx, &edx.full);
> >>>>> +	/*
> >>>>> +	 * Query CPUID_Fn80000020_EDX_x01 for MBA and
> >>>>> +	 * CPUID_Fn80000020_EDX_x02 for SMBA
> >>>>> +	 */
> >>>>> +	subleaf = (r->rid == RDT_RESOURCE_SMBA) ? 2 :  1;
> >>>>> +
> >>>>> +	cpuid_count(0x80000020, subleaf, &eax.full, &ebx, &ecx,
> >>>>> +&edx.full);
> >>>>>  	hw_res->num_closid = edx.split.cos_max + 1;
> >>>>>  	r->default_ctrl = MAX_MBA_BW_AMD;
> >>>>>
> >>>>> @@ -750,6 +763,19 @@ static __init bool get_mem_config(void)
> >>>>>  	return false;
> >>>>>  }
> >>>>>
> >>>>> +static __init bool get_slow_mem_config(void) {
> >>>>> +	struct rdt_hw_resource *hw_res =
> >>>>> +&rdt_resources_all[RDT_RESOURCE_SMBA];
> >>>>> +
> >>>>> +	if (!rdt_cpu_has(X86_FEATURE_SMBA))
> >>>>> +		return false;
> >>>>> +
> >>>>> +	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
> >>>>> +		return __rdt_get_mem_config_amd(&hw_res-
> >r_resctrl);
> >>>>> +
> >>>>> +	return false;
> >>>>> +}
> >>>>> +
> >>>>>  static __init bool get_rdt_alloc_resources(void)  {
> >>>>>  	struct rdt_resource *r;
> >>>>> @@ -780,6 +806,9 @@ static __init bool get_rdt_alloc_resources(void)
> >>>>>  	if (get_mem_config())
> >>>>>  		ret = true;
> >>>>>
> >>>>> +	if (get_slow_mem_config())
> >>>>> +		ret = true;
> >>>>> +
> >>>>>  	return ret;
> >>>>>  }
> >>>>>
> >>>>> @@ -869,6 +898,9 @@ static __init void rdt_init_res_defs_amd(void)
> >>>>>  		} else if (r->rid == RDT_RESOURCE_MBA) {
> >>>>>  			hw_res->msr_base = MSR_IA32_MBA_BW_BASE;
> >>>>>  			hw_res->msr_update = mba_wrmsr_amd;
> >>>>> +		} else if (r->rid == RDT_RESOURCE_SMBA) {
> >>>>> +			hw_res->msr_base =
> MSR_IA32_SMBA_BW_BASE;
> >>>>> +			hw_res->msr_update = mba_wrmsr_amd;
> >>>>>  		}
> >>>>>  	}
> >>>>>  }
> >>>> I mentioned earlier that this can be moved to init of
> >>>> rdt_resources_all[]. No strong preference, leaving here works also.
> >>> I am little confused about this comment. Initialization of
> >>> rdt_resources_all in core.c is mostly generic initialization. The
> >>> msr_base and msr_update routines here are vendor specific. I would
> >>> prefer to keep this in
> >> This is a contradiction. Yes, rdt_resources_all[] initialization in
> >> core.c is indeed generic initialization, so why is SMBA there? If
> >> this was really generic initialization then the entire initialization
> >> of SMBA resource should rather move to AMD specific code.
> >>
> >> SMBA is an AMD only feature yet its resource initialization is
> >> fragmented with one portion treated as generic and another portion
> >> treated as vendor specific while it all is vendor specific.
> >>
> >> The current fragmentation is not clear to me. Keeping the
> >> initialization as you have in patch #2 is the simplest and that is
> >> what prompted me to suggest the move to keep initialization together at
> that location.
> >>
> >>> rdt_init_res_defs_amd.Is that ok?
> >> The generic vs non-generic initialization argument is not convincing to me.
> >> Could you please elaborate why you prefer it this way? I already
> >> mentioned that I do not have a strong preference but I would like to
> >> understand what the motivation for this split initialization is.
> >>
> > I dont have any strong argument. I was thinking, in case Intel
> > supports this resource in the future then they only have to change
> > rdt_init_res_defs_intel.
> 
> I agree that this is not a strong argument. If this happens then Intel can split the
> initialization also. This is also not the only bits that would need changing since
> only __rdt_get_mem_config_amd() can initialize an SMBA resource.
> 
> It does not sound like there is a clear winner. To answer your earlier question
> more succinctly, yes, from my perspective you can keep the change to
> rdt_init_res_defs_amd(). At least with this change things would be more
> familiar between MBA and SMBA and it will be obvious that SMBA is not
> supported by Intel.

Will do. Thanks
Babu
diff mbox series

Patch

diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index e31c98e2fafc..6571d08e2b0d 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -162,6 +162,13 @@  bool is_mba_sc(struct rdt_resource *r)
 	if (!r)
 		return rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl.membw.mba_sc;
 
+	/*
+	 * The software controller support is only applicable to MBA resource.
+	 * Make sure to check for resource type again.
+	 */
+	if (r->rid != RDT_RESOURCE_MBA)
+		return false;
+
 	return r->membw.mba_sc;
 }
 
@@ -225,9 +232,15 @@  static bool __rdt_get_mem_config_amd(struct rdt_resource *r)
 	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
 	union cpuid_0x10_3_eax eax;
 	union cpuid_0x10_x_edx edx;
-	u32 ebx, ecx;
+	u32 ebx, ecx, subleaf;
 
-	cpuid_count(0x80000020, 1, &eax.full, &ebx, &ecx, &edx.full);
+	/*
+	 * Query CPUID_Fn80000020_EDX_x01 for MBA and
+	 * CPUID_Fn80000020_EDX_x02 for SMBA
+	 */
+	subleaf = (r->rid == RDT_RESOURCE_SMBA) ? 2 :  1;
+
+	cpuid_count(0x80000020, subleaf, &eax.full, &ebx, &ecx, &edx.full);
 	hw_res->num_closid = edx.split.cos_max + 1;
 	r->default_ctrl = MAX_MBA_BW_AMD;
 
@@ -750,6 +763,19 @@  static __init bool get_mem_config(void)
 	return false;
 }
 
+static __init bool get_slow_mem_config(void)
+{
+	struct rdt_hw_resource *hw_res = &rdt_resources_all[RDT_RESOURCE_SMBA];
+
+	if (!rdt_cpu_has(X86_FEATURE_SMBA))
+		return false;
+
+	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
+		return __rdt_get_mem_config_amd(&hw_res->r_resctrl);
+
+	return false;
+}
+
 static __init bool get_rdt_alloc_resources(void)
 {
 	struct rdt_resource *r;
@@ -780,6 +806,9 @@  static __init bool get_rdt_alloc_resources(void)
 	if (get_mem_config())
 		ret = true;
 
+	if (get_slow_mem_config())
+		ret = true;
+
 	return ret;
 }
 
@@ -869,6 +898,9 @@  static __init void rdt_init_res_defs_amd(void)
 		} else if (r->rid == RDT_RESOURCE_MBA) {
 			hw_res->msr_base = MSR_IA32_MBA_BW_BASE;
 			hw_res->msr_update = mba_wrmsr_amd;
+		} else if (r->rid == RDT_RESOURCE_SMBA) {
+			hw_res->msr_base = MSR_IA32_SMBA_BW_BASE;
+			hw_res->msr_update = mba_wrmsr_amd;
 		}
 	}
 }
diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
index 1df0e3262bca..2dd4b8c47f23 100644
--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
@@ -209,7 +209,7 @@  static int parse_line(char *line, struct resctrl_schema *s,
 	unsigned long dom_id;
 
 	if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP &&
-	    r->rid == RDT_RESOURCE_MBA) {
+	    (r->rid == RDT_RESOURCE_MBA || r->rid == RDT_RESOURCE_SMBA)) {
 		rdt_last_cmd_puts("Cannot pseudo-lock MBA resource\n");
 		return -EINVAL;
 	}
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 43d9f6f5a931..16e3c6e03c79 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -14,6 +14,7 @@ 
 #define MSR_IA32_L2_CBM_BASE		0xd10
 #define MSR_IA32_MBA_THRTL_BASE		0xd50
 #define MSR_IA32_MBA_BW_BASE		0xc0000200
+#define MSR_IA32_SMBA_BW_BASE		0xc0000280
 
 #define MSR_IA32_QM_CTR			0x0c8e
 #define MSR_IA32_QM_EVTSEL		0x0c8d
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index e5a48f05e787..8a3dafc0dbf7 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1213,7 +1213,7 @@  static bool rdtgroup_mode_test_exclusive(struct rdtgroup *rdtgrp)
 
 	list_for_each_entry(s, &resctrl_schema_all, list) {
 		r = s->res;
-		if (r->rid == RDT_RESOURCE_MBA)
+		if (r->rid == RDT_RESOURCE_MBA || r->rid == RDT_RESOURCE_SMBA)
 			continue;
 		has_cache = true;
 		list_for_each_entry(d, &r->domains, list) {
@@ -1402,7 +1402,8 @@  static int rdtgroup_size_show(struct kernfs_open_file *of,
 					ctrl = resctrl_arch_get_config(r, d,
 								       closid,
 								       type);
-				if (r->rid == RDT_RESOURCE_MBA)
+				if (r->rid == RDT_RESOURCE_MBA ||
+				    r->rid == RDT_RESOURCE_SMBA)
 					size = ctrl;
 				else
 					size = rdtgroup_cbm_to_size(r, d, ctrl);
@@ -2845,7 +2846,8 @@  static int rdtgroup_init_alloc(struct rdtgroup *rdtgrp)
 
 	list_for_each_entry(s, &resctrl_schema_all, list) {
 		r = s->res;
-		if (r->rid == RDT_RESOURCE_MBA) {
+		if (r->rid == RDT_RESOURCE_MBA ||
+		    r->rid == RDT_RESOURCE_SMBA) {
 			rdtgroup_init_mba(r, rdtgrp->closid);
 			if (is_mba_sc(r))
 				continue;