diff mbox

[kvm-unit-tests] x86: apic: APIC ID tests

Message ID 20160707172453.15642-1-rkrcmar@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Radim Krčmář July 7, 2016, 5:24 p.m. UTC
KVM commit 000000000000 ("KVM: x86: reset xAPIC ID") fixed xAPIC ID
value after reset.

QEMU commit 5232d00a041c ("target-i386: Implement CPUID[0xB] (Extended
Topology Enumeration)") added initial x2APIC to CPUID.

KVM commit 000000000000 ("KVM: x86: use hardware-compatible format for
APIC ID register") changed internal format of APIC ID register, so make
sure that guest-visible APIC ID was not been affected.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 I'll send v2 with hashes when KVM patches are merged.

 Should I remove the hunk that depends on QEMU version?
 (It introduces a FAIL.)

 lib/x86/apic.c |  9 +++++++++
 lib/x86/apic.h |  1 +
 x86/apic.c     | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+)

Comments

Paolo Bonzini July 14, 2016, 9:50 a.m. UTC | #1
On 07/07/2016 19:24, Radim Krčmář wrote:
> KVM commit 000000000000 ("KVM: x86: reset xAPIC ID") fixed xAPIC ID
> value after reset.
> 
> QEMU commit 5232d00a041c ("target-i386: Implement CPUID[0xB] (Extended
> Topology Enumeration)") added initial x2APIC to CPUID.
> 
> KVM commit 000000000000 ("KVM: x86: use hardware-compatible format for
> APIC ID register") changed internal format of APIC ID register, so make
> sure that guest-visible APIC ID was not been affected.
> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  I'll send v2 with hashes when KVM patches are merged.
> 
>  Should I remove the hunk that depends on QEMU version?
>  (It introduces a FAIL.)

No, it's always possible to revert it locally if desired.

Applied with fixed hashes, thanks.

Paolo

>  lib/x86/apic.c |  9 +++++++++
>  lib/x86/apic.h |  1 +
>  x86/apic.c     | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 67 insertions(+)
> 
> diff --git a/lib/x86/apic.c b/lib/x86/apic.c
> index 2ceba395e0dc..a9ed28d67727 100644
> --- a/lib/x86/apic.c
> +++ b/lib/x86/apic.c
> @@ -1,6 +1,7 @@
>  #include "libcflat.h"
>  #include "apic.h"
>  #include "msr.h"
> +#include "processor.h"
>  
>  static void *g_apic = (void *)0xfee00000;
>  static void *g_ioapic = (void *)0xfec00000;
> @@ -129,6 +130,14 @@ int enable_x2apic(void)
>      }
>  }
>  
> +void reset_apic(void)
> +{
> +    u64 disabled = rdmsr(MSR_IA32_APICBASE) & ~(APIC_EN | APIC_EXTD);
> +    wrmsr(MSR_IA32_APICBASE, disabled);
> +    apic_ops = &xapic_ops;
> +    wrmsr(MSR_IA32_APICBASE, disabled | APIC_EN);
> +}
> +
>  u32 ioapic_read_reg(unsigned reg)
>  {
>      *(volatile u32 *)g_ioapic = reg;
> diff --git a/lib/x86/apic.h b/lib/x86/apic.h
> index 2d0504c2088d..dbd6c9b6e7e4 100644
> --- a/lib/x86/apic.h
> +++ b/lib/x86/apic.h
> @@ -37,5 +37,6 @@ void apic_icr_write(uint32_t val, uint32_t dest);
>  uint32_t apic_id(void);
>  
>  int enable_x2apic(void);
> +void reset_apic(void);
>  
>  #endif
> diff --git a/x86/apic.c b/x86/apic.c
> index 8b08a950a0c7..5fc83c681ce8 100644
> --- a/x86/apic.c
> +++ b/x86/apic.c
> @@ -136,6 +136,62 @@ static void test_apicbase(void)
>      report_prefix_pop();
>  }
>  
> +static void do_write_apic_id(void *id)
> +{
> +    apic_write(APIC_ID, *(u32 *)id);
> +}
> +
> +static void __test_apic_id(void * unused)
> +{
> +    u32 id, newid;
> +    u8  initial_xapic_id = cpuid(1).b >> 24;
> +    u32 initial_x2apic_id = cpuid(0xb).d;
> +    bool x2apic_mode = rdmsr(MSR_IA32_APICBASE) & APIC_EXTD;
> +
> +    if (x2apic_mode)
> +        reset_apic();
> +
> +    id = apic_id();
> +    report("xapic id matches cpuid", initial_xapic_id == id);
> +
> +    newid = (id + 1) << 24;
> +    report("writeable xapic id",
> +            !test_for_exception(GP_VECTOR, do_write_apic_id, &newid) &&
> +            id + 1 == apic_id());
> +
> +    if (!enable_x2apic())
> +        goto out;
> +
> +    report("non-writeable x2apic id",
> +            test_for_exception(GP_VECTOR, do_write_apic_id, &newid));
> +    report("sane x2apic id", initial_xapic_id == (apic_id() & 0xff));
> +
> +    /* old QEMUs do not set initial x2APIC ID */
> +    report("x2apic id matches cpuid",
> +           initial_xapic_id == (initial_x2apic_id & 0xff) &&
> +           initial_x2apic_id == apic_id());
> +
> +out:
> +    reset_apic();
> +
> +    report("correct xapic id after reset", initial_xapic_id == apic_id());
> +
> +    /* old KVMs do not reset xAPIC ID */
> +    if (id != apic_id())
> +        apic_write(APIC_ID, id << 24);
> +
> +    if (x2apic_mode)
> +        enable_x2apic();
> +}
> +
> +static void test_apic_id(void)
> +{
> +    if (cpu_count() < 2)
> +        return;
> +
> +    on_cpu(1, __test_apic_id, NULL);
> +}
> +
>  static int ipi_count;
>  
>  static void self_ipi_isr(isr_regs_t *regs)
> @@ -303,6 +359,7 @@ int main()
>      test_lapic_existence();
>  
>      mask_pic_interrupts();
> +    test_apic_id();
>      test_enable_x2apic();
>      test_apicbase();
>  
> 
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/lib/x86/apic.c b/lib/x86/apic.c
index 2ceba395e0dc..a9ed28d67727 100644
--- a/lib/x86/apic.c
+++ b/lib/x86/apic.c
@@ -1,6 +1,7 @@ 
 #include "libcflat.h"
 #include "apic.h"
 #include "msr.h"
