[kvm-unit-tests PATCH 0/9] SMP Support for x86 UEFI Tests

[kvm-unit-tests PATCH 0/9] SMP Support for x86 UEFI Tests

[Varad Gautam]

This series brings multi-vcpu support to UEFI tests on x86.

Most of the necessary AP bringup code already exists within kvm-unit-tests'
cstart64.S, and has now been either rewritten in C or moved to a common location
to be shared between EFI and non-EFI test builds.

A call gate is used to transition from 16-bit to 32-bit mode, since EFI may
not load the 32-bit entrypoint low enough to be reachable from the SIPI vector.

Git branch: https://github.com/varadgautam/kvm-unit-tests/commits/ap-boot-v1

Varad Gautam (9):
  x86: Move ap_init() to smp.c
  x86: Move load_idt() to desc.c
  x86: desc: Split IDT entry setup into a generic helper
  x86: efi, smp: Transition APs from 16-bit to 32-bit mode
  x86: Move 32-bit bringup routines to start32.S
  x86: efi, smp: Transition APs from 32-bit to 64-bit mode
  x86: Move load_gdt_tss() to desc.c
  x86: Provide a common 64-bit AP entrypoint for EFI and non-EFI
  x86: setup: Serialize ap_start64 with a spinlock

 lib/x86/asm/setup.h       |   3 ++
 lib/x86/desc.c            |  39 +++++++++++---
 lib/x86/desc.h            |   3 ++
 lib/x86/setup.c           |  65 +++++++++++++++++-----
 lib/x86/smp.c             |  89 +++++++++++++++++++++++++++++-
 lib/x86/smp.h             |   1 +
 x86/cstart64.S            | 111 ++------------------------------------
 x86/efi/crt0-efi-x86_64.S |   3 ++
 x86/efi/efistart64.S      |  73 ++++++++++++++++++++-----
 x86/start32.S             | 102 +++++++++++++++++++++++++++++++++++
 10 files changed, 348 insertions(+), 141 deletions(-)
 create mode 100644 x86/start32.S

-- 
2.32.0

[kvm-unit-tests PATCH 1/9] x86: Move ap_init() to smp.c

[Varad Gautam]

ap_init() copies the SIPI vector to lowmem, sends INIT/SIPI to APs
and waits on the APs to come up.

Port this routine to C from asm and move it to smp.c to allow sharing
this functionality between the EFI (-fPIC) and non-EFI builds.

Call ap_init() from the EFI setup path to reset the APs to a known
location.

Signed-off-by: Varad Gautam <varad.gautam@suse.com>
---
 lib/x86/setup.c      |  1 +
 lib/x86/smp.c        | 28 ++++++++++++++++++++++++++--
 lib/x86/smp.h        |  1 +
 x86/cstart64.S       | 20 ++------------------
 x86/efi/efistart64.S |  9 +++++++++
 5 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index 0044b64..8be39cb 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -323,6 +323,7 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
 	load_idt();
 	mask_pic_interrupts();
 	enable_apic();
+	ap_init();
 	enable_x2apic();
 	smp_init();
 	setup_page_table();
diff --git a/lib/x86/smp.c b/lib/x86/smp.c
index 2ac0ef7..1a68557 100644
--- a/lib/x86/smp.c
+++ b/lib/x86/smp.c
@@ -18,6 +18,9 @@ static volatile int ipi_done;
 static volatile bool ipi_wait;
 static int _cpu_count;
 static atomic_t active_cpus;
+extern u8 sipi_entry;
+extern u8 sipi_end;
+volatile unsigned cpu_online_count = 1;
 
 static __attribute__((used)) void ipi(void)
 {
@@ -118,8 +121,6 @@ void smp_init(void)
     int i;
     void ipi_entry(void);
 
-    _cpu_count = fwcfg_get_nb_cpus();
-
     setup_idt();
     init_apic_map();
     set_idt_entry(IPI_VECTOR, ipi_entry, 0);
@@ -146,3 +147,26 @@ void smp_reset_apic(void)
 
     atomic_inc(&active_cpus);
 }
+
+void ap_init(void)
+{
+    u8 *dst_addr = 0;
+    size_t sipi_sz = (&sipi_end - &sipi_entry) + 1;
+
+    asm volatile("cld");
+
+    /* Relocate SIPI vector to dst_addr so it can run in 16-bit mode. */
+    memcpy(dst_addr, &sipi_entry, sipi_sz);
+
+    /* INIT */
+    apic_icr_write(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_INIT | APIC_INT_ASSERT, 0);
+
+    /* SIPI */
+    apic_icr_write(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_STARTUP, 0);
+
+    _cpu_count = fwcfg_get_nb_cpus();
+
+    while (_cpu_count != cpu_online_count) {
+        ;
+    }
+}
diff --git a/lib/x86/smp.h b/lib/x86/smp.h
index f74845e..40255c3 100644
--- a/lib/x86/smp.h
+++ b/lib/x86/smp.h
@@ -11,5 +11,6 @@ void on_cpu(int cpu, void (*function)(void *data), void *data);
 void on_cpu_async(int cpu, void (*function)(void *data), void *data);
 void on_cpus(void (*function)(void *data), void *data);
 void smp_reset_apic(void);
+void ap_init(void);
 
 #endif
diff --git a/x86/cstart64.S b/x86/cstart64.S
index 238cebf..06daa7c 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -160,6 +160,7 @@ gdt32:
 gdt32_end:
 
 .code16
+.globl sipi_entry
 sipi_entry:
 	mov %cr0, %eax
 	or $1, %eax
@@ -171,6 +172,7 @@ gdt32_descr:
 	.word gdt32_end - gdt32 - 1
 	.long gdt32
 
+.globl sipi_end
 sipi_end:
 
 .code32
