[PATCH v3 0/3] support booting more than 255 CPUs with QEMU

[Igor Mammedov]

Changelog since:
  v2:
    * rebase on top of current master /smp_scan() changes/
  v1:
    * s/count_cpu/apic_id_init/
    * merge handle_x2apic() into apic_id_init()
  RFC:
    * move out max-cpus check out of mptable_setup()
    * factor out CPU counting/apic ID detection in separate function
    * return back accidentially deleted debug message with APIC ID
    * drop unused code in smp_setup()

According to SDM, if CPUs have APIC ID more than 254
firmware should pass control to OS in x2APIC mode.
This series adds x2APIC bootstrap initialization.

Rebased QEMU side of x2APIC support for the next merge window:
https://www.mail-archive.com/[e-mail]/msg390671.html

Igor Mammedov (2):
  paravirt: disable legacy bios tables in case of more than 255 CPUs
  support booting with more than 255 CPUs

Kevin O'Connor (1):
  smp: refactor present CPU APIC ID detection and counting

src/x86.h         |  1 +
 src/fw/paravirt.c |  6 ++++--
 src/fw/smp.c      | 52 +++++++++++++++++++++++++++++++++++++---------------
 3 files changed, 42 insertions(+), 17 deletions(-)

[PATCH v3 1/3] paravirt: disable legacy bios tables in case of more than 255 CPUs

[Igor Mammedov]

MPTable doesn't support more than 255 CPUs and
QEMU supplies an alternative MADT table which
guest will use instead of it. So do not install
legacy tables if more than 254 CPUs are provided

Signed-off-by: Igor Mammedov 
---
 src/fw/paravirt.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
index 73a08f0..33a471b 100644
--- a/src/fw/paravirt.c
+++ b/src/fw/paravirt.c
@@ -164,8 +164,10 @@ qemu_platform_setup(void)
     smp_setup();

// Create bios tables
-    pirtable_setup();
-    mptable_setup();
+    if (MaxCountCPUs <= 255) {
+        pirtable_setup();
+        mptable_setup();
+    }
     smbios_setup();

if (CONFIG_FW_ROMFILE_LOAD) {

[PATCH v3 2/3] smp: refactor present CPU APIC ID detection and counting

[Igor Mammedov]

From: Kevin O'Connor

Signed-off-by: "Kevin O'Connor" 
Signed-off-by: Igor Mammedov 
---
v2:
  * s/count_cpu/apic_id_init/
  * call apic_id_init() after sending SIPI,
    it will be needed for switching BSP into x2APIC mode
---
 src/fw/smp.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/src/fw/smp.c b/src/fw/smp.c
index 719d51d..2c5670c 100644
--- a/src/fw/smp.c
+++ b/src/fw/smp.c
@@ -55,24 +55,32 @@ int apic_id_is_present(u8 apic_id)
     return !!(FoundAPICIDs[apic_id/32] & (1ul << (apic_id % 32)));
 }

+static int
+apic_id_init(void)
+{
+    CountCPUs++;
+
+    // Track found apic id for use in legacy internal bios tables
+    u32 eax, ebx, ecx, cpuid_features;
+    cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
+    u8 apic_id = ebx>>24;
+    FoundAPICIDs[apic_id/32] |= 1 << (apic_id % 32);
+
+    return apic_id;
+}
+
 void VISIBLE32FLAT
 handle_smp(void)
 {
     if (!CONFIG_QEMU)
         return;

-    // Detect apic_id
-    u32 eax, ebx, ecx, cpuid_features;
-    cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
-    u8 apic_id = ebx>>24;
+    // Track this CPU and detect the apic_id
+    int apic_id = apic_id_init();
     dprintf(DEBUG_HDL_smp, "handle_smp: apic_id=%d\n", apic_id);

smp_write_msrs();

-    // Set bit on FoundAPICIDs
-    FoundAPICIDs[apic_id/32] |= (1 << (apic_id % 32));
-
-    CountCPUs++;
 }

// Atomic lock for shared stack across processors.
@@ -93,11 +101,6 @@ smp_scan(void)
         return;
     }

-    // mark the BSP initial APIC ID as found, too:
-    u8 apic_id = ebx>>24;
-    FoundAPICIDs[apic_id/32] |= (1 << (apic_id % 32));
-    CountCPUs = 1;
-
     // Setup jump trampoline to counter code.
     u64 old = *(u64*)BUILD_AP_BOOT_ADDR;
     // ljmpw $SEG_BIOS, $(entry_smp - BUILD_BIOS_ADDR)
