All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 kvm-unit-tests 0/8] x86: Move IDT, GDT and TSS to C code
@ 2021-10-21 11:49 Paolo Bonzini
  2021-10-21 11:49 ` [PATCH kvm-unit-tests 1/9] x86: cleanup handling of 16-byte GDT descriptors Paolo Bonzini
                   ` (9 more replies)
  0 siblings, 10 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-10-21 11:49 UTC (permalink / raw)
  To: kvm
  Cc: aaronlewis, jmattson, zxwang42, marcorr, seanjc, jroedel, varad.gautam

Patches 1-4 clean up tss_descr; it is declared as a struct
descriptor_table_ptr but it is actualy pointing to an _entry_ in the GDT.
Also it is different per CPU, but tss_descr does not recognize that.
Fix both by reusing the code (already present e.g. in the vmware_backdoors
test) that extracts the base from the GDT entry; and also provide a
helper to retrieve the limit, which is needed in vmx.c.

Patches 5-9 move the IDT, GDT and TSS to C code.  This was originally done
by Zixuan Wang for the UEFI port, which is 64-bit only.  The series extends
this to 32-bit code for consistency and to avoid duplicating code between
C and assembly.

Paolo

v2->v3: cleaned up handling of 16 byte descriptors (new patch 1)
	get TR limit from GDT
	rename high four bits of segment limit to "limit2"
	included Zixuan's work to port GDT/TSS/IDT to C, extended to 32-bit

Paolo Bonzini (8):
  x86: cleanup handling of 16-byte GDT descriptors
  x86: fix call to set_gdt_entry
  unify field names and definitions for GDT descriptors
  replace tss_descr global with a function
  x86: Move IDT to desc.c
  x86: unify name of 32-bit and 64-bit GDT
  x86: get rid of ring0stacktop
  x86: Move 32-bit GDT and TSS to desc.c

Zixuan Wang (1):
  x86: Move 64-bit GDT and TSS to desc.c

 lib/x86/asm/setup.h    |   6 +++
 lib/x86/desc.c         | 116 +++++++++++++++++++++++++++++++++++------
 lib/x86/desc.h         |  31 +++++------
 lib/x86/setup.c        |  49 +++++++++++++++++
 lib/x86/usermode.c     |   9 ++--
 x86/access.c           |  16 +++---
 x86/cstart.S           | 115 ++++++----------------------------------
 x86/cstart64.S         |  97 ++++------------------------------
 x86/smap.c             |   2 +-
 x86/svm_tests.c        |  15 ++----
 x86/taskswitch.c       |   4 +-
 x86/umip.c             |  19 ++++---
 x86/vmware_backdoors.c |  22 +++-----
 x86/vmx.c              |  17 +++---
 x86/vmx_tests.c        |   4 +-
 15 files changed, 244 insertions(+), 278 deletions(-)
 create mode 100644 lib/x86/asm/setup.h

-- 
2.27.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH kvm-unit-tests 1/9] x86: cleanup handling of 16-byte GDT descriptors
  2021-10-21 11:49 [PATCH v3 kvm-unit-tests 0/8] x86: Move IDT, GDT and TSS to C code Paolo Bonzini
@ 2021-10-21 11:49 ` Paolo Bonzini
  2021-11-29 21:46   ` Marc Orr
  2021-10-21 11:49 ` [PATCH kvm-unit-tests 2/9] x86: fix call to set_gdt_entry Paolo Bonzini
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2021-10-21 11:49 UTC (permalink / raw)
  To: kvm
  Cc: aaronlewis, jmattson, zxwang42, marcorr, seanjc, jroedel, varad.gautam

Look them up using a gdt_entry_t pointer, so that the address of
the descriptor is correct even for "odd" selectors (e.g. 0x98).
Rename the struct from segment_desc64 to system_desc64,
highlighting that it is only used in the case of S=0 (system
descriptor).  Rename the "limit" bitfield to "limit2", matching
the convention used for the various parts of the base field.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 lib/x86/desc.h         |  4 ++--
 x86/svm_tests.c        | 12 ++++++------
 x86/vmware_backdoors.c |  8 ++++----
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/lib/x86/desc.h b/lib/x86/desc.h
index a6ffb38..1755486 100644
--- a/lib/x86/desc.h
+++ b/lib/x86/desc.h
@@ -172,7 +172,7 @@ typedef struct {
 	u8 base_high;
 } gdt_entry_t;
 
-struct segment_desc64 {
+struct system_desc64 {
 	uint16_t limit1;
 	uint16_t base1;
 	uint8_t  base2;
@@ -183,7 +183,7 @@ struct segment_desc64 {
 			uint16_t s:1;
 			uint16_t dpl:2;
 			uint16_t p:1;
-			uint16_t limit:4;
+			uint16_t limit2:4;
 			uint16_t avl:1;
 			uint16_t l:1;
 			uint16_t db:1;
diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index afdd359..2fdb0dc 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -1876,22 +1876,22 @@ static bool reg_corruption_check(struct svm_test *test)
 static void get_tss_entry(void *data)
 {
     struct descriptor_table_ptr gdt;
-    struct segment_desc64 *gdt_table;
-    struct segment_desc64 *tss_entry;
+    gdt_entry_t *gdt_table;
+    struct system_desc64 *tss_entry;
     u16 tr = 0;
 
     sgdt(&gdt);
     tr = str();
-    gdt_table = (struct segment_desc64 *) gdt.base;
-    tss_entry = &gdt_table[tr / sizeof(struct segment_desc64)];
-    *((struct segment_desc64 **)data) = tss_entry;
+    gdt_table = (gdt_entry_t *) gdt.base;
+    tss_entry = (struct system_desc64 *) &gdt_table[tr / 8];
+    *((struct system_desc64 **)data) = tss_entry;
 }
 
 static int orig_cpu_count;
 
 static void init_startup_prepare(struct svm_test *test)
 {
-    struct segment_desc64 *tss_entry;
+    struct system_desc64 *tss_entry;
     int i;
 
     on_cpu(1, get_tss_entry, &tss_entry);
diff --git a/x86/vmware_backdoors.c b/x86/vmware_backdoors.c
index b4902a9..b1433cd 100644
--- a/x86/vmware_backdoors.c
+++ b/x86/vmware_backdoors.c
@@ -133,8 +133,8 @@ struct fault_test vmware_backdoor_tests[] = {
 static void set_tss_ioperm(void)
 {
 	struct descriptor_table_ptr gdt;
-	struct segment_desc64 *gdt_table;
-	struct segment_desc64 *tss_entry;
+	gdt_entry_t *gdt_table;
+	struct system_desc64 *tss_entry;
 	u16 tr = 0;
 	tss64_t *tss;
 	unsigned char *ioperm_bitmap;
@@ -142,8 +142,8 @@ static void set_tss_ioperm(void)
 
 	sgdt(&gdt);
 	tr = str();
-	gdt_table = (struct segment_desc64 *) gdt.base;
-	tss_entry = &gdt_table[tr / sizeof(struct segment_desc64)];
+	gdt_table = (gdt_entry_t *) gdt.base;
+	tss_entry = (struct system_desc64 *) &gdt_table[tr / 8];
 	tss_base = ((uint64_t) tss_entry->base1 |
 			((uint64_t) tss_entry->base2 << 16) |
 			((uint64_t) tss_entry->base3 << 24) |
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH kvm-unit-tests 2/9] x86: fix call to set_gdt_entry
  2021-10-21 11:49 [PATCH v3 kvm-unit-tests 0/8] x86: Move IDT, GDT and TSS to C code Paolo Bonzini
  2021-10-21 11:49 ` [PATCH kvm-unit-tests 1/9] x86: cleanup handling of 16-byte GDT descriptors Paolo Bonzini
@ 2021-10-21 11:49 ` Paolo Bonzini
  2021-10-21 11:49 ` [PATCH kvm-unit-tests 3/9] unify field names and definitions for GDT descriptors Paolo Bonzini
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-10-21 11:49 UTC (permalink / raw)
  To: kvm
  Cc: aaronlewis, jmattson, zxwang42, marcorr, seanjc, jroedel, varad.gautam

The low four bits of the fourth argument are unused, make them
zero in all the callers.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 lib/x86/desc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index e7378c1..b691c9b 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -336,7 +336,7 @@ void setup_tss32(void)
 	tss_intr.ds = tss_intr.es = tss_intr.fs = tss_intr.ss = 0x10;
 	tss_intr.gs = read_gs();
 	tss_intr.iomap_base = (u16)desc_size;
-	set_gdt_entry(TSS_INTR, (u32)&tss_intr, desc_size - 1, 0x89, 0x0f);
+	set_gdt_entry(TSS_INTR, (u32)&tss_intr, desc_size - 1, 0x89, 0);
 }
 
 void set_intr_task_gate(int e, void *fn)
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH kvm-unit-tests 3/9] unify field names and definitions for GDT descriptors
  2021-10-21 11:49 [PATCH v3 kvm-unit-tests 0/8] x86: Move IDT, GDT and TSS to C code Paolo Bonzini
  2021-10-21 11:49 ` [PATCH kvm-unit-tests 1/9] x86: cleanup handling of 16-byte GDT descriptors Paolo Bonzini
  2021-10-21 11:49 ` [PATCH kvm-unit-tests 2/9] x86: fix call to set_gdt_entry Paolo Bonzini
@ 2021-10-21 11:49 ` Paolo Bonzini
  2021-10-21 11:49 ` [PATCH kvm-unit-tests 4/9] replace tss_descr global with a function Paolo Bonzini
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-10-21 11:49 UTC (permalink / raw)
  To: kvm
  Cc: aaronlewis, jmattson, zxwang42, marcorr, seanjc, jroedel, varad.gautam

Use the same names and definitions (apart from the high base field)
for GDT descriptors in both 32-bit and 64-bit code.  The next patch
will also reuse gdt_entry_t in the 16-byte struct definition, for now
some duplication remains.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 lib/x86/desc.c   | 18 +++++++-----------
 lib/x86/desc.h   | 28 +++++++++++++++++++++-------
 x86/taskswitch.c |  2 +-
 3 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index b691c9b..ba0db65 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -280,22 +280,18 @@ bool exception_rflags_rf(void)
 static char intr_alt_stack[4096];
 
 #ifndef __x86_64__
-void set_gdt_entry(int sel, u32 base,  u32 limit, u8 access, u8 gran)
+void set_gdt_entry(int sel, u32 base,  u32 limit, u8 type, u8 flags)
 {
 	int num = sel >> 3;
 
 	/* Setup the descriptor base address */
-	gdt32[num].base_low = (base & 0xFFFF);
-	gdt32[num].base_middle = (base >> 16) & 0xFF;
-	gdt32[num].base_high = (base >> 24) & 0xFF;
+	gdt32[num].base1 = (base & 0xFFFF);
+	gdt32[num].base2 = (base >> 16) & 0xFF;
+	gdt32[num].base3 = (base >> 24) & 0xFF;
 
-	/* Setup the descriptor limits */
-	gdt32[num].limit_low = (limit & 0xFFFF);
-	gdt32[num].granularity = ((limit >> 16) & 0x0F);
-
-	/* Finally, set up the granularity and access flags */
-	gdt32[num].granularity |= (gran & 0xF0);
-	gdt32[num].access = access;
+	/* Setup the descriptor limits, type and flags */
+	gdt32[num].limit1 = (limit & 0xFFFF);
+	gdt32[num].type_limit_flags = ((limit & 0xF0000) >> 8) | ((flags & 0xF0) << 8) | type;
 }
 
 void set_gdt_task_gate(u16 sel, u16 tss_sel)