@@ -249,21 +251,3 @@ lvl5:
 
 online_cpus:
 	.fill (max_cpus + 7) / 8, 1, 0
-
-ap_init:
-	cld
-	lea sipi_entry, %rsi
-	xor %rdi, %rdi
-	mov $(sipi_end - sipi_entry), %rcx
-	rep movsb
-	mov $APIC_DEFAULT_PHYS_BASE, %eax
-	movl $(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_INIT | APIC_INT_ASSERT), APIC_ICR(%rax)
-	movl $(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_STARTUP), APIC_ICR(%rax)
-	call fwcfg_get_nb_cpus
-1:	pause
-	cmpw %ax, cpu_online_count
-	jne 1b
-	ret
-
-.align 2
-cpu_online_count:	.word 1
diff --git a/x86/efi/efistart64.S b/x86/efi/efistart64.S
index 017abba..0425153 100644
--- a/x86/efi/efistart64.S
+++ b/x86/efi/efistart64.S
@@ -57,3 +57,12 @@ load_gdt_tss:
 	pushq $0x08 /* 2nd entry in gdt64: 64-bit code segment */
 	pushq %rdi
 	lretq
+
+.code16
+
+.globl sipi_entry
+sipi_entry:
+	jmp sipi_entry
+
+.globl sipi_end
+sipi_end:
-- 
2.32.0

[kvm-unit-tests PATCH 2/9] x86: Move load_idt() to desc.c

[Varad Gautam]

This allows sharing IDT setup code between EFI (-fPIC) and
non-EFI builds.

Signed-off-by: Varad Gautam <varad.gautam@suse.com>
---
 lib/x86/desc.c       | 5 +++++
 lib/x86/desc.h       | 1 +
 lib/x86/setup.c      | 1 -
 x86/cstart64.S       | 3 ++-
 x86/efi/efistart64.S | 5 -----
 5 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index c2eb16e..355a428 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -293,6 +293,11 @@ void setup_idt(void)
     handle_exception(13, check_exception_table);
 }
 