@@ -125,6 +128,9 @@ smp_scan(void)
     u32 sipi_vector = BUILD_AP_BOOT_ADDR >> 12;
     writel(APIC_ICR_LOW, 0x000C4600 | sipi_vector);

+    // Put BSP into APIC ID bitmap and CPUs counter
+    apic_id_init();
+
     // Wait for other CPUs to process the SIPI.
     u8 cmos_smp_count = rtc_read(CMOS_BIOS_SMP_COUNT) + 1;
     while (cmos_smp_count != CountCPUs)

[PATCH v3 3/3] support booting with more than 255 CPUs

[Igor Mammedov]

SDM[*1] says that if there are CPUs with APIC ID
greater than 254, BIOS is to pass control to OS
in x2APIC mode. Use the fact that QEMU passes in
"etc/max-cpus" max possible "APIC ID + 1" to
detect need for x2APIC mode. Also instead of
CMOS_BIOS_SMP_COUNT which is limited to 256 CPUs
use a new rom file "etc/boot-cpus" that QEMU
supporting more than 256 CPUs will provide.

*1) SDM: Volume 3: EXTENDED XAPIC (X2APIC):
     Initialization by System Software

Signed-off-by: Igor Mammedov 
---
v2:
  * merge handle_x2apic() into apic_id_init()
---
 src/x86.h    |  1 +
 src/fw/smp.c | 26 +++++++++++++++++++++-----
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/src/x86.h b/src/x86.h
index 53378e9..a770e6f 100644
--- a/src/x86.h
+++ b/src/x86.h
@@ -68,6 +68,7 @@ static inline void wbinvd(void)
 #define CPUID_MSR (1 << 5)
 #define CPUID_APIC (1 << 9)
 #define CPUID_MTRR (1 << 12)
