From: Andrew Cooper Date: Fri, 2 Mar 2018 14:19:56 +0000 (+0000) Subject: Introduce update_desc() for updates to live descriptor entries X-Git-Url: http://xenbits.xensource.com/gitweb?a=commitdiff_plain;h=067488fa25621c6a566e2f555fcb5a1088f7abb7;p=xtf.git 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 Signed-off-by: Andrew Cooper --- diff --git a/arch/x86/hvm/traps.c b/arch/x86/hvm/traps.c index f936d4d..9d732e6 100644 --- 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 36a0a50..c9b9b89 100644 --- 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 416fad3..826dc30 100644 --- 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 579f460..82ee9f4 100644 --- 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 81c112f..ec55089 100644 --- 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 8eef883..96a72be 100644 --- 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 30bec6f..438c151 100644 --- 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 7cc6c5c..de42903 100644 --- 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);