diff --git a/lib/x86/desc.h b/lib/x86/desc.h
index 1755486..c339e0e 100644
--- a/lib/x86/desc.h
+++ b/lib/x86/desc.h
@@ -164,14 +164,27 @@ typedef struct {
 } idt_entry_t;
 
 typedef struct {
-	u16 limit_low;
-	u16 base_low;
-	u8 base_middle;
-	u8 access;
-	u8 granularity;
-	u8 base_high;
-} gdt_entry_t;
+	uint16_t limit1;
+	uint16_t base1;
+	uint8_t  base2;
+	union {
+		uint16_t  type_limit_flags;      /* Type and limit flags */
+		struct {
+			uint16_t type:4;
+			uint16_t s:1;
+			uint16_t dpl:2;
+			uint16_t p:1;
+			uint16_t limit2:4;
+			uint16_t avl:1;
+			uint16_t l:1;
+			uint16_t db:1;
+			uint16_t g:1;
+		} __attribute__((__packed__));
+	} __attribute__((__packed__));
+	uint8_t  base3;
+} __attribute__((__packed__)) gdt_entry_t;
 
+#ifdef __x86_64__
 struct system_desc64 {
 	uint16_t limit1;
 	uint16_t base1;
@@ -194,6 +207,7 @@ struct system_desc64 {
 	uint32_t base4;
 	uint32_t zero;
 } __attribute__((__packed__));
+#endif
 
 #define DESC_BUSY ((uint64_t) 1 << 41)
 
diff --git a/x86/taskswitch.c b/x86/taskswitch.c
index 889831e..b6b3451 100644
--- a/x86/taskswitch.c
+++ b/x86/taskswitch.c
@@ -21,7 +21,7 @@ fault_handler(unsigned long error_code)
 
 	tss.eip += 2;
 
-	gdt32[TSS_MAIN / 8].access &= ~2;
+	gdt32[TSS_MAIN / 8].type &= ~2;
 
 	set_gdt_task_gate(TSS_RETURN, tss_intr.prev);
 }
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH kvm-unit-tests 4/9] replace tss_descr global with a function
  2021-10-21 11:49 [PATCH v3 kvm-unit-tests 0/8] x86: Move IDT, GDT and TSS to C code Paolo Bonzini
                   ` (2 preceding siblings ...)
  2021-10-21 11:49 ` [PATCH kvm-unit-tests 3/9] unify field names and definitions for GDT descriptors Paolo Bonzini
@ 2021-10-21 11:49 ` Paolo Bonzini
  2021-10-21 11:49 ` [PATCH kvm-unit-tests 5/9] x86: Move IDT to desc.c Paolo Bonzini
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-10-21 11:49 UTC (permalink / raw)
  To: kvm
  Cc: aaronlewis, jmattson, zxwang42, marcorr, seanjc, jroedel, varad.gautam