+#include "processor.h"
 
 static void *g_apic = (void *)0xfee00000;
 static void *g_ioapic = (void *)0xfec00000;
@@ -129,6 +130,14 @@  int enable_x2apic(void)
     }
 }
 
+void reset_apic(void)
+{
+    u64 disabled = rdmsr(MSR_IA32_APICBASE) & ~(APIC_EN | APIC_EXTD);
+    wrmsr(MSR_IA32_APICBASE, disabled);
+    apic_ops = &xapic_ops;
+    wrmsr(MSR_IA32_APICBASE, disabled | APIC_EN);
+}
+
 u32 ioapic_read_reg(unsigned reg)
 {
     *(volatile u32 *)g_ioapic = reg;
diff --git a/lib/x86/apic.h b/lib/x86/apic.h
index 2d0504c2088d..dbd6c9b6e7e4 100644
--- a/lib/x86/apic.h
+++ b/lib/x86/apic.h
@@ -37,5 +37,6 @@  void apic_icr_write(uint32_t val, uint32_t dest);
 uint32_t apic_id(void);
 
 int enable_x2apic(void);
+void reset_apic(void);
 
 #endif
diff --git a/x86/apic.c b/x86/apic.c
index 8b08a950a0c7..5fc83c681ce8 100644
--- a/x86/apic.c
+++ b/x86/apic.c
@@ -136,6 +136,62 @@  static void test_apicbase(void)
     report_prefix_pop();
 }
 
+static void do_write_apic_id(void *id)
+{
+    apic_write(APIC_ID, *(u32 *)id);
+}
+
+static void __test_apic_id(void * unused)
+{
+    u32 id, newid;
+    u8  initial_xapic_id = cpuid(1).b >> 24;
+    u32 initial_x2apic_id = cpuid(0xb).d;
+    bool x2apic_mode = rdmsr(MSR_IA32_APICBASE) & APIC_EXTD;
+
+    if (x2apic_mode)
+        reset_apic();
+
+    id = apic_id();
+    report("xapic id matches cpuid", initial_xapic_id == id);
+
+    newid = (id + 1) << 24;
+    report("writeable xapic id",
+            !test_for_exception(GP_VECTOR, do_write_apic_id, &newid) &&
+            id + 1 == apic_id());
+
+    if (!enable_x2apic())
+        goto out;
+
+    report("non-writeable x2apic id",
+            test_for_exception(GP_VECTOR, do_write_apic_id, &newid));
+    report("sane x2apic id", initial_xapic_id == (apic_id() & 0xff));
+
+    /* old QEMUs do not set initial x2APIC ID */
+    report("x2apic id matches cpuid",
+           initial_xapic_id == (initial_x2apic_id & 0xff) &&
+           initial_x2apic_id == apic_id());
+
+out:
+    reset_apic();
+
+    report("correct xapic id after reset", initial_xapic_id == apic_id());
+
+    /* old KVMs do not reset xAPIC ID */
+    if (id != apic_id())
+        apic_write(APIC_ID, id << 24);
+
+    if (x2apic_mode)
+        enable_x2apic();
+}
+
+static void test_apic_id(void)
+{
+    if (cpu_count() < 2)
+        return;
+
+    on_cpu(1, __test_apic_id, NULL);
+}
+
 static int ipi_count;
 
 static void self_ipi_isr(isr_regs_t *regs)
@@ -303,6 +359,7 @@  int main()
     test_lapic_existence();
 
     mask_pic_interrupts();
+    test_apic_id();
     test_enable_x2apic();
     test_apicbase();