+void load_idt(void)
+{
+	lidt(&idt_descr);
+}
+
 unsigned exception_vector(void)
 {
     unsigned char vector;
diff --git a/lib/x86/desc.h b/lib/x86/desc.h
index ad6277b..602e9f7 100644
--- a/lib/x86/desc.h
+++ b/lib/x86/desc.h
@@ -4,6 +4,7 @@
 #include <setjmp.h>
 
 void setup_idt(void);
+void load_idt(void);
 void setup_alt_stack(void);
 
 struct ex_regs {
diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index 8be39cb..eab035f 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -170,7 +170,6 @@ void setup_multiboot(struct mbi_bootinfo *bi)
 #ifdef CONFIG_EFI
 
 /* From x86/efi/efistart64.S */
-extern void load_idt(void);
 extern void load_gdt_tss(size_t tss_offset);
 
 static efi_status_t setup_memory_allocator(efi_bootinfo_t *efi_bootinfo)
diff --git a/x86/cstart64.S b/x86/cstart64.S
index 06daa7c..b867791 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -69,7 +69,6 @@ MSR_GS_BASE = 0xc0000101
 .endm
 
 .macro load_tss
-	lidtq idt_descr
 	movq %rsp, %rdi
 	call setup_tss
 	ltr %ax
@@ -198,6 +197,7 @@ ap_start64:
 	lock btsl %eax, ap_lock
 	jc .retry
 	call reset_apic
+	call load_idt
 	load_tss
 	call enable_apic
 	call save_id
@@ -213,6 +213,7 @@ ap_start64:
 
 start64:
 	call reset_apic
+	call load_idt
 	load_tss
 	call mask_pic_interrupts
 	call enable_apic
diff --git a/x86/efi/efistart64.S b/x86/efi/efistart64.S
index 0425153..ea3d1c0 100644
--- a/x86/efi/efistart64.S
+++ b/x86/efi/efistart64.S
@@ -26,11 +26,6 @@ ptl4:
 .code64
 .text
 
-.globl load_idt
-load_idt:
-	lidtq idt_descr(%rip)
-	retq
-
 .globl load_gdt_tss
 load_gdt_tss:
 	/* Load GDT */
-- 
2.32.0

[kvm-unit-tests PATCH 3/9] x86: desc: Split IDT entry setup into a generic helper

[Varad Gautam]

EFI bootstrapping code configures a call gate in a later commit to jump
from 16-bit to 32-bit code.

Introduce a set_idt_entry_t() routine which can be used to fill both
an interrupt descriptor and a call gate descriptor on x86.

Signed-off-by: Varad Gautam <varad.gautam@suse.com>
---
 lib/x86/desc.c | 28 ++++++++++++++++++++++------
 lib/x86/desc.h |  1 +
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 355a428..713ad0b 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -56,22 +56,38 @@ __attribute__((regparm(1)))
 #endif
 void do_handle_exception(struct ex_regs *regs);
 
-void set_idt_entry(int vec, void *addr, int dpl)
+/*
+ * Fill an idt_entry_t, clearing e_sz bytes first.
+ *
+ * This can also be used to set up x86 call gates, since the gate
+ * descriptor layout is identical to idt_entry_t, except for the
+ * absence of .offset2 and .reserved fields. To do so, pass in e_sz
+ * according to the gate descriptor size.
+ */
+void set_idt_entry_t(idt_entry_t *e, size_t e_sz, void *addr,
+		u16 sel, u16 type, u16 dpl)
 {
-    idt_entry_t *e = &boot_idt[vec];
-    memset(e, 0, sizeof *e);
+    memset(e, 0, e_sz);
     e->offset0 = (unsigned long)addr;
-    e->selector = read_cs();
+    e->selector = sel;
     e->ist = 0;
-    e->type = 14;
+    e->type = type;
     e->dpl = dpl;
     e->p = 1;
     e->offset1 = (unsigned long)addr >> 16;
 #ifdef __x86_64__
-    e->offset2 = (unsigned long)addr >> 32;
+    if (e_sz == sizeof(*e)) {
+        e->offset2 = (unsigned long)addr >> 32;
+    }
 #endif
 }
 
+void set_idt_entry(int vec, void *addr, int dpl)
+{
+    idt_entry_t *e = &boot_idt[vec];
+    set_idt_entry_t(e, sizeof *e, addr, read_cs(), 14, dpl);
+}
+
 void set_idt_dpl(int vec, u16 dpl)
 {
     idt_entry_t *e = &boot_idt[vec];
diff --git a/lib/x86/desc.h b/lib/x86/desc.h
index 602e9f7..5eb21e4 100644
--- a/lib/x86/desc.h
+++ b/lib/x86/desc.h
@@ -217,6 +217,7 @@ unsigned exception_vector(void);
 int write_cr4_checking(unsigned long val);
 unsigned exception_error_code(void);
 bool exception_rflags_rf(void);
+void set_idt_entry_t(idt_entry_t *e, size_t e_sz, void *addr, u16 sel, u16 type, u16 dpl);
 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);
-- 
2.32.0

[kvm-unit-tests PATCH 4/9] x86: efi, smp: Transition APs from 16-bit to 32-bit mode

[Varad Gautam]

Sending INIT/SIPI to APs from ap_init() resets them into 16-bit mode
to loop within sipi_entry().

To drive the APs into 32-bit mode, the SIPI vector needs:
1. A GDT descriptor reachable from 16-bit code (gdt32_descr).
2. A 32-bit entrypoint reachable from 16-bit code (ap_start32).
3. The locations of GDT and the 32-bit entrypoint.

Setting these up at compile time (like on non-EFI builds) is not
possible since EFI builds with -shared -fPIC and efistart64.S cannot
reference any absolute addresses.

Relative addressing is unavailable on 16-bit mode.

Moreover, EFI may not load the 32-bit entrypoint to be reachable from
16-bit mode.

To overcome these problems,
1. Fill the GDT descriptor at runtime after relocating
   [sipi_entry-sipi_end] to lowmem. Since sipi_entry does not know the
   address of this descriptor, use the last two bytes of SIPI page to
   communicate it.
2. Place a call gate in the GDT to point to ap_start32.
3. Popluate sipi_entry() to lcall to ap_start32.

With this, the APs can transition to 32-bit mode and loop at a known
location.

Signed-off-by: Varad Gautam <varad.gautam@suse.com>
---
 lib/x86/smp.c        | 56 ++++++++++++++++++++++++++++++++++++++++++++
 x86/efi/efistart64.S | 29 ++++++++++++++++++++++-
 2 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/lib/x86/smp.c b/lib/x86/smp.c
index 1a68557..33251cb 100644
--- a/lib/x86/smp.c
+++ b/lib/x86/smp.c
@@ -6,6 +6,7 @@
 #include "apic.h"
 #include "fwcfg.h"
 #include "desc.h"
+#include "asm/page.h"
 
 #define IPI_VECTOR 0x20
 
@@ -148,16 +149,71 @@ void smp_reset_apic(void)
     atomic_inc(&active_cpus);
 }
 
+#ifdef CONFIG_EFI
+extern u8 gdt32_descr, gdt32, gdt32_end;
+extern u8 ap_start32;
+#endif
+
 void ap_init(void)
 {
     u8 *dst_addr = 0;
     size_t sipi_sz = (&sipi_end - &sipi_entry) + 1;
 
+    assert(sipi_sz < PAGE_SIZE);
+
     asm volatile("cld");
 
     /* Relocate SIPI vector to dst_addr so it can run in 16-bit mode. */
+    memset(dst_addr, 0, PAGE_SIZE);
     memcpy(dst_addr, &sipi_entry, sipi_sz);
 
+#ifdef CONFIG_EFI
+    volatile struct descriptor_table_ptr *gdt32_descr_rel;
+    idt_entry_t *gate_descr;
+    u16 *gdt32_descr_reladdr = (u16 *) (PAGE_SIZE - sizeof(u16));
+
+    /*
+     * gdt32_descr for CONFIG_EFI needs to be filled here dynamically
+     * since compile time calculation of offsets is not allowed when
+     * building with -shared, and rip-relative addressing is not supported
+     * in 16-bit mode.
+     *
+     * Use the last two bytes of SIPI page to store relocated gdt32_descr
+     * addr.
+     */
+    *gdt32_descr_reladdr = (&gdt32_descr - &sipi_entry);
+
+    gdt32_descr_rel = (struct descriptor_table_ptr *) ((u64) *gdt32_descr_reladdr);
+    gdt32_descr_rel->limit = (u16) (&gdt32_end - &gdt32 - 1);
+    gdt32_descr_rel->base = (ulong) ((u32) (&gdt32 - &sipi_entry));
+
+    /*
+     * EFI may not load the 32-bit AP entrypoint (ap_start32) low enough
+     * to be reachable from the SIPI vector. Since we build with -shared, this
+     * location needs to be fetched at runtime, and rip-relative addressing is
+     * not supported in 16-bit mode.
+     * To perform 16-bit -> 32-bit far jump, our options are:
+     * - ljmpl $cs, $label : unusable since $label is not known at build time.
+     * - push $cs; push $label; lret : requires an intermediate trampoline since
+     *     $label must still be within 0 - 0xFFFF for 16-bit far return to work.
+     * - lcall into a call-gate : best suited.
+     *
+     * Set up call gate to ap_start32 within GDT.
+     *
+     * gdt32 layout:
+     *
+     * Entry | Segment
+     * 0     | NULL descr
+     * 1     | Code segment descr
+     * 2     | Data segment descr
+     * 3     | Call gate descr
+     */
+    gate_descr = (idt_entry_t *) ((u8 *)(&gdt32 - &sipi_entry)
+                + 3 * sizeof(gdt_entry_t));
+    set_idt_entry_t(gate_descr, sizeof(gdt_entry_t), (void *) &ap_start32,
+        0x8 /* sel */, 0xc /* type */, 0 /* dpl */);
+#endif
+
     /* INIT */
     apic_icr_write(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_INIT | APIC_INT_ASSERT, 0);
 
diff --git a/x86/efi/efistart64.S b/x86/efi/efistart64.S
index ea3d1c0..9a0cf98 100644
--- a/x86/efi/efistart64.S
+++ b/x86/efi/efistart64.S
@@ -57,7 +57,34 @@ load_gdt_tss:
 
 .globl sipi_entry
 sipi_entry:
-	jmp sipi_entry
+	mov %cr0, %eax
+	or $1, %eax
+	mov %eax, %cr0
+
+	/* Retrieve relocated gdt32_descr address at (PAGE_SIZE - 2). */
+	mov (PAGE_SIZE - 2), %ebx
+	lgdtl (%ebx)
+
+	lcall $0x18, $0x0
+
+.globl gdt32
+gdt32:
+	.quad 0
+	.quad 0x00cf9b000000ffff // flat 32-bit code segment
+	.quad 0x00cf93000000ffff // flat 32-bit data segment
+	.quad 0                  // call gate to 32-bit AP entrypoint
+.globl gdt32_end
+gdt32_end:
+
+.globl gdt32_descr
+gdt32_descr:
+	.word 0
+	.long 0
 
 .globl sipi_end
 sipi_end:
+
+.code32
+.globl ap_start32
+ap_start32:
+	jmp ap_start32
-- 
2.32.0

[kvm-unit-tests PATCH 5/9] x86: Move 32-bit bringup routines to start32.S

[Varad Gautam]

These can be shared across EFI and non-EFI builds.

Signed-off-by: Varad Gautam <varad.gautam@suse.com>
---
 x86/cstart64.S | 60 +-----------------------------------------------
 x86/start32.S  | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 59 deletions(-)
 create mode 100644 x86/start32.S

diff --git a/x86/cstart64.S b/x86/cstart64.S
index b867791..45009d4 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -59,35 +59,13 @@ mb_flags = 0x0
 	.long mb_magic, mb_flags, 0 - (mb_magic + mb_flags)
 mb_cmdline = 16
 
-MSR_GS_BASE = 0xc0000101
-
-.macro setup_percpu_area
-	lea -4096(%esp), %eax
-	mov $0, %edx
-	mov $MSR_GS_BASE, %ecx
-	wrmsr
-.endm
-
 .macro load_tss
 	movq %rsp, %rdi
 	call setup_tss
 	ltr %ax
 .endm
 
-.macro setup_segments
-	mov $MSR_GS_BASE, %ecx
-	rdmsr
-
-	mov $0x10, %bx
-	mov %bx, %ds
-	mov %bx, %es
-	mov %bx, %fs
-	mov %bx, %gs
-	mov %bx, %ss
-
-	/* restore MSR_GS_BASE */
-	wrmsr
-.endm
+#include "start32.S"
 
 .globl start
 start:
@@ -121,33 +99,6 @@ switch_to_5level:
 	call enter_long_mode
 	jmpl $8, $lvl5
 
-prepare_64:
-	lgdt gdt_descr
-	setup_segments
-
-	xor %eax, %eax
-	mov %eax, %cr4
-
-enter_long_mode:
-	mov %cr4, %eax
-	bts $5, %eax  // pae
-	mov %eax, %cr4
-
-	mov pt_root, %eax
-	mov %eax, %cr3
-
-efer = 0xc0000080
-	mov $efer, %ecx
-	rdmsr
-	bts $8, %eax
-	wrmsr
-
-	mov %cr0, %eax
-	bts $0, %eax
-	bts $31, %eax
-	mov %eax, %cr0
-	ret
-
 smp_stacktop:	.long stacktop - 4096
 
 .align 16
@@ -174,15 +125,6 @@ gdt32_descr:
 .globl sipi_end
 sipi_end:
 
-.code32
-ap_start32:
-	setup_segments
-	mov $-4096, %esp
-	lock xaddl %esp, smp_stacktop
-	setup_percpu_area
-	call prepare_64
-	ljmpl $8, $ap_start64
-
 .code64
 save_id:
 	movl $(APIC_DEFAULT_PHYS_BASE + APIC_ID), %eax
diff --git a/x86/start32.S b/x86/start32.S
new file mode 100644
index 0000000..9e00474
--- /dev/null
+++ b/x86/start32.S
@@ -0,0 +1,62 @@
+/* Common 32-bit code between EFI and non-EFI bootstrapping. */
+
+.code32
+
+MSR_GS_BASE = 0xc0000101
+
+.macro setup_percpu_area
+	lea -4096(%esp), %eax
+	mov $0, %edx
+	mov $MSR_GS_BASE, %ecx
+	wrmsr
+.endm
+
+.macro setup_segments
+	mov $MSR_GS_BASE, %ecx
+	rdmsr
+
+	mov $0x10, %bx
+	mov %bx, %ds
+	mov %bx, %es
+	mov %bx, %fs
+	mov %bx, %gs
+	mov %bx, %ss
+
+	/* restore MSR_GS_BASE */
+	wrmsr
+.endm
+
+prepare_64:
+	lgdt gdt_descr
+	setup_segments
+
+	xor %eax, %eax
+	mov %eax, %cr4
+
+enter_long_mode:
+	mov %cr4, %eax
+	bts $5, %eax  // pae
+	mov %eax, %cr4
+
+	mov pt_root, %eax
+	mov %eax, %cr3
+
+efer = 0xc0000080
+	mov $efer, %ecx
+	rdmsr
+	bts $8, %eax
+	wrmsr
+
+	mov %cr0, %eax
+	bts $0, %eax
+	bts $31, %eax
+	mov %eax, %cr0
+	ret
+
+ap_start32:
+	setup_segments
+	mov $-4096, %esp
+	lock xaddl %esp, smp_stacktop
+	setup_percpu_area
+	call prepare_64
+	ljmpl $8, $ap_start64
-- 
2.32.0

[kvm-unit-tests PATCH 6/9] x86: efi, smp: Transition APs from 32-bit to 64-bit mode

[Varad Gautam]

Reaching 64-bit mode requires setting up a valid stack and percpu
regions for each CPU and configuring a page table before far-jumping to
the 64-bit entrypoint.

This functionality is already present as prepare_64() and ap_start32()
routines in start32.S for non-EFI test builds.

However since EFI builds (-fPIC) cannot use absolute addressing, and
32-bit mode does not allow RIP-relative addressing, these routines need
some changes.

Modify prepare_64() and ap_start32() asm routines to calculate label
addresses during runtime on CONFIG_EFI. To ease the common case, replace
the far-jump to ap_start64() with a far-return.

Signed-off-by: Varad Gautam <varad.gautam@suse.com>
---
 lib/x86/setup.c           |  2 +-
 lib/x86/smp.c             |  4 ++++
 x86/efi/crt0-efi-x86_64.S |  3 +++
 x86/efi/efistart64.S      | 21 +++++++++++++++---
 x86/start32.S             | 46 ++++++++++++++++++++++++++++++++++++---
 5 files changed, 69 insertions(+), 7 deletions(-)

diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index eab035f..3f3b1e2 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -321,11 +321,11 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
 	setup_idt();
 	load_idt();
 	mask_pic_interrupts();
+	setup_page_table();
 	enable_apic();
 	ap_init();
 	enable_x2apic();
 	smp_init();
-	setup_page_table();
 
 	return EFI_SUCCESS;
 }
diff --git a/lib/x86/smp.c b/lib/x86/smp.c
index 33251cb..4ddb459 100644
--- a/lib/x86/smp.c
+++ b/lib/x86/smp.c
@@ -152,6 +152,8 @@ void smp_reset_apic(void)
 #ifdef CONFIG_EFI
 extern u8 gdt32_descr, gdt32, gdt32_end;
 extern u8 ap_start32;
+extern u32 smp_stacktop;
+extern u8 stacktop;
 #endif
 
 void ap_init(void)
@@ -172,6 +174,8 @@ void ap_init(void)
     idt_entry_t *gate_descr;
     u16 *gdt32_descr_reladdr = (u16 *) (PAGE_SIZE - sizeof(u16));
 
+    smp_stacktop = ((u64) (&stacktop)) - 4096;
+
     /*
      * gdt32_descr for CONFIG_EFI needs to be filled here dynamically
      * since compile time calculation of offsets is not allowed when
diff --git a/x86/efi/crt0-efi-x86_64.S b/x86/efi/crt0-efi-x86_64.S
index eaf1656..1708ed5 100644
--- a/x86/efi/crt0-efi-x86_64.S
+++ b/x86/efi/crt0-efi-x86_64.S
@@ -58,6 +58,9 @@ _start:
 	popq %rdi
 	popq %rsi
 
+	/* Switch away from EFI stack. */
+	lea stacktop(%rip), %rsp
+
 	call efi_main
 	addq $8, %rsp
 
diff --git a/x86/efi/efistart64.S b/x86/efi/efistart64.S
index 9a0cf98..7e924dc 100644
--- a/x86/efi/efistart64.S
+++ b/x86/efi/efistart64.S
@@ -6,6 +6,17 @@
 
 .data
 
+max_cpus = MAX_TEST_CPUS
+
+/* Reserve stack in .data */
+	. = . + 4096 * max_cpus
+	.align 16
+.globl stacktop
+stacktop:
+
+.globl smp_stacktop
+smp_stacktop:	.long 0
+
 .align PAGE_SIZE
 .globl ptl2
 ptl2:
@@ -85,6 +96,10 @@ gdt32_descr:
 sipi_end:
 
 .code32
-.globl ap_start32
-ap_start32:
-	jmp ap_start32
+
+#include "../start32.S"
+
+.code64:
+
+ap_start64:
+	jmp ap_start64
diff --git a/x86/start32.S b/x86/start32.S
index 9e00474..2089be7 100644
--- a/x86/start32.S
+++ b/x86/start32.S
@@ -27,7 +27,16 @@ MSR_GS_BASE = 0xc0000101
 .endm
 
 prepare_64:
-	lgdt gdt_descr
+#ifdef CONFIG_EFI
+	call prepare_64_1
+prepare_64_1:
+	pop %edx
+	add $gdt_descr - prepare_64_1, %edx
+#else
+	mov $gdt_descr, %edx
+#endif
+	lgdtl (%edx)
+
 	setup_segments
 
 	xor %eax, %eax
@@ -38,7 +47,14 @@ enter_long_mode:
 	bts $5, %eax  // pae
 	mov %eax, %cr4
 
+#ifdef CONFIG_EFI
+	call prepare_64_2
+prepare_64_2:
+	pop %eax
+	add $ptl4 - prepare_64_2, %eax
+#else
 	mov pt_root, %eax
+#endif
 	mov %eax, %cr3
 
 efer = 0xc0000080
@@ -53,10 +69,34 @@ efer = 0xc0000080
 	mov %eax, %cr0
 	ret
 
+.globl ap_start32
 ap_start32:
 	setup_segments
+
+#ifdef CONFIG_EFI
+	call ap_start32_1
+ap_start32_1:
+	pop %edx
+	add $smp_stacktop - ap_start32_1, %edx
+#else
+	mov $smp_stacktop, %edx
+#endif
 	mov $-4096, %esp
-	lock xaddl %esp, smp_stacktop
+	lock xaddl %esp, (%edx)
+
 	setup_percpu_area
 	call prepare_64
-	ljmpl $8, $ap_start64
+
+#ifdef CONFIG_EFI
+	call ap_start32_2
+ap_start32_2:
+	pop %edx
+	add $ap_start64 - ap_start32_2, %edx
+#else
+	mov $ap_start64, %edx
+#endif
+
+	pushl $0x08
+	pushl %edx
+
+	lretl
-- 
2.32.0

[kvm-unit-tests PATCH 7/9] x86: Move load_gdt_tss() to desc.c

[Varad Gautam]

Split load_gdt_tss() functionality into:
1. Load gdt/tss.
2. Setup segments in 64-bit mode.
3. Update cs segment via far-return.

and move load_gdt_tss() to desc.c to share this code between
EFI and non-EFI tests.

Signed-off-by: Varad Gautam <varad.gautam@suse.com>
---
 lib/x86/desc.c       |  6 ++++++
 lib/x86/desc.h       |  1 +
 lib/x86/setup.c      |  9 ++++++++-
 x86/efi/efistart64.S | 22 +++++++++++++---------
 4 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 713ad0b..d627a22 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -370,6 +370,12 @@ void set_gdt_entry(int sel, unsigned long base,  u32 limit, u8 type, u8 flags)
 #endif
 }
 