tss_descr is declared as a struct descriptor_table_ptr but it is actualy
pointing to an _entry_ in the GDT.  Also it is different per CPU, but
tss_descr does not recognize that.  Fix both by reusing the code
(already present e.g. in the vmware_backdoors test) that extracts
the base from the GDT entry; and also provide a helper to retrieve
the limit, which is needed in vmx.c.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 lib/x86/desc.c         | 32 ++++++++++++++++++++++++++++++++
 lib/x86/desc.h         | 25 ++++++-------------------
 x86/cstart64.S         |  1 -
 x86/svm_tests.c        | 15 +++------------
 x86/taskswitch.c       |  2 +-
 x86/vmware_backdoors.c | 22 ++++++----------------
 x86/vmx.c              |  9 +++++----
 7 files changed, 53 insertions(+), 53 deletions(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index ba0db65..94f0ddb 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -409,3 +409,35 @@ void __set_exception_jmpbuf(jmp_buf *addr)
 {
 	exception_jmpbuf = addr;
 }
+
+gdt_entry_t *get_tss_descr(void)
+{
+	struct descriptor_table_ptr gdt_ptr;
+	gdt_entry_t *gdt;
+
+	sgdt(&gdt_ptr);
+	gdt = (gdt_entry_t *)gdt_ptr.base;
+	return &gdt[str() / 8];
+}
+
+unsigned long get_gdt_entry_base(gdt_entry_t *entry)
+{
+	unsigned long base;
+	base = entry->base1 | ((u32)entry->base2 << 16) | ((u32)entry->base3 << 24);
+#ifdef __x86_64__
+	if (!entry->s) {
+		base |= (u64)((struct system_desc64 *)entry)->base4 << 32;
+	}
+#endif
+	return base;
+}
+
+unsigned long get_gdt_entry_limit(gdt_entry_t *entry)
+{
+	unsigned long limit;
+	limit = entry->limit1 | ((u32)entry->limit2 << 16);
+	if (entry->g) {
+		limit = (limit << 12) | 0xFFF;
+	}
+	return limit;
+}
diff --git a/lib/x86/desc.h b/lib/x86/desc.h
index c339e0e..51148d1 100644
--- a/lib/x86/desc.h
+++ b/lib/x86/desc.h
@@ -186,30 +186,13 @@ typedef struct {
 
 #ifdef __x86_64__
 struct system_desc64 {
-	uint16_t limit1;
-	uint16_t base1;
-	uint8_t  base2;
-	union {
-		uint16_t  type_limit_flags;      /* Type and limit flags */
-		struct {
-			uint16_t type:4;
-			uint16_t s:1;
-			uint16_t dpl:2;
-			uint16_t p:1;
-			uint16_t limit2:4;
-			uint16_t avl:1;
-			uint16_t l:1;
-			uint16_t db:1;
-			uint16_t g:1;
-		} __attribute__((__packed__));
-	} __attribute__((__packed__));
-	uint8_t  base3;
+	gdt_entry_t common;
 	uint32_t base4;
 	uint32_t zero;
 } __attribute__((__packed__));
 #endif
 
-#define DESC_BUSY ((uint64_t) 1 << 41)
+#define DESC_BUSY 2
 
 extern idt_entry_t boot_idt[256];
 
@@ -253,4 +236,8 @@ static inline void *get_idt_addr(idt_entry_t *entry)
 	return (void *)addr;
 }
 
+extern gdt_entry_t *get_tss_descr(void);
+extern unsigned long get_gdt_entry_base(gdt_entry_t *entry);
+extern unsigned long get_gdt_entry_limit(gdt_entry_t *entry);
+
 #endif
diff --git a/x86/cstart64.S b/x86/cstart64.S
index 5c6ad38..cf38bae 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -4,7 +4,6 @@
 .globl boot_idt
 
 .globl idt_descr
-.globl tss_descr
 .globl gdt64_desc
 .globl online_cpus
 .globl cpu_online_count
diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index 2fdb0dc..8ad6122 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -1875,23 +1875,14 @@ static bool reg_corruption_check(struct svm_test *test)
 
 static void get_tss_entry(void *data)
 {
-    struct descriptor_table_ptr gdt;
-    gdt_entry_t *gdt_table;
-    struct system_desc64 *tss_entry;
-    u16 tr = 0;
-
-    sgdt(&gdt);
-    tr = str();
-    gdt_table = (gdt_entry_t *) gdt.base;
-    tss_entry = (struct system_desc64 *) &gdt_table[tr / 8];
-    *((struct system_desc64 **)data) = tss_entry;
+    *((gdt_entry_t **)data) = get_tss_descr();
 }
 
 static int orig_cpu_count;
 
 static void init_startup_prepare(struct svm_test *test)
 {
-    struct system_desc64 *tss_entry;
+    gdt_entry_t *tss_entry;
     int i;
 
     on_cpu(1, get_tss_entry, &tss_entry);
@@ -1905,7 +1896,7 @@ static void init_startup_prepare(struct svm_test *test)
 
     --cpu_online_count;
 
-    *(uint64_t *)tss_entry &= ~DESC_BUSY;
+    tss_entry->type &= ~DESC_BUSY;
 
     apic_icr_write(APIC_DEST_PHYSICAL | APIC_DM_STARTUP, id_map[1]);
 
diff --git a/x86/taskswitch.c b/x86/taskswitch.c
index b6b3451..0fa818d 100644
--- a/x86/taskswitch.c
+++ b/x86/taskswitch.c
@@ -21,7 +21,7 @@ fault_handler(unsigned long error_code)
 
 	tss.eip += 2;
 
-	gdt32[TSS_MAIN / 8].type &= ~2;
+	gdt32[TSS_MAIN / 8].type &= ~DESC_BUSY;
 
 	set_gdt_task_gate(TSS_RETURN, tss_intr.prev);
 }
diff --git a/x86/vmware_backdoors.c b/x86/vmware_backdoors.c
index b1433cd..bc10020 100644
--- a/x86/vmware_backdoors.c
+++ b/x86/vmware_backdoors.c
@@ -132,23 +132,13 @@ struct fault_test vmware_backdoor_tests[] = {
  */
 static void set_tss_ioperm(void)
 {
-	struct descriptor_table_ptr gdt;
-	gdt_entry_t *gdt_table;
-	struct system_desc64 *tss_entry;
-	u16 tr = 0;
+	gdt_entry_t *tss_entry;
 	tss64_t *tss;
 	unsigned char *ioperm_bitmap;
-	uint64_t tss_base;
-
-	sgdt(&gdt);
-	tr = str();
-	gdt_table = (gdt_entry_t *) gdt.base;
-	tss_entry = (struct system_desc64 *) &gdt_table[tr / 8];
-	tss_base = ((uint64_t) tss_entry->base1 |
-			((uint64_t) tss_entry->base2 << 16) |
-			((uint64_t) tss_entry->base3 << 24) |
-			((uint64_t) tss_entry->base4 << 32));
-	tss = (tss64_t *)tss_base;
+	u16 tr = str();
+
+	tss_entry = get_tss_descr();
+	tss = (tss64_t *)get_gdt_entry_base(tss_entry);
 	tss->iomap_base = sizeof(*tss);
 	ioperm_bitmap = ((unsigned char *)tss+tss->iomap_base);
 
@@ -157,7 +147,7 @@ static void set_tss_ioperm(void)
 		1 << (RANDOM_IO_PORT % 8);
 	ioperm_bitmap[VMWARE_BACKDOOR_PORT / 8] |=
 		1 << (VMWARE_BACKDOOR_PORT % 8);
-	*(uint64_t *)tss_entry &= ~DESC_BUSY;
+	tss_entry->type &= ~DESC_BUSY;
 
 	/* Update TSS */
 	ltr(tr);
diff --git a/x86/vmx.c b/x86/vmx.c
index 20dc677..d45c6de 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -75,7 +75,6 @@ union vmx_ept_vpid  ept_vpid;
 
 extern struct descriptor_table_ptr gdt64_desc;
 extern struct descriptor_table_ptr idt_descr;
-extern struct descriptor_table_ptr tss_descr;
 extern void *vmx_return;
 extern void *entry_sysenter;
 extern void *guest_entry;
@@ -1275,7 +1274,7 @@ static void init_vmcs_host(void)
 	vmcs_write(HOST_SEL_FS, KERNEL_DS);
 	vmcs_write(HOST_SEL_GS, KERNEL_DS);
 	vmcs_write(HOST_SEL_TR, TSS_MAIN);
-	vmcs_write(HOST_BASE_TR, tss_descr.base);
+	vmcs_write(HOST_BASE_TR, get_gdt_entry_base(get_tss_descr()));
 	vmcs_write(HOST_BASE_GDTR, gdt64_desc.base);
 	vmcs_write(HOST_BASE_IDTR, idt_descr.base);
 	vmcs_write(HOST_BASE_FS, 0);
@@ -1291,6 +1290,8 @@ static void init_vmcs_host(void)
 
 static void init_vmcs_guest(void)
 {
+	gdt_entry_t *tss_descr = get_tss_descr();
+
 	/* 26.3 CHECKING AND LOADING GUEST STATE */
 	ulong guest_cr0, guest_cr4, guest_cr3;
 	/* 26.3.1.1 */
@@ -1331,7 +1332,7 @@ static void init_vmcs_guest(void)
 	vmcs_write(GUEST_BASE_DS, 0);
 	vmcs_write(GUEST_BASE_FS, 0);
 	vmcs_write(GUEST_BASE_GS, 0);
-	vmcs_write(GUEST_BASE_TR, tss_descr.base);
+	vmcs_write(GUEST_BASE_TR, get_gdt_entry_base(tss_descr));
 	vmcs_write(GUEST_BASE_LDTR, 0);
 
 	vmcs_write(GUEST_LIMIT_CS, 0xFFFFFFFF);
@@ -1341,7 +1342,7 @@ static void init_vmcs_guest(void)
 	vmcs_write(GUEST_LIMIT_FS, 0xFFFFFFFF);
 	vmcs_write(GUEST_LIMIT_GS, 0xFFFFFFFF);
 	vmcs_write(GUEST_LIMIT_LDTR, 0xffff);
-	vmcs_write(GUEST_LIMIT_TR, tss_descr.limit);
+	vmcs_write(GUEST_LIMIT_TR, get_gdt_entry_limit(tss_descr));
 
 	vmcs_write(GUEST_AR_CS, 0xa09b);
 	vmcs_write(GUEST_AR_DS, 0xc093);
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH kvm-unit-tests 5/9] x86: Move IDT to desc.c
  2021-10-21 11:49 [PATCH v3 kvm-unit-tests 0/8] x86: Move IDT, GDT and TSS to C code Paolo Bonzini
                   ` (3 preceding siblings ...)
  2021-10-21 11:49 ` [PATCH kvm-unit-tests 4/9] replace tss_descr global with a function Paolo Bonzini
@ 2021-10-21 11:49 ` Paolo Bonzini
  2021-10-21 11:49 ` [PATCH kvm-unit-tests 6/9] x86: unify name of 32-bit and 64-bit GDT Paolo Bonzini
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-10-21 11:49 UTC (permalink / raw)
  To: kvm
  Cc: aaronlewis, jmattson, zxwang42, marcorr, seanjc, jroedel, varad.gautam

Move the IDT data structures from x86/cstart.S and x86/cstart64.S to
lib/x86/desc.c, so that the follow-up UEFI support commits can reuse
these definitions, without re-defining them in UEFI's boot up assembly
code.

Extracted by a patch by Zixuan Wang <zxwang42@gmail.com> and ported
to 32-bit too.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 lib/x86/desc.c | 10 ++++++++++
 x86/cstart.S   | 11 -----------
 x86/cstart64.S | 14 --------------
 3 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 94f0ddb..2ef5aad 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -3,6 +3,16 @@
 #include "processor.h"
 #include <setjmp.h>
 
+/* Boot-related data structures */
+
+/* IDT and IDT descriptor */
+idt_entry_t boot_idt[256] = {0};
+
+struct descriptor_table_ptr idt_descr = {
+	.limit = sizeof(boot_idt) - 1,
+	.base = (unsigned long)boot_idt,
+};
+
 #ifndef __x86_64__
 __attribute__((regparm(1)))
 #endif
diff --git a/x86/cstart.S b/x86/cstart.S
index bcf7218..4461c38 100644
--- a/x86/cstart.S
+++ b/x86/cstart.S
@@ -1,7 +1,6 @@
 
 #include "apic-defs.h"
 
-.globl boot_idt
 .global online_cpus
 
 ipi_vector = 0x20
@@ -28,12 +27,6 @@ i = 0
         i = i + 1
         .endr
 
-boot_idt:
-	.rept 256
-	.quad 0
-	.endr
-end_boot_idt:
-
 .globl gdt32
 gdt32:
 	.quad 0
@@ -78,10 +71,6 @@ tss:
         .endr
 tss_end:
 
-idt_descr:
-	.word end_boot_idt - boot_idt - 1
-	.long boot_idt
-
 .section .init
 
 .code32
diff --git a/x86/cstart64.S b/x86/cstart64.S
index cf38bae..b98a0d3 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -1,9 +1,6 @@
 
 #include "apic-defs.h"
 
-.globl boot_idt
-
-.globl idt_descr
 .globl gdt64_desc
 .globl online_cpus
 .globl cpu_online_count
@@ -50,13 +47,6 @@ ptl5:
 
 .align 4096
 
-boot_idt:
-	.rept 256
-	.quad 0
-	.quad 0
-	.endr
-end_boot_idt:
-
 gdt64_desc:
 	.word gdt64_end - gdt64 - 1
 	.quad gdt64
@@ -290,10 +280,6 @@ setup_5level_page_table:
 lvl5:
 	retq
 
-idt_descr:
-	.word end_boot_idt - boot_idt - 1
-	.quad boot_idt
-
 online_cpus:
 	.fill (max_cpus + 7) / 8, 1, 0
 
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH kvm-unit-tests 6/9] x86: unify name of 32-bit and 64-bit GDT
  2021-10-21 11:49 [PATCH v3 kvm-unit-tests 0/8] x86: Move IDT, GDT and TSS to C code Paolo Bonzini
                   ` (4 preceding siblings ...)
  2021-10-21 11:49 ` [PATCH kvm-unit-tests 5/9] x86: Move IDT to desc.c Paolo Bonzini
@ 2021-10-21 11:49 ` Paolo Bonzini
  2021-10-21 11:49 ` [PATCH kvm-unit-tests 7/9] x86: get rid of ring0stacktop Paolo Bonzini
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-10-21 11:49 UTC (permalink / raw)
  To: kvm
  Cc: aaronlewis, jmattson, zxwang42, marcorr, seanjc, jroedel, varad.gautam

There's no need to distinguish gdt32 and gdt64, since the same C functions
operate on both and selector numbers are mostly unified between 32-
and 64-bit versions.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 lib/x86/desc.c   | 12 ++++++------
 lib/x86/desc.h   |  2 +-
 x86/cstart.S     | 20 ++++++++++----------
 x86/cstart64.S   | 17 +++++++++--------
 x86/taskswitch.c |  2 +-
 x86/vmx.c        |  8 ++++----
 x86/vmx_tests.c  |  4 ++--
 7 files changed, 33 insertions(+), 32 deletions(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 2ef5aad..ac167d0 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -292,16 +292,16 @@ static char intr_alt_stack[4096];
 #ifndef __x86_64__
 void set_gdt_entry(int sel, u32 base,  u32 limit, u8 type, u8 flags)
 {
-	int num = sel >> 3;
+	gdt_entry_t *entry = &gdt[sel >> 3];
 
 	/* Setup the descriptor base address */
-	gdt32[num].base1 = (base & 0xFFFF);
-	gdt32[num].base2 = (base >> 16) & 0xFF;
-	gdt32[num].base3 = (base >> 24) & 0xFF;
+	entry->base1 = (base & 0xFFFF);
+	entry->base2 = (base >> 16) & 0xFF;
+	entry->base3 = (base >> 24) & 0xFF;
 
 	/* Setup the descriptor limits, type and flags */
-	gdt32[num].limit1 = (limit & 0xFFFF);
-	gdt32[num].type_limit_flags = ((limit & 0xF0000) >> 8) | ((flags & 0xF0) << 8) | type;
+	entry->limit1 = (limit & 0xFFFF);
+	entry->type_limit_flags = ((limit & 0xF0000) >> 8) | ((flags & 0xF0) << 8) | type;
 }
 
 void set_gdt_task_gate(u16 sel, u16 tss_sel)
diff --git a/lib/x86/desc.h b/lib/x86/desc.h
index 51148d1..c0817d8 100644
--- a/lib/x86/desc.h
+++ b/lib/x86/desc.h
@@ -197,7 +197,6 @@ struct system_desc64 {
 extern idt_entry_t boot_idt[256];
 
 #ifndef __x86_64__
-extern gdt_entry_t gdt32[];
 extern tss32_t tss;
 extern tss32_t tss_intr;
 void set_gdt_task_gate(u16 tss_sel, u16 sel);
@@ -207,6 +206,7 @@ void setup_tss32(void);
 #else
 extern tss64_t tss;
 #endif
+extern gdt_entry_t gdt[];
 
 unsigned exception_vector(void);
 int write_cr4_checking(unsigned long val);
diff --git a/x86/cstart.S b/x86/cstart.S
index 4461c38..5e925d8 100644
--- a/x86/cstart.S
+++ b/x86/cstart.S
@@ -27,8 +27,8 @@ i = 0
         i = i + 1
         .endr
 
-.globl gdt32
-gdt32:
+.globl gdt
+gdt:
 	.quad 0
 	.quad 0x00cf9b000000ffff // flat 32-bit code segment
 	.quad 0x00cf93000000ffff // flat 32-bit data segment
@@ -55,7 +55,7 @@ percpu_descr:
         .rept max_cpus
         .quad 0x00cf93000000ffff // 32-bit data segment for perCPU area
         .endr
-gdt32_end:
+gdt_end:
 
 i = 0
 .globl tss
@@ -94,7 +94,7 @@ mb_cmdline = 16
 	mov %al, percpu_descr+4(,%ecx,8)
 	mov %ah, percpu_descr+7(,%ecx,8)
 
-	lea percpu_descr-gdt32(,%ecx,8), %eax
+	lea percpu_descr-gdt(,%ecx,8), %eax
 	mov %ax, %gs
 
 .endm
@@ -110,7 +110,7 @@ mb_cmdline = 16
 
 .globl start
 start:
-        lgdtl gdt32_descr
+        lgdtl gdt_descr
         setup_segments
         mov $stacktop, %esp
         setup_percpu_area
@@ -195,7 +195,7 @@ load_tss:
 	shr $16, %eax
 	mov %al, tss_descr+4(,%ebx,8)
 	mov %ah, tss_descr+7(,%ebx,8)
-	lea tss_descr-gdt32(,%ebx,8), %eax
+	lea tss_descr-gdt(,%ebx,8), %eax
 	ltr %ax
 	ret
 
@@ -224,11 +224,11 @@ sipi_entry:
 	mov %cr0, %eax
 	or $1, %eax
 	mov %eax, %cr0
-	lgdtl gdt32_descr - sipi_entry
+	lgdtl gdt_descr - sipi_entry
 	ljmpl $8, $ap_start32
 
-gdt32_descr:
-	.word gdt32_end - gdt32 - 1
-	.long gdt32
+gdt_descr:
+	.word gdt_end - gdt - 1
+	.long gdt
 
 sipi_end:
diff --git a/x86/cstart64.S b/x86/cstart64.S
index b98a0d3..46b9d9b 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -1,7 +1,8 @@
 
 #include "apic-defs.h"
 
-.globl gdt64_desc
+.globl gdt
+.globl gdt_descr
 .globl online_cpus
 .globl cpu_online_count
 
@@ -47,11 +48,11 @@ ptl5:
 
 .align 4096
 
-gdt64_desc:
-	.word gdt64_end - gdt64 - 1
-	.quad gdt64
+gdt_descr:
+	.word gdt_end - gdt - 1
+	.quad gdt
 
-gdt64:
+gdt:
 	.quad 0
 	.quad 0x00af9b000000ffff // 64-bit code segment
 	.quad 0x00cf93000000ffff // 32/64-bit data segment
@@ -75,7 +76,7 @@ tss_descr:
 	.quad 0x000089000000ffff // 64-bit avail tss
 	.quad 0                  // tss high addr
 	.endr
-gdt64_end:
+gdt_end:
 
 i = 0
 .globl tss
@@ -162,7 +163,7 @@ switch_to_5level:
 	jmpl $8, $lvl5
 
 prepare_64:
-	lgdt gdt64_desc
+	lgdt gdt_descr
 	setup_segments
 
 	xor %eax, %eax
@@ -300,7 +301,7 @@ load_tss:
 	mov %al, tss_descr+7(%rbx)
 	shr $8, %rax
 	mov %eax, tss_descr+8(%rbx)
-	lea tss_descr-gdt64(%rbx), %rax
+	lea tss_descr-gdt(%rbx), %rax
 	ltr %ax
 	ret
 
diff --git a/x86/taskswitch.c b/x86/taskswitch.c
index 0fa818d..1d6e6e2 100644
--- a/x86/taskswitch.c
+++ b/x86/taskswitch.c
@@ -21,7 +21,7 @@ fault_handler(unsigned long error_code)
 
 	tss.eip += 2;
 
-	gdt32[TSS_MAIN / 8].type &= ~DESC_BUSY;
+	gdt[TSS_MAIN / 8].type &= ~DESC_BUSY;
 
 	set_gdt_task_gate(TSS_RETURN, tss_intr.prev);
 }
diff --git a/x86/vmx.c b/x86/vmx.c
index d45c6de..7a2f7a3 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -73,7 +73,7 @@ union vmx_ctrl_msr ctrl_exit_rev;
 union vmx_ctrl_msr ctrl_enter_rev;
 union vmx_ept_vpid  ept_vpid;
 
-extern struct descriptor_table_ptr gdt64_desc;
+extern struct descriptor_table_ptr gdt_descr;
 extern struct descriptor_table_ptr idt_descr;
 extern void *vmx_return;
 extern void *entry_sysenter;
@@ -1275,7 +1275,7 @@ static void init_vmcs_host(void)
 	vmcs_write(HOST_SEL_GS, KERNEL_DS);
 	vmcs_write(HOST_SEL_TR, TSS_MAIN);
 	vmcs_write(HOST_BASE_TR, get_gdt_entry_base(get_tss_descr()));
-	vmcs_write(HOST_BASE_GDTR, gdt64_desc.base);
+	vmcs_write(HOST_BASE_GDTR, gdt_descr.base);
 	vmcs_write(HOST_BASE_IDTR, idt_descr.base);
 	vmcs_write(HOST_BASE_FS, 0);
 	vmcs_write(HOST_BASE_GS, 0);
@@ -1354,9 +1354,9 @@ static void init_vmcs_guest(void)
 	vmcs_write(GUEST_AR_TR, 0x8b);
 
 	/* 26.3.1.3 */
-	vmcs_write(GUEST_BASE_GDTR, gdt64_desc.base);
+	vmcs_write(GUEST_BASE_GDTR, gdt_descr.base);
 	vmcs_write(GUEST_BASE_IDTR, idt_descr.base);
-	vmcs_write(GUEST_LIMIT_GDTR, gdt64_desc.limit);
+	vmcs_write(GUEST_LIMIT_GDTR, gdt_descr.limit);
 	vmcs_write(GUEST_LIMIT_IDTR, idt_descr.limit);
 
 	/* 26.3.1.4 */
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index ac2b0b4..9ee6653 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -797,8 +797,8 @@ asm(
 	"insn_wbinvd: wbinvd;ret\n\t"
 	"insn_cpuid: mov $10, %eax; cpuid;ret\n\t"
 	"insn_invd: invd;ret\n\t"
-	"insn_sgdt: sgdt gdt64_desc;ret\n\t"
-	"insn_lgdt: lgdt gdt64_desc;ret\n\t"
+	"insn_sgdt: sgdt gdt_descr;ret\n\t"
+	"insn_lgdt: lgdt gdt_descr;ret\n\t"
 	"insn_sidt: sidt idt_descr;ret\n\t"
 	"insn_lidt: lidt idt_descr;ret\n\t"
 	"insn_sldt: sldt %ax;ret\n\t"
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH kvm-unit-tests 7/9] x86: get rid of ring0stacktop
  2021-10-21 11:49 [PATCH v3 kvm-unit-tests 0/8] x86: Move IDT, GDT and TSS to C code Paolo Bonzini
                   ` (5 preceding siblings ...)
  2021-10-21 11:49 ` [PATCH kvm-unit-tests 6/9] x86: unify name of 32-bit and 64-bit GDT Paolo Bonzini
@ 2021-10-21 11:49 ` Paolo Bonzini
  2021-10-21 23:08   ` Aaron Lewis
  2021-10-21 11:49 ` [PATCH kvm-unit-tests 8/9] x86: Move 64-bit GDT and TSS to desc.c Paolo Bonzini
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2021-10-21 11:49 UTC (permalink / raw)
  To: kvm
  Cc: aaronlewis, jmattson, zxwang42, marcorr, seanjc, jroedel, varad.gautam

The ring3 switch code relied on a special stack page that was used
for the ring0 stack during the ring3 part of the test.  This special
stack page was used if an exception handler ran during the ring3 part
of the test.

This method is quite complex; it is easier to just use the same
stack for the "outer" part of the test and the exception handler.
To do so, store esp/rsp in the TSS just before doing the PUSH/IRET
sequence.  On 64-bit, the TSS can also be used to restore rsp after
coming back from ring3.

Unifying the three copies of the ring switching code is left as an
exercise to the reader.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 lib/x86/usermode.c |  9 +++++----
 x86/access.c       | 16 ++++++++--------
 x86/cstart.S       |  6 +-----
 x86/cstart64.S     |  6 +-----
 x86/umip.c         | 19 ++++++++++++-------
 5 files changed, 27 insertions(+), 29 deletions(-)

diff --git a/lib/x86/usermode.c b/lib/x86/usermode.c
index f032523..d511f4f 100644
--- a/lib/x86/usermode.c
+++ b/lib/x86/usermode.c
@@ -47,8 +47,8 @@ uint64_t run_in_user(usermode_func func, unsigned int fault_vector,
 	}
 
 	asm volatile (
-			/* Backing Up Stack in rdi */
-			"mov %%rsp, %%rdi\n\t"
+			/* Prepare kernel SP for exception handlers */
+			"mov %%rsp, %[rsp0]\n\t"
 			/* Load user_ds to DS and ES */
 			"mov %[user_ds], %%ax\n\t"
 			"mov %%ax, %%ds\n\t"
@@ -92,9 +92,10 @@ uint64_t run_in_user(usermode_func func, unsigned int fault_vector,
 			"int %[kernel_entry_vector]\n\t"
 			/* Kernel Mode */
 			"ret_to_kernel:\n\t"
-			"mov %%rdi, %%rsp\n\t"
+			"mov %[rsp0], %%rsp\n\t"
 			:
-			"+a"(rax)
+			"+a"(rax),
+			[rsp0]"=m"(tss.rsp0),
 			:
 			[arg1]"m"(arg1),
 			[arg2]"m"(arg2),
diff --git a/x86/access.c b/x86/access.c
index 4725bbd..49d31b1 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -711,7 +711,7 @@ static int ac_test_do_access(ac_test_t *at)
     }
 
     asm volatile ("mov $fixed1, %%rsi \n\t"
-		  "mov %%rsp, %%rdx \n\t"
+		  "mov %%rsp, %[rsp0] \n\t"
 		  "cmp $0, %[user] \n\t"
 		  "jz do_access \n\t"
 		  "push %%rax; mov %[user_ds], %%ax; mov %%ax, %%ds; pop %%rax  \n\t"
@@ -734,8 +734,14 @@ static int ac_test_do_access(ac_test_t *at)
 		  "done: \n"
 		  "fixed1: \n"
 		  "int %[kernel_entry_vector] \n\t"
+		  ".section .text.entry \n\t"
+		  "kernel_entry: \n\t"
+		  "mov %[rsp0], %%rsp \n\t"
+		  "jmp back_to_kernel \n\t"
+		  ".section .text \n\t"
 		  "back_to_kernel:"
-		  : [reg]"+r"(r), "+a"(fault), "=b"(e), "=&d"(rsp)
+		  : [reg]"+r"(r), "+a"(fault), "=b"(e), "=&d"(rsp),
+		    [rsp0]"=m"(tss.rsp0)
 		  : [addr]"r"(at->virt),
 		    [write]"r"(F(AC_ACCESS_WRITE)),
 		    [user]"r"(F(AC_ACCESS_USER)),
@@ -754,12 +760,6 @@ static int ac_test_do_access(ac_test_t *at)
 		  "iretq \n\t"
 		  ".section .text");
 
-    asm volatile (".section .text.entry \n\t"
-		  "kernel_entry: \n\t"
-		  "mov %rdx, %rsp \n\t"
-		  "jmp back_to_kernel \n\t"
-		  ".section .text");
-
     ac_test_check(at, &success, fault && !at->expected_fault,
                   "unexpected fault");
     ac_test_check(at, &success, !fault && at->expected_fault,
diff --git a/x86/cstart.S b/x86/cstart.S
index 5e925d8..e9100a4 100644
--- a/x86/cstart.S
+++ b/x86/cstart.S
@@ -13,10 +13,6 @@ max_cpus = MAX_TEST_CPUS
 	.align 16
 stacktop:
 
-	. = . + 4096
-	.align 16
-ring0stacktop:
-
 .data
 
 .align 4096
@@ -62,7 +58,7 @@ i = 0
 tss:
         .rept max_cpus
         .long 0
-        .long ring0stacktop - i * 4096
+        .long 0
         .long 16
         .quad 0, 0
         .quad 0, 0, 0, 0, 0, 0, 0, 0
diff --git a/x86/cstart64.S b/x86/cstart64.S
index 46b9d9b..18c7457 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -16,10 +16,6 @@ max_cpus = MAX_TEST_CPUS
 	.align 16
 stacktop:
 
-	. = . + 4096 * max_cpus
-	.align 16
-ring0stacktop:
-
 .data
 
 .align 4096
@@ -83,7 +79,7 @@ i = 0
 tss:
 	.rept max_cpus
 	.long 0
-	.quad ring0stacktop - i * 4096
+	.quad 0
 	.quad 0, 0
 	.quad 0, 0, 0, 0, 0, 0, 0, 0
 	.long 0, 0, 0
diff --git a/x86/umip.c b/x86/umip.c
index 0fc1f65..79e288d 100644
--- a/x86/umip.c
+++ b/x86/umip.c
@@ -124,7 +124,7 @@ static noinline int do_ring3(void (*fn)(const char *), const char *arg)
 		  "mov %%dx, %%es\n\t"
 		  "mov %%dx, %%fs\n\t"
 		  "mov %%dx, %%gs\n\t"
-		  "mov %%" R "sp, %%" R "cx\n\t"
+		  "mov %%" R "sp, %[sp0]\n\t" /* kernel sp for exception handlers */
 		  "push" W " %%" R "dx \n\t"
 		  "lea %[user_stack_top], %%" R "dx \n\t"
 		  "push" W " %%" R "dx \n\t"
@@ -133,8 +133,6 @@ static noinline int do_ring3(void (*fn)(const char *), const char *arg)
 		  "push" W " $1f \n\t"
 		  "iret" W "\n"
 		  "1: \n\t"
-		  "push %%" R "cx\n\t"   /* save kernel SP */
-
 #ifndef __x86_64__
 		  "push %[arg]\n\t"
 #endif
@@ -142,13 +140,15 @@ static noinline int do_ring3(void (*fn)(const char *), const char *arg)
 #ifndef __x86_64__
 		  "pop %%ecx\n\t"
 #endif
-
-		  "pop %%" R "cx\n\t"
 		  "mov $1f, %%" R "dx\n\t"
 		  "int %[kernel_entry_vector]\n\t"
 		  ".section .text.entry \n\t"
 		  "kernel_entry: \n\t"
-		  "mov %%" R "cx, %%" R "sp \n\t"
+#ifdef __x86_64__
+		  "mov %[sp0], %%" R "sp\n\t"
+#else
+		  "add $(5 * " S "), %%esp\n\t"
+#endif
 		  "mov %[kernel_ds], %%cx\n\t"
 		  "mov %%cx, %%ds\n\t"
 		  "mov %%cx, %%es\n\t"
@@ -157,7 +157,12 @@ static noinline int do_ring3(void (*fn)(const char *), const char *arg)
 		  "jmp *%%" R "dx \n\t"
 		  ".section .text\n\t"
 		  "1:\n\t"
-		  : [ret] "=&a" (ret)
+		  : [ret] "=&a" (ret),
+#ifdef __x86_64__
+		    [sp0] "=m" (tss.rsp0),
+#else
+		    [sp0] "=m" (tss.esp0),
+#endif
 		  : [user_ds] "i" (USER_DS),
 		    [user_cs] "i" (USER_CS),
 		    [user_stack_top]"m"(user_stack[sizeof(user_stack) -
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH kvm-unit-tests 8/9] x86: Move 64-bit GDT and TSS to desc.c
  2021-10-21 11:49 [PATCH v3 kvm-unit-tests 0/8] x86: Move IDT, GDT and TSS to C code Paolo Bonzini
                   ` (6 preceding siblings ...)
  2021-10-21 11:49 ` [PATCH kvm-unit-tests 7/9] x86: get rid of ring0stacktop Paolo Bonzini
@ 2021-10-21 11:49 ` Paolo Bonzini
  2021-10-21 11:49 ` [PATCH kvm-unit-tests 9/9] x86: Move 32-bit " Paolo Bonzini
  2021-10-21 14:37 ` [PATCH v3 kvm-unit-tests 0/8] x86: Move IDT, GDT and TSS to C code Paolo Bonzini
  9 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-10-21 11:49 UTC (permalink / raw)
  To: kvm
  Cc: aaronlewis, jmattson, zxwang42, marcorr, seanjc, jroedel,
	varad.gautam, Zixuan Wang

From: Zixuan Wang <zixuanwang@google.com>

Move the GDT and TSS data structures from x86/cstart64.S to
lib/x86/desc.c, so that the follow-up UEFI support commits can reuse
these definitions, without re-defining them in UEFI's boot up assembly
code.

Signed-off-by: Zixuan Wang <zixuanwang@google.com>
Message-Id: <20211004204931.1537823-2-zxwang42@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 lib/x86/asm/setup.h |  6 ++++
 lib/x86/desc.c      | 38 +++++++++++++++++++++--
 lib/x86/desc.h      |  4 +--
 lib/x86/setup.c     | 25 +++++++++++++++
 lib/x86/usermode.c  |  2 +-
 x86/access.c        |  2 +-
 x86/cstart64.S      | 76 +++++----------------------------------------
 x86/umip.c          |  2 +-
 8 files changed, 79 insertions(+), 76 deletions(-)
 create mode 100644 lib/x86/asm/setup.h

diff --git a/lib/x86/asm/setup.h b/lib/x86/asm/setup.h
new file mode 100644
index 0000000..e3c3bfb
--- /dev/null
+++ b/lib/x86/asm/setup.h
@@ -0,0 +1,6 @@
+#ifndef _X86_ASM_SETUP_H_
+#define _X86_ASM_SETUP_H_
+
+unsigned long setup_tss(void);
+
+#endif /* _X86_ASM_SETUP_H_ */
diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index ac167d0..c185c01 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -2,6 +2,7 @@
 #include "desc.h"
 #include "processor.h"
 #include <setjmp.h>
+#include "apic-defs.h"
 
 /* Boot-related data structures */
 
@@ -13,6 +14,29 @@ struct descriptor_table_ptr idt_descr = {
 	.base = (unsigned long)boot_idt,
 };
 
+#ifdef __x86_64__
+/* GDT, TSS and descriptors */
+gdt_entry_t gdt[TSS_MAIN / 8 + MAX_TEST_CPUS * 2] = {
+	{     0, 0, 0, .type_limit_flags = 0x0000}, /* 0x00 null */
+	{0xffff, 0, 0, .type_limit_flags = 0xaf9b}, /* 0x08 64-bit code segment */
+	{0xffff, 0, 0, .type_limit_flags = 0xcf93}, /* 0x10 32/64-bit data segment */
+	{0xffff, 0, 0, .type_limit_flags = 0xaf1b}, /* 0x18 64-bit code segment, not present */
+	{0xffff, 0, 0, .type_limit_flags = 0xcf9b}, /* 0x20 32-bit code segment */
+	{0xffff, 0, 0, .type_limit_flags = 0x8f9b}, /* 0x28 16-bit code segment */
+	{0xffff, 0, 0, .type_limit_flags = 0x8f93}, /* 0x30 16-bit data segment */
+	{0xffff, 0, 0, .type_limit_flags = 0xcffb}, /* 0x38 32-bit code segment (user) */
+	{0xffff, 0, 0, .type_limit_flags = 0xcff3}, /* 0x40 32/64-bit data segment (user) */
+	{0xffff, 0, 0, .type_limit_flags = 0xaffb}, /* 0x48 64-bit code segment (user) */
+};
+
+tss64_t tss[MAX_TEST_CPUS] = {0};
+
+struct descriptor_table_ptr gdt_descr = {
+	.limit = sizeof(gdt) - 1,
+	.base = (unsigned long)gdt,
+};
+#endif
+
 #ifndef __x86_64__
 __attribute__((regparm(1)))
 #endif
@@ -289,8 +313,7 @@ bool exception_rflags_rf(void)
 
 static char intr_alt_stack[4096];
 
-#ifndef __x86_64__
-void set_gdt_entry(int sel, u32 base,  u32 limit, u8 type, u8 flags)
+void set_gdt_entry(int sel, unsigned long base,  u32 limit, u8 type, u8 flags)
 {
 	gdt_entry_t *entry = &gdt[sel >> 3];
 
@@ -302,8 +325,17 @@ void set_gdt_entry(int sel, u32 base,  u32 limit, u8 type, u8 flags)
 	/* Setup the descriptor limits, type and flags */
 	entry->limit1 = (limit & 0xFFFF);
 	entry->type_limit_flags = ((limit & 0xF0000) >> 8) | ((flags & 0xF0) << 8) | type;
+
+#ifdef __x86_64__
+	if (!entry->s) {
+		struct system_desc64 *entry16 = (struct system_desc64 *)entry;
+		entry16->zero = 0;
+		entry16->base4 = base >> 32;
+	}
+#endif
 }
 
+#ifndef __x86_64__
 void set_gdt_task_gate(u16 sel, u16 tss_sel)
 {
     set_gdt_entry(sel, tss_sel, 0, 0x85, 0); // task, present
@@ -380,7 +412,7 @@ void set_intr_alt_stack(int e, void *addr)
 
 void setup_alt_stack(void)
 {
-	tss.ist1 = (u64)intr_alt_stack + 4096;
+	tss[0].ist1 = (u64)intr_alt_stack + 4096;
 }
 #endif
 
diff --git a/lib/x86/desc.h b/lib/x86/desc.h
index c0817d8..ddfae04 100644
--- a/lib/x86/desc.h
+++ b/lib/x86/desc.h
@@ -204,7 +204,7 @@ void set_idt_task_gate(int vec, u16 sel);
 void set_intr_task_gate(int vec, void *fn);
 void setup_tss32(void);
 #else
-extern tss64_t tss;
+extern tss64_t tss[];
 #endif
 extern gdt_entry_t gdt[];
 
@@ -215,7 +215,7 @@ bool exception_rflags_rf(void);
 void set_idt_entry(int vec, void *addr, int dpl);
 void set_idt_sel(int vec, u16 sel);
 void set_idt_dpl(int vec, u16 dpl);
-void set_gdt_entry(int sel, u32 base,  u32 limit, u8 access, u8 gran);
+void set_gdt_entry(int sel, unsigned long base, u32 limit, u8 access, u8 gran);
 void set_intr_alt_stack(int e, void *fn);
 void print_current_tss_info(void);
 handler handle_exception(u8 v, handler fn);
diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index 7befe09..ec005b5 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -2,6 +2,7 @@
  * Initialize machine setup information
  *
  * Copyright (C) 2017, Red Hat Inc, Andrew Jones <drjones@redhat.com>
+ * Copyright (C) 2021, Google Inc, Zixuan Wang <zixuanwang@google.com>
  *
  * This work is licensed under the terms of the GNU LGPL, version 2.
  */
@@ -9,6 +10,10 @@
 #include "fwcfg.h"
 #include "alloc_phys.h"
 #include "argv.h"
+#include "desc.h"
+#include "apic.h"
+#include "apic-defs.h"
+#include "asm/setup.h"
 
 extern char edata;
 
@@ -97,6 +102,26 @@ void find_highmem(void)
 		phys_alloc_init(best_start, best_end - best_start);
 	}
 }
+
+/* Setup TSS for the current processor, and return TSS offset within GDT */
+unsigned long setup_tss(void)
+{
+	u32 id;
+	tss64_t *tss_entry;
+
+	id = apic_id();
+
+	/* Runtime address of current TSS */
+	tss_entry = &tss[id];
+
+	/* Update TSS */
+	memset((void *)tss_entry, 0, sizeof(tss64_t));
+
+	/* Update TSS descriptors; each descriptor takes up 2 entries */
+	set_gdt_entry(TSS_MAIN + id * 16, (unsigned long)tss_entry, 0xffff, 0x89, 0);
+
+	return TSS_MAIN + id * 16;
+}
 #endif
 
 void setup_multiboot(struct mbi_bootinfo *bi)
diff --git a/lib/x86/usermode.c b/lib/x86/usermode.c
index d511f4f..c44faba 100644
--- a/lib/x86/usermode.c
+++ b/lib/x86/usermode.c
@@ -95,7 +95,7 @@ uint64_t run_in_user(usermode_func func, unsigned int fault_vector,
 			"mov %[rsp0], %%rsp\n\t"
 			:
 			"+a"(rax),
-			[rsp0]"=m"(tss.rsp0),
+			[rsp0]"=m"(tss[0].rsp0),
 			:
 			[arg1]"m"(arg1),
 			[arg2]"m"(arg2),
diff --git a/x86/access.c b/x86/access.c
index 49d31b1..a781a0c 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -741,7 +741,7 @@ static int ac_test_do_access(ac_test_t *at)
 		  ".section .text \n\t"
 		  "back_to_kernel:"
 		  : [reg]"+r"(r), "+a"(fault), "=b"(e), "=&d"(rsp),
-		    [rsp0]"=m"(tss.rsp0)
+		    [rsp0]"=m"(tss[0].rsp0)
 		  : [addr]"r"(at->virt),
 		    [write]"r"(F(AC_ACCESS_WRITE)),
 		    [user]"r"(F(AC_ACCESS_USER)),
diff --git a/x86/cstart64.S b/x86/cstart64.S
index 18c7457..c6daa34 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -1,8 +1,6 @@
 
 #include "apic-defs.h"
 
-.globl gdt
-.globl gdt_descr
 .globl online_cpus
 .globl cpu_online_count
 
@@ -44,49 +42,6 @@ ptl5:
 
 .align 4096
 
-gdt_descr:
-	.word gdt_end - gdt - 1
-	.quad gdt
-
-gdt:
-	.quad 0
-	.quad 0x00af9b000000ffff // 64-bit code segment
-	.quad 0x00cf93000000ffff // 32/64-bit data segment
-	.quad 0x00af1b000000ffff // 64-bit code segment, not present
-	.quad 0x00cf9b000000ffff // 32-bit code segment
-	.quad 0x008f9b000000FFFF // 16-bit code segment
-	.quad 0x008f93000000FFFF // 16-bit data segment
-	.quad 0x00cffb000000ffff // 32-bit code segment (user)
-	.quad 0x00cff3000000ffff // 32/64-bit data segment (user)
-	.quad 0x00affb000000ffff // 64-bit code segment (user)
-
-	.quad 0			 // 6 spare selectors
-	.quad 0
-	.quad 0
-	.quad 0
-	.quad 0
-	.quad 0
-
-tss_descr:
-	.rept max_cpus
-	.quad 0x000089000000ffff // 64-bit avail tss
-	.quad 0                  // tss high addr
-	.endr
-gdt_end:
-
-i = 0
-.globl tss
-tss:
-	.rept max_cpus
-	.long 0
-	.quad 0
-	.quad 0, 0
-	.quad 0, 0, 0, 0, 0, 0, 0, 0
-	.long 0, 0, 0
-i = i + 1
-	.endr
-tss_end:
-
 mb_boot_info:	.quad 0
 
 pt_root:	.quad ptl4
@@ -111,6 +66,12 @@ MSR_GS_BASE = 0xc0000101
 	wrmsr
 .endm
 
+.macro load_tss
+	lidtq idt_descr
+	call setup_tss
+	ltr %ax
+.endm
+
 .macro setup_segments
 	mov $MSR_GS_BASE, %ecx
 	rdmsr
@@ -228,7 +189,7 @@ save_id:
 
 ap_start64:
 	call reset_apic
-	call load_tss
+	load_tss
 	call enable_apic
 	call save_id
 	call enable_x2apic
@@ -241,7 +202,7 @@ ap_start64:
 
 start64:
 	call reset_apic
-	call load_tss
+	load_tss
 	call mask_pic_interrupts
 	call enable_apic
 	call save_id
@@ -280,27 +241,6 @@ lvl5:
 online_cpus:
 	.fill (max_cpus + 7) / 8, 1, 0
 
-load_tss:
-	lidtq idt_descr
-	mov $(APIC_DEFAULT_PHYS_BASE + APIC_ID), %eax
-	mov (%rax), %eax
-	shr $24, %eax
-	mov %eax, %ebx
-	shl $4, %ebx
-	mov $((tss_end - tss) / max_cpus), %edx
-	imul %edx
-	add $tss, %rax
-	mov %ax, tss_descr+2(%rbx)
-	shr $16, %rax
-	mov %al, tss_descr+4(%rbx)
-	shr $8, %rax
-	mov %al, tss_descr+7(%rbx)
-	shr $8, %rax
-	mov %eax, tss_descr+8(%rbx)
-	lea tss_descr-gdt(%rbx), %rax
-	ltr %ax
-	ret
-
 ap_init:
 	cld
 	lea sipi_entry, %rsi
diff --git a/x86/umip.c b/x86/umip.c
index 79e288d..54f3884 100644
--- a/x86/umip.c
+++ b/x86/umip.c
@@ -159,7 +159,7 @@ static noinline int do_ring3(void (*fn)(const char *), const char *arg)
 		  "1:\n\t"
 		  : [ret] "=&a" (ret),
 #ifdef __x86_64__
-		    [sp0] "=m" (tss.rsp0),
+		    [sp0] "=m" (tss[0].rsp0),
 #else
 		    [sp0] "=m" (tss.esp0),
 #endif
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH kvm-unit-tests 9/9] x86: Move 32-bit GDT and TSS to desc.c
  2021-10-21 11:49 [PATCH v3 kvm-unit-tests 0/8] x86: Move IDT, GDT and TSS to C code Paolo Bonzini
                   ` (7 preceding siblings ...)
  2021-10-21 11:49 ` [PATCH kvm-unit-tests 8/9] x86: Move 64-bit GDT and TSS to desc.c Paolo Bonzini
@ 2021-10-21 11:49 ` Paolo Bonzini
  2021-10-21 14:37 ` [PATCH v3 kvm-unit-tests 0/8] x86: Move IDT, GDT and TSS to C code Paolo Bonzini
  9 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-10-21 11:49 UTC (permalink / raw)
  To: kvm
  Cc: aaronlewis, jmattson, zxwang42, marcorr, seanjc, jroedel, varad.gautam

Move the GDT and TSS data structures from x86/cstart.S to
lib/x86/desc.c, for consistency with the 64-bit version.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 lib/x86/asm/setup.h |  2 +-
 lib/x86/desc.c      | 22 ++++++++--
 lib/x86/desc.h      |  2 +-
 lib/x86/setup.c     | 26 +++++++++++-
 x86/cstart.S        | 98 +++++++--------------------------------------
 x86/cstart64.S      |  1 +
 x86/smap.c          |  2 +-
 x86/taskswitch.c    |  2 +-
 x86/umip.c          |  2 +-
 9 files changed, 63 insertions(+), 94 deletions(-)

diff --git a/lib/x86/asm/setup.h b/lib/x86/asm/setup.h
index e3c3bfb..4310132 100644
--- a/lib/x86/asm/setup.h
+++ b/lib/x86/asm/setup.h
@@ -1,6 +1,6 @@
 #ifndef _X86_ASM_SETUP_H_
 #define _X86_ASM_SETUP_H_
 
-unsigned long setup_tss(void);
+unsigned long setup_tss(u8 *stacktop);
 
 #endif /* _X86_ASM_SETUP_H_ */
diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index c185c01..16b7256 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -14,8 +14,22 @@ struct descriptor_table_ptr idt_descr = {
 	.base = (unsigned long)boot_idt,
 };
 
-#ifdef __x86_64__
+#ifndef __x86_64__
 /* GDT, TSS and descriptors */
+gdt_entry_t gdt[TSS_MAIN / 8 + MAX_TEST_CPUS * 2] = {
+	{     0, 0, 0, .type_limit_flags = 0x0000}, /* 0x00 null */
+	{0xffff, 0, 0, .type_limit_flags = 0xcf9b}, /* flat 32-bit code segment */
+	{0xffff, 0, 0, .type_limit_flags = 0xcf93}, /* flat 32-bit data segment */
+	{0xffff, 0, 0, .type_limit_flags = 0xcf1b}, /* flat 32-bit code segment, not present */
+	{     0, 0, 0, .type_limit_flags = 0x0000}, /* TSS for task gates */
+	{0xffff, 0, 0, .type_limit_flags = 0x8f9b}, /* 16-bit code segment */
+	{0xffff, 0, 0, .type_limit_flags = 0x8f93}, /* 16-bit data segment */
+	{0xffff, 0, 0, .type_limit_flags = 0xcffb}, /* 32-bit code segment (user) */
+	{0xffff, 0, 0, .type_limit_flags = 0xcff3}, /* 32-bit data segment (user) */
+};
+
+tss32_t tss[MAX_TEST_CPUS] = {0};
+#else
 gdt_entry_t gdt[TSS_MAIN / 8 + MAX_TEST_CPUS * 2] = {
 	{     0, 0, 0, .type_limit_flags = 0x0000}, /* 0x00 null */
 	{0xffff, 0, 0, .type_limit_flags = 0xaf9b}, /* 0x08 64-bit code segment */
@@ -30,12 +44,12 @@ gdt_entry_t gdt[TSS_MAIN / 8 + MAX_TEST_CPUS * 2] = {
 };
 
 tss64_t tss[MAX_TEST_CPUS] = {0};
+#endif
 
 struct descriptor_table_ptr gdt_descr = {
 	.limit = sizeof(gdt) - 1,
 	.base = (unsigned long)gdt,
 };
-#endif
 
 #ifndef __x86_64__
 __attribute__((regparm(1)))
@@ -365,7 +379,7 @@ void setup_tss32(void)
 {
 	u16 desc_size = sizeof(tss32_t);
 
-	tss.cr3 = read_cr3();
+	tss[0].cr3 = read_cr3();
 	tss_intr.cr3 = read_cr3();
 	tss_intr.ss0 = tss_intr.ss1 = tss_intr.ss2 = 0x10;
 	tss_intr.esp = tss_intr.esp0 = tss_intr.esp1 = tss_intr.esp2 =
@@ -401,7 +415,7 @@ void print_current_tss_info(void)
 		printf("Unknown TSS %x\n", tr);
 	else
 		printf("TR=%x (%s) Main TSS back link %x. Intr TSS back link %x\n",
-		       tr, tr ? "interrupt" : "main", tss.prev, tss_intr.prev);
+		       tr, tr ? "interrupt" : "main", tss[0].prev, tss_intr.prev);
 }
 #else
 void set_intr_alt_stack(int e, void *addr)
diff --git a/lib/x86/desc.h b/lib/x86/desc.h
index ddfae04..b65539e 100644
--- a/lib/x86/desc.h
+++ b/lib/x86/desc.h
@@ -197,7 +197,7 @@ struct system_desc64 {
 extern idt_entry_t boot_idt[256];
 
 #ifndef __x86_64__
-extern tss32_t tss;
+extern tss32_t tss[];
 extern tss32_t tss_intr;
 void set_gdt_task_gate(u16 tss_sel, u16 sel);
 void set_idt_task_gate(int vec, u16 sel);
diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index ec005b5..9c4393f 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -104,7 +104,7 @@ void find_highmem(void)
 }
 
 /* Setup TSS for the current processor, and return TSS offset within GDT */
-unsigned long setup_tss(void)
+unsigned long setup_tss(u8 *stacktop)
 {
 	u32 id;
 	tss64_t *tss_entry;
@@ -122,6 +122,30 @@ unsigned long setup_tss(void)
 
 	return TSS_MAIN + id * 16;
 }
+#else
+/* Setup TSS for the current processor, and return TSS offset within GDT */
+unsigned long setup_tss(u8 *stacktop)
+{
+	u32 id;
+	tss32_t *tss_entry;
+
+	id = apic_id();
+
+	/* Runtime address of current TSS */
+	tss_entry = &tss[id];
+
+	/* Update TSS */
+	memset((void *)tss_entry, 0, sizeof(tss32_t));
+	tss_entry->ss0 = KERNEL_DS;
+
+	/* Update descriptors for TSS and percpu data segment.  */
+	set_gdt_entry(TSS_MAIN + id * 8,
+		      (unsigned long)tss_entry, 0xffff, 0x89, 0);
+	set_gdt_entry(TSS_MAIN + MAX_TEST_CPUS * 8 + id * 8,
+		      (unsigned long)stacktop - 4096, 0xfffff, 0x93, 0xc0);
+
+	return TSS_MAIN + id * 8;
+}
 #endif
 
 void setup_multiboot(struct mbi_bootinfo *bi)
diff --git a/x86/cstart.S b/x86/cstart.S
index e9100a4..2c0eec7 100644
--- a/x86/cstart.S
+++ b/x86/cstart.S
@@ -23,50 +23,6 @@ i = 0
         i = i + 1
         .endr
 
-.globl gdt
-gdt:
-	.quad 0
-	.quad 0x00cf9b000000ffff // flat 32-bit code segment
-	.quad 0x00cf93000000ffff // flat 32-bit data segment
-	.quad 0x00cf1b000000ffff // flat 32-bit code segment, not present
-	.quad 0                  // TSS for task gates
-	.quad 0x008f9b000000FFFF // 16-bit code segment
-	.quad 0x008f93000000FFFF // 16-bit data segment
-	.quad 0x00cffb000000ffff // 32-bit code segment (user)
-	.quad 0x00cff3000000ffff // 32-bit data segment (user)
-	.quad 0                  // unused
-
-	.quad 0			 // 6 spare selectors
-	.quad 0
-	.quad 0
-	.quad 0
-	.quad 0
-	.quad 0
-
-tss_descr:
-        .rept max_cpus
-        .quad 0x000089000000ffff // 32-bit avail tss
-        .endr
-percpu_descr:
-        .rept max_cpus
-        .quad 0x00cf93000000ffff // 32-bit data segment for perCPU area
-        .endr
-gdt_end:
-
-i = 0
-.globl tss
-tss:
-        .rept max_cpus
-        .long 0
-        .long 0
-        .long 16
-        .quad 0, 0
-        .quad 0, 0, 0, 0, 0, 0, 0, 0
-        .long 0, 0, 0
-        i = i + 1
-        .endr
-tss_end:
-
 .section .init
 
 .code32
@@ -78,21 +34,14 @@ mb_flags = 0x0
 	.long mb_magic, mb_flags, 0 - (mb_magic + mb_flags)
 mb_cmdline = 16
 
-.macro setup_percpu_area
-	lea -4096(%esp), %eax
-
-	/* fill GS_BASE in the GDT, do not clobber %ebx (multiboot info) */
-	mov (APIC_DEFAULT_PHYS_BASE + APIC_ID), %ecx
-	shr $24, %ecx
-	mov %ax, percpu_descr+2(,%ecx,8)
-
-	shr $16, %eax
-	mov %al, percpu_descr+4(,%ecx,8)
-	mov %ah, percpu_descr+7(,%ecx,8)
-
-	lea percpu_descr-gdt(,%ecx,8), %eax
+.macro setup_tr_and_percpu
+	lidt idt_descr
+	push %esp
+	call setup_tss
+	addl $4, %esp
+	ltr %ax
+	add $(max_cpus * 8), %ax
 	mov %ax, %gs
-
 .endm
 
 .macro setup_segments
@@ -109,7 +58,6 @@ start:
         lgdtl gdt_descr
         setup_segments
         mov $stacktop, %esp
-        setup_percpu_area
 
         push %ebx
         call setup_multiboot
@@ -147,11 +95,10 @@ ap_start32:
 	setup_segments
 	mov $-4096, %esp
 	lock xaddl %esp, smp_stacktop
-	setup_percpu_area
+	setup_tr_and_percpu
 	call prepare_32
 	call reset_apic
 	call save_id
-	call load_tss
 	call enable_apic
 	call enable_x2apic
 	sti
@@ -162,9 +109,9 @@ ap_start32:
 	jmp 1b
 
 start32:
+	setup_tr_and_percpu
 	call reset_apic
 	call save_id
-	call load_tss
 	call mask_pic_interrupts
 	call enable_apic
 	call ap_init
@@ -177,26 +124,9 @@ start32:
 	push %eax
 	call exit
 
-load_tss:
-	lidt idt_descr
-	mov $16, %eax
-	mov %ax, %ss
-	mov (APIC_DEFAULT_PHYS_BASE + APIC_ID), %eax
-	shr $24, %eax
-	mov %eax, %ebx
-	mov $((tss_end - tss) / max_cpus), %edx
-	imul %edx
-	add $tss, %eax
-	mov %ax, tss_descr+2(,%ebx,8)
-	shr $16, %eax
-	mov %al, tss_descr+4(,%ebx,8)
-	mov %ah, tss_descr+7(,%ebx,8)
-	lea tss_descr-gdt(,%ebx,8), %eax
-	ltr %ax
-	ret
-
 ap_init:
 	cld
+	sgdtl ap_gdt_descr // must be close to sipi_entry for real mode access to work
 	lea sipi_entry, %esi
 	xor %edi, %edi
 	mov $(sipi_end - sipi_entry), %ecx
@@ -220,11 +150,11 @@ sipi_entry:
 	mov %cr0, %eax
 	or $1, %eax
 	mov %eax, %cr0
-	lgdtl gdt_descr - sipi_entry
+	lgdtl ap_gdt_descr - sipi_entry
 	ljmpl $8, $ap_start32
 
-gdt_descr:
-	.word gdt_end - gdt - 1
-	.long gdt
+ap_gdt_descr:
+	.word 0
+	.long 0
 
 sipi_end:
diff --git a/x86/cstart64.S b/x86/cstart64.S
index c6daa34..ddb83a0 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -68,6 +68,7 @@ MSR_GS_BASE = 0xc0000101
 
 .macro load_tss
 	lidtq idt_descr
+	movq %rsp, %rdi
 	call setup_tss
 	ltr %ax
 .endm
diff --git a/x86/smap.c b/x86/smap.c
index ac2c8d5..c6ddf38 100644
--- a/x86/smap.c
+++ b/x86/smap.c
@@ -20,7 +20,7 @@ void do_pf_tss(unsigned long error_code)
 	save = test;
 
 #ifndef __x86_64__
-	tss.eflags |= X86_EFLAGS_AC;
+	tss[0].eflags |= X86_EFLAGS_AC;
 #endif
 }
 
diff --git a/x86/taskswitch.c b/x86/taskswitch.c
index 1d6e6e2..0d31149 100644
--- a/x86/taskswitch.c
+++ b/x86/taskswitch.c
@@ -19,7 +19,7 @@ fault_handler(unsigned long error_code)
 	print_current_tss_info();
 	printf("error code %lx\n", error_code);
 
-	tss.eip += 2;
+	tss[0].eip += 2;
 
 	gdt[TSS_MAIN / 8].type &= ~DESC_BUSY;
 
diff --git a/x86/umip.c b/x86/umip.c
index 54f3884..55332f8 100644
--- a/x86/umip.c
+++ b/x86/umip.c
@@ -161,7 +161,7 @@ static noinline int do_ring3(void (*fn)(const char *), const char *arg)
 #ifdef __x86_64__
 		    [sp0] "=m" (tss[0].rsp0),
 #else
-		    [sp0] "=m" (tss.esp0),
+		    [sp0] "=m" (tss[0].esp0),
 #endif
 		  : [user_ds] "i" (USER_DS),
 		    [user_cs] "i" (USER_CS),
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 kvm-unit-tests 0/8] x86: Move IDT, GDT and TSS to C code
  2021-10-21 11:49 [PATCH v3 kvm-unit-tests 0/8] x86: Move IDT, GDT and TSS to C code Paolo Bonzini
                   ` (8 preceding siblings ...)
  2021-10-21 11:49 ` [PATCH kvm-unit-tests 9/9] x86: Move 32-bit " Paolo Bonzini