+#define CPUID_X2APIC (1 << 21)
 static inline void __cpuid(u32 index, u32 *eax, u32 *ebx, u32 *ecx, u32 *edx)
 {
     asm("cpuid"
diff --git a/src/fw/smp.c b/src/fw/smp.c
index 2c5670c..352f52e 100644
--- a/src/fw/smp.c
+++ b/src/fw/smp.c
@@ -19,6 +19,9 @@
 #define APIC_LINT1   ((u8*)BUILD_APIC_ADDR + 0x360)

#define APIC_ENABLED 0x0100
+#define MSR_IA32_APIC_BASE 0x01B
+#define MSR_LOCAL_APIC_ID 0x802
+#define MSR_IA32_APICBASE_EXTD (1ULL << 10) /* Enable x2APIC mode */

static struct { u32 index; u64 val; } smp_msr[32];
 static u32 smp_msr_count;
@@ -46,6 +49,7 @@ smp_write_msrs(void)
 }

u32 MaxCountCPUs;
+static u16 boot_cpus_count;
 static u32 CountCPUs;
 // 256 bits for the found APIC IDs
 static u32 FoundAPICIDs[256/32];
@@ -58,13 +62,24 @@ int apic_id_is_present(u8 apic_id)
 static int
 apic_id_init(void)
 {
+    u32 apic_id;
     CountCPUs++;

-    // Track found apic id for use in legacy internal bios tables
     u32 eax, ebx, ecx, cpuid_features;
     cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
-    u8 apic_id = ebx>>24;
-    FoundAPICIDs[apic_id/32] |= 1 << (apic_id % 32);
+    apic_id = ebx>>24;
+    if (MaxCountCPUs < 256) { // xAPIC mode
+        // Track found apic id for use in legacy internal bios tables
+        FoundAPICIDs[apic_id/32] |= 1 << (apic_id % 32);
+    } else if (ecx & CPUID_X2APIC) {
+        // switch to x2APIC mode
+        u64 apic_base = rdmsr(MSR_IA32_APIC_BASE);
+        wrmsr(MSR_IA32_APIC_BASE, apic_base | MSR_IA32_APICBASE_EXTD);
+        apic_id = rdmsr(MSR_LOCAL_APIC_ID);
+    } else {
+        // x2APIC is masked by CPUID
+        apic_id = -1;
+    }

return apic_id;
 }
@@ -132,8 +147,7 @@ smp_scan(void)
     apic_id_init();

// Wait for other CPUs to process the SIPI.
-    u8 cmos_smp_count = rtc_read(CMOS_BIOS_SMP_COUNT) + 1;
-    while (cmos_smp_count != CountCPUs)
+    while (boot_cpus_count != CountCPUs)
         asm volatile(
             // Release lock and allow other processors to use the stack.
             "  movl %%esp, %1\n"
@@ -164,6 +178,8 @@ smp_setup(void)
     if (MaxCountCPUs < cmos_smp_count)
         MaxCountCPUs = cmos_smp_count;

+    boot_cpus_count = romfile_loadint("etc/boot-cpus", cmos_smp_count);
+
     smp_scan();
 }

[PATCH v3 1/3] paravirt: disable legacy bios tables in case of more than 255 CPUs

[Michael S. Tsirkin]

On Fri, Aug 05, 2016 at 12:47:27PM +0200, Igor Mammedov wrote:
> MPTable doesn't support more than 255 CPUs and
> QEMU supplies an alternative MADT table which
> guest will use instead of it. So do not install
> legacy tables if more than 254 CPUs are provided
>
> Signed-off-by: Igor Mammedov

Acked-by: Michael S. Tsirkin

In fact, should we disable these when loading
acpi from guest?
> src/fw/paravirt.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
> index 73a08f0..33a471b 100644
> --- a/src/fw/paravirt.c
> +++ b/src/fw/paravirt.c
> @@ -164,8 +164,10 @@ qemu_platform_setup(void)
>      smp_setup();
>  // Create bios tables
> pirtable_setup();
> mptable_setup();
>
> if (MaxCountCPUs <= 255) {
>    pirtable_setup();
>    mptable_setup();
>
> }
> smbios_setup();
>
> if (CONFIG_FW_ROMFILE_LOAD) {

[PATCH v3 1/3] paravirt: disable legacy bios tables in case of more than 255 CPUs

[Paolo Bonzini]

On 07/08/2016 07:57, Michael S. Tsirkin wrote:
> On Fri, Aug 05, 2016 at 12:47:27PM +0200, Igor Mammedov wrote:
> > MPTable doesn't support more than 255 CPUs and
> > QEMU supplies an alternative MADT table which
> > guest will use instead of it. So do not install
> > legacy tables if more than 254 CPUs are provided
> >
> > Signed-off-by: Igor Mammedov
>
> Acked-by: Michael S. Tsirkin
>
> In fact, should we disable these when loading
> acpi from guest?

Old guests (and guests with no ACPI support) still require these tables.
 I think RHEL3 (which has a 2.4.x kernel) was one.

Paolo
> > src/fw/paravirt.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
> > index 73a08f0..33a471b 100644
> > --- a/src/fw/paravirt.c
> > +++ b/src/fw/paravirt.c
> > @@ -164,8 +164,10 @@ qemu_platform_setup(void)
> >      smp_setup();
> >  // Create bios tables
> > pirtable_setup();
> > mptable_setup();
> >
> > if (MaxCountCPUs <= 255) {
> >    pirtable_setup();
> >    mptable_setup();
> >
> > }
> > smbios_setup();
> >
> > if (CONFIG_FW_ROMFILE_LOAD) {

[PATCH v3 2/3] smp: refactor present CPU APIC ID detection and counting

[Kevin O'Connor]

On Fri, Aug 05, 2016 at 12:47:28PM +0200, Igor Mammedov wrote:
> From: Kevin O'Connor
>
> Signed-off-by: "Kevin O'Connor" 
> Signed-off-by: Igor Mammedov
>
> v2:
> s/count_cpu/apic_id_init/
> call apic_id_init() after sending SIPI,
> it will be needed for switching BSP into x2APIC mode
>
> src/fw/smp.c | 32 +++++++++++++++++++-------------
>  1 file changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/src/fw/smp.c b/src/fw/smp.c
> index 719d51d..2c5670c 100644
> --- a/src/fw/smp.c
> +++ b/src/fw/smp.c
> @@ -55,24 +55,32 @@ int apic_id_is_present(u8 apic_id)
>      return !!(FoundAPICIDs[apic_id/32] & (1ul << (apic_id % 32)));
>  }
>
> +static int
> +apic_id_init(void)
> +{
> CountCPUs++;
> // Track found apic id for use in legacy internal bios tables
> u32 eax, ebx, ecx, cpuid_features;
> cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
> u8 apic_id = ebx>>24;
> FoundAPICIDs[apic_id/32] |= 1 << (apic_id % 32);
> return apic_id;
>
> +}
>
> void VISIBLE32FLAT
>  handle_smp(void)
>  {
>      if (!CONFIG_QEMU)
>          return;
> // Detect apic_id
> u32 eax, ebx, ecx, cpuid_features;
> cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
> u8 apic_id = ebx>>24;
>
> // Track this CPU and detect the apic_id
>
> int apic_id = apic_id_init();
> dprintf(DEBUG_HDL_smp, "handle_smp: apic_id=%d\n", apic_id);
>
> smp_write_msrs();
> // Set bit on FoundAPICIDs
> FoundAPICIDs[apic_id/32] |= (1 << (apic_id % 32));
> CountCPUs++;
>
> }
>
> // Atomic lock for shared stack across processors.
> @@ -93,11 +101,6 @@ smp_scan(void)
>          return;
>      }
> // mark the BSP initial APIC ID as found, too:
> u8 apic_id = ebx>>24;
> FoundAPICIDs[apic_id/32] |= (1 << (apic_id % 32));
> CountCPUs = 1;

Now that scan_smp() is called from the resume path, we do have to
initialize CountCPUs here.  Probably best to not move the updating of
CountCPUs into apic_id_init().

-Kevin

[PATCH v3 3/3] support booting with more than 255 CPUs

[Kevin O'Connor]

On Fri, Aug 05, 2016 at 12:47:29PM +0200, Igor Mammedov wrote:
> SDM[*1] says that if there are CPUs with APIC ID
> greater than 254, BIOS is to pass control to OS
> in x2APIC mode. Use the fact that QEMU passes in
> "etc/max-cpus" max possible "APIC ID + 1" to
> detect need for x2APIC mode. Also instead of
> CMOS_BIOS_SMP_COUNT which is limited to 256 CPUs
> use a new rom file "etc/boot-cpus" that QEMU
> supporting more than 256 CPUs will provide.
>
> *1) SDM: Volume 3: EXTENDED XAPIC (X2APIC):
>      Initialization by System Software
> Signed-off-by: Igor Mammedov
>
> v2:
> merge handle_x2apic() into apic_id_init()
>
> src/x86.h    |  1 +
>  src/fw/smp.c | 26 +++++++++++++++++++++-----
>  2 files changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/src/x86.h b/src/x86.h
> index 53378e9..a770e6f 100644
> --- a/src/x86.h
> +++ b/src/x86.h
> @@ -68,6 +68,7 @@ static inline void wbinvd(void)
>  #define CPUID_MSR (1 << 5)
>  #define CPUID_APIC (1 << 9)
>  #define CPUID_MTRR (1 << 12)
> +#define CPUID_X2APIC (1 << 21)
>  static inline void __cpuid(u32 index, u32 *eax, u32 *ebx, u32 *ecx, u32 *edx)
>  {
>      asm("cpuid"
> diff --git a/src/fw/smp.c b/src/fw/smp.c
> index 2c5670c..352f52e 100644
> --- a/src/fw/smp.c
> +++ b/src/fw/smp.c
> @@ -19,6 +19,9 @@
>  #define APIC_LINT1   ((u8*)BUILD_APIC_ADDR + 0x360)
>
> #define APIC_ENABLED 0x0100
> +#define MSR_IA32_APIC_BASE 0x01B
> +#define MSR_LOCAL_APIC_ID 0x802
> +#define MSR_IA32_APICBASE_EXTD (1ULL << 10) /* Enable x2APIC mode */
>
> static struct { u32 index; u64 val; } smp_msr[32];
>  static u32 smp_msr_count;
> @@ -46,6 +49,7 @@ smp_write_msrs(void)
>  }
>
> u32 MaxCountCPUs;
> +static u16 boot_cpus_count;
>  static u32 CountCPUs;
>  // 256 bits for the found APIC IDs
>  static u32 FoundAPICIDs[256/32];
> @@ -58,13 +62,24 @@ int apic_id_is_present(u8 apic_id)
>  static int
>  apic_id_init(void)
>  {
> u32 apic_id;
> CountCPUs++;
> // Track found apic id for use in legacy internal bios tables
> u32 eax, ebx, ecx, cpuid_features;
> cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
> u8 apic_id = ebx>>24;
> FoundAPICIDs[apic_id/32] |= 1 << (apic_id % 32);
>
> apic_id = ebx>>24;
>
> if (MaxCountCPUs < 256) { // xAPIC mode
>    // Track found apic id for use in legacy internal bios tables
>    FoundAPICIDs[apic_id/32] |= 1 << (apic_id % 32);
>
> } else if (ecx & CPUID_X2APIC) {
>    // switch to x2APIC mode
>    u64 apic_base = rdmsr(MSR_IA32_APIC_BASE);
>    wrmsr(MSR_IA32_APIC_BASE, apic_base | MSR_IA32_APICBASE_EXTD);
>    apic_id = rdmsr(MSR_LOCAL_APIC_ID);
>
> } else {
>    // x2APIC is masked by CPUID
>    apic_id = -1;
>
> }
>
> return apic_id;
>
> }
> @@ -132,8 +147,7 @@ smp_scan(void)
>      apic_id_init();
>  // Wait for other CPUs to process the SIPI.
> u8 cmos_smp_count = rtc_read(CMOS_BIOS_SMP_COUNT) + 1;
> while (cmos_smp_count != CountCPUs)
> while (boot_cpus_count != CountCPUs)
>     asm volatile(
>         // Release lock and allow other processors to use the stack.
>         "  movl %%esp, %1\n"
>
> @@ -164,6 +178,8 @@ smp_setup(void)
>      if (MaxCountCPUs < cmos_smp_count)
>          MaxCountCPUs = cmos_smp_count;
> boot_cpus_count = romfile_loadint("etc/boot-cpus", cmos_smp_count);

Wouldn't boot_cpus_count then also need to be updated in smp_resume()?
If the user hotplugs a new cpu between system start and prior to an s3
resume event, then the new could should be handled during that resume.

-Kevin

[PATCH v3 2/3] smp: refactor present CPU APIC ID detection and counting

[Igor Mammedov]

On Mon, 8 Aug 2016 18:37:13 -0400
"Kevin O'Connor"  wrote:
> On Fri, Aug 05, 2016 at 12:47:28PM +0200, Igor Mammedov wrote:
> > From: Kevin O'Connor
> >
> > Signed-off-by: "Kevin O'Connor" 
> > Signed-off-by: Igor Mammedov
> >
> > v2:
> > s/count_cpu/apic_id_init/
> > call apic_id_init() after sending SIPI,
> > it will be needed for switching BSP into x2APIC mode
> >
> > src/fw/smp.c | 32 +++++++++++++++++++-------------
> >  1 file changed, 19 insertions(+), 13 deletions(-)
> >
> > diff --git a/src/fw/smp.c b/src/fw/smp.c
> > index 719d51d..2c5670c 100644
> > --- a/src/fw/smp.c
> > +++ b/src/fw/smp.c
> > @@ -55,24 +55,32 @@ int apic_id_is_present(u8 apic_id)
> >      return !!(FoundAPICIDs[apic_id/32] & (1ul << (apic_id % 32)));
> >  }
> >
> > +static int
> > +apic_id_init(void)
> > +{
> > CountCPUs++;
> > // Track found apic id for use in legacy internal bios tables
> > u32 eax, ebx, ecx, cpuid_features;
> > cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
> > u8 apic_id = ebx>>24;
> > FoundAPICIDs[apic_id/32] |= 1 << (apic_id % 32);
> > return apic_id;
> >
> > +}
> >
> > void VISIBLE32FLAT
> >  handle_smp(void)
> >  {
> >      if (!CONFIG_QEMU)
> >          return;
> > // Detect apic_id
> > u32 eax, ebx, ecx, cpuid_features;
> > cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
> > u8 apic_id = ebx>>24;
> >
> > // Track this CPU and detect the apic_id
> >
> > int apic_id = apic_id_init();
> > dprintf(DEBUG_HDL_smp, "handle_smp: apic_id=%d\n", apic_id);
> >
> > smp_write_msrs();
> > // Set bit on FoundAPICIDs
> > FoundAPICIDs[apic_id/32] |= (1 << (apic_id % 32));
> > CountCPUs++;
> >
> > }
> >
> > // Atomic lock for shared stack across processors.
> > @@ -93,11 +101,6 @@ smp_scan(void)
> >          return;
> >      }
> > // mark the BSP initial APIC ID as found, too:
> > u8 apic_id = ebx>>24;
> > FoundAPICIDs[apic_id/32] |= (1 << (apic_id % 32));
> > CountCPUs = 1;
>
> Now that scan_smp() is called from the resume path, we do have to
> initialize CountCPUs here.  Probably best to not move the updating of
> CountCPUs into apic_id_init().

Indeed with this patch guest hung on resume.

adding CountCPUs = 0 here here fixes it,
but I can just not move CountCPUs as you suggest.

What would you prefer?
> -Kevin

[PATCH v3 3/3] support booting with more than 255 CPUs

[Igor Mammedov]

On Mon, 8 Aug 2016 18:42:07 -0400
"Kevin O'Connor"  wrote:
> On Fri, Aug 05, 2016 at 12:47:29PM +0200, Igor Mammedov wrote:
> > SDM[*1] says that if there are CPUs with APIC ID
> > greater than 254, BIOS is to pass control to OS
> > in x2APIC mode. Use the fact that QEMU passes in
> > "etc/max-cpus" max possible "APIC ID + 1" to
> > detect need for x2APIC mode. Also instead of
> > CMOS_BIOS_SMP_COUNT which is limited to 256 CPUs
> > use a new rom file "etc/boot-cpus" that QEMU
> > supporting more than 256 CPUs will provide.
> >
> > *1) SDM: Volume 3: EXTENDED XAPIC (X2APIC):
> >      Initialization by System Software
> > Signed-off-by: Igor Mammedov
> >
> > v2:
> > merge handle_x2apic() into apic_id_init()
> >
> > src/x86.h    |  1 +
> >  src/fw/smp.c | 26 +++++++++++++++++++++-----
> >  2 files changed, 22 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/x86.h b/src/x86.h
> > index 53378e9..a770e6f 100644
> > --- a/src/x86.h
> > +++ b/src/x86.h
> > @@ -68,6 +68,7 @@ static inline void wbinvd(void)
> >  #define CPUID_MSR (1 << 5)
> >  #define CPUID_APIC (1 << 9)
> >  #define CPUID_MTRR (1 << 12)
> > +#define CPUID_X2APIC (1 << 21)
> >  static inline void __cpuid(u32 index, u32 *eax, u32 *ebx, u32 *ecx, u32 *edx)
> >  {
> >      asm("cpuid"
> > diff --git a/src/fw/smp.c b/src/fw/smp.c
> > index 2c5670c..352f52e 100644
> > --- a/src/fw/smp.c
> > +++ b/src/fw/smp.c
> > @@ -19,6 +19,9 @@
> >  #define APIC_LINT1   ((u8*)BUILD_APIC_ADDR + 0x360)
> >
> > #define APIC_ENABLED 0x0100
> > +#define MSR_IA32_APIC_BASE 0x01B
> > +#define MSR_LOCAL_APIC_ID 0x802
> > +#define MSR_IA32_APICBASE_EXTD (1ULL << 10) /* Enable x2APIC mode */
> >
> > static struct { u32 index; u64 val; } smp_msr[32];
> >  static u32 smp_msr_count;
> > @@ -46,6 +49,7 @@ smp_write_msrs(void)
> >  }
> >
> > u32 MaxCountCPUs;
> > +static u16 boot_cpus_count;
> >  static u32 CountCPUs;
> >  // 256 bits for the found APIC IDs
> >  static u32 FoundAPICIDs[256/32];
> > @@ -58,13 +62,24 @@ int apic_id_is_present(u8 apic_id)
> >  static int
> >  apic_id_init(void)
> >  {
> > u32 apic_id;
> > CountCPUs++;
> > // Track found apic id for use in legacy internal bios tables
> > u32 eax, ebx, ecx, cpuid_features;
> > cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
> > u8 apic_id = ebx>>24;
> > FoundAPICIDs[apic_id/32] |= 1 << (apic_id % 32);
> >
> > apic_id = ebx>>24;
> >
> > if (MaxCountCPUs < 256) { // xAPIC mode
> >    // Track found apic id for use in legacy internal bios tables
> >    FoundAPICIDs[apic_id/32] |= 1 << (apic_id % 32);
> >
> > } else if (ecx & CPUID_X2APIC) {
> >    // switch to x2APIC mode
> >    u64 apic_base = rdmsr(MSR_IA32_APIC_BASE);
> >    wrmsr(MSR_IA32_APIC_BASE, apic_base | MSR_IA32_APICBASE_EXTD);
> >    apic_id = rdmsr(MSR_LOCAL_APIC_ID);
> >
> > } else {
> >    // x2APIC is masked by CPUID
> >    apic_id = -1;
> >
> > }
> >
> > return apic_id;
> >
> > }
> > @@ -132,8 +147,7 @@ smp_scan(void)
> >      apic_id_init();
> >  // Wait for other CPUs to process the SIPI.
> > u8 cmos_smp_count = rtc_read(CMOS_BIOS_SMP_COUNT) + 1;
> > while (cmos_smp_count != CountCPUs)
> > while (boot_cpus_count != CountCPUs)
> >     asm volatile(
> >         // Release lock and allow other processors to use the stack.
> >         "  movl %%esp, %1\n"
> >
> > @@ -164,6 +178,8 @@ smp_setup(void)
> >      if (MaxCountCPUs < cmos_smp_count)
> >          MaxCountCPUs = cmos_smp_count;
> > boot_cpus_count = romfile_loadint("etc/boot-cpus", cmos_smp_count);
>
> Wouldn't boot_cpus_count then also need to be updated in smp_resume()?
> If the user hotplugs a new cpu between system start and prior to an s3
> resume event, then the new could should be handled during that resume.

when I try to put romfile_loadint() into smp_scan(), it breaks compilation with:

ERROR: .data.varinit../src/romfile.c.13 is VARVERIFY32INIT but used from ['.text.runtime../src/resume.c.142', '.text.smp_scan', '.text.romfile_loadint', '.text.romfile_find']

I'm not sure how to fix it

[PATCH v3 3/3] support booting with more than 255 CPUs

[Kevin O'Connor]

On Wed, Aug 10, 2016 at 12:59:16PM +0200, Igor Mammedov wrote:
> On Mon, 8 Aug 2016 18:42:07 -0400
> "Kevin O'Connor"  wrote:
> > On Fri, Aug 05, 2016 at 12:47:29PM +0200, Igor Mammedov wrote:
> > > @@ -164,6 +178,8 @@ smp_setup(void)
> > >      if (MaxCountCPUs < cmos_smp_count)
> > >          MaxCountCPUs = cmos_smp_count;
> > > boot_cpus_count = romfile_loadint("etc/boot-cpus", cmos_smp_count);
> >
> > Wouldn't boot_cpus_count then also need to be updated in smp_resume()?
> > If the user hotplugs a new cpu between system start and prior to an s3
> > resume event, then the new could should be handled during that resume.
>
> when I try to put romfile_loadint() into smp_scan(), it breaks compilation with:
>
> ERROR: .data.varinit../src/romfile.c.13 is VARVERIFY32INIT but used from ['.text.runtime../src/resume.c.142', '.text.smp_scan', '.text.romfile_loadint', '.text.romfile_find']
>
> I'm not sure how to fix it

Right - one can't use the romfile code on resume.  I forgot about
that.

I don't have a good solution.  One option is to record the fw_cfg
'select' and then call qemu_cfg_read_entry() directly on the resume
path.  Another option would be to pass the number of active CPUs using
some mechanism other than fw_cfg.  Neither seems like a great option.

-Kevin

[PATCH v3 2/3] smp: refactor present CPU APIC ID detection and counting

[Kevin O'Connor]

On Wed, Aug 10, 2016 at 12:33:22PM +0200, Igor Mammedov wrote:
> On Mon, 8 Aug 2016 18:37:13 -0400
> "Kevin O'Connor"  wrote:
> > On Fri, Aug 05, 2016 at 12:47:28PM +0200, Igor Mammedov wrote:
> > > From: Kevin O'Connor
> > >
> > > Signed-off-by: "Kevin O'Connor" 
> > > Signed-off-by: Igor Mammedov
> > >
> > > v2:
> > > s/count_cpu/apic_id_init/
> > > call apic_id_init() after sending SIPI,
> > > it will be needed for switching BSP into x2APIC mode
> > >
> > > src/fw/smp.c | 32 +++++++++++++++++++-------------
> > >  1 file changed, 19 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/src/fw/smp.c b/src/fw/smp.c
> > > index 719d51d..2c5670c 100644
> > > --- a/src/fw/smp.c
> > > +++ b/src/fw/smp.c
> > > @@ -55,24 +55,32 @@ int apic_id_is_present(u8 apic_id)
> > >      return !!(FoundAPICIDs[apic_id/32] & (1ul << (apic_id % 32)));
> > >  }
> > >
> > > +static int
> > > +apic_id_init(void)
> > > +{
> > > CountCPUs++;
> > > // Track found apic id for use in legacy internal bios tables
> > > u32 eax, ebx, ecx, cpuid_features;
> > > cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
> > > u8 apic_id = ebx>>24;
> > > FoundAPICIDs[apic_id/32] |= 1 << (apic_id % 32);
> > > return apic_id;
> > >
> > > +}
> > >
> > > void VISIBLE32FLAT
> > >  handle_smp(void)
> > >  {
> > >      if (!CONFIG_QEMU)
> > >          return;
> > > // Detect apic_id
> > > u32 eax, ebx, ecx, cpuid_features;
> > > cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
> > > u8 apic_id = ebx>>24;
> > >
> > > // Track this CPU and detect the apic_id
> > >
> > > int apic_id = apic_id_init();
> > > dprintf(DEBUG_HDL_smp, "handle_smp: apic_id=%d\n", apic_id);
> > >
> > > smp_write_msrs();
> > > // Set bit on FoundAPICIDs
> > > FoundAPICIDs[apic_id/32] |= (1 << (apic_id % 32));
> > > CountCPUs++;
> > >
> > > }
> > >
> > > // Atomic lock for shared stack across processors.
> > > @@ -93,11 +101,6 @@ smp_scan(void)
> > >          return;
> > >      }
> > > // mark the BSP initial APIC ID as found, too:
> > > u8 apic_id = ebx>>24;
> > > FoundAPICIDs[apic_id/32] |= (1 << (apic_id % 32));
> > > CountCPUs = 1;
> >
> > Now that scan_smp() is called from the resume path, we do have to
> > initialize CountCPUs here.  Probably best to not move the updating of
> > CountCPUs into apic_id_init().
>
> Indeed with this patch guest hung on resume.
>
> adding CountCPUs = 0 here here fixes it,
> but I can just not move CountCPUs as you suggest.
>
> What would you prefer?

I'd say the latter.

-Kevin

[PATCH v3 3/3] support booting with more than 255 CPUs

[Igor Mammedov]

On Wed, 10 Aug 2016 11:25:41 -0400
"Kevin O'Connor"  wrote:
> On Wed, Aug 10, 2016 at 12:59:16PM +0200, Igor Mammedov wrote:
> > On Mon, 8 Aug 2016 18:42:07 -0400
> > "Kevin O'Connor"  wrote:
> > > On Fri, Aug 05, 2016 at 12:47:29PM +0200, Igor Mammedov wrote:
> > > > @@ -164,6 +178,8 @@ smp_setup(void)
> > > >      if (MaxCountCPUs < cmos_smp_count)
> > > >          MaxCountCPUs = cmos_smp_count;
> > > > boot_cpus_count = romfile_loadint("etc/boot-cpus", cmos_smp_count);
> > >
> > > Wouldn't boot_cpus_count then also need to be updated in smp_resume()?
> > > If the user hotplugs a new cpu between system start and prior to an s3
> > > resume event, then the new could should be handled during that resume.
> >
> > when I try to put romfile_loadint() into smp_scan(), it breaks compilation with:
> >
> > ERROR: .data.varinit../src/romfile.c.13 is VARVERIFY32INIT but used from ['.text.runtime../src/resume.c.142', '.text.smp_scan', '.text.romfile_loadint', '.text.romfile_find']
> >
> > I'm not sure how to fix it
>
> Right - one can't use the romfile code on resume.  I forgot about
> that.
>
> I don't have a good solution.  One option is to record the fw_cfg
> 'select' and then call qemu_cfg_read_entry() directly on the resume
> path.  Another option would be to pass the number of active CPUs using
> some mechanism other than fw_cfg.  Neither seems like a great option.

QEMU has cpu hotplug MMIO registers, what we can do is to
replace fwcfg(etc/boot_cpus) with registers address fwcfg(etc/cphp_addr)
and add a command to return number of cpus in QEMU:
  https://github.com/qemu/qemu/blob/master/docs/specs/acpi_cpu_hotplug.txt
Then Seabios would be able to query it at resume time.
But that's even more complicated than
 "record the fw_cfg 'select' and then call qemu_cfg_read_entry()"
> -Kevin