+void load_gdt_tss(size_t tss_offset)
+{
+	lgdt(&gdt_descr);
+	ltr(tss_offset);
+}
+
 #ifndef __x86_64__
 void set_gdt_task_gate(u16 sel, u16 tss_sel)
 {
diff --git a/lib/x86/desc.h b/lib/x86/desc.h
index 5eb21e4..30a0c90 100644
--- a/lib/x86/desc.h
+++ b/lib/x86/desc.h
@@ -222,6 +222,7 @@ 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, unsigned long base, u32 limit, u8 access, u8 gran);
+void load_gdt_tss(size_t tss_offset);
 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 3f3b1e2..e5a690a 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -170,7 +170,9 @@ void setup_multiboot(struct mbi_bootinfo *bi)
 #ifdef CONFIG_EFI
 
 /* From x86/efi/efistart64.S */
-extern void load_gdt_tss(size_t tss_offset);
+extern void update_cs(void);
+extern void setup_segments64(u64 gs_base);
+extern u8 stacktop;
 
 static efi_status_t setup_memory_allocator(efi_bootinfo_t *efi_bootinfo)
 {
@@ -271,10 +273,15 @@ static void setup_page_table(void)
 static void setup_gdt_tss(void)
 {
 	size_t tss_offset;
+	u64 gs_base;
 
 	/* 64-bit setup_tss does not use the stacktop argument.  */
 	tss_offset = setup_tss(NULL);
 	load_gdt_tss(tss_offset);
+
+	update_cs();
+	gs_base = (u64)(&stacktop) - (PAGE_SIZE * (apic_id() + 1));
+	setup_segments64(gs_base);
 }
 
 efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
diff --git a/x86/efi/efistart64.S b/x86/efi/efistart64.S
index 7e924dc..c8fd3a2 100644
--- a/x86/efi/efistart64.S
+++ b/x86/efi/efistart64.S
@@ -37,15 +37,8 @@ ptl4:
 .code64
 .text
 
-.globl load_gdt_tss
-load_gdt_tss:
-	/* Load GDT */
-	lgdt gdt_descr(%rip)
-
-	/* Load TSS */
-	mov %rdi, %rax
-	ltr %ax
-
+.globl setup_segments64
+setup_segments64:
 	/* Update data segments */
 	mov $0x10, %ax /* 3rd entry in gdt64: 32/64-bit data segment */
 	mov %ax, %ds
@@ -54,6 +47,17 @@ load_gdt_tss:
 	mov %ax, %gs
 	mov %ax, %ss
 
+	/* Setup percpu base */
+	MSR_GS_BASE = 0xc0000101
+	mov %rdi, %rax
+	mov $0, %edx
+	mov $MSR_GS_BASE, %ecx
+	wrmsr
+
+	ret
+
+.globl update_cs
+update_cs:
 	/*
 	 * Update the code segment by putting it on the stack before the return
 	 * address, then doing a far return: this will use the new code segment
-- 
2.32.0

[kvm-unit-tests PATCH 8/9] x86: Provide a common 64-bit AP entrypoint for EFI and non-EFI

[Varad Gautam]

ap_start64() currently serves as the 64-bit entrypoint for non-EFI
tests.

Having ap_start64() and save_id() written in asm prevents sharing these
routines between EFI and non-EFI tests.

Rewrite them in C and use ap_start64 as the 64-bit entrypoint in the EFI
boot flow.

With this, EFI tests support -smp > 1. smptest.efi now passes.

Signed-off-by: Varad Gautam <varad.gautam@suse.com>
---
 lib/x86/asm/setup.h  |  3 +++
 lib/x86/setup.c      | 57 +++++++++++++++++++++++++++++++++-----------
 lib/x86/smp.c        |  1 +
 x86/cstart64.S       | 30 -----------------------
 x86/efi/efistart64.S |  5 ----
 5 files changed, 47 insertions(+), 49 deletions(-)

diff --git a/lib/x86/asm/setup.h b/lib/x86/asm/setup.h
index 24d4fa9..8502e7d 100644
--- a/lib/x86/asm/setup.h
+++ b/lib/x86/asm/setup.h
@@ -16,4 +16,7 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo);
 void setup_5level_page_table(void);
 #endif /* CONFIG_EFI */
 
+void save_id(void);
+void ap_start64(void);
+
 #endif /* _X86_ASM_SETUP_H_ */
diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index e5a690a..261fd9b 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -14,8 +14,12 @@
 #include "apic.h"
 #include "apic-defs.h"
 #include "asm/setup.h"
+#include "processor.h"
+#include "atomic.h"
 
 extern char edata;
+extern unsigned char online_cpus[(MAX_TEST_CPUS + 7) / 8];
+extern unsigned cpu_online_count;
 
 struct mbi_bootinfo {
 	u32 flags;
@@ -170,10 +174,27 @@ void setup_multiboot(struct mbi_bootinfo *bi)
 #ifdef CONFIG_EFI
 
 /* From x86/efi/efistart64.S */
+
 extern void update_cs(void);
 extern void setup_segments64(u64 gs_base);
 extern u8 stacktop;
+#endif
+
+static void setup_gdt_tss(void)
+{
+	size_t tss_offset;
+
+	/* 64-bit setup_tss does not use the stacktop argument.  */
+	tss_offset = setup_tss(NULL);
+	load_gdt_tss(tss_offset);
+#ifdef CONFIG_EFI
+	update_cs();
+	u64 gs_base = (u64)(&stacktop) - (PAGE_SIZE * (apic_id() + 1));
+	setup_segments64(gs_base);
+#endif
+}
 
+#ifdef CONFIG_EFI
 static efi_status_t setup_memory_allocator(efi_bootinfo_t *efi_bootinfo)
 {
 	int i;
@@ -270,20 +291,6 @@ static void setup_page_table(void)
 	write_cr3((ulong)&ptl4);
 }
 
-static void setup_gdt_tss(void)
-{
-	size_t tss_offset;
-	u64 gs_base;
-
-	/* 64-bit setup_tss does not use the stacktop argument.  */
-	tss_offset = setup_tss(NULL);
-	load_gdt_tss(tss_offset);
-
-	update_cs();
-	gs_base = (u64)(&stacktop) - (PAGE_SIZE * (apic_id() + 1));
-	setup_segments64(gs_base);
-}
-
 efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
 {
 	efi_status_t status;
@@ -330,6 +337,7 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
 	mask_pic_interrupts();
 	setup_page_table();
 	enable_apic();
+	save_id();
 	ap_init();
 	enable_x2apic();
 	smp_init();
@@ -352,3 +360,24 @@ void setup_libcflat(void)
 			add_setup_arg("bootloader");
 	}
 }
+
+void save_id(void)
+{
+	u32 id = apic_id();
+
+	/* atomic_fetch_or() emits `lock or %dl, (%eax)` */
+	atomic_fetch_or(&online_cpus[id / 8], (1 << (id % 8)));
+}
+
+void ap_start64(void)
+{
+	reset_apic();
+	load_idt();
+	setup_gdt_tss();
+	save_id();
+	enable_apic();
+	enable_x2apic();
+	sti();
+	atomic_fetch_inc(&cpu_online_count);
+	asm volatile("1: hlt; jmp 1b");
+}
diff --git a/lib/x86/smp.c b/lib/x86/smp.c
index 4ddb459..eec8d2b 100644
--- a/lib/x86/smp.c
+++ b/lib/x86/smp.c
@@ -22,6 +22,7 @@ static atomic_t active_cpus;
 extern u8 sipi_entry;
 extern u8 sipi_end;
 volatile unsigned cpu_online_count = 1;
+unsigned char online_cpus[(MAX_TEST_CPUS + 7) / 8];
 
 static __attribute__((used)) void ipi(void)
 {
diff --git a/x86/cstart64.S b/x86/cstart64.S
index 45009d4..e0d4ab5 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -126,33 +126,6 @@ gdt32_descr:
 sipi_end:
 
 .code64
-save_id:
-	movl $(APIC_DEFAULT_PHYS_BASE + APIC_ID), %eax
-	movl (%rax), %eax
-	shrl $24, %eax
-	lock btsl %eax, online_cpus
-	retq
-
-ap_start64:
-.retry:
-	xor %eax, %eax
-	lock btsl %eax, ap_lock
-	jc .retry
-	call reset_apic
-	call load_idt
-	load_tss
-	call enable_apic
-	call save_id
-	call enable_x2apic
-	sti
-	xor %eax, %eax
-	lock btr %eax, ap_lock
-	nop
-	lock incw cpu_online_count
-
-1:	hlt
-	jmp 1b
-
 start64:
 	call reset_apic
 	call load_idt
@@ -191,6 +164,3 @@ setup_5level_page_table:
 	lretq
 lvl5:
 	retq
-
-online_cpus:
-	.fill (max_cpus + 7) / 8, 1, 0
diff --git a/x86/efi/efistart64.S b/x86/efi/efistart64.S
index c8fd3a2..99a3802 100644
--- a/x86/efi/efistart64.S
+++ b/x86/efi/efistart64.S
@@ -102,8 +102,3 @@ sipi_end:
 .code32
 
 #include "../start32.S"
-
-.code64:
-
-ap_start64:
-	jmp ap_start64
-- 
2.32.0

[kvm-unit-tests PATCH 9/9] x86: setup: Serialize ap_start64 with a spinlock

[Varad Gautam]

Since apic.c:apic_ops is not guarded against concurrent accesses,
there exists a race between reset_apic(), enable_apic() and
enable_x2apic() which results in APs crashing or getting blocked
in various scenarios (eg, enabling x2apic while disabling xapic).

The bug is rare with vcpu count < 32, but becomes easier to
reproduce with vcpus > 64 and the following thunk:

lib/x86/apic.c:
 void enable_apic(void)
 {
-    printf("enabling apic\n");
     xapic_write(APIC_SPIV, 0x1ff);
 }

Serialize the bringup code in ap_start64 to fix this.

Signed-off-by: Varad Gautam <varad.gautam@suse.com>
Link: https://lore.kernel.org/kvm/20220406124002.13741-1-varad.gautam@suse.com/
---
Note that this is a C port of 20220406124002.13741-1-varad.gautam@suse.com
which is not present upstream. I can squash it into the previous patch once
the asm version is upstream.

 lib/x86/setup.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index 261fd9b..b08290a 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -16,6 +16,9 @@
 #include "asm/setup.h"
 #include "processor.h"
 #include "atomic.h"
+#include "asm/spinlock.h"
+
+struct spinlock ap_lock;
 
 extern char edata;
 extern unsigned char online_cpus[(MAX_TEST_CPUS + 7) / 8];
@@ -371,12 +374,14 @@ void save_id(void)
 
 void ap_start64(void)
 {
+	spin_lock(&ap_lock);
 	reset_apic();
 	load_idt();
 	setup_gdt_tss();
 	save_id();
 	enable_apic();
 	enable_x2apic();
+	spin_unlock(&ap_lock);
 	sti();
 	atomic_fetch_inc(&cpu_online_count);
 	asm volatile("1: hlt; jmp 1b");
-- 
2.32.0

Re: [kvm-unit-tests PATCH 0/9] SMP Support for x86 UEFI Tests

[Sean Christopherson]

On Fri, Apr 08, 2022, Varad Gautam wrote:
> This series brings multi-vcpu support to UEFI tests on x86.
> 
> Most of the necessary AP bringup code already exists within kvm-unit-tests'
> cstart64.S, and has now been either rewritten in C or moved to a common location
> to be shared between EFI and non-EFI test builds.
> 
> A call gate is used to transition from 16-bit to 32-bit mode, since EFI may
> not load the 32-bit entrypoint low enough to be reachable from the SIPI vector.
> 
> Git branch: https://github.com/varadgautam/kvm-unit-tests/commits/ap-boot-v1
> 
> Varad Gautam (9):
>   x86: Move ap_init() to smp.c
>   x86: Move load_idt() to desc.c
>   x86: desc: Split IDT entry setup into a generic helper
>   x86: efi, smp: Transition APs from 16-bit to 32-bit mode
>   x86: Move 32-bit bringup routines to start32.S
>   x86: efi, smp: Transition APs from 32-bit to 64-bit mode
>   x86: Move load_gdt_tss() to desc.c
>   x86: Provide a common 64-bit AP entrypoint for EFI and non-EFI
>   x86: setup: Serialize ap_start64 with a spinlock

This series doesn't apply cleanly on upstream master.  I feel bad for asking, but
in addition to rebasing to master, can you also rebase on top of my series[*] that
fixes SMP bugs that were introduced by the initial UEFI support?  I don't think
there will be semantic conflicts, but the whitespace cleanups (spaces => tabs) do
conflict, and I'd really like to start purging the spaces mess from KUT.

Paolo / Andrew, ping on my series, it still applies cleanly.

[*] https://lore.kernel.org/all/20220121231852.1439917-1-seanjc@google.com

Re: [kvm-unit-tests PATCH 0/9] SMP Support for x86 UEFI Tests

[Varad Gautam]

Hi Sean,

On 4/8/22 4:35 PM, Sean Christopherson wrote:
> On Fri, Apr 08, 2022, Varad Gautam wrote:
>> This series brings multi-vcpu support to UEFI tests on x86.
>>
>> Most of the necessary AP bringup code already exists within kvm-unit-tests'
>> cstart64.S, and has now been either rewritten in C or moved to a common location
>> to be shared between EFI and non-EFI test builds.
>>
>> A call gate is used to transition from 16-bit to 32-bit mode, since EFI may
>> not load the 32-bit entrypoint low enough to be reachable from the SIPI vector.
>>
>> Git branch: https://github.com/varadgautam/kvm-unit-tests/commits/ap-boot-v1
>>
>> Varad Gautam (9):
>>   x86: Move ap_init() to smp.c
>>   x86: Move load_idt() to desc.c
>>   x86: desc: Split IDT entry setup into a generic helper
>>   x86: efi, smp: Transition APs from 16-bit to 32-bit mode
>>   x86: Move 32-bit bringup routines to start32.S
>>   x86: efi, smp: Transition APs from 32-bit to 64-bit mode
>>   x86: Move load_gdt_tss() to desc.c
>>   x86: Provide a common 64-bit AP entrypoint for EFI and non-EFI
>>   x86: setup: Serialize ap_start64 with a spinlock
> 
> This series doesn't apply cleanly on upstream master.  I feel bad for asking, but
> in addition to rebasing to master, can you also rebase on top of my series[*] that
> fixes SMP bugs that were introduced by the initial UEFI support?  I don't think
> there will be semantic conflicts, but the whitespace cleanups (spaces => tabs) do
> conflict, and I'd really like to start purging the spaces mess from KUT.
> 

I'd based the v1 on [1], which is no longer required after your apic_ops percpu
conversion series [2].

I've now based my series on yours and posted a v2 here [3].

[1] https://lore.kernel.org/kvm/20220406124002.13741-1-varad.gautam@suse.com/
[2] https://lore.kernel.org/all/20220121231852.1439917-1-seanjc@google.com/
[3] https://lore.kernel.org/kvm/20220412173407.13637-1-varad.gautam@suse.com/

> Paolo / Andrew, ping on my series, it still applies cleanly.
> 
> [*] https://lore.kernel.org/all/20220121231852.1439917-1-seanjc@google.com
>