@ 2021-10-21 14:37 ` Paolo Bonzini
  9 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-10-21 14:37 UTC (permalink / raw)
  To: kvm
  Cc: aaronlewis, jmattson, zxwang42, marcorr, seanjc, jroedel, varad.gautam

On 21/10/21 13:49, Paolo Bonzini wrote:
> Patches 1-4 clean up tss_descr; it is declared as a struct
> descriptor_table_ptr but it is actualy pointing to an _entry_ in the GDT.
> Also it is different per CPU, but tss_descr does not recognize that.
> Fix both by reusing the code (already present e.g. in the vmware_backdoors
> test) that extracts the base from the GDT entry; and also provide a
> helper to retrieve the limit, which is needed in vmx.c.
> 
> Patches 5-9 move the IDT, GDT and TSS to C code.  This was originally done
> by Zixuan Wang for the UEFI port, which is 64-bit only.  The series extends
> this to 32-bit code for consistency and to avoid duplicating code between
> C and assembly.
> 
> Paolo

I'm going to post a v4, since the gitlab CI showed some small issues.

Paolo


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH kvm-unit-tests 7/9] x86: get rid of ring0stacktop
  2021-10-21 11:49 ` [PATCH kvm-unit-tests 7/9] x86: get rid of ring0stacktop Paolo Bonzini
