x86: Set "APIC Software Enable" after APIC reset

From: Nadav Amit <[email protected]>

After the APIC is reset, some of its registers might be reset. As the SDM says: "When IA32_APIC_BASE[11] is set to 0, prior initialization to the APIC may be lost and the APIC may return to the state described in Section 10.4.7.1". The SDM also says that after APIC reset "the spurious-interrupt vector register is initialized to 000000FFH". This means that after the APIC is reset it needs to be software-enabled through the SPIV.

This is done one occasion, but there are (at least) two occasions that do not software-enable the APIC after reset (__test_apic_id() and main() in vmx.c).

Move APIC SPIV reinitialization into reset_apic(). Remove SPIV settings which are unnecessary after reset_apic() is modified.

Cc Sean Christopherson <[email protected]>

Signed-off-by Nadav Amit <[email protected]>

---
  lib/x86/apic.c | 1 +
  x86/apic.c     | 1 -
  2 files changed, 1 insertion(+), 1 deletion(-)

lib/x86/apic.c +1 -0

index 2aeffbd..4e7d43c 100644
@@ -161,6 +161,7 @@ void reset_apic(void)
  {
      disable_apic();
      wrmsr(MSR_IA32_APICBASE, rdmsr(MSR_IA32_APICBASE) | APIC_EN);
+    apic_write(APIC_SPIV, 0x1ff);
  }
 
  u32 ioapic_read_reg(unsigned reg)

x86/apic.c +0 -0

index 3eff588..7ef4a27 100644

Quoted 40 lines · On 05/02/2019 07:08 AM, [email protected] wrote:

From: Nadav Amit <[email protected]>

After the APIC is reset, some of its registers might be reset. As the SDM says: "When IA32_APIC_BASE[11] is set to 0, prior initialization to the APIC may be lost and the APIC may return to the state described in Section 10.4.7.1". The SDM also says that after APIC reset "the spurious-interrupt vector register is initialized to 000000FFH". This means that after the APIC is reset it needs to be software-enabled through the SPIV.

This is done one occasion, but there are (at least) two occasions that do not software-enable the APIC after reset (__test_apic_id() and main() in vmx.c).

Move APIC SPIV reinitialization into reset_apic(). Remove SPIV settings which are unnecessary after reset_apic() is modified.

Cc Sean Christopherson <[email protected]>

Signed-off-by Nadav Amit <[email protected]>

---
 lib/x86/apic.c | 1 +
 x86/apic.c     | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

lib/x86/apic.c +1 -0

index 2aeffbd..4e7d43c 100644
@@ -161,6 +161,7 @@ void reset_apic(void)
 {
     disable_apic();
     wrmsr(MSR_IA32_APICBASE, rdmsr(MSR_IA32_APICBASE) | APIC_EN);
+    apic_write(APIC_SPIV, 0x1ff);
 }
   u32 ioapic_read_reg(unsigned reg)

x86/apic.c +0 -0

index 3eff588..7ef4a27 100644

x86/apic.c static void test_apic_disable(void) · line 152

Quoted 6 lines

x86/apic.c +0 -1

@@ -148,7 +148,6 @@ static void test_apic_disable(void)
     verify_disabled_apic_mmio();
       reset_apic();
-    apic_write(APIC_SPIV, 0x1ff);
     report("Local apic enabled in xAPIC mode",
 	   (rdmsr(MSR_IA32_APICBASE) & (APIC_EN | APIC_EXTD)) == APIC_EN);
     report("CPUID.1H:EDX.APIC[bit 9] is set", cpuid(1).d & (1 << 9));

While you are at it, would you mind replacing "0xf0" with APIC_SPIV in enable_apic() also ?

Reviewed-by Krish Sadhukhan <[email protected]>

Quoted 55 lines · Krish Sadhukhan

Quoted 40 lines · On 05/02/2019 07:08 AM, [email protected] wrote:

From: Nadav Amit <[email protected]>

After the APIC is reset, some of its registers might be reset. As the SDM says: "When IA32_APIC_BASE[11] is set to 0, prior initialization to the APIC may be lost and the APIC may return to the state described in Section 10.4.7.1". The SDM also says that after APIC reset "the spurious-interrupt vector register is initialized to 000000FFH". This means that after the APIC is reset it needs to be software-enabled through the SPIV.

This is done one occasion, but there are (at least) two occasions that do not software-enable the APIC after reset (__test_apic_id() and main() in vmx.c).

Move APIC SPIV reinitialization into reset_apic(). Remove SPIV settings which are unnecessary after reset_apic() is modified.

Cc Sean Christopherson <[email protected]>

Signed-off-by Nadav Amit <[email protected]>

---
lib/x86/apic.c | 1 +
x86/apic.c | 1 -

2 files changed, 1 insertion(+), 1 deletion(-)

lib/x86/apic.c +1 -0

index 2aeffbd..4e7d43c 100644
@@ -161,6 +161,7 @@ void reset_apic(void)
 {
    disable_apic();
    wrmsr(MSR_IA32_APICBASE, rdmsr(MSR_IA32_APICBASE) | APIC_EN);
+    apic_write(APIC_SPIV, 0x1ff);
 }
  u32 ioapic_read_reg(unsigned reg)

x86/apic.c +0 -0

index 3eff588..7ef4a27 100644

x86/apic.c static void test_apic_disable(void) · line 152

Quoted 6 lines

x86/apic.c +0 -1

@@ -148,7 +148,6 @@ static void test_apic_disable(void)
    verify_disabled_apic_mmio();
      reset_apic();
-    apic_write(APIC_SPIV, 0x1ff);
    report("Local apic enabled in xAPIC mode",
 	   (rdmsr(MSR_IA32_APICBASE) & (APIC_EN | APIC_EXTD)) == APIC_EN);
    report("CPUID.1H:EDX.APIC[bit 9] is set", cpuid(1).d & (1 << 9));

While you are at it, would you mind replacing "0xf0" with APIC_SPIV in enable_apic() also ?

Quoted 1 line

Reviewed-by Krish Sadhukhan <[email protected]>

Will do. Thanks.