linux-kernel - Re: [PATCH 01/15] x86/resctrl: Remove hard-coded memory bandwidth limit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <68f2c64b-9d46-4077-a183-0abc21ff0535@intel.com>
Date:   Wed, 6 Dec 2023 09:09:23 -0800
From:   Reinette Chatre <reinette.chatre@...el.com>
To:     <babu.moger@....com>, <corbet@....net>, <fenghua.yu@...el.com>,
        <tglx@...utronix.de>, <mingo@...hat.com>, <bp@...en8.de>,
        <dave.hansen@...ux.intel.com>
CC:     <x86@...nel.org>, <hpa@...or.com>, <paulmck@...nel.org>,
        <rdunlap@...radead.org>, <tj@...nel.org>, <peterz@...radead.org>,
        <seanjc@...gle.com>, <kim.phillips@....com>, <jmattson@...gle.com>,
        <ilpo.jarvinen@...ux.intel.com>, <jithu.joseph@...el.com>,
        <kan.liang@...ux.intel.com>, <nikunj@....com>,
        <daniel.sneddon@...ux.intel.com>, <pbonzini@...hat.com>,
        <rick.p.edgecombe@...el.com>, <rppt@...nel.org>,
        <maciej.wieczor-retman@...el.com>, <linux-doc@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <eranian@...gle.com>,
        <peternewman@...gle.com>, <dhagiani@....com>
Subject: Re: [PATCH 01/15] x86/resctrl: Remove hard-coded memory bandwidth
 limit

Hi Babu,

On 12/6/2023 8:29 AM, Moger, Babu wrote:
> On 12/5/23 17:18, Reinette Chatre wrote:
>> On 11/30/2023 4:57 PM, Babu Moger wrote:

>>> Signed-off-by: Babu Moger <babu.moger@....com>
>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
>>> ---
>>>  arch/x86/kernel/cpu/resctrl/core.c     | 2 +-
>>>  arch/x86/kernel/cpu/resctrl/internal.h | 1 -
>>>  2 files changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
>>> index 19e0681f0435..3fbae10b662d 100644
>>> --- a/arch/x86/kernel/cpu/resctrl/core.c
>>> +++ b/arch/x86/kernel/cpu/resctrl/core.c
>>> @@ -243,7 +243,7 @@ static bool __rdt_get_mem_config_amd(struct rdt_resource *r)
>>>  
>>>  	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;
>>> +	r->default_ctrl = 1 << eax.full;
>>
>> This does not seem appropriate. You are using eax because it
>> it convenient but if you take a look at its definition it does not
>> match the AMD CPUID instruction output at all.
> 
> Not sure where you see it. Here it is.
> https://bugzilla.kernel.org/attachment.cgi?id=303986
> 
> Here is the definition.
> 
> CPUID_Fn80000020_EAX_x01 [Platform QoS Enforcement for Memory Bandwidth]
> (Core::X86::Cpuid::PqeBandwidthEax1)
> Read-only. Reset: 0000_000Bh.
> _ccd[11:0]_lthree0_core[7:0]_thread[1:0]; CPUID_Fn80000020_EAX_x01
> Bits Description
> 31:0 BW_LEN: QOS Memory Bandwidth Enforcement Limit Size. Read-only.
> Reset: 0000_000Bh. Size of the QOS Memory Bandwidth Enforcement Limit.
> 
> In this case, limit size is 12 (0BH) bits. Max limit is 1 << 12.
> 

I see it in the definition of the data type you are using. Specifically
it is:

	/* CPUID.(EAX=10H, ECX=ResID=3).EAX */
	union cpuid_0x10_3_eax {
		struct {
			unsigned int max_delay:12;
		} split;
		unsigned int full;
	};

How the kernel interprets the register does not match with what you paste
from the spec. This is an AMD specific function, __rdt_get_mem_config_amd().
Tt does not seem appropriate to use the register definition of Intel
systems if the Intel and AMD registers do not have the same format.

Reinette