Currently the MTRRs and MSR_IA32_FEATURE_CONTROL are not restored on S3 resume. Because these have to be applied to all processors, SMP setup has to be added to S3 resume.
There are two differences between the boot and resume paths. First, romfile_* is not usable in the resume paths so we move the call out of smp_setup and into a new function smp_boot_setup. Second, smp_msr has to be walked on the BSP as well, so we extract that out of handle_smp and into a new function smp_write_msrs. Then, resume can call smp_write_msrs on the BSP followed by smp_setup to initialize the APs.
Reported-by: Laszlo Ersek lersek@redhat.com Signed-off-by: Paolo Bonzini pbonzini@redhat.com --- src/fw/paravirt.c | 1 + src/fw/smp.c | 28 +++++++++++++++++++++++----- src/resume.c | 1 + src/util.h | 2 ++ 4 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c index e8e419e..9189672 100644 --- a/src/fw/paravirt.c +++ b/src/fw/paravirt.c @@ -160,6 +160,7 @@ qemu_platform_setup(void) // Initialize mtrr, msr_feature_control and smp mtrr_setup(); msr_feature_control_setup(); + smp_boot_setup(); smp_setup();
// Create bios tables diff --git a/src/fw/smp.c b/src/fw/smp.c index 6e706e4..64e76b6 100644 --- a/src/fw/smp.c +++ b/src/fw/smp.c @@ -46,6 +46,15 @@ int apic_id_is_present(u8 apic_id) return !!(FoundAPICIDs[apic_id/32] & (1ul << (apic_id % 32))); }
+static void +smp_write_msrs(void) +{ + // MTRR and MSR_IA32_FEATURE_CONTROL setup + int i; + for (i=0; i<smp_msr_count; i++) + wrmsr(smp_msr[i].index, smp_msr[i].val); +} + void VISIBLE32FLAT handle_smp(void) { @@ -58,10 +67,7 @@ handle_smp(void) u8 apic_id = ebx>>24; dprintf(DEBUG_HDL_smp, "handle_smp: apic_id=%d\n", apic_id);
- // MTRR and MSR_IA32_FEATURE_CONTROL setup - int i; - for (i=0; i<smp_msr_count; i++) - wrmsr(smp_msr[i].index, smp_msr[i].val); + smp_write_msrs();
// Set bit on FoundAPICIDs FoundAPICIDs[apic_id/32] |= (1 << (apic_id % 32)); @@ -73,6 +79,12 @@ handle_smp(void) u32 SMPLock __VISIBLE; u32 SMPStack __VISIBLE;
+void +smp_boot_setup(void) +{ + MaxCountCPUs = romfile_loadint("etc/max-cpus", 0); +} + // find and initialize the CPUs by launching a SIPI to them void smp_setup(void) @@ -141,10 +153,16 @@ smp_setup(void) // Restore memory. *(u64*)BUILD_AP_BOOT_ADDR = old;
- MaxCountCPUs = romfile_loadint("etc/max-cpus", 0); if (!MaxCountCPUs || MaxCountCPUs < CountCPUs) MaxCountCPUs = CountCPUs;
dprintf(1, "Found %d cpu(s) max supported %d cpu(s)\n", CountCPUs, MaxCountCPUs); } + +void +smp_resume(void) +{ + smp_write_msrs(); + smp_setup(); +} diff --git a/src/resume.c b/src/resume.c index a5465d8..3a26f84 100644 --- a/src/resume.c +++ b/src/resume.c @@ -97,6 +97,7 @@ s3_resume(void)
pic_setup(); smm_setup(); + smp_resume();
pci_resume();
diff --git a/src/util.h b/src/util.h index cba3359..510da73 100644 --- a/src/util.h +++ b/src/util.h @@ -131,7 +131,9 @@ void smm_setup(void); // fw/smp.c extern u32 MaxCountCPUs; void wrmsr_smp(u32 index, u64 val); +void smp_boot_setup(void); void smp_setup(void); +void smp_resume(void); int apic_id_is_present(u8 apic_id);
// hw/dma.c
On Thu, Jul 07, 2016 at 04:00:40PM +0200, Paolo Bonzini wrote:
Currently the MTRRs and MSR_IA32_FEATURE_CONTROL are not restored on S3 resume. Because these have to be applied to all processors, SMP setup has to be added to S3 resume.
There are two differences between the boot and resume paths. First, romfile_* is not usable in the resume paths so we move the call out of smp_setup and into a new function smp_boot_setup. Second, smp_msr has to be walked on the BSP as well, so we extract that out of handle_smp and into a new function smp_write_msrs. Then, resume can call smp_write_msrs on the BSP followed by smp_setup to initialize the APs.
In general, looks good to me. Are you okay with the slightly modified patch below? Three differences - check CONFIG_QEMU in smp_resume(), only modify MaxCountCPUs during the init phase, and only export smp_setup() and smp_resume() from smp.c.
BTW, this will conflict with Igor's pending series, but it doesn't look too invasive.
-Kevin
commit 54e3a88609da074aaae2f04e592026ebf82169dc Author: Paolo Bonzini pbonzini@redhat.com Date: Thu Jul 7 16:00:40 2016 +0200
smp: restore MSRs on S3 resume
Currently the MTRRs and MSR_IA32_FEATURE_CONTROL are not restored on S3 resume. Because these have to be applied to all processors, SMP setup has to be added to S3 resume.
There are two differences between the boot and resume paths. First, romfile_* is not usable in the resume paths so we separate out the remaining common code to a new smp_scan function. Second, smp_msr has to be walked on the BSP as well, so we extract that out of handle_smp and into a new function smp_write_msrs. Then, resume can call smp_write_msrs on the BSP followed by smp_scan to initialize the APs.
Reported-by: Laszlo Ersek lersek@redhat.com Signed-off-by: Paolo Bonzini pbonzini@redhat.com Signed-off-by: Kevin O'Connor kevin@koconnor.net
diff --git a/src/fw/smp.c b/src/fw/smp.c index 6e706e4..719d51d 100644 --- a/src/fw/smp.c +++ b/src/fw/smp.c @@ -36,6 +36,15 @@ wrmsr_smp(u32 index, u64 val) smp_msr_count++; }
+static void +smp_write_msrs(void) +{ + // MTRR and MSR_IA32_FEATURE_CONTROL setup + int i; + for (i=0; i<smp_msr_count; i++) + wrmsr(smp_msr[i].index, smp_msr[i].val); +} + u32 MaxCountCPUs; static u32 CountCPUs; // 256 bits for the found APIC IDs @@ -58,10 +67,7 @@ handle_smp(void) u8 apic_id = ebx>>24; dprintf(DEBUG_HDL_smp, "handle_smp: apic_id=%d\n", apic_id);
- // MTRR and MSR_IA32_FEATURE_CONTROL setup - int i; - for (i=0; i<smp_msr_count; i++) - wrmsr(smp_msr[i].index, smp_msr[i].val); + smp_write_msrs();
// Set bit on FoundAPICIDs FoundAPICIDs[apic_id/32] |= (1 << (apic_id % 32)); @@ -74,12 +80,9 @@ u32 SMPLock __VISIBLE; u32 SMPStack __VISIBLE;
// find and initialize the CPUs by launching a SIPI to them -void -smp_setup(void) +static void +smp_scan(void) { - if (!CONFIG_QEMU) - return; - ASSERT32FLAT(); u32 eax, ebx, ecx, cpuid_features; cpuid(1, &eax, &ebx, &ecx, &cpuid_features); @@ -87,7 +90,6 @@ smp_setup(void) // No apic - only the main cpu is present. dprintf(1, "No apic - only the main cpu is present.\n"); CountCPUs= 1; - MaxCountCPUs = 1; return; }
@@ -141,10 +143,30 @@ smp_setup(void) // Restore memory. *(u64*)BUILD_AP_BOOT_ADDR = old;
- MaxCountCPUs = romfile_loadint("etc/max-cpus", 0); - if (!MaxCountCPUs || MaxCountCPUs < CountCPUs) - MaxCountCPUs = CountCPUs; - dprintf(1, "Found %d cpu(s) max supported %d cpu(s)\n", CountCPUs, MaxCountCPUs); } + +void +smp_setup(void) +{ + if (!CONFIG_QEMU) + return; + + MaxCountCPUs = romfile_loadint("etc/max-cpus", 0); + u8 cmos_smp_count = rtc_read(CMOS_BIOS_SMP_COUNT) + 1; + if (MaxCountCPUs < cmos_smp_count) + MaxCountCPUs = cmos_smp_count; + + smp_scan(); +} + +void +smp_resume(void) +{ + if (!CONFIG_QEMU) + return; + + smp_write_msrs(); + smp_scan(); +} diff --git a/src/resume.c b/src/resume.c index afeadcf..e67cfce 100644 --- a/src/resume.c +++ b/src/resume.c @@ -97,6 +97,7 @@ s3_resume(void)
pic_setup(); smm_setup(); + smp_resume();
pci_resume();
diff --git a/src/util.h b/src/util.h index 7b41207..7bfd2b8 100644 --- a/src/util.h +++ b/src/util.h @@ -135,6 +135,7 @@ void smm_setup(void); extern u32 MaxCountCPUs; void wrmsr_smp(u32 index, u64 val); void smp_setup(void); +void smp_resume(void); int apic_id_is_present(u8 apic_id);
// hw/dma.c
On Thu, Jul 07, 2016 at 04:00:40PM +0200, Paolo Bonzini wrote:
Currently the MTRRs and MSR_IA32_FEATURE_CONTROL are not restored on S3 resume. Because these have to be applied to all processors, SMP setup has to be added to S3 resume.
There are two differences between the boot and resume paths. First, romfile_* is not usable in the resume paths so we move the call out of smp_setup and into a new function smp_boot_setup. Second, smp_msr has to be walked on the BSP as well, so we extract that out of handle_smp and into a new function smp_write_msrs. Then, resume can call smp_write_msrs on the BSP followed by smp_setup to initialize the APs.
In general, looks good to me. Are you okay with the slightly modified patch below? Three differences - check CONFIG_QEMU in smp_resume(), only modify MaxCountCPUs during the init phase, and only export smp_setup() and smp_resume() from smp.c.
Yes, of course. Thanks! Gerd, can you get this in QEMU 2.7 too?
Paolo
BTW, this will conflict with Igor's pending series, but it doesn't look too invasive.
-Kevin
commit 54e3a88609da074aaae2f04e592026ebf82169dc Author: Paolo Bonzini pbonzini@redhat.com Date: Thu Jul 7 16:00:40 2016 +0200
smp: restore MSRs on S3 resume Currently the MTRRs and MSR_IA32_FEATURE_CONTROL are not restored on S3 resume. Because these have to be applied to all processors, SMP setup has to be added to S3 resume. There are two differences between the boot and resume paths. First, romfile_* is not usable in the resume paths so we separate out the remaining common code to a new smp_scan function. Second, smp_msr has to be walked on the BSP as well, so we extract that out of handle_smp and into a new function smp_write_msrs. Then, resume can call smp_write_msrs on the BSP followed by smp_scan to initialize the APs. Reported-by: Laszlo Ersek <lersek@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin O'Connor <kevin@koconnor.net>diff --git a/src/fw/smp.c b/src/fw/smp.c index 6e706e4..719d51d 100644 --- a/src/fw/smp.c +++ b/src/fw/smp.c @@ -36,6 +36,15 @@ wrmsr_smp(u32 index, u64 val) smp_msr_count++; }
+static void +smp_write_msrs(void) +{
- // MTRR and MSR_IA32_FEATURE_CONTROL setup
- int i;
- for (i=0; i<smp_msr_count; i++)
wrmsr(smp_msr[i].index, smp_msr[i].val);+}
u32 MaxCountCPUs; static u32 CountCPUs; // 256 bits for the found APIC IDs @@ -58,10 +67,7 @@ handle_smp(void) u8 apic_id = ebx>>24; dprintf(DEBUG_HDL_smp, "handle_smp: apic_id=%d\n", apic_id);
- // MTRR and MSR_IA32_FEATURE_CONTROL setup
- int i;
- for (i=0; i<smp_msr_count; i++)
wrmsr(smp_msr[i].index, smp_msr[i].val);
smp_write_msrs();
// Set bit on FoundAPICIDs FoundAPICIDs[apic_id/32] |= (1 << (apic_id % 32));
@@ -74,12 +80,9 @@ u32 SMPLock __VISIBLE; u32 SMPStack __VISIBLE;
// find and initialize the CPUs by launching a SIPI to them -void -smp_setup(void) +static void +smp_scan(void) {
- if (!CONFIG_QEMU)
return;- ASSERT32FLAT(); u32 eax, ebx, ecx, cpuid_features; cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
@@ -87,7 +90,6 @@ smp_setup(void) // No apic - only the main cpu is present. dprintf(1, "No apic - only the main cpu is present.\n"); CountCPUs= 1;
MaxCountCPUs = 1; return;@@ -141,10 +143,30 @@ smp_setup(void) // Restore memory. *(u64*)BUILD_AP_BOOT_ADDR = old;
- MaxCountCPUs = romfile_loadint("etc/max-cpus", 0);
- if (!MaxCountCPUs || MaxCountCPUs < CountCPUs)
MaxCountCPUs = CountCPUs;- dprintf(1, "Found %d cpu(s) max supported %d cpu(s)\n", CountCPUs, MaxCountCPUs);
}
+void +smp_setup(void) +{
- if (!CONFIG_QEMU)
return;- MaxCountCPUs = romfile_loadint("etc/max-cpus", 0);
- u8 cmos_smp_count = rtc_read(CMOS_BIOS_SMP_COUNT) + 1;
- if (MaxCountCPUs < cmos_smp_count)
MaxCountCPUs = cmos_smp_count;- smp_scan();
+}
+void +smp_resume(void) +{
- if (!CONFIG_QEMU)
return;- smp_write_msrs();
- smp_scan();
+} diff --git a/src/resume.c b/src/resume.c index afeadcf..e67cfce 100644 --- a/src/resume.c +++ b/src/resume.c @@ -97,6 +97,7 @@ s3_resume(void)
pic_setup(); smm_setup();
smp_resume();
pci_resume();
diff --git a/src/util.h b/src/util.h index 7b41207..7bfd2b8 100644 --- a/src/util.h +++ b/src/util.h @@ -135,6 +135,7 @@ void smm_setup(void); extern u32 MaxCountCPUs; void wrmsr_smp(u32 index, u64 val); void smp_setup(void); +void smp_resume(void); int apic_id_is_present(u8 apic_id);
// hw/dma.c
On Thu, Jul 07, 2016 at 12:15:24PM -0400, Paolo Bonzini wrote:
On Thu, Jul 07, 2016 at 04:00:40PM +0200, Paolo Bonzini wrote:
Currently the MTRRs and MSR_IA32_FEATURE_CONTROL are not restored on S3 resume. Because these have to be applied to all processors, SMP setup has to be added to S3 resume.
There are two differences between the boot and resume paths. First, romfile_* is not usable in the resume paths so we move the call out of smp_setup and into a new function smp_boot_setup. Second, smp_msr has to be walked on the BSP as well, so we extract that out of handle_smp and into a new function smp_write_msrs. Then, resume can call smp_write_msrs on the BSP followed by smp_setup to initialize the APs.
In general, looks good to me. Are you okay with the slightly modified patch below? Three differences - check CONFIG_QEMU in smp_resume(), only modify MaxCountCPUs during the init phase, and only export smp_setup() and smp_resume() from smp.c.
Yes, of course. Thanks! Gerd, can you get this in QEMU 2.7 too?
Thanks. I committed this change to master.
-Kevin
-
Gerd Hoffmann -
Kevin O'Connor -
Paolo Bonzini