Hi Reinette, On 9/29/22 16:58, Reinette Chatre wrote: > Hi Babu, > > On 9/27/2022 1:25 PM, Babu Moger wrote: >> Add the new AMD feature X86_FEATURE_SMBA. With this feature, the QOS >> enforcement policies can be applied to external slow memory connected >> to the host. QOS enforcement is accomplished by assigning a Class Of >> Service (COS) to a processor and specifying allocations or limits for >> that COS for each resource to be allocated. >> >> This feature is identified by the CPUID Function 8000_0020_EBX_x0. >> >> CPUID Fn8000_0020_EBX_x0 AMD Bandwidth Enforcement Feature Identifiers >> (ECX=0) >> >> Bits Field Name Description >> 2 L3SBE L3 external slow memory bandwidth enforcement >> >> >> Currently, CXL.memory is the only supported "slow" memory device. With >> the support of SMBA feature, the hardware enables bandwidth allocation >> on the slow memory devices. If there are multiple slow memory devices >> in the system, then the throttling logic groups all the slow sources >> together and applies the limit on them as a whole. >> >> The presence of the SMBA feature(with CXL.memory) is independent of >> whether slow memory device is actually present in the system. If there >> is no slow memory in the system, then setting a SMBA limit will have no >> impact on the performance of the system. >> >> Presence of CXL memory can be identified by numactl command. >> >> $numactl -H >> available: 2 nodes (0-1) >> node 0 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 >> node 0 size: 63678 MB node 0 free: 59542 MB >> node 1 cpus: >> node 1 size: 16122 MB >> node 1 free: 15627 MB >> node distances: >> node 0 1 >> 0: 10 50 >> 1: 50 10 >> >> CPU list for CXL memory will be empty. The cpu-cxl node distance is >> greater than cpu-to-cpu distances. Node 1 has the CXL memory in this >> case. CXL memory can also be identified using ACPI SRAT table and >> memory maps. >> >> Feature description is available in the specification, "AMD64 >> Technology Platform Quality of Service Extensions, Revision: 1.03 >> Publication # 56375 Revision: 1.03 Issue Date: February 2022". >> >> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fen%2Fsupport%2Ftech-docs%2Famd64-technology-platform-quality-service-extensions&data=05%7C01%7Cbabu.moger%40amd.com%7Cdf869c35332b477dc5e808daa265c0cd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638000855157338562%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=xFd6dFAZtT4jE9cPQ2LEkfxWAbG3ypQ0Mhl3K780YxI%3D&reserved=0 >> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D206537&data=05%7C01%7Cbabu.moger%40amd.com%7Cdf869c35332b477dc5e808daa265c0cd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638000855157338562%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=ZwBrgjVUvfriC0rM4z7XMP85CVhhI0erM%2BSFtdWa5%2B0%3D&reserved=0 >> Signed-off-by: Babu Moger <babu.moger@xxxxxxx> >> --- >> arch/x86/include/asm/cpufeatures.h | 1 + >> arch/x86/kernel/cpu/scattered.c | 1 + >> 2 files changed, 2 insertions(+) >> >> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h >> index ef4775c6db01..349852b9daa4 100644 >> --- a/arch/x86/include/asm/cpufeatures.h >> +++ b/arch/x86/include/asm/cpufeatures.h >> @@ -304,6 +304,7 @@ >> #define X86_FEATURE_UNRET (11*32+15) /* "" AMD BTB untrain return */ >> #define X86_FEATURE_USE_IBPB_FW (11*32+16) /* "" Use IBPB during runtime firmware calls */ >> #define X86_FEATURE_RSB_VMEXIT_LITE (11*32+17) /* "" Fill RSB on VM exit when EIBRS is enabled */ >> +#define X86_FEATURE_SMBA (11*32+18) /* Slow Memory Bandwidth Allocation */ >> >> /* Intel-defined CPU features, CPUID level 0x00000007:1 (EAX), word 12 */ >> #define X86_FEATURE_AVX_VNNI (12*32+ 4) /* AVX VNNI instructions */ >> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c >> index fd44b54c90d5..885ecf46abb2 100644 >> --- a/arch/x86/kernel/cpu/scattered.c >> +++ b/arch/x86/kernel/cpu/scattered.c >> @@ -44,6 +44,7 @@ static const struct cpuid_bit cpuid_bits[] = { >> { X86_FEATURE_CPB, CPUID_EDX, 9, 0x80000007, 0 }, >> { X86_FEATURE_PROC_FEEDBACK, CPUID_EDX, 11, 0x80000007, 0 }, >> { X86_FEATURE_MBA, CPUID_EBX, 6, 0x80000008, 0 }, >> + { X86_FEATURE_SMBA, CPUID_EBX, 2, 0x80000020, 0 }, >> { X86_FEATURE_PERFMON_V2, CPUID_EAX, 0, 0x80000022, 0 }, >> { 0, 0, 0, 0, 0 } >> }; >> >> > Please respect the coding style of the area you are modifying. > This is the same feedback as provided in v4 in > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2Fba36c68c-0b13-e8a2-fb45-8b84ea9f7259%40intel.com%2F&data=05%7C01%7Cbabu.moger%40amd.com%7Cdf869c35332b477dc5e808daa265c0cd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638000855157338562%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=WPFwZq1KCIhrGyGt5Qle9GYZBeXqTtHWGCTaK7vAeZY%3D&reserved=0 > > Looking ahead the same issue also remains in patch 3 as previously > mentioned in v4 feedback. Hmm.. I ran "./scripts/checkpatch.pl --strict --codespell" on all the patches. The checkpatch didn't complain about this. Now, looking at the line again, I see it should have been tabs between those texts you are referring. I will take care of it next revision. > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2Fc4a9ea23-4280-d54c-263b-354ea321f746%40intel.com%2F&data=05%7C01%7Cbabu.moger%40amd.com%7Cdf869c35332b477dc5e808daa265c0cd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638000855157338562%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=4tuK0pMrJsiW44IKCkeZV8ujS4z9STOa3mKfRKbHulk%3D&reserved=0 > > Also missing is highlighting that configuration has changed from > per-domain to per-CPU and why. Already responded about this in patch 10. Thanks Babu > > It does not seem as though this series is ready. I will wait > for next version to have existing review comments addressed before > trying to look at new changes. > > Reinette -- Thanks Babu Moger
