Re: [PATCH v5 02/10] x86/resctrl: Protect against bad shift

[Borislav Petkov]

On Tue, Jul 21, 2026 at 09:14:20AM -0700, Reinette Chatre wrote:
> Hi Boris,
> 
> On 7/20/26 6:59 PM, Borislav Petkov wrote:
> > On Tue, Jun 30, 2026 at 09:27:02PM -0700, Reinette Chatre wrote:
> >> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> >> index bca782050198..9b9495174041 100644
> >> --- a/arch/x86/kernel/cpu/resctrl/core.c
> >> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> >> @@ -247,7 +247,11 @@ static __init bool __rdt_get_mem_config_amd(struct rdt_resource *r)
> >>  
> >>  	cpuid_count(0x80000020, subleaf, &eax, &ebx, &ecx, &edx);
> >>  	hw_res->num_closid = edx + 1;
> >> -	r->membw.max_bw = 1 << eax;
> >> +	if (BITS_PER_TYPE(r->membw.max_bw) <= eax) {
> >> +		pr_warn("Unable to support hardware's maximum bandwidth\n");
> >> +		return false;
> >> +	}
> >> +	r->membw.max_bw = BIT(eax);
> > 
> > Why?
> > 
> > Why go all the effort if the field can be 64 bits?
> > 
> > Why not make max_bw u64 and handle *all* possible values naturally?
> > 
> 
> resctrl does not support values larger than u32.
> 
> The value represented by max_bw provides the upper limit of control values that
> user space can configure via the schemata file, which in turn represents the
> control value programmed to hardware.
>
> u32 is expected in several places:
> - Parsing of user input in bw_validate() is done via kstrtou32() placing result in
>   "u32 bw". It is this value that is compared against max_bw and would thus need to
>   change as a consequence.
> - Parsing of user input to schemata file via parse_bw() places parsed bandwidth in
>   "u32 bw_val".
> - Upon successful parsing the control value is staged by resctrl fs in
>   resctrl_staged_config::new_ctrl that is u32.
> - On receipt of programming request from resctrl fs, the x86 architectural specific
>   code programs the control value to rdt_hw_ctrl_domain::ctrl_val[] that are u32.
> 
> resctrl, from fs to x86 architecture and now also MPAM, would thus require changes to
> support these AMD systems. If I get the math right this will be systems that support 
> 1073741823.875GB/s and up?
> 
> If resctrl is required to support AMD hardware that can return up to 64bits here
> then that requires more changes than what I am able to take on at this time. From
> my side I prefer to stick to this resctrl safeguard for the scenario and leave support
> for this hardware to AMD.

So that bandwidth limit thing is read from those L3QOS_BW_CONTROL_n MSRs which
have

BW_LEN-1:0 as the range which is the bandwidth limit, at
BW_LEN there's the unlimited bandwidth limit

and that BW_LEN is read from CPUID.

And that is, kinda 12, right now.

And your worry is that some day that limit BW_LEN might go over 31?

Which your math shows how absurd this is.

So what are we actually fixing here?

Something some AI hallucinated that it might be possible at some point in the
very distant, dark future?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette