amd: uncore: Fix for AMD Uncore driver
Subsystems (2)
v3 cover amd: uncore: Fix for AMD Uncore driver
The following series provides fixes for the AMD Uncore driver.
* Renamed cache counters cpufeatures macro to better reflect different last level cache for different families.
* Get correct information about number of cores sharing last level of cache.
v2->v3
* Updated patch description and cleaned up code based on feedback from Borislav Petkov.
v1->v2
* Replace while(1) with finite for loop based on feedback from Peter Zijlstra and Borislav Petkov.
Janakarajan Natarajan (2):
amd: uncore: Rename cpufeatures macro for cache counters
amd: uncore: Get correct number of cores sharing last level cache
arch/x86/events/amd/uncore.c | 21 +++++++++++++++++---- arch/x86/include/asm/cpufeatures.h | 2 +- 2 files changed, 18 insertions(+), 5 deletions(-)
Signature · 1 line
2.7.4
PATCH v3 1/2 amd: uncore: Rename cpufeatures macro for cache counters
In Family 17h, L3 is the last level cache as opposed to L2 in previous families. Avoid this name confusion and rename X86_FEATURE_PERFCTR_L2 to X86_FEATURE_PERFCTR_LLC to indicate the performance counter on the last level of cache.
--- arch/x86/events/amd/uncore.c | 2 +- arch/x86/include/asm/cpufeatures.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
arch/x86/events/amd/uncore.c +1 -1
@@ -555,7 +555,7 @@ static int __init amd_uncore_init(void) ret =0 ; } -if (boot_cpu_has (X86_FEATURE_PERFCTR_L2 )) { +if (boot_cpu_has (X86_FEATURE_PERFCTR_LLC )) { amd_uncore_llc = alloc_percpu(struct amd_uncore *);if (!amd_uncore_llc) { ret = -ENOMEM ;
arch/x86/include/asm/cpufeatures.h +1 -1
@@ -177,7 +177,7 @@#define X86_FEATURE_PERFCTR_NB ( 6*32+24)/* NB performance counter extensions */ #define X86_FEATURE_BPEXT (6*32+26)/* data breakpoint extension */ #define X86_FEATURE_PTSC ( 6*32+27)/* performance time-stamp counter */ -#define X86_FEATURE_PERFCTR_L2 ( 6*32+28)/* L2 performance counter extensions */ +#define X86_FEATURE_PERFCTR_LLC ( 6*32+28)/* Last Level Cache performance counter extensions */ #define X86_FEATURE_MWAITX ( 6*32+29)/* MWAIT extension (MONITORX/MWAITX) */ /*
Signature · 1 line
2.7.4
Borislav Petkov
maintainerRe: [PATCH v3 1/2] amd: uncore: Rename cpufeatures macro for cache counters
Quoted 10 lines · Janakarajan Natarajan
In Family 17h, L3 is the last level cache as opposed to L2 in previous families. Avoid this name confusion and rename
X86_FEATURE_PERFCTR_L2toX86_FEATURE_PERFCTR_LLCto indicate the performance counter on the last level of cache.Signed-off-by Janakarajan Natarajan <[email protected]>--- arch/x86/events/amd/uncore.c | 2 +- arch/x86/include/asm/cpufeatures.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
A hint for the future:
when a reviewer gives you a Reviewed-by tag, you should add it to your when patch you submit.
Thanks.
Signature · 4 lines
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.PATCH v3 2/2 amd: uncore: Get correct number of cores sharing last level cache
In reply to Janakarajan NatarajanIn Family 17h, the number of cores sharing a cache level is obtained from the Cache Properties CPUID leaf (0x8000001d) by passing in the cache level in ECX. In prior families, a cache level of 2 was used to determine this information.
To get the right information, irrespective of Family, iterate over the cache levels using CPUID 0x8000001d. The last level cache is the last value to return a non-zero value in EAX.
--- arch/x86/events/amd/uncore.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)
arch/x86/events/amd/uncore.c +16 -3
@@ -400,11 +400,24 @@ static int amd_uncore_cpu_starting(unsigned int cpu)if (amd_uncore_llc) {unsigned int apicid =cpu_data (cpu).apicid; -unsigned int nshared; +unsigned int nshared, subleaf, prev_eax =0 ; uncore = *per_cpu_ptr (amd_uncore_llc, cpu); -cpuid_count (0x8000001d ,2 , &eax, &ebx, &ecx, &edx); - nshared = ((eax >>14 ) &0xfff ) +1 ; +/* +* Iterate over Cache Topology Definition leaves until no +* more cache descriptions are available. +*/ +for (subleaf =0 ; subleaf <5 ; subleaf++) { +cpuid_count (0x8000001d , subleaf, &eax, &ebx, &ecx, &edx); + +/* EAX[0:4] gives type of cache */ +if (!(eax &0x1f )) +break ; + + prev_eax = eax; + } + nshared = ((prev_eax >>14 ) &0xfff ) +1 ; + uncore->id = apicid - (apicid % nshared); uncore =amd_uncore_find_online_sibling (uncore, amd_uncore_llc);
Signature · 1 line
2.7.4
Borislav Petkov
maintainerRe: [PATCH v3 2/2] amd: uncore: Get correct number of cores sharing last level cache
Quoted 23 lines · Janakarajan Natarajan
In Family 17h, the number of cores sharing a cache level is obtained from the Cache Properties CPUID leaf (
0x8000001d) by passing in the cache level in ECX. In prior families, a cache level of 2 was used to determine this information.To get the right information, irrespective of Family, iterate over the cache levels using CPUID
0x8000001d. The last level cache is the last value to return a non-zero value in EAX.Signed-off-by Janakarajan Natarajan <[email protected]>--- arch/x86/events/amd/uncore.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)arch/x86/events/amd/uncore.c +0
-1@@ -400,11 +400,24 @@ static int amd_uncore_cpu_starting(unsigned int cpu)if (amd_uncore_llc) {unsigned int apicid =cpu_data (cpu).apicid; -unsigned int nshared;
arch/x86/events/amd/uncore.c static int amd_uncore_cpu_starting(unsigned int cpu) · line 410
Quoted 10 lines
arch/x86/events/amd/uncore.c +6
-2+unsigned int nshared, subleaf, prev_eax =0 ; uncore = *per_cpu_ptr (amd_uncore_llc, cpu); -cpuid_count (0x8000001d ,2 , &eax, &ebx, &ecx, &edx); - nshared = ((eax >>14 ) &0xfff ) +1 ; +/* +* Iterate over Cache Topology Definition leaves until no +* more cache descriptions are available. +*/ +for (subleaf =0 ; subleaf <5 ; subleaf++) {
ERROR: space required before the open parenthesis '(' #51: FILE: arch/x86/events/amd/uncore.c:410:
+ for(subleaf = 0; subleaf < 5; subleaf++) {
Please integrate scripts/checkpatch.pl in your patch creation workflow. Some of the warnings/errors *actually* make sense.
Thanks.
Signature · 4 lines
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.