Improved guest segment state setup

include/x86/cr.h

@@ -5,6 +5,8 @@
 
 #define CR4_VMXE (1 << 13)
 
+// TODO properly handle segment registers as 16-bit values
+
 #define DECLARE_REGISTER_ACCESSORS(regname)\
 	static inline uint64_t read##regname(void) {\
 		uint64_t v;\
@@ -40,6 +42,14 @@ DECLARE_REGISTER_ACCESSORS(dr5)
 DECLARE_REGISTER_ACCESSORS(dr6)
 DECLARE_REGISTER_ACCESSORS(dr7)
 
+static inline uint16_t
+readtr(void)
+{
+	uint16_t v;
+	__asm__ ("str %0\n\t" : "=a"(v));
+	return v;
+}
+
 static inline uint64_t
 readflags(void)
 {

include/x86/gdt.h

@@ -2,6 +2,7 @@
 #define _VISOR_GDT_H_
 
 #include <stdint.h>
+#include <stdbool.h>
 
 __attribute__ ((packed))
 struct segdescr {
@@ -10,15 +11,13 @@ struct segdescr {
 	uint8_t  base1;
 	uint8_t  access;
 	uint8_t  flags;
-	uint16_t base2;
+	uint8_t  base2;
-	uint32_t base3;
-	uint32_t reserved;
 };
 
 __attribute__ ((packed))
 struct GDTR {
 	uint16_t limit;
-	uint64_t base;
+	struct segdescr *base;
 };
 
 __attribute__ ((packed))
@@ -49,7 +48,7 @@ storegdt(void)
 }
 
 static inline void
-setsegdescr(struct segdescr *descr, uint64_t base, uint32_t limit, uint8_t access, uint8_t flags)
+setsegdescr(struct segdescr *descr, uint32_t base, uint32_t limit, uint8_t access, uint8_t flags)
 {
 	descr->limit0   = limit;
 	descr->base0    = base;
@@ -57,8 +56,32 @@ setsegdescr(struct segdescr *descr, uint64_t base, uint32_t limit, uint8_t acces
 	descr->access   = access;
 	descr->flags    = flags | (limit >> 16);
 	descr->base2    = base >> 24;
-	descr->base3    = base >> 32;
+}
-	descr->reserved = 0;
+
+static inline uint32_t
+getsegbase(struct segdescr *descr)
+{
+	return descr->base0 | ((uint32_t)descr->base1 << 16) | ((uint32_t)descr->base2 << 24);
+}
+
+static inline uint32_t
+getsegbaseext(struct segdescr *descr)
+{
+	uint32_t *words = (void *)descr;
+	return words[0];
+}
+
+static inline uint32_t
+getseglimit(struct segdescr *descr)
+{
+	return descr->limit0 | ((uint32_t)(descr->flags & 0xf) << 16);
+}
+
+static inline uint32_t
+getsegaccess(struct segdescr *descr, bool unusable)
+{
+	uint32_t *words = (void *)descr;
+	return (words[1] & 0x00ffffff) | (unusable ? 0x10000 : 0);
 }
 
 #endif

include/x86/idt.h

@@ -17,7 +17,7 @@ struct intdescr {
 __attribute__ ((packed))
 struct IDTR {
 	uint16_t limit;
-	uint64_t base;
+	struct intdescr *base;
 };
 
 static inline struct IDTR

src/proc.c

@@ -12,20 +12,18 @@ struct procvisor *procvisors[256];
 static void
 proc_fill_host_gdt(struct GDTR gdtr, struct TSS *tss)
 {
-	struct segdescr *descrs = (void *)gdtr.base;
+	setsegdescr(&gdtr.base[0], 0, 0, 0, 0); // null
-	setsegdescr(&descrs[0], 0, 0, 0, 0); // null
+	setsegdescr(&gdtr.base[1], 0, 0xffffff, 0x9B, 0xA0); // code
-	setsegdescr(&descrs[1], 0, 0xffffff, 0x9B, 0xA0); // code
+	setsegdescr(&gdtr.base[2], 0, 0xffffff, 0x93, 0xC0); // data
-	setsegdescr(&descrs[2], 0, 0xffffff, 0x93, 0xC0); // data
+	setsegdescr(&gdtr.base[3], (uintptr_t)tss, TSS_SIZE - 1, 0x89, 0x00); // TSS
-	setsegdescr(&descrs[3], (uintptr_t)tss, TSS_SIZE - 1, 0x89, 0x00); // TSS
+	*(uint64_t *)(&gdtr.base[4]) = (uintptr_t)tss >> 32;// TSS (continued)
-	*(uint64_t *)(&descrs[4]) = (uintptr_t)tss >> 32;// TSS (continued)
 }
 
 static void
 proc_fill_host_idt(struct IDTR idtr)
 {
-	struct intdescr *descrs = (void *)idtr.base;
 	for (int i = 0; i < 256; i++) {
-		setintdescr(&descrs[i], 0, 0, 0, 0);
+		setintdescr(&idtr.base[i], 0, 0, 0, 0);
 	}
 }
 
@@ -42,12 +40,12 @@ proc_setup(void)
 
 	// Carve out IDT
 	top -= IDT_SIZE;
-	pv->host_idtr.base  = (uintptr_t)top;
+	pv->host_idtr.base  = (void *)top;
 	pv->host_idtr.limit = IDT_SIZE - 1;
 
 	// Carve out GDT
 	top -= GDT_SIZE;
-	pv->host_gdtr.base  = (uintptr_t)top;
+	pv->host_gdtr.base  = (void *)top;
 	pv->host_gdtr.limit = GDT_SIZE - 1;
 
 	// Carve out TSS

src/vintel.c

@@ -83,6 +83,10 @@ vintel_fix_cr_bits(void)
 #define GUEST_FS_SELECTOR 0x0808
 #define GUEST_GS_SELECTOR 0x080A
 
+#define GUEST_ES_LIMIT  0x4800
+#define GUEST_ES_ACCESS 0x4814
+#define GUEST_ES_BASE   0x6806
+
 #define GUEST_GDTR_BASE 0x6816
 #define GUEST_IDTR_BASE 0x6818
 
@@ -97,19 +101,44 @@ guest_main(void)
 	for (;;) {}
 }
 
+static void
+vintel_write_guest_segment(struct GDTR gdtr, int reg, int ss, uint32_t hibase)
+{
+	struct segdescr *descr = &gdtr.base[ss >> 3];
+
+	vmwrite(GUEST_ES_SELECTOR + 2 * reg, ss & 0xfff8);
+	vmwrite(GUEST_ES_BASE     + 2 * reg, getsegbase(descr) | ((uint64_t)hibase << 32));
+	vmwrite(GUEST_ES_LIMIT    + 2 * reg, getseglimit(descr));
+	vmwrite(GUEST_ES_ACCESS   + 2 * reg, getsegaccess(descr, false));
+}
+
 static void
 vintel_init_guest(void)
 {
+	struct GDTR gdtr = storegdt();
+	struct IDTR idtr = storeidt();
+
 	uint64_t guestSP = (uintptr_t)guest_stack + 4096;
 	uint64_t guestIP = (uintptr_t)(void *)guest_main;
 
 	// Set segment selectors
-	vmwrite(GUEST_ES_SELECTOR, reades() & 0xfff8);
+	vintel_write_guest_segment(gdtr, 0, reades(), 0);
-	vmwrite(GUEST_CS_SELECTOR, readcs() & 0xfff8);
+	vintel_write_guest_segment(gdtr, 1, readcs(), 0);
-	vmwrite(GUEST_SS_SELECTOR, readss() & 0xfff8);
+	vintel_write_guest_segment(gdtr, 2, readss(), 0);
-	vmwrite(GUEST_DS_SELECTOR, readds() & 0xfff8);
+	vintel_write_guest_segment(gdtr, 3, readds(), 0);
-	vmwrite(GUEST_FS_SELECTOR, readfs() & 0xfff8);
+	vintel_write_guest_segment(gdtr, 4, readfs(), 0);
-	vmwrite(GUEST_GS_SELECTOR, readgs() & 0xfff8);
+	vintel_write_guest_segment(gdtr, 5, readgs(), 0);
+	
+	/* VMX requires a usable task register.
+	 * UEFI requires us to use their GDT, which does not include a task register.
+	 * So we try something stupid: load temporary but valid values into the TR shadow registers.
+	 * Then later reload the GDT to make the shadow registers consistent again.
+	 * Sigh.
+	 */
+	vmwrite(GUEST_ES_SELECTOR + 2 * 7, readtr() & 0xfff8);
+	vmwrite(GUEST_ES_BASE + 2 * 7, (uintptr_t)myprocvisor()->host_tss); // just abuse the host's TSS for this
+	vmwrite(GUEST_ES_LIMIT + 2 * 7, sizeof (struct TSS) - 1);
+	vmwrite(GUEST_ES_ACCESS + 2 * 7, 0x00000089);
 
 	// Set control registers
 	vmwrite(GUEST_CR0, readcr0());
@@ -131,10 +160,8 @@ vintel_init_guest(void)
 	vmwrite(GUEST_SYSENTER_EIP,  readmsr64(0x176));
 
 	// Set GDT and IDT
-	struct GDTR gdtr = storegdt();
+	vmwrite(GUEST_GDTR_BASE, (uintptr_t)gdtr.base);
-	struct IDTR idtr = storeidt();
+	vmwrite(GUEST_IDTR_BASE, (uintptr_t)idtr.base);
-	vmwrite(GUEST_GDTR_BASE, gdtr.base);
-	vmwrite(GUEST_IDTR_BASE, idtr.base);
 }
 
 #define HOST_ES_SELECTOR 0xC00
@@ -197,8 +224,8 @@ vintel_init_host(void)
 	vmwrite(HOST_IA32_EFER, readmsr64(IA32_EFER));
 
 	// Set GDT and IDT
-	vmwrite(HOST_GDTR_BASE, pv->host_gdtr.base);
+	vmwrite(HOST_GDTR_BASE, (uintptr_t)pv->host_gdtr.base);
-	vmwrite(HOST_IDTR_BASE, pv->host_idtr.base);
+	vmwrite(HOST_IDTR_BASE, (uintptr_t)pv->host_idtr.base);
 }
 
 void