Introduce update_desc() for updates to live descriptor entries

GCC 4.4 from CentOS 6 is clever enough to turn the invlpg test's

  gdt[GDTE_AVAIL0] = GDTE_SYM(0, 1, COMMON, DATA, DPL0, B, W);
  write_fs(GDTE_AVAIL0 << 3);

into

  103927:       b8 48 00 00 00                  mov    $0x48,%eax
  10392c:       c7 05 48 f0 10 00 01 00 00 00   movl   $0x1,0x10f048
  103936:       8e e0                           mov    %eax,%fs
  103938:       c7 05 4c f0 10 00 00 93 c0 00   movl   $0xc09300,0x10f04c

which hardware rightfully complains about, as the descriptor isn't valid at
the point that %fs is loaded.

Introduce update_desc() which copes with PV and HVM differences, and enforces
a compiler barrier to prevent reordering of later operations.

Reported-by: Glenn Enright <glenn@rimuhosting.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff --git a/arch/x86/hvm/traps.c b/arch/x86/hvm/traps.c
index f936d4d74d0f9e2e41047d71eab4ba68ec34fda9..9d732e6002492f250f610105ba2d0ca673717cee 100644 (file)
--- a/arch/x86/hvm/traps.c
+++ b/arch/x86/hvm/traps.c
@@ -80,6 +80,7 @@ static void setup_doublefault(void)
     if ( IS_DEFINED(CONFIG_32BIT) )
     {
         gdt[GDTE_TSS_DF] = GDTE(_u(&tss_DF), 0x67, 0x89);
+        barrier();
 
         pack_task_gate(&idt[X86_EXC_DF], GDTE_TSS_DF * 8);
     }
@@ -121,6 +122,7 @@ void arch_init_traps(void)
     lidt(&idt_ptr);
 
     gdt[GDTE_TSS] = GDTE(_u(&tss), 0x67, 0x89);
+    barrier();
     ltr(GDTE_TSS * 8);
 
     /*

diff --git a/arch/x86/include/arch/xtf.h b/arch/x86/include/arch/xtf.h
index 36a0a500d6bdb90e6b62b200f0e99b223fba5749..c9b9b8924fd57f5f956ca495680bb4cd8bccff39 100644 (file)
--- a/arch/x86/include/arch/xtf.h
+++ b/arch/x86/include/arch/xtf.h
@@ -13,6 +13,31 @@
 
 extern char _end[];
 
+/*** Misc helpers which are library code, but really want to be inline. ***/
+
+/**
+ * Helper to update a live LDT/GDT entry.
+ */
+static inline void update_desc(user_desc *ptr, const user_desc new)
+{
+    if ( IS_DEFINED(CONFIG_HVM) )
+    {
+        *ptr = new;
+
+        /*
+         * Prevent the compiler reordering later operations which refer to the
+         * descriptor which has been updated.
+         */
+        barrier();
+    }
+    else
+    {
+        int rc = hypercall_update_descriptor(virt_to_maddr(ptr), new);
+        if ( rc )
+            panic("Update descriptor failed: %d\n", rc);
+    }
+}
+
 #endif /* XTF_X86_XTF_H */
 
 /*

diff --git a/tests/invlpg/main.c b/tests/invlpg/main.c
index 416fad38b7992655fba810c01b06d8c631ade108..826dc3027f7d730428ed714ad853fc86596feb00 100644 (file)
--- a/tests/invlpg/main.c
+++ b/tests/invlpg/main.c
@@ -234,7 +234,7 @@ static void test_tlb_refill(void)
         const struct tlb_refill_fs_test *t = &tlb_refill_fs_tests[i];
 
         printk("  Test: %%fs %s\n", t->desc);
-        gdt[GDTE_AVAIL0] = t->seg;
+        update_desc(&gdt[GDTE_AVAIL0], t->seg);
         write_fs(GDTE_AVAIL0 << 3);
         run_tlb_refill_test(invlpg_fs_refill, t->mapping);
     }
@@ -271,12 +271,12 @@ static void test_no_fault(void)
     invlpg_fs_checked(0);
 
     printk("  Test: Past segment limit\n");
-    gdt[GDTE_AVAIL0] = GDTE_SYM(0, 1, COMMON, DATA, DPL0, B, W);
+    update_desc(&gdt[GDTE_AVAIL0], GDTE_SYM(0, 1, COMMON, DATA, DPL0, B, W));
     write_fs(GDTE_AVAIL0 << 3);
     invlpg_fs_checked(0x2000);
 
     printk("  Test: Before expand-down segment limit\n");
-    gdt[GDTE_AVAIL0] = GDTE_SYM(0, 1, COMMON, DATA, DPL0, B, W, E);
+    update_desc(&gdt[GDTE_AVAIL0], GDTE_SYM(0, 1, COMMON, DATA, DPL0, B, W, E));
     write_fs(GDTE_AVAIL0 << 3);
     invlpg_fs_checked(0);
 

diff --git a/tests/memop-seg/main.c b/tests/memop-seg/main.c
index 579f4609780c9d1a870ad5516aaec12d27c5d59e..82ee9f44fd8fe93a366f2dcd93cc3f309cb34d39 100644 (file)
--- a/tests/memop-seg/main.c
+++ b/tests/memop-seg/main.c
@@ -260,28 +260,14 @@ void test_main(void)
     /* For 32bit, use segments with a limit of 2GB. */
     if ( IS_DEFINED(CONFIG_32BIT) )
     {
-        user_desc code = GDTE_SYM(0, 0x7ffff, COMMON, CODE, DPL3, R, D);
-        user_desc data = GDTE_SYM(0, 0x7ffff, COMMON, DATA, DPL3, B, W);
-
-        if ( IS_DEFINED(CONFIG_HVM) )
-        {
-            gdt[GDTE_AVAIL0] = code;
-            gdt[GDTE_AVAIL1] = data;
-        }
-        else
-        {
-            int rc = hypercall_update_descriptor(virt_to_maddr(
-                                                     &gdt[GDTE_AVAIL0]), code);
-
-            if ( !rc )
-                rc = hypercall_update_descriptor(virt_to_maddr(
-                                                     &gdt[GDTE_AVAIL1]), data);
-
-            if ( rc )
-                return xtf_error("Error: Update descriptor failed: %d\n", rc);
-        }
-
+        /* Code selector in AVAIL0 */
+        update_desc(&gdt[GDTE_AVAIL0],
+                    GDTE_SYM(0, 0x7ffff, COMMON, CODE, DPL3, R, D));
         exec_user_cs = GDTE_AVAIL0 << 3 | 3;
+
+        /* Data selector in AVAIL1 */
+        update_desc(&gdt[GDTE_AVAIL1],
+                    GDTE_SYM(0, 0x7ffff, COMMON, DATA, DPL3, B, W));
         exec_user_ss = GDTE_AVAIL1 << 3 | 3;
     }
 

