diff mbox

x86/SMP: CPU0's scratch mask is needed earlier

Message ID 5857BA4B020000780012A761@prv-mh.provo.novell.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Beulich Dec. 19, 2016, 9:45 a.m. UTC 
When putting together commit 3b61726458 ("x86: introduce and use
scratch CPU mask") I failed to remember that AMD IOMMU setups needs the
scratch mask prior to smp_prepare_cpus() having run. Use a static mask
for the boot CPU instead.

Note that the definition of scratch_cpu0mask could also be put inside a
"NR_CPUS > 2 * BITS_PER_LONG" conditional, but it seems preferable to
me to carry the extra variable in all cases and avoid the #ifdef-ary.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
I'm not particularly happy about the remaining #ifdef, but I don't see
a way to avoid it.
x86/SMP: CPU0's scratch mask is needed earlier

When putting together commit 3b61726458 ("x86: introduce and use
scratch CPU mask") I failed to remember that AMD IOMMU setups needs the
scratch mask prior to smp_prepare_cpus() having run. Use a static mask
for the boot CPU instead.

Note that the definition of scratch_cpu0mask could also be put inside a
"NR_CPUS > 2 * BITS_PER_LONG" conditional, but it seems preferable to
me to carry the extra variable in all cases and avoid the #ifdef-ary.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
I'm not particularly happy about the remaining #ifdef, but I don't see
a way to avoid it.

--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -57,6 +57,7 @@ DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t
 DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_core_mask);
 
 DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, scratch_cpumask);
+static cpumask_t scratch_cpu0mask;
 
 cpumask_t cpu_online_map __read_mostly;
 EXPORT_SYMBOL(cpu_online_map);
@@ -648,7 +649,8 @@ static void cpu_smpboot_free(unsigned in
 
     free_cpumask_var(per_cpu(cpu_sibling_mask, cpu));
     free_cpumask_var(per_cpu(cpu_core_mask, cpu));
-    free_cpumask_var(per_cpu(scratch_cpumask, cpu));
+    if ( per_cpu(scratch_cpumask, cpu) != &scratch_cpu0mask )
+        free_cpumask_var(per_cpu(scratch_cpumask, cpu));
 
     if ( per_cpu(stubs.addr, cpu) )
     {
@@ -795,8 +797,7 @@ void __init smp_prepare_cpus(unsigned in
         panic("No memory for socket CPU siblings map");
 
     if ( !zalloc_cpumask_var(&per_cpu(cpu_sibling_mask, 0)) ||
-         !zalloc_cpumask_var(&per_cpu(cpu_core_mask, 0)) ||
-         !alloc_cpumask_var(&per_cpu(scratch_cpumask, 0)) )
+         !zalloc_cpumask_var(&per_cpu(cpu_core_mask, 0)) )
         panic("No memory for boot CPU sibling/core maps");
 
     set_cpu_sibling_map(0);
@@ -850,8 +851,13 @@ void __init smp_prepare_cpus(unsigned in
 
 void __init smp_prepare_boot_cpu(void)
 {
-    cpumask_set_cpu(smp_processor_id(), &cpu_online_map);
-    cpumask_set_cpu(smp_processor_id(), &cpu_present_map);
+    unsigned int cpu = smp_processor_id();
+
+    cpumask_set_cpu(cpu, &cpu_online_map);
+    cpumask_set_cpu(cpu, &cpu_present_map);
+#if NR_CPUS > 2 * BITS_PER_LONG
+    per_cpu(scratch_cpumask, cpu) = &scratch_cpu0mask;
+#endif
 }
 
 static void

Comments

Andrew Cooper Dec. 19, 2016, 10:38 a.m. UTC | #1 
On 19/12/16 09:45, Jan Beulich wrote:
> When putting together commit 3b61726458 ("x86: introduce and use
> scratch CPU mask") I failed to remember that AMD IOMMU setups needs the
> scratch mask prior to smp_prepare_cpus() having run. Use a static mask
> for the boot CPU instead.
>
> Note that the definition of scratch_cpu0mask could also be put inside a
> "NR_CPUS > 2 * BITS_PER_LONG" conditional, but it seems preferable to
> me to carry the extra variable in all cases and avoid the #ifdef-ary.
>
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> I'm not particularly happy about the remaining #ifdef, but I don't see
> a way to avoid it.

Nor me.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Lets unblock staging while considering if there is a better way of doing
this.

~Andrew
diff mbox

Patch

--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -57,6 +57,7 @@  DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t
 DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_core_mask);
 
 DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, scratch_cpumask);
+static cpumask_t scratch_cpu0mask;
 
 cpumask_t cpu_online_map __read_mostly;
 EXPORT_SYMBOL(cpu_online_map);
@@ -648,7 +649,8 @@  static void cpu_smpboot_free(unsigned in
 
     free_cpumask_var(per_cpu(cpu_sibling_mask, cpu));
     free_cpumask_var(per_cpu(cpu_core_mask, cpu));
-    free_cpumask_var(per_cpu(scratch_cpumask, cpu));
+    if ( per_cpu(scratch_cpumask, cpu) != &scratch_cpu0mask )
+        free_cpumask_var(per_cpu(scratch_cpumask, cpu));
 
     if ( per_cpu(stubs.addr, cpu) )
     {
@@ -795,8 +797,7 @@  void __init smp_prepare_cpus(unsigned in
         panic("No memory for socket CPU siblings map");
 
     if ( !zalloc_cpumask_var(&per_cpu(cpu_sibling_mask, 0)) ||
-         !zalloc_cpumask_var(&per_cpu(cpu_core_mask, 0)) ||
-         !alloc_cpumask_var(&per_cpu(scratch_cpumask, 0)) )
+         !zalloc_cpumask_var(&per_cpu(cpu_core_mask, 0)) )
         panic("No memory for boot CPU sibling/core maps");
 
     set_cpu_sibling_map(0);
@@ -850,8 +851,13 @@  void __init smp_prepare_cpus(unsigned in
 
 void __init smp_prepare_boot_cpu(void)
 {
-    cpumask_set_cpu(smp_processor_id(), &cpu_online_map);
-    cpumask_set_cpu(smp_processor_id(), &cpu_present_map);
+    unsigned int cpu = smp_processor_id();
+
+    cpumask_set_cpu(cpu, &cpu_online_map);
+    cpumask_set_cpu(cpu, &cpu_present_map);
+#if NR_CPUS > 2 * BITS_PER_LONG
+    per_cpu(scratch_cpumask, cpu) = &scratch_cpu0mask;
+#endif
 }
 
 static void