Hi Reinetee, On 12/6/23 11:09, Reinette Chatre wrote: > 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@xxxxxxx> >>>> 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. > Yes. You are right. Our current code has the problem already. union cpuid_0x10_3_eax eax; union cpuid_0x10_x_edx edx; u32 ebx, ecx, subleaf; Will fix both. Just "u32 eax, edx" should be fine for AMD. Thanks for pointing. -- Thanks Babu Moger