diff --git a/tests/nmi-taskswitch-priv/main.c b/tests/nmi-taskswitch-priv/main.c
index 81c112f91ecf90c460c3be050802d962059107ff..ec5508959ba0318e2fda9f8c5647292dc1c1ccc8 100644 (file)
--- a/tests/nmi-taskswitch-priv/main.c
+++ b/tests/nmi-taskswitch-priv/main.c
@@ -146,9 +146,8 @@ void test_main(void)
      * Set up NMI handling to be a task gate.
      */
     xtf_unhandled_exception_hook = unhandled_exception;
-    gdt[GDTE_AVAIL0] = GDTE(_u(&nmi_tss), 0x67, 0x89);
+    update_desc(&gdt[GDTE_AVAIL0], GDTE(_u(&nmi_tss), 0x67, 0x89));
     pack_task_gate(&idt[X86_EXC_NMI], GDTE_AVAIL0 * 8);
-    barrier();
 
     /*
      * Send an NMI from supervisor mode, checking that we task switch back to

diff --git a/tests/xsa-186/main.c b/tests/xsa-186/main.c
index 8eef883d38d0af034aa80b173c626b7675026eb4..96a72bed66c9bc327b5162065ae06b84bcce3e2d 100644 (file)
--- a/tests/xsa-186/main.c
+++ b/tests/xsa-186/main.c
@@ -191,7 +191,8 @@ void test_main(void)
      * to execute the code with.  The stub still runs with 32bit data
      * segments, which is perfectly valid.
      */
-    gdt[GDTE_AVAIL0] = GDTE_SYM(0, 0xfffff, COMMON, CODE, DPL0, R);
+    update_desc(&gdt[GDTE_AVAIL0],
+                GDTE_SYM(0, 0xfffff, COMMON, CODE, DPL0, R));
 
     asm volatile ("push $%c[cs16];"
                   "push $1f;"

diff --git a/tests/xsa-191/main.c b/tests/xsa-191/main.c
index 30bec6fd4c014aa4e22eca73bf49247fcdf99f35..438c151fa968bff9395e846e6a9da87e78f70354 100644 (file)
--- a/tests/xsa-191/main.c
+++ b/tests/xsa-191/main.c
@@ -64,8 +64,7 @@ void test_main(void)
 
     user_desc ldt[1] = { gdt[__KERN_DS >> 3] };
 
-    gdt[GDTE_AVAIL0] = GDTE(_u(ldt), sizeof(ldt) - 1, 0x82);
-    barrier();
+    update_desc(&gdt[GDTE_AVAIL0], GDTE(_u(ldt), sizeof(ldt) - 1, 0x82));
 
     lldt(GDTE_AVAIL0 << 3);
     lldt(0);

diff --git a/tests/xsa-192/main.c b/tests/xsa-192/main.c
index 7cc6c5c4cc159006e928736a8e12e3a439a05d64..de42903210e46273c7a57a0c674218ea3a2e4188 100644 (file)
--- a/tests/xsa-192/main.c
+++ b/tests/xsa-192/main.c
@@ -86,7 +86,7 @@ void test_main(void)
     xtf_set_idte(X86_VEC_AVAIL, &idte);
 
     /* Create the vm86 TSS descriptor. */
-    gdt[GDTE_AVAIL0] = GDTE(_u(&vm86_tss), 0x67, 0x89);
+    update_desc(&gdt[GDTE_AVAIL0], GDTE(_u(&vm86_tss), 0x67, 0x89));
 
     /* Copy a stub to somewhere vm86 can actually reach. */
     uint8_t insn_buf[] = { 0xcd, X86_VEC_AVAIL }; /* `int $X86_VEC_AVAIL` */
@@ -98,7 +98,7 @@ void test_main(void)
      */
     if ( vendor_is_amd )
     {
-        gdt[GDTE_AVAIL1] = GDTE(0, 0, 0x82);
+        update_desc(&gdt[GDTE_AVAIL1], GDTE(0, 0, 0x82));
         lldt(GDTE_AVAIL1 << 3);
     }
     lldt(0);