cpu_llc_shared_map is always declared and defined, but populated in
arch/x86/kernel/smpboot.c which only builds for CONFIG_SMP=y. For
UniProcessor builds this mask is never populated and hence invalid.
Current code doesn't handle the case of AMD UniProcessor correctly,
which results "shared_cpu_map" and "shared_cpu_list" files missing from
sysfs entries for l3 cache. This patch fixes this issue.
This code used to work because of a another bug in 'cpumask_next',
where in the CONFIG_SMP=n case the cpumask_next() ignores empty mask
that results as if CPU 0 was set. This bug in 'cpumask_next' was fixed by
following commit, which exposes the cpu_llc_shared_map bug.
b81dce77ced ("cpumask: Fix invalid uniprocessor mask assumption")
Fixes: 2b83809a5e ("x86/cpu/amd: Derive L3 shared_cpu_map from cpu_llc_shared_mask")
Signed-off-by: Saurabh Sengar <[email protected]>
---
arch/x86/kernel/cpu/cacheinfo.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/cacheinfo.c b/arch/x86/kernel/cpu/cacheinfo.c
index 66556833d7af..8753bf33fec4 100644
--- a/arch/x86/kernel/cpu/cacheinfo.c
+++ b/arch/x86/kernel/cpu/cacheinfo.c
@@ -889,10 +889,12 @@ static int __cache_amd_cpumap_setup(unsigned int cpu, int index,
int i, sibling;
/*
- * For L3, always use the pre-calculated cpu_llc_shared_mask
- * to derive shared_cpu_map.
+ * For L3, in case of SMP systems use the pre-calculated
+ * cpu_llc_shared_mask to derive shared_cpu_map. In case
+ * of UP simply set the only cpu in mask.
*/
if (index == 3) {
+#ifdef CONFIG_SMP
for_each_cpu(i, cpu_llc_shared_mask(cpu)) {
this_cpu_ci = get_cpu_cacheinfo(i);
if (!this_cpu_ci->info_list)
@@ -905,6 +907,14 @@ static int __cache_amd_cpumap_setup(unsigned int cpu, int index,
&this_leaf->shared_cpu_map);
}
}
+#else
+ this_cpu_ci = get_cpu_cacheinfo(cpu);
+ WARN_ON(!this_cpu_ci->info_list);
+ if (!this_cpu_ci->info_list)
+ return 0;
+ this_leaf = this_cpu_ci->info_list + index;
+ cpumask_set_cpu(cpu, &this_leaf->shared_cpu_map);
+#endif
} else if (boot_cpu_has(X86_FEATURE_TOPOEXT)) {
unsigned int apicid, nshared, first, last;
--
2.25.1
From: Saurabh Sengar <[email protected]> Sent: Wednesday, August 10, 2022 9:15 AM
>
> cpu_llc_shared_map is always declared and defined, but populated in
> arch/x86/kernel/smpboot.c which only builds for CONFIG_SMP=y. For
> UniProcessor builds this mask is never populated and hence invalid.
> Current code doesn't handle the case of AMD UniProcessor correctly,
> which results "shared_cpu_map" and "shared_cpu_list" files missing from
> sysfs entries for l3 cache. This patch fixes this issue.
> This code used to work because of a another bug in 'cpumask_next',
> where in the CONFIG_SMP=n case the cpumask_next() ignores empty mask
> that results as if CPU 0 was set. This bug in 'cpumask_next' was fixed by
> following commit, which exposes the cpu_llc_shared_map bug.
>
> b81dce77ced ("cpumask: Fix invalid uniprocessor mask assumption")
>
> Fixes: 2b83809a5e ("x86/cpu/amd: Derive L3 shared_cpu_map from
> cpu_llc_shared_mask")
> Signed-off-by: Saurabh Sengar <[email protected]>
> ---
> arch/x86/kernel/cpu/cacheinfo.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/cacheinfo.c b/arch/x86/kernel/cpu/cacheinfo.c
> index 66556833d7af..8753bf33fec4 100644
> --- a/arch/x86/kernel/cpu/cacheinfo.c
> +++ b/arch/x86/kernel/cpu/cacheinfo.c
> @@ -889,10 +889,12 @@ static int __cache_amd_cpumap_setup(unsigned int cpu, int
> index,
> int i, sibling;
>
> /*
> - * For L3, always use the pre-calculated cpu_llc_shared_mask
> - * to derive shared_cpu_map.
> + * For L3, in case of SMP systems use the pre-calculated
> + * cpu_llc_shared_mask to derive shared_cpu_map. In case
> + * of UP simply set the only cpu in mask.
> */
> if (index == 3) {
> +#ifdef CONFIG_SMP
> for_each_cpu(i, cpu_llc_shared_mask(cpu)) {
> this_cpu_ci = get_cpu_cacheinfo(i);
> if (!this_cpu_ci->info_list)
> @@ -905,6 +907,14 @@ static int __cache_amd_cpumap_setup(unsigned int cpu, int
> index,
> &this_leaf->shared_cpu_map);
> }
> }
> +#else
> + this_cpu_ci = get_cpu_cacheinfo(cpu);
> + WARN_ON(!this_cpu_ci->info_list);
> + if (!this_cpu_ci->info_list)
> + return 0;
> + this_leaf = this_cpu_ci->info_list + index;
> + cpumask_set_cpu(cpu, &this_leaf->shared_cpu_map);
> +#endif
> } else if (boot_cpu_has(X86_FEATURE_TOPOEXT)) {
> unsigned int apicid, nshared, first, last;
>
> --
> 2.25.1
Reviewed-by: Michael Kelley <[email protected]>
On Wed, Aug 10, 2022 at 09:15:15AM -0700, Saurabh Sengar wrote:
> cpu_llc_shared_map is always declared and defined, but populated in
> arch/x86/kernel/smpboot.c which only builds for CONFIG_SMP=y. For
> UniProcessor builds this mask is never populated and hence invalid.
> Current code doesn't handle the case of AMD UniProcessor correctly,
What is "AMD UniProcessor"?
> which results "shared_cpu_map" and "shared_cpu_list" files missing from
> sysfs entries for l3 cache. This patch fixes this issue.
What issue exactly?
What is the real life use case for this?
> This code used to work because of a another bug in 'cpumask_next',
> where in the CONFIG_SMP=n case the cpumask_next() ignores empty mask
> that results as if CPU 0 was set. This bug in 'cpumask_next' was fixed by
> following commit, which exposes the cpu_llc_shared_map bug.
>
> b81dce77ced ("cpumask: Fix invalid uniprocessor mask assumption")
>
> Fixes: 2b83809a5e ("x86/cpu/amd: Derive L3 shared_cpu_map from cpu_llc_shared_mask")
Add
---
[core]
abbrev = 12
[alias]
one = show -s --pretty='format:%h (\"%s\")'
---
to your git .config so that when you do
$ git one <commit id>
you can get the proper formatting and abbreviated sha1 for commits.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Thanks for review, please find my comments inline.
On Wed, Aug 17, 2022 at 09:40:29PM +0200, Borislav Petkov wrote:
> On Wed, Aug 10, 2022 at 09:15:15AM -0700, Saurabh Sengar wrote:
> > cpu_llc_shared_map is always declared and defined, but populated in
> > arch/x86/kernel/smpboot.c which only builds for CONFIG_SMP=y. For
> > UniProcessor builds this mask is never populated and hence invalid.
> > Current code doesn't handle the case of AMD UniProcessor correctly,
>
> What is "AMD UniProcessor"?
Shall I mention here "Non-SMP AMD processor" instead ?
>
> > which results "shared_cpu_map" and "shared_cpu_list" files missing from
> > sysfs entries for l3 cache. This patch fixes this issue.
>
> What issue exactly?
>
> What is the real life use case for this?
We observed that lscpu version 2.34 was causing segfault with latest kernel,
which motivate us to debug this issue. Root cause being shared_cpu_map file
missing. There could be more usecases where this file is getting queried for
L3 cache information but at the moment I am aware of only lscpu.
Do we need to add this info in commit.
>
> > This code used to work because of a another bug in 'cpumask_next',
> > where in the CONFIG_SMP=n case the cpumask_next() ignores empty mask
> > that results as if CPU 0 was set. This bug in 'cpumask_next' was fixed by
> > following commit, which exposes the cpu_llc_shared_map bug.
> >
> > b81dce77ced ("cpumask: Fix invalid uniprocessor mask assumption")
> >
> > Fixes: 2b83809a5e ("x86/cpu/amd: Derive L3 shared_cpu_map from cpu_llc_shared_mask")
>
> Add
>
> ---
> [core]
> abbrev = 12
>
> [alias]
> one = show -s --pretty='format:%h (\"%s\")'
> ---
>
> to your git .config so that when you do
>
> $ git one <commit id>
>
> you can get the proper formatting and abbreviated sha1 for commits.
Thanks for suggestion, will fix this in V2.
>
> Thx.
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
On Thu, Aug 18, 2022 at 01:57:47PM +0200, Borislav Petkov wrote:
> On Wed, Aug 17, 2022 at 09:52:25PM -0700, Saurabh Singh Sengar wrote:
> > Shall I mention here "Non-SMP AMD processor" instead ?
>
> You should explain what that is.
>
> Is that a CONFIG_SMP=n kernel which you boot on a AMD CPU?
Yes, thats correct.
>
> IOW, how do I reproduce the issue you're describing below, here locally?
Boot latest linux kernel on AMD CPU, having CONGIG_SMP=n.
Once booted execute lscpu (just lscpu without any argument ), this will cause segfault.
Please make a note issue is observed with 2.34 version (default lscpu version with
Ubuntu 20.04.4) of lscpu.
Also, if for the above test we take linux kernel code before below commit,
shared_cpu_map file gets created and lscpu just works fine.
b81dce77ced ("cpumask: Fix invalid uniprocessor mask assumption")
>
> Send dmesg pls.
>
dmesg :
We get only 2 lines of dmesg for this segfault.
Step 1: dmesg -c
Step 2: lscpu
Step 3: dmesg
below is the output of step 3.
Dmesg:
[ 6951.530138] lscpu[99034]: segfault at 0 ip 00005557bc9c35ba sp 00007ffced2681c8 error 4 in lscpu[5557bc9c0000+e000]
[ 6951.530145] Code: 4d 39 c2 76 33 48 c1 e9 06 49 89 c8 4c 8d 2c cd 00 00 00 00 44 89 c9 4f 8b 04 c3 83 e1 3f 4d 0f a3 c8 73 14 4c 8b 02 4d 8b 00 <4f> 8b 04 28 49 d3 e8 41 83 e0 01 44 01 c3 48 89 c1 49 39 c4 74 38
> Also, what is the use case?
>
Explained above, putting here more precise steps:
Step 1: Build linux kernel for AMD processor supporting L3 cache with CONFIG_SMP=n.
Step 2: Boot this kernel.
Step 3: Once in command prompt, enter lscpu
If lspu is version 2.34 like the one I have (default with Ubuntu 20.04.4),
segfault will be observed.
> Why would you even build with SMP off?
This is caught in our testing.
>
> Feel free to explain more verbosely what you're trying to accomplish.
>
> Thx.
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
Please let me know if any thing is unclear or need more info reproducing the scenario.
- Saurabh
On Wed, Aug 17, 2022 at 09:52:25PM -0700, Saurabh Singh Sengar wrote:
> Shall I mention here "Non-SMP AMD processor" instead ?
You should explain what that is.
Is that a CONFIG_SMP=n kernel which you boot on a AMD CPU?
IOW, how do I reproduce the issue you're describing below, here locally?
Send dmesg pls.
Also, what is the use case?
Why would you even build with SMP off?
Feel free to explain more verbosely what you're trying to accomplish.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Thu, Aug 18, 2022 at 05:29:25AM -0700, Saurabh Singh Sengar wrote:
> Boot latest linux kernel on AMD CPU, having CONGIG_SMP=n. Once booted
> execute lscpu (just lscpu without any argument ), this will cause
> segfault. Please make a note issue is observed with 2.34 version
> (default lscpu version with Ubuntu 20.04.4) of lscpu.
Does that fix it?
---
diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index 81a0211a372d..a73bced40e24 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -21,16 +21,6 @@ DECLARE_PER_CPU_READ_MOSTLY(u16, cpu_llc_id);
DECLARE_PER_CPU_READ_MOSTLY(u16, cpu_l2c_id);
DECLARE_PER_CPU_READ_MOSTLY(int, cpu_number);
-static inline struct cpumask *cpu_llc_shared_mask(int cpu)
-{
- return per_cpu(cpu_llc_shared_map, cpu);
-}
-
-static inline struct cpumask *cpu_l2c_shared_mask(int cpu)
-{
- return per_cpu(cpu_l2c_shared_map, cpu);
-}
-
DECLARE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_cpu_to_apicid);
DECLARE_EARLY_PER_CPU_READ_MOSTLY(u32, x86_cpu_to_acpiid);
DECLARE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_bios_cpu_apicid);
@@ -172,6 +162,16 @@ extern int safe_smp_processor_id(void);
# define safe_smp_processor_id() smp_processor_id()
#endif
+static inline struct cpumask *cpu_llc_shared_mask(int cpu)
+{
+ return per_cpu(cpu_llc_shared_map, cpu);
+}
+
+static inline struct cpumask *cpu_l2c_shared_mask(int cpu)
+{
+ return per_cpu(cpu_l2c_shared_map, cpu);
+}
+
#else /* !CONFIG_SMP */
#define wbinvd_on_cpu(cpu) wbinvd()
static inline int wbinvd_on_all_cpus(void)
@@ -179,6 +179,11 @@ static inline int wbinvd_on_all_cpus(void)
wbinvd();
return 0;
}
+
+static inline struct cpumask *cpu_llc_shared_mask(int cpu)
+{
+ return (struct cpumask *)cpumask_of(0);
+}
#endif /* CONFIG_SMP */
extern unsigned disabled_cpus;
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Fri, Aug 19, 2022 at 07:16:13PM +0200, Borislav Petkov wrote:
> On Thu, Aug 18, 2022 at 05:29:25AM -0700, Saurabh Singh Sengar wrote:
> > Boot latest linux kernel on AMD CPU, having CONGIG_SMP=n. Once booted
> > execute lscpu (just lscpu without any argument ), this will cause
> > segfault. Please make a note issue is observed with 2.34 version
> > (default lscpu version with Ubuntu 20.04.4) of lscpu.
>
> Does that fix it?
Yes, this patch fixes this segfault.
- Saurabh
>
> ---
> diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
> index 81a0211a372d..a73bced40e24 100644
> --- a/arch/x86/include/asm/smp.h
> +++ b/arch/x86/include/asm/smp.h
> @@ -21,16 +21,6 @@ DECLARE_PER_CPU_READ_MOSTLY(u16, cpu_llc_id);
> DECLARE_PER_CPU_READ_MOSTLY(u16, cpu_l2c_id);
> DECLARE_PER_CPU_READ_MOSTLY(int, cpu_number);
>
> -static inline struct cpumask *cpu_llc_shared_mask(int cpu)
> -{
> - return per_cpu(cpu_llc_shared_map, cpu);
> -}
> -
> -static inline struct cpumask *cpu_l2c_shared_mask(int cpu)
> -{
> - return per_cpu(cpu_l2c_shared_map, cpu);
> -}
> -
> DECLARE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_cpu_to_apicid);
> DECLARE_EARLY_PER_CPU_READ_MOSTLY(u32, x86_cpu_to_acpiid);
> DECLARE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_bios_cpu_apicid);
> @@ -172,6 +162,16 @@ extern int safe_smp_processor_id(void);
> # define safe_smp_processor_id() smp_processor_id()
> #endif
>
> +static inline struct cpumask *cpu_llc_shared_mask(int cpu)
> +{
> + return per_cpu(cpu_llc_shared_map, cpu);
> +}
> +
> +static inline struct cpumask *cpu_l2c_shared_mask(int cpu)
> +{
> + return per_cpu(cpu_l2c_shared_map, cpu);
> +}
> +
> #else /* !CONFIG_SMP */
> #define wbinvd_on_cpu(cpu) wbinvd()
> static inline int wbinvd_on_all_cpus(void)
> @@ -179,6 +179,11 @@ static inline int wbinvd_on_all_cpus(void)
> wbinvd();
> return 0;
> }
> +
> +static inline struct cpumask *cpu_llc_shared_mask(int cpu)
> +{
> + return (struct cpumask *)cpumask_of(0);
> +}
> #endif /* CONFIG_SMP */
>
> extern unsigned disabled_cpus;
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
On Fri, Aug 19, 2022 at 10:46:59AM -0700, Saurabh Singh Sengar wrote:
> Yes, this patch fixes this segfault.
Thx!
---
From: Borislav Petkov <[email protected]>
On a CONFIG_SMP=n kernel, the LLC shared mask is 0, which prevents
__cache_amd_cpumap_setup() from doing the L3 masks setup, and more
specifically from setting up the shared_cpu_map and shared_cpu_list
files in sysfs, leading to lscpu from util-linux getting confused and
segfaulting.
Add a cpu_llc_shared_mask() UP variant which returns a mask with a
single bit set, i.e., for CPU0.
Fixes: 2b83809a5e6d ("x86/cpu/amd: Derive L3 shared_cpu_map from cpu_llc_shared_mask")
Reported-by: Saurabh Sengar <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Cc: <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/smp.h | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index 81a0211a372d..a73bced40e24 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -21,16 +21,6 @@ DECLARE_PER_CPU_READ_MOSTLY(u16, cpu_llc_id);
DECLARE_PER_CPU_READ_MOSTLY(u16, cpu_l2c_id);
DECLARE_PER_CPU_READ_MOSTLY(int, cpu_number);
-static inline struct cpumask *cpu_llc_shared_mask(int cpu)
-{
- return per_cpu(cpu_llc_shared_map, cpu);
-}
-
-static inline struct cpumask *cpu_l2c_shared_mask(int cpu)
-{
- return per_cpu(cpu_l2c_shared_map, cpu);
-}
-
DECLARE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_cpu_to_apicid);
DECLARE_EARLY_PER_CPU_READ_MOSTLY(u32, x86_cpu_to_acpiid);
DECLARE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_bios_cpu_apicid);
@@ -172,6 +162,16 @@ extern int safe_smp_processor_id(void);
# define safe_smp_processor_id() smp_processor_id()
#endif
+static inline struct cpumask *cpu_llc_shared_mask(int cpu)
+{
+ return per_cpu(cpu_llc_shared_map, cpu);
+}
+
+static inline struct cpumask *cpu_l2c_shared_mask(int cpu)
+{
+ return per_cpu(cpu_l2c_shared_map, cpu);
+}
+
#else /* !CONFIG_SMP */
#define wbinvd_on_cpu(cpu) wbinvd()
static inline int wbinvd_on_all_cpus(void)
@@ -179,6 +179,11 @@ static inline int wbinvd_on_all_cpus(void)
wbinvd();
return 0;
}
+
+static inline struct cpumask *cpu_llc_shared_mask(int cpu)
+{
+ return (struct cpumask *)cpumask_of(0);
+}
#endif /* CONFIG_SMP */
extern unsigned disabled_cpus;
--
2.35.1
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: df5b035b5683d6a25f077af889fb88e09827f8bc
Gitweb: https://git.kernel.org/tip/df5b035b5683d6a25f077af889fb88e09827f8bc
Author: Borislav Petkov <[email protected]>
AuthorDate: Fri, 19 Aug 2022 19:47:44 +02:00
Committer: Borislav Petkov <[email protected]>
CommitterDate: Wed, 28 Sep 2022 18:35:37 +02:00
x86/cacheinfo: Add a cpu_llc_shared_mask() UP variant
On a CONFIG_SMP=n kernel, the LLC shared mask is 0, which prevents
__cache_amd_cpumap_setup() from doing the L3 masks setup, and more
specifically from setting up the shared_cpu_map and shared_cpu_list
files in sysfs, leading to lscpu from util-linux getting confused and
segfaulting.
Add a cpu_llc_shared_mask() UP variant which returns a mask with a
single bit set, i.e., for CPU0.
Fixes: 2b83809a5e6d ("x86/cpu/amd: Derive L3 shared_cpu_map from cpu_llc_shared_mask")
Reported-by: Saurabh Sengar <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Cc: <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/smp.h | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index 81a0211..a73bced 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -21,16 +21,6 @@ DECLARE_PER_CPU_READ_MOSTLY(u16, cpu_llc_id);
DECLARE_PER_CPU_READ_MOSTLY(u16, cpu_l2c_id);
DECLARE_PER_CPU_READ_MOSTLY(int, cpu_number);
-static inline struct cpumask *cpu_llc_shared_mask(int cpu)
-{
- return per_cpu(cpu_llc_shared_map, cpu);
-}
-
-static inline struct cpumask *cpu_l2c_shared_mask(int cpu)
-{
- return per_cpu(cpu_l2c_shared_map, cpu);
-}
-
DECLARE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_cpu_to_apicid);
DECLARE_EARLY_PER_CPU_READ_MOSTLY(u32, x86_cpu_to_acpiid);
DECLARE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_bios_cpu_apicid);
@@ -172,6 +162,16 @@ extern int safe_smp_processor_id(void);
# define safe_smp_processor_id() smp_processor_id()
#endif
+static inline struct cpumask *cpu_llc_shared_mask(int cpu)
+{
+ return per_cpu(cpu_llc_shared_map, cpu);
+}
+
+static inline struct cpumask *cpu_l2c_shared_mask(int cpu)
+{
+ return per_cpu(cpu_l2c_shared_map, cpu);
+}
+
#else /* !CONFIG_SMP */
#define wbinvd_on_cpu(cpu) wbinvd()
static inline int wbinvd_on_all_cpus(void)
@@ -179,6 +179,11 @@ static inline int wbinvd_on_all_cpus(void)
wbinvd();
return 0;
}
+
+static inline struct cpumask *cpu_llc_shared_mask(int cpu)
+{
+ return (struct cpumask *)cpumask_of(0);
+}
#endif /* CONFIG_SMP */
extern unsigned disabled_cpus;