@ 2021-10-21 23:08   ` Aaron Lewis
  0 siblings, 0 replies; 16+ messages in thread
From: Aaron Lewis @ 2021-10-21 23:08 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kvm, jmattson, zxwang42, marcorr, seanjc, jroedel, varad.gautam

> --- a/lib/x86/usermode.c
> +++ b/lib/x86/usermode.c
> @@ -47,8 +47,8 @@ uint64_t run_in_user(usermode_func func, unsigned int fault_vector,
>         }
>
>         asm volatile (
> -                       /* Backing Up Stack in rdi */
> -                       "mov %%rsp, %%rdi\n\t"
> +                       /* Prepare kernel SP for exception handlers */
> +                       "mov %%rsp, %[rsp0]\n\t"
>                         /* Load user_ds to DS and ES */
>                         "mov %[user_ds], %%ax\n\t"
>                         "mov %%ax, %%ds\n\t"
> @@ -92,9 +92,10 @@ uint64_t run_in_user(usermode_func func, unsigned int fault_vector,
>                         "int %[kernel_entry_vector]\n\t"
>                         /* Kernel Mode */
>                         "ret_to_kernel:\n\t"
> -                       "mov %%rdi, %%rsp\n\t"
> +                       "mov %[rsp0], %%rsp\n\t"
>                         :
> -                       "+a"(rax)
> +                       "+a"(rax),
> +                       [rsp0]"=m"(tss.rsp0),
>                         :

The compiler didn't like the comma:
-                       [rsp0]"=m"(tss.rsp0),
+                       [rsp0]"=m"(tss.rsp0)

>                         [arg1]"m"(arg1),
>                         [arg2]"m"(arg2),

> --- a/x86/umip.c
> +++ b/x86/umip.c
> @@ -124,7 +124,7 @@ static noinline int do_ring3(void (*fn)(const char *), const char *arg)
>                   "mov %%dx, %%es\n\t"
>                   "mov %%dx, %%fs\n\t"
>                   "mov %%dx, %%gs\n\t"
> -                 "mov %%" R "sp, %%" R "cx\n\t"
> +                 "mov %%" R "sp, %[sp0]\n\t" /* kernel sp for exception handlers */
>                   "push" W " %%" R "dx \n\t"
>                   "lea %[user_stack_top], %%" R "dx \n\t"
>                   "push" W " %%" R "dx \n\t"
> @@ -133,8 +133,6 @@ static noinline int do_ring3(void (*fn)(const char *), const char *arg)
>                   "push" W " $1f \n\t"
>                   "iret" W "\n"
>                   "1: \n\t"
> -                 "push %%" R "cx\n\t"   /* save kernel SP */
> -
>  #ifndef __x86_64__
>                   "push %[arg]\n\t"
>  #endif
> @@ -142,13 +140,15 @@ static noinline int do_ring3(void (*fn)(const char *), const char *arg)
>  #ifndef __x86_64__
>                   "pop %%ecx\n\t"
>  #endif
> -
> -                 "pop %%" R "cx\n\t"
>                   "mov $1f, %%" R "dx\n\t"
>                   "int %[kernel_entry_vector]\n\t"
>                   ".section .text.entry \n\t"
>                   "kernel_entry: \n\t"
> -                 "mov %%" R "cx, %%" R "sp \n\t"
> +#ifdef __x86_64__
> +                 "mov %[sp0], %%" R "sp\n\t"
> +#else
> +                 "add $(5 * " S "), %%esp\n\t"
> +#endif
>                   "mov %[kernel_ds], %%cx\n\t"
>                   "mov %%cx, %%ds\n\t"
>                   "mov %%cx, %%es\n\t"
> @@ -157,7 +157,12 @@ static noinline int do_ring3(void (*fn)(const char *), const char *arg)
>                   "jmp *%%" R "dx \n\t"
>                   ".section .text\n\t"
>                   "1:\n\t"
> -                 : [ret] "=&a" (ret)
> +                 : [ret] "=&a" (ret),
> +#ifdef __x86_64__
> +                   [sp0] "=m" (tss.rsp0),
> +#else
> +                   [sp0] "=m" (tss.esp0),
> +#endif
>                   : [user_ds] "i" (USER_DS),

Same here:
-                   [sp0] "=m" (tss.rsp0),
-                   [sp0] "=m" (tss.esp0),
+                   [sp0] "=m" (tss.rsp0)
+                   [sp0] "=m" (tss.esp0)

>                     [user_cs] "i" (USER_CS),
>                     [user_stack_top]"m"(user_stack[sizeof(user_stack) -
> --
> 2.27.0
>
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH kvm-unit-tests 1/9] x86: cleanup handling of 16-byte GDT descriptors
  2021-10-21 11:49 ` [PATCH kvm-unit-tests 1/9] x86: cleanup handling of 16-byte GDT descriptors Paolo Bonzini
@ 2021-11-29 21:46   ` Marc Orr
  2021-11-30 10:55     ` Paolo Bonzini
  0 siblings, 1 reply; 16+ messages in thread
From: Marc Orr @ 2021-11-29 21:46 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kvm, aaronlewis, jmattson, zxwang42, seanjc, jroedel, varad.gautam

On Thu, Oct 21, 2021 at 4:49 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Look them up using a gdt_entry_t pointer, so that the address of
> the descriptor is correct even for "odd" selectors (e.g. 0x98).
> Rename the struct from segment_desc64 to system_desc64,
> highlighting that it is only used in the case of S=0 (system
> descriptor).  Rename the "limit" bitfield to "limit2", matching
> the convention used for the various parts of the base field.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  lib/x86/desc.h         |  4 ++--
>  x86/svm_tests.c        | 12 ++++++------
>  x86/vmware_backdoors.c |  8 ++++----
>  3 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/lib/x86/desc.h b/lib/x86/desc.h
> index a6ffb38..1755486 100644
> --- a/lib/x86/desc.h
> +++ b/lib/x86/desc.h
> @@ -172,7 +172,7 @@ typedef struct {
>         u8 base_high;
>  } gdt_entry_t;
>
> -struct segment_desc64 {
> +struct system_desc64 {
>         uint16_t limit1;
>         uint16_t base1;
>         uint8_t  base2;
> @@ -183,7 +183,7 @@ struct segment_desc64 {
>                         uint16_t s:1;
>                         uint16_t dpl:2;
>                         uint16_t p:1;
> -                       uint16_t limit:4;
> +                       uint16_t limit2:4;
>                         uint16_t avl:1;
>                         uint16_t l:1;
>                         uint16_t db:1;
> diff --git a/x86/svm_tests.c b/x86/svm_tests.c
> index afdd359..2fdb0dc 100644
> --- a/x86/svm_tests.c
> +++ b/x86/svm_tests.c
> @@ -1876,22 +1876,22 @@ static bool reg_corruption_check(struct svm_test *test)
>  static void get_tss_entry(void *data)
>  {
>      struct descriptor_table_ptr gdt;
> -    struct segment_desc64 *gdt_table;
> -    struct segment_desc64 *tss_entry;
> +    gdt_entry_t *gdt_table;
> +    struct system_desc64 *tss_entry;
>      u16 tr = 0;
>
>      sgdt(&gdt);
>      tr = str();
> -    gdt_table = (struct segment_desc64 *) gdt.base;
> -    tss_entry = &gdt_table[tr / sizeof(struct segment_desc64)];
> -    *((struct segment_desc64 **)data) = tss_entry;
> +    gdt_table = (gdt_entry_t *) gdt.base;
> +    tss_entry = (struct system_desc64 *) &gdt_table[tr / 8];
> +    *((struct system_desc64 **)data) = tss_entry;
>  }
>
>  static int orig_cpu_count;
>
>  static void init_startup_prepare(struct svm_test *test)
>  {
> -    struct segment_desc64 *tss_entry;
> +    struct system_desc64 *tss_entry;
>      int i;
>
>      on_cpu(1, get_tss_entry, &tss_entry);
> diff --git a/x86/vmware_backdoors.c b/x86/vmware_backdoors.c
> index b4902a9..b1433cd 100644
> --- a/x86/vmware_backdoors.c
> +++ b/x86/vmware_backdoors.c
> @@ -133,8 +133,8 @@ struct fault_test vmware_backdoor_tests[] = {
>  static void set_tss_ioperm(void)
>  {
>         struct descriptor_table_ptr gdt;
> -       struct segment_desc64 *gdt_table;
> -       struct segment_desc64 *tss_entry;
> +       gdt_entry_t *gdt_table;
> +       struct system_desc64 *tss_entry;
>         u16 tr = 0;
>         tss64_t *tss;
>         unsigned char *ioperm_bitmap;
> @@ -142,8 +142,8 @@ static void set_tss_ioperm(void)
>
>         sgdt(&gdt);
>         tr = str();
> -       gdt_table = (struct segment_desc64 *) gdt.base;
> -       tss_entry = &gdt_table[tr / sizeof(struct segment_desc64)];
> +       gdt_table = (gdt_entry_t *) gdt.base;
> +       tss_entry = (struct system_desc64 *) &gdt_table[tr / 8];
>         tss_base = ((uint64_t) tss_entry->base1 |
>                         ((uint64_t) tss_entry->base2 << 16) |
>                         ((uint64_t) tss_entry->base3 << 24) |
> --
> 2.27.0
>
>

I think this patch series was what was blocking the `uefi` branch from
being merged into the `master` branch. I was just trying to apply it
locally, so I could review it, and now see that it's been merged
already. Any reason not to go ahead and merge the `uefi` branch into
the mainline branch?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH kvm-unit-tests 1/9] x86: cleanup handling of 16-byte GDT descriptors
  2021-11-29 21:46   ` Marc Orr
@ 2021-11-30 10:55     ` Paolo Bonzini
  2021-11-30 17:22       ` Marc Orr
  0 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2021-11-30 10:55 UTC (permalink / raw)
  To: Marc Orr
  Cc: kvm, aaronlewis, jmattson, zxwang42, seanjc, jroedel, varad.gautam

On 11/29/21 22:46, Marc Orr wrote:
> I think this patch series was what was blocking the `uefi` branch from
> being merged into the `master` branch. I was just trying to apply it
> locally, so I could review it, and now see that it's been merged
> already.

Yes, Aaron used it in some patches of his so I took that as a review.

> Any reason not to go ahead and merge the `uefi` branch into
> the mainline branch?

Mostly the fact that "./run_tests.sh" does not work out of the box.

Paolo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH kvm-unit-tests 1/9] x86: cleanup handling of 16-byte GDT descriptors
  2021-11-30 10:55     ` Paolo Bonzini
@ 2021-11-30 17:22       ` Marc Orr
  2021-11-30 17:26         ` Paolo Bonzini
  0 siblings, 1 reply; 16+ messages in thread
From: Marc Orr @ 2021-11-30 17:22 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kvm, aaronlewis, jmattson, zxwang42, seanjc, jroedel, varad.gautam

On Tue, Nov 30, 2021 at 2:55 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 11/29/21 22:46, Marc Orr wrote:
> > I think this patch series was what was blocking the `uefi` branch from
> > being merged into the `master` branch. I was just trying to apply it
> > locally, so I could review it, and now see that it's been merged
> > already.
>
> Yes, Aaron used it in some patches of his so I took that as a review.

Makes sense.

> > Any reason not to go ahead and merge the `uefi` branch into
> > the mainline branch?
>
> Mostly the fact that "./run_tests.sh" does not work out of the box.

Ack. The recent patch set posted by Zixuan [1] makes "./run_tests.sh"
work out of the box.

[1] https://lore.kernel.org/all/20211116204053.220523-1-zxwang42@gmail.com/

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH kvm-unit-tests 1/9] x86: cleanup handling of 16-byte GDT descriptors
  2021-11-30 17:22       ` Marc Orr
@ 2021-11-30 17:26         ` Paolo Bonzini
  0 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-11-30 17:26 UTC (permalink / raw)
  To: Marc Orr
  Cc: kvm, aaronlewis, jmattson, zxwang42, seanjc, jroedel, varad.gautam

On 11/30/21 18:22, Marc Orr wrote:
>> Mostly the fact that "./run_tests.sh" does not work out of the box.
> Ack. The recent patch set posted by Zixuan [1] makes "./run_tests.sh"
> work out of the box.
> 
> [1]https://lore.kernel.org/all/20211116204053.220523-1-zxwang42@gmail.com/
> 

Yep, I'll take a look.  I also want to merge it. :)

Paolo

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-11-30 17:26 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-21 11:49 [PATCH v3 kvm-unit-tests 0/8] x86: Move IDT, GDT and TSS to C code Paolo Bonzini
2021-10-21 11:49 ` [PATCH kvm-unit-tests 1/9] x86: cleanup handling of 16-byte GDT descriptors Paolo Bonzini
2021-11-29 21:46   ` Marc Orr
2021-11-30 10:55     ` Paolo Bonzini
2021-11-30 17:22       ` Marc Orr
2021-11-30 17:26         ` Paolo Bonzini
2021-10-21 11:49 ` [PATCH kvm-unit-tests 2/9] x86: fix call to set_gdt_entry Paolo Bonzini
2021-10-21 11:49 ` [PATCH kvm-unit-tests 3/9] unify field names and definitions for GDT descriptors Paolo Bonzini
2021-10-21 11:49 ` [PATCH kvm-unit-tests 4/9] replace tss_descr global with a function Paolo Bonzini
2021-10-21 11:49 ` [PATCH kvm-unit-tests 5/9] x86: Move IDT to desc.c Paolo Bonzini
2021-10-21 11:49 ` [PATCH kvm-unit-tests 6/9] x86: unify name of 32-bit and 64-bit GDT Paolo Bonzini
2021-10-21 11:49 ` [PATCH kvm-unit-tests 7/9] x86: get rid of ring0stacktop Paolo Bonzini
2021-10-21 23:08   ` Aaron Lewis
2021-10-21 11:49 ` [PATCH kvm-unit-tests 8/9] x86: Move 64-bit GDT and TSS to desc.c Paolo Bonzini
2021-10-21 11:49 ` [PATCH kvm-unit-tests 9/9] x86: Move 32-bit " Paolo Bonzini
2021-10-21 14:37 ` [PATCH v3 kvm-unit-tests 0/8] x86: Move IDT, GDT and TSS to C code Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.