[kvm-unit-tests PATCH v4 00/13] x86: SMP Support for x86 UEFI Tests

[kvm-unit-tests PATCH v4 00/13] x86: SMP Support for x86 UEFI Tests

[Sean Christopherson]

This is Varad's series to bring multi-vcpu support to UEFI tests on x86,
with some relatively minor massaging by me.  The only change I made that
I suspect might be controversial is squashing start16.S and start32.S into
a single trampolines.S.

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.

v4:
 - Add an explict magic string in dummy.c for probing.
 - Reduce #ifdefs via macros
 - Consolidate final "AP online" code.
 - Misc tweaks, e.g. to adhere to preferred style.

v3:
 - Unbreak i386 build, ingest seanjc's reviews from v2.a
 - https://lore.kernel.org/all/20220426114352.1262-1-varad.gautam@suse.com

v2: https://lore.kernel.org/kvm/20220412173407.13637-1-varad.gautam@suse.com/

Sean Christopherson (3):
  x86: Use an explicit magic string to detect that dummy.efi passes
  x86: Rename ap_init() to bringup_aps()
  x86: Add ap_online() to consolidate final "AP is alive!" code

Varad Gautam (10):
  x86: Share realmode trampoline between i386 and x86_64
  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: Move load_gdt_tss() to desc.c
  x86: efi: Provide a stack within testcase memory
  x86: efi: Provide percpu storage
  x86: Move 32-bit => 64-bit transition code to trampolines.S
  x86: efi, smp: Transition APs from 16-bit to 32-bit mode
  x86: Provide a common 64-bit AP entrypoint for EFI and non-EFI

 lib/alloc_page.h          |   3 +
 lib/x86/apic.c            |   2 -
 lib/x86/asm/setup.h       |   3 +
 lib/x86/desc.c            |  38 ++++++++--
 lib/x86/desc.h            |   4 +
 lib/x86/setup.c           |  82 +++++++++++++++++----
 lib/x86/smp.c             | 150 +++++++++++++++++++++++++++++++++++++-
 lib/x86/smp.h             |  11 +++
 scripts/runtime.bash      |   2 +-
 x86/cstart.S              |  48 ++----------
 x86/cstart64.S            | 125 +------------------------------
 x86/dummy.c               |   8 ++
 x86/efi/crt0-efi-x86_64.S |   3 +
 x86/efi/efistart64.S      |  79 ++++++++++++--------
 x86/svm_tests.c           |  10 +--
 x86/trampolines.S         | 129 ++++++++++++++++++++++++++++++++
 16 files changed, 470 insertions(+), 227 deletions(-)
 create mode 100644 x86/trampolines.S


base-commit: 610c15284a537484682adfb4b6d6313991ab954f
-- 
2.36.1.476.g0c4daa206d-goog

[kvm-unit-tests PATCH v4 01/13] x86: Use an explicit magic string to detect that dummy.efi passes

[Sean Christopherson]

Print an explicit "Dummy Hello World!" from the dummy "test" that is used
by x86 EFI to probe the basic setup.  Relying on the last line to match an
arbitrary, undocumented string in x86's boot flow is evil and fragile,
e.g. a future patch to share boot code between EFI and !EFI will print
something AP bringup info after the "enabling apic" line.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 scripts/runtime.bash | 2 +-
 x86/dummy.c          | 8 ++++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 7d0180b..bbf87cf 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -132,7 +132,7 @@ function run()
 
     last_line=$(premature_failure > >(tail -1)) && {
         skip=true
-        if [ "${CONFIG_EFI}" == "y" ] && [[ "${last_line}" =~ "enabling apic" ]]; then
+        if [ "${CONFIG_EFI}" == "y" ] && [[ "${last_line}" =~ "Dummy Hello World!" ]]; then
             skip=false
         fi
         if [ ${skip} == true ]; then
diff --git a/x86/dummy.c b/x86/dummy.c
index 5019e79..7033bb7 100644
--- a/x86/dummy.c
+++ b/x86/dummy.c
@@ -1,4 +1,12 @@
+#include "libcflat.h"
+
 int main(int argc, char **argv)
 {
+	/*
+	 * scripts/runtime.bash uses this test as a canary to determine if the
+	 * basic setup is functional.  Print a magic string to let runtime.bash
+	 * know that all is well.
+	 */
+	printf("Dummy Hello World!");
 	return 0;
 }
-- 
2.36.1.476.g0c4daa206d-goog

[kvm-unit-tests PATCH v4 02/13] x86: Share realmode trampoline between i386 and x86_64

[Sean Christopherson]

From: Varad Gautam <varad.gautam@suse.com>

i386 and x86_64 each maintain their own copy of the realmode trampoline
(sipi_entry). Move the 16-bit SIPI vector and GDT to a new trampolines.S
to be shared by both.  The common trampoline file will also be used to
shared 32-bit to 64-bit trampoline code.

Signed-off-by: Varad Gautam <varad.gautam@suse.com>
[sean: rename to trampolines.S to avoid naming conundrum]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/cstart.S      | 20 ++++----------------
 x86/cstart64.S    | 21 ++++-----------------
 x86/trampolines.S | 30 ++++++++++++++++++++++++++++++
 3 files changed, 38 insertions(+), 33 deletions(-)
 create mode 100644 x86/trampolines.S

diff --git a/x86/cstart.S b/x86/cstart.S
index 6db6a38..8aa29ee 100644
--- a/x86/cstart.S
+++ b/x86/cstart.S
@@ -126,10 +126,10 @@ start32:
 
 ap_init:
 	cld
-	sgdtl ap_gdt_descr // must be close to sipi_entry for real mode access to work
-	lea sipi_entry, %esi
+	sgdtl ap_rm_gdt_descr // must be close to rm_trampoline for real mode access to work
+	lea rm_trampoline, %esi
 	xor %edi, %edi
-	mov $(sipi_end - sipi_entry), %ecx
+	mov $(rm_trampoline_end - rm_trampoline), %ecx
 	rep movsb
 	mov $APIC_DEFAULT_PHYS_BASE, %eax
 	movl $(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_INIT | APIC_INT_ASSERT), APIC_ICR(%eax)
@@ -146,16 +146,4 @@ online_cpus:
 .align 2
 cpu_online_count:	.word 1
 
-.code16
-sipi_entry:
-	mov %cr0, %eax
-	or $1, %eax
-	mov %eax, %cr0
-	lgdtl ap_gdt_descr - sipi_entry
-	ljmpl $8, $ap_start32
-
-ap_gdt_descr:
-	.word 0
-	.long 0
-
-sipi_end:
+#include "trampolines.S"
diff --git a/x86/cstart64.S b/x86/cstart64.S
index 7272452..129ef0b 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -45,8 +45,9 @@ mb_boot_info:	.quad 0
 
 pt_root:	.quad ptl4
 
+#include "trampolines.S"
+
 .section .init
-
 .code32
 
 mb_magic = 0x1BADB002
@@ -156,20 +157,6 @@ gdt32:
 	.quad 0x00cf93000000ffff // flat 32-bit data segment
 gdt32_end:
 
-.code16
-sipi_entry:
-	mov %cr0, %eax
-	or $1, %eax
-	mov %eax, %cr0
-	lgdtl gdt32_descr - sipi_entry
-	ljmpl $8, $ap_start32
-
-gdt32_descr:
-	.word gdt32_end - gdt32 - 1
-	.long gdt32
-
-sipi_end:
-
 .code32
 ap_start32:
 	setup_segments
@@ -243,9 +230,9 @@ online_cpus:
 
 ap_init:
 	cld
-	lea sipi_entry, %rsi
+	lea rm_trampoline, %rsi
 	xor %rdi, %rdi
-	mov $(sipi_end - sipi_entry), %rcx
+	mov $(rm_trampoline_end - rm_trampoline), %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)
diff --git a/x86/trampolines.S b/x86/trampolines.S
new file mode 100644
index 0000000..9640c66
--- /dev/null
+++ b/x86/trampolines.S
@@ -0,0 +1,30 @@
+/* Common bootstrapping code to transition from 16-bit to 32-bit code. */
+
+/* EFI provides it's own SIPI sequence to handle relocation. */
+#ifndef CONFIG_EFI
+.code16
+.globl rm_trampoline
+rm_trampoline:
+
+/* Store SIPI vector code at the beginning of trampoline. */
+sipi_entry:
+	mov %cr0, %eax
+	or $1, %eax
+	mov %eax, %cr0
+	lgdtl ap_rm_gdt_descr - sipi_entry
+	ljmpl $8, $ap_start32
+sipi_end:
+
+.globl ap_rm_gdt_descr
+ap_rm_gdt_descr:
+#ifdef __i386__
+	.word 0
+	.long 0
+#else
+	.word gdt32_end - gdt32 - 1
+	.long gdt32
+#endif
+
+.globl rm_trampoline_end
+rm_trampoline_end:
+#endif
-- 
2.36.1.476.g0c4daa206d-goog

[kvm-unit-tests PATCH v4 03/13] x86: Move ap_init() to smp.c

[Sean Christopherson]

From: Varad Gautam <varad.gautam@suse.com>

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>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 lib/alloc_page.h     |  3 ++
 lib/x86/setup.c      |  1 +
 lib/x86/smp.c        | 72 ++++++++++++++++++++++++++++++++++++++++++--
 lib/x86/smp.h        |  5 +++
 x86/cstart.S         | 19 ------------
 x86/cstart64.S       | 19 ------------
 x86/efi/efistart64.S | 15 +++++++++
 x86/svm_tests.c      | 10 +++---
 8 files changed, 98 insertions(+), 46 deletions(-)

diff --git a/lib/alloc_page.h b/lib/alloc_page.h
index eed2ba0..060e041 100644
--- a/lib/alloc_page.h
+++ b/lib/alloc_page.h
@@ -13,6 +13,9 @@
 
 #define AREA_ANY_NUMBER	0xff
 
+/* The realmode trampoline gets relocated here during bootup. */
+#define RM_TRAMPOLINE_ADDR 0
+
 #define AREA_ANY	0x00000
 #define AREA_MASK	0x0ffff
 
diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index 8a4fa3c..2004e7f 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 683b25d..81dacef 100644
--- a/lib/x86/smp.c
+++ b/lib/x86/smp.c
@@ -1,11 +1,16 @@
 
 #include <libcflat.h>
+
+#include <asm/barrier.h>
+
 #include "processor.h"
 #include "atomic.h"
 #include "smp.h"
 #include "apic.h"
 #include "fwcfg.h"
 #include "desc.h"
+#include "alloc_page.h"
+#include "asm/page.h"
 
 #define IPI_VECTOR 0x20
 
@@ -18,6 +23,13 @@ static volatile int ipi_done;
 static volatile bool ipi_wait;
 static int _cpu_count;
 static atomic_t active_cpus;
+extern u8 rm_trampoline, rm_trampoline_end;
+#ifdef __i386__
+extern u8 ap_rm_gdt_descr;
+#endif
+
+/* The BSP is online from time zero. */
+atomic_t cpu_online_count = { .counter = 1 };
 
 static __attribute__((used)) void ipi(void)
 {
@@ -114,8 +126,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);
@@ -142,3 +152,61 @@ void smp_reset_apic(void)
 
 	atomic_inc(&active_cpus);
 }
+
+static void setup_rm_gdt(void)
+{
+#ifdef __i386__
+	struct descriptor_table_ptr *rm_gdt =
+		(struct descriptor_table_ptr *) (&ap_rm_gdt_descr - &rm_trampoline);
+	/*
+	 * On i386, place the gdt descriptor to be loaded from SIPI vector right after
+	 * the vector code.
+	 */
+	sgdt(rm_gdt);
+#endif
+}
+
+void ap_init(void)
+{
+	void *rm_trampoline_dst = RM_TRAMPOLINE_ADDR;
+	size_t rm_trampoline_size = (&rm_trampoline_end - &rm_trampoline) + 1;
+	assert(rm_trampoline_size < PAGE_SIZE);
+
+	asm volatile("cld");
+
+	/*
+	 * Fill the trampoline page with with INT3 (0xcc) so that any AP
+	 * that goes astray within the first page gets a fault.
+	 */
+	memset(rm_trampoline_dst, 0xcc /* INT3 */, PAGE_SIZE);
+
+	memcpy(rm_trampoline_dst, &rm_trampoline, rm_trampoline_size);
+
+	setup_rm_gdt();
+
+#ifdef CONFIG_EFI
+	/*
+	 * apic_icr_write() is unusable on CONFIG_EFI until percpu area gets set up.
+	 * Use raw writes to APIC_ICR to send IPIs for time being.
+	 */
+	volatile u32 *apic_icr = (volatile u32 *) (APIC_DEFAULT_PHYS_BASE + APIC_ICR);
+
+	/* INIT */
+	*apic_icr = APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_INIT | APIC_INT_ASSERT;
+
+	/* SIPI */
+	*apic_icr = APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_STARTUP;
+#else
+	/* 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);
+#endif
+
+	_cpu_count = fwcfg_get_nb_cpus();
+
+	printf("smp: waiting for %d APs\n", _cpu_count - 1);
+	while (_cpu_count != atomic_read(&cpu_online_count))
+		cpu_relax();
+}
diff --git a/lib/x86/smp.h b/lib/x86/smp.h
index bd303c2..77f8a19 100644
--- a/lib/x86/smp.h
+++ b/lib/x86/smp.h
@@ -3,6 +3,8 @@
 
 #include <stddef.h>
 #include <asm/spinlock.h>
+#include "libcflat.h"
+#include "atomic.h"
 
 /* Offsets into the per-cpu page. */
 struct percpu_data {
@@ -78,5 +80,8 @@ 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);
+
+extern atomic_t cpu_online_count;
 
 #endif
diff --git a/x86/cstart.S b/x86/cstart.S
index 8aa29ee..88d25fc 100644
--- a/x86/cstart.S
+++ b/x86/cstart.S
@@ -124,26 +124,7 @@ start32:
 	push %eax
 	call exit
 
-ap_init:
-	cld
-	sgdtl ap_rm_gdt_descr // must be close to rm_trampoline for real mode access to work
-	lea rm_trampoline, %esi
-	xor %edi, %edi
-	mov $(rm_trampoline_end - rm_trampoline), %ecx
-	rep movsb
-	mov $APIC_DEFAULT_PHYS_BASE, %eax
-	movl $(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_INIT | APIC_INT_ASSERT), APIC_ICR(%eax)
-	movl $(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_STARTUP), APIC_ICR(%eax)
-	call fwcfg_get_nb_cpus
-1:	pause
-	cmpw %ax, cpu_online_count
-	jne 1b
-	ret
-
 online_cpus:
 	.fill (max_cpus + 7) / 8, 1, 0
 
-.align 2
-cpu_online_count:	.word 1
-
 #include "trampolines.S"
diff --git a/x86/cstart64.S b/x86/cstart64.S
index 129ef0b..f0d12fb 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -2,7 +2,6 @@
 #include "apic-defs.h"
 
 .globl online_cpus
-.globl cpu_online_count
 
 ipi_vector = 0x20
 
@@ -227,21 +226,3 @@ lvl5:
 
 online_cpus:
 	.fill (max_cpus + 7) / 8, 1, 0
-
-ap_init:
-	cld
-	lea rm_trampoline, %rsi
-	xor %rdi, %rdi
-	mov $(rm_trampoline_end - rm_trampoline), %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..7d50f96 100644
--- a/x86/efi/efistart64.S
+++ b/x86/efi/efistart64.S
@@ -57,3 +57,18 @@ load_gdt_tss:
 	pushq $0x08 /* 2nd entry in gdt64: 64-bit code segment */
 	pushq %rdi
 	lretq
+
+.code16
+
+.globl rm_trampoline
+rm_trampoline:
+
+.globl sipi_entry
+sipi_entry:
+	jmp sipi_entry
+
+.globl sipi_end
+sipi_end:
+
+.globl rm_trampoline_end
+rm_trampoline_end:
diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index 1bd4d3b..f8c0589 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -17,8 +17,6 @@ static void *scratch_page;
 
 #define LATENCY_RUNS 1000000
 
-extern u16 cpu_online_count;
-
 u64 tsc_start;
 u64 tsc_end;
 
@@ -1955,20 +1953,20 @@ static void init_startup_prepare(struct svm_test *test)
 
     on_cpu(1, get_tss_entry, &tss_entry);
 
-    orig_cpu_count = cpu_online_count;
+    orig_cpu_count = atomic_read(&cpu_online_count);
 
     apic_icr_write(APIC_DEST_PHYSICAL | APIC_DM_INIT | APIC_INT_ASSERT,
                    id_map[1]);
 
     delay(100000000ULL);
 
-    --cpu_online_count;
+    atomic_dec(&cpu_online_count);
 
     tss_entry->type &= ~DESC_BUSY;
 
     apic_icr_write(APIC_DEST_PHYSICAL | APIC_DM_STARTUP, id_map[1]);
 
-    for (i = 0; i < 5 && cpu_online_count < orig_cpu_count; i++)
+    for (i = 0; i < 5 && atomic_read(&cpu_online_count) < orig_cpu_count; i++)
        delay(100000000ULL);
 }
 
@@ -1979,7 +1977,7 @@ static bool init_startup_finished(struct svm_test *test)
 
 static bool init_startup_check(struct svm_test *test)
 {
-    return cpu_online_count == orig_cpu_count;
+    return atomic_read(&cpu_online_count) == orig_cpu_count;
 }
 
 static volatile bool init_intercept;
-- 
2.36.1.476.g0c4daa206d-goog

[kvm-unit-tests PATCH v4 04/13] x86: Move load_idt() to desc.c

[Sean Christopherson]

From: Varad Gautam <varad.gautam@suse.com>

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

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

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 0677fcd..087e85c 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -294,6 +294,11 @@ void setup_idt(void)
 	handle_exception(13, check_exception_table);
 }
 
+void load_idt(void)
+{
+	lidt(&idt_descr);
+}
+
 unsigned exception_vector(void)
 {
 	return this_cpu_read_exception_vector();
diff --git a/lib/x86/desc.h b/lib/x86/desc.h
index 5224b58..3044409 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 2004e7f..dd2b916 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/cstart.S b/x86/cstart.S
index 88d25fc..20fcb64 100644
--- a/x86/cstart.S
+++ b/x86/cstart.S
@@ -35,7 +35,7 @@ mb_flags = 0x0
 mb_cmdline = 16
 
 .macro setup_tr_and_percpu
-	lidt idt_descr
+	call load_idt
 	push %esp
 	call setup_tss
 	addl $4, %esp
diff --git a/x86/cstart64.S b/x86/cstart64.S
index f0d12fb..8ac6e9c 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -66,7 +66,6 @@ MSR_GS_BASE = 0xc0000101
 .endm
 
 .macro load_tss
-	lidtq idt_descr
 	movq %rsp, %rdi
 	call setup_tss
 	ltr %ax
@@ -175,6 +174,7 @@ save_id:
 
 ap_start64:
 	call reset_apic
+	call load_idt
 	load_tss
 	call enable_apic
 	call save_id
@@ -188,6 +188,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 7d50f96..98cc965 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.36.1.476.g0c4daa206d-goog

[kvm-unit-tests PATCH v4 05/13] x86: desc: Split IDT entry setup into a generic helper

[Sean Christopherson]

From: Varad Gautam <varad.gautam@suse.com>

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

Introduce a set_desc_entry() 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>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 lib/x86/desc.c | 27 +++++++++++++++++++++------
 lib/x86/desc.h |  2 ++
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 087e85c..9512363 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -57,22 +57,37 @@ __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 or call gate entry, clearing e_sz bytes first.
+ *
+ * This can be used for both IDT entries and call gate entries, 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_desc_entry(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_desc_entry(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 3044409..1dc1ea0 100644
--- a/lib/x86/desc.h
+++ b/lib/x86/desc.h
@@ -217,6 +217,8 @@ unsigned exception_vector(void);
 int write_cr4_checking(unsigned long val);
 unsigned exception_error_code(void);
 bool exception_rflags_rf(void);
+void set_desc_entry(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.36.1.476.g0c4daa206d-goog

[kvm-unit-tests PATCH v4 06/13] x86: Move load_gdt_tss() to desc.c

[Sean Christopherson]

From: Varad Gautam <varad.gautam@suse.com>

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

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

Move the segment setup code specific to EFI into
setup.c:setup_segments64().

Signed-off-by: Varad Gautam <varad.gautam@suse.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 lib/x86/desc.c       |  6 ++++++
 lib/x86/desc.h       |  1 +
 lib/x86/setup.c      | 25 +++++++++++++++++++++++--
 x86/efi/efistart64.S | 27 ---------------------------
 4 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 9512363..a7c3480 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -361,6 +361,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 1dc1ea0..9dcc92b 100644
--- a/lib/x86/desc.h
+++ b/lib/x86/desc.h
@@ -223,6 +223,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 dd2b916..9724465 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -169,8 +169,27 @@ void setup_multiboot(struct mbi_bootinfo *bi)
 
 #ifdef CONFIG_EFI
 
-/* From x86/efi/efistart64.S */
-extern void load_gdt_tss(size_t tss_offset);
+static void setup_segments64(void)
+{
+	/* Update data segments */
+	write_ds(KERNEL_DS);
+	write_es(KERNEL_DS);
+	write_fs(KERNEL_DS);
+	write_gs(KERNEL_DS);
+	write_ss(KERNEL_DS);
+
+	/*
+	 * 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
+	 * along with the address.
+	 */
+	asm volatile("pushq %1\n\t"
+		     "lea 1f(%%rip), %0\n\t"
+		     "pushq %0\n\t"
+		     "lretq\n\t"
+		     "1:"
+		     :: "r" ((u64)KERNEL_DS), "i" (KERNEL_CS));
+}
 
 static efi_status_t setup_memory_allocator(efi_bootinfo_t *efi_bootinfo)
 {
@@ -275,6 +294,8 @@ static void setup_gdt_tss(void)
 	/* 64-bit setup_tss does not use the stacktop argument.  */
 	tss_offset = setup_tss(NULL);
 	load_gdt_tss(tss_offset);
+
+	setup_segments64();
 }
 
 efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
diff --git a/x86/efi/efistart64.S b/x86/efi/efistart64.S
index 98cc965..b94c5ab 100644
--- a/x86/efi/efistart64.S
+++ b/x86/efi/efistart64.S
@@ -26,33 +26,6 @@ ptl4:
 .code64
 .text
 
-.globl load_gdt_tss
-load_gdt_tss:
-	/* Load GDT */
-	lgdt gdt_descr(%rip)
-
-	/* Load TSS */
-	mov %rdi, %rax
-	ltr %ax
-
-	/* Update data segments */
-	mov $0x10, %ax /* 3rd entry in gdt64: 32/64-bit data segment */
-	mov %ax, %ds
-	mov %ax, %es
-	mov %ax, %fs
-	mov %ax, %gs
-	mov %ax, %ss
-
-	/*
-	 * 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
-	 * along with the address.
-	 */
-	popq %rdi
-	pushq $0x08 /* 2nd entry in gdt64: 64-bit code segment */
-	pushq %rdi
-	lretq
-
 .code16
 
 .globl rm_trampoline
-- 
2.36.1.476.g0c4daa206d-goog

[kvm-unit-tests PATCH v4 07/13] x86: efi: Provide a stack within testcase memory

[Sean Christopherson]

From: Varad Gautam <varad.gautam@suse.com>

UEFI test builds currently use the stack pointer configured by the
UEFI implementation.

Reserve stack space in .data for EFI testcases and switch %rsp to
use this memory on early boot. This provides one 4K page per CPU
to store its stack.

Signed-off-by: Varad Gautam <varad.gautam@suse.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/efi/crt0-efi-x86_64.S | 3 +++
 x86/efi/efistart64.S      | 6 ++++++
 2 files changed, 9 insertions(+)

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 b94c5ab..3a4135e 100644
--- a/x86/efi/efistart64.S
+++ b/x86/efi/efistart64.S
@@ -4,7 +4,13 @@
 #include "asm-generic/page.h"
 #include "crt0-efi-x86_64.S"
 
+
+/* Reserve stack in .data */
 .data
+.align PAGE_SIZE
+	. = . + PAGE_SIZE * MAX_TEST_CPUS
+.globl stacktop
+stacktop:
 
 .align PAGE_SIZE
 .globl ptl2
-- 
2.36.1.476.g0c4daa206d-goog

[kvm-unit-tests PATCH v4 08/13] x86: efi: Provide percpu storage

[Sean Christopherson]

From: Varad Gautam <varad.gautam@suse.com>

UEFI tests do not update MSR_GS_BASE during bringup, and continue
using the GS_BASE set up by the UEFI implementation for percpu
storage.

Update this MSR during setup_segments64() to allow storing percpu
data at a sane location reserved by the testcase, and ensure that
this happens before any operation that ends up storing to the percpu
space.

Since apic_ops (touched by reset_apic()) is percpu, move reset_apic()
to happen after setup_gdt_tss(). With this, ap_init() can now use
percpu apic_ops via apic_icr_write().

Signed-off-by: Varad Gautam <varad.gautam@suse.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 lib/x86/setup.c |  7 ++++++-
 lib/x86/smp.c   | 14 --------------
 2 files changed, 6 insertions(+), 15 deletions(-)

diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index 9724465..c7c0983 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -169,6 +169,8 @@ void setup_multiboot(struct mbi_bootinfo *bi)
 
 #ifdef CONFIG_EFI
 
+static struct percpu_data __percpu_data[MAX_TEST_CPUS];
+
 static void setup_segments64(void)
 {
 	/* Update data segments */
@@ -178,6 +180,9 @@ static void setup_segments64(void)
 	write_gs(KERNEL_DS);
 	write_ss(KERNEL_DS);
 
+	/* Setup percpu base */
+	wrmsr(MSR_GS_BASE, (u64)&__percpu_data[pre_boot_apic_id()]);
+
 	/*
 	 * 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
@@ -337,8 +342,8 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
 		return status;
 	}
 
-	reset_apic();
 	setup_gdt_tss();
+	reset_apic();
 	setup_idt();
 	load_idt();
 	mask_pic_interrupts();
diff --git a/lib/x86/smp.c b/lib/x86/smp.c
index 81dacef..2f554ce 100644
--- a/lib/x86/smp.c
+++ b/lib/x86/smp.c
@@ -184,25 +184,11 @@ void ap_init(void)
 
 	setup_rm_gdt();
 
-#ifdef CONFIG_EFI
-	/*
-	 * apic_icr_write() is unusable on CONFIG_EFI until percpu area gets set up.
-	 * Use raw writes to APIC_ICR to send IPIs for time being.
-	 */
-	volatile u32 *apic_icr = (volatile u32 *) (APIC_DEFAULT_PHYS_BASE + APIC_ICR);
-
-	/* INIT */
-	*apic_icr = APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_INIT | APIC_INT_ASSERT;
-
-	/* SIPI */
-	*apic_icr = APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_STARTUP;
-#else
 	/* 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);
-#endif
 
 	_cpu_count = fwcfg_get_nb_cpus();
 
-- 
2.36.1.476.g0c4daa206d-goog

[kvm-unit-tests PATCH v4 09/13] x86: Move 32-bit => 64-bit transition code to trampolines.S

[Sean Christopherson]

From: Varad Gautam <varad.gautam@suse.com>

Move the code for transitioning from unpaged 32-bit mode to paged 64-bit
mode to trampoline.S, it can be shared across EFI and non-EFI builds.

Leave 5-level paging behind for the time being, EFI doesn't yet support
support disabling paging, which is required to get from 4-level to
5-level paging.

Signed-off-by: Varad Gautam <varad.gautam@suse.com>
[sean: move to trampolines.S instead of start32.S, reword changelog]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/cstart64.S    | 60 ---------------------------------------
 x86/trampolines.S | 71 +++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 69 insertions(+), 62 deletions(-)

diff --git a/x86/cstart64.S b/x86/cstart64.S
index 8ac6e9c..0a561ce 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -56,36 +56,12 @@ 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
-
 .globl start
 start:
 	mov %ebx, mb_boot_info
@@ -118,33 +94,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
@@ -155,15 +104,6 @@ gdt32:
 	.quad 0x00cf93000000ffff // flat 32-bit data segment
 gdt32_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/trampolines.S b/x86/trampolines.S
index 9640c66..deb5057 100644
--- a/x86/trampolines.S
+++ b/x86/trampolines.S
@@ -1,6 +1,9 @@
-/* Common bootstrapping code to transition from 16-bit to 32-bit code. */
+/*
+ * Common bootstrapping code to transition from 16-bit to 32-bit code, and to
+ * transition from 32-bit to 64-bit code (x86-64 only)
+ */
 
-/* EFI provides it's own SIPI sequence to handle relocation. */
+ /* EFI provides it's own SIPI sequence to handle relocation. */
 #ifndef CONFIG_EFI
 .code16
 .globl rm_trampoline
@@ -28,3 +31,67 @@ ap_rm_gdt_descr:
 .globl rm_trampoline_end
 rm_trampoline_end:
 #endif
+
+/* The 32-bit => 64-bit trampoline is x86-64 only. */
+#ifdef __x86_64__
+.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
+#endif
-- 
2.36.1.476.g0c4daa206d-goog

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

[Sean Christopherson]

From: Varad Gautam <varad.gautam@suse.com>

Sending INIT/SIPI to APs from ap_init() resets them into 16-bit mode
to loop into 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>
Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 lib/x86/setup.c      |  2 +-
 lib/x86/smp.c        | 81 +++++++++++++++++++++++++++++++++++++++++++-
 lib/x86/smp.h        |  3 ++
 x86/efi/efistart64.S | 35 ++++++++++++++++++-
 x86/trampolines.S    | 38 +++++++++++++++++++--
 5 files changed, 153 insertions(+), 6 deletions(-)

diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index c7c0983..2d199b4 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -347,11 +347,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 2f554ce..022d627 100644
--- a/lib/x86/smp.c
+++ b/lib/x86/smp.c
@@ -24,10 +24,17 @@ static volatile bool ipi_wait;
 static int _cpu_count;
 static atomic_t active_cpus;
 extern u8 rm_trampoline, rm_trampoline_end;
-#ifdef __i386__
+#if defined(__i386__) || defined(CONFIG_EFI)
 extern u8 ap_rm_gdt_descr;
 #endif
 
+#ifdef CONFIG_EFI
+extern u8 ap_rm_gdt, ap_rm_gdt_end;
+extern u8 ap_start32;
+extern u32 smp_stacktop;
+extern u8 stacktop;
+#endif
+
 /* The BSP is online from time zero. */
 atomic_t cpu_online_count = { .counter = 1 };
 
@@ -163,6 +170,74 @@ static void setup_rm_gdt(void)
 	 * the vector code.
 	 */
 	sgdt(rm_gdt);
+#elif defined(CONFIG_EFI)
+	idt_entry_t *gate_descr;
+
+	/*
+	 * The realmode trampoline on EFI has the following layout:
+	 *
+	 * |rm_trampoline:
+	 * |sipi_entry:
+	 * |  <AP bootstrapping code called from SIPI>
+	 * |ap_rm_gdt:
+	 * |  <GDT used for 16-bit -> 32-bit trasition>
+	 * |ap_rm_gdt_descr:
+	 * |  <GDT descriptor for ap_rm_gdt>
+	 * |sipi_end:
+	 * |  <End of trampoline>
+	 * |rm_trampoline_end:
+	 *
+	 * After relocating to the lowmem address pointed to by realmode_trampoline,
+	 * the realmode GDT descriptor needs to contain the relocated address of
+	 * ap_rm_gdt.
+	 */
+	volatile struct descriptor_table_ptr *rm_gdt_descr =
+			(struct descriptor_table_ptr *) (&ap_rm_gdt_descr - &rm_trampoline);
+	rm_gdt_descr->base = (ulong) ((u32) (&ap_rm_gdt - &rm_trampoline));
+	rm_gdt_descr->limit = (u16) (&ap_rm_gdt_end - &ap_rm_gdt - 1);
+
+	/*
+	 * Since 1. compile time calculation of offsets is not allowed when
+	 * building with -shared, and 2. rip-relative addressing is not supported in
+	 * 16-bit mode, the relocated address of ap_rm_gdt_descr needs to be stored at
+	 * a location known to / accessible from the trampoline.
+	 *
+	 * Use the last two bytes of the trampoline page (REALMODE_GDT_LOWMEM) to store
+	 * a pointer to relocated ap_rm_gdt_descr addr. This way, the trampoline code can
+	 * find the relocated descriptor using the lowmem address at pa=REALMODE_GDT_LOWMEM,
+	 * and this relocated descriptor points to the relocated GDT.
+	 */
+	*((u16 *)(REALMODE_GDT_LOWMEM)) = (u16) (u64) rm_gdt_descr;
+
+	/*
+	 * Set up a call gate to the 32-bit entrypoint (ap_start32) within GDT, since
+	 * EFI may not load the 32-bit AP entrypoint (ap_start32) low enough
+	 * to be reachable from the SIPI vector.
+	 *
+	 * Since kvm-unit-tests builds with -shared, this location needs to be fetched
+	 * at runtime, and rip-relative addressing is not supported in 16-bit mode. This
+	 * prevents using a long jump to ap_start32 (`ljmpl $cs, $ap_start32`).
+	 *
+	 * As an alternative, a far return via `push $cs; push $label; lret` would require
+	 * an intermediate trampoline since $label must still be within 0 - 0xFFFF for
+	 * 16-bit far return to work.
+	 *
+	 * Using a call gate allows for an easier 16-bit -> 32-bit transition via `lcall`.
+	 *
+	 * GDT layout:
+	 *
+	 * Entry | Segment
+	 * 0	 | NULL descr
+	 * 1	 | Code segment descr
+	 * 2	 | Data segment descr
+	 * 3	 | Call gate descr
+	 *
+	 * This layout is only used for reaching 32-bit mode. APs load a 64-bit GDT
+	 * later during boot, which does not need to follow this layout.
+	 */
+	gate_descr = ((void *)(&ap_rm_gdt - &rm_trampoline) + 3 * sizeof(gdt_entry_t));
+	set_desc_entry(gate_descr, sizeof(gdt_entry_t), (void *) &ap_start32,
+		       0x8 /* sel */, 0xc /* type */, 0 /* dpl */);
 #endif
 }
 
@@ -184,6 +259,10 @@ void ap_init(void)
 
 	setup_rm_gdt();
 
+#ifdef CONFIG_EFI
+	smp_stacktop = ((u64) (&stacktop)) - PAGE_SIZE;
+#endif
+
 	/* INIT */
 	apic_icr_write(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_INIT | APIC_INT_ASSERT, 0);
 
diff --git a/lib/x86/smp.h b/lib/x86/smp.h
index 77f8a19..b805be5 100644
--- a/lib/x86/smp.h
+++ b/lib/x86/smp.h
@@ -6,6 +6,9 @@
 #include "libcflat.h"
 #include "atomic.h"
 
+/* Address where to store the address of realmode GDT descriptor. */
+#define REALMODE_GDT_LOWMEM (PAGE_SIZE - 2)
+
 /* Offsets into the per-cpu page. */
 struct percpu_data {
 	uint32_t  smp_id;
diff --git a/x86/efi/efistart64.S b/x86/efi/efistart64.S
index 3a4135e..a514612 100644
--- a/x86/efi/efistart64.S
+++ b/x86/efi/efistart64.S
@@ -12,6 +12,9 @@
 .globl stacktop
 stacktop:
 
+.globl smp_stacktop
+smp_stacktop:	.long 0
+
 .align PAGE_SIZE
 .globl ptl2
 ptl2:
@@ -33,16 +36,46 @@ ptl4:
 .text
 
 .code16
+REALMODE_GDT_LOWMEM = PAGE_SIZE - 2
 
 .globl rm_trampoline
 rm_trampoline:
 
 .globl sipi_entry
 sipi_entry:
-	jmp sipi_entry
+	mov %cr0, %eax
+	or $1, %eax
+	mov %eax, %cr0
+
+	/* Retrieve relocated ap_rm_gdt_descr address at REALMODE_GDT_LOWMEM. */
+	mov (REALMODE_GDT_LOWMEM), %ebx
+	lgdtl (%ebx)
+
+	lcall $0x18, $0x0
+
+.globl ap_rm_gdt
+ap_rm_gdt:
+	.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 ap_rm_gdt_end
+ap_rm_gdt_end:
+
+.globl ap_rm_gdt_descr
+ap_rm_gdt_descr:
+	.word 0
+	.long 0
 
 .globl sipi_end
 sipi_end:
 
 .globl rm_trampoline_end
 rm_trampoline_end:
+
+#include "../trampolines.S"
+
+.code64:
+
+ap_start64:
+	jmp ap_start64
diff --git a/x86/trampolines.S b/x86/trampolines.S
index deb5057..6a3df9c 100644
--- a/x86/trampolines.S
+++ b/x86/trampolines.S
@@ -36,6 +36,23 @@ rm_trampoline_end:
 #ifdef __x86_64__
 .code32
 
+/*
+ * EFI builds with "-shared -fPIC" and so cannot directly reference any absolute
+ * address.  In 64-bit mode, RIP-relative addressing neatly solves the problem,
+ * but 32-bit code doesn't have that luxury.  Make a dummy CALL to get RIP into
+ * a GPR in order to emulate RIP-relative for 32-bit transition code.
+ */
+.macro load_absolute_addr, addr, reg
+#ifdef CONFIG_EFI
+	call 1f
+1:
+	pop \reg
+	add \addr - 1b, \reg
+#else
+	mov \addr, \reg
+#endif
+.endm
+
 MSR_GS_BASE = 0xc0000101
 
 .macro setup_percpu_area
@@ -61,7 +78,9 @@ MSR_GS_BASE = 0xc0000101
 .endm
 
 prepare_64:
-	lgdt gdt_descr
+	load_absolute_addr $gdt_descr, %edx
+	lgdtl (%edx)
+
 	setup_segments
 
 	xor %eax, %eax
@@ -72,7 +91,12 @@ enter_long_mode:
 	bts $5, %eax  // pae
 	mov %eax, %cr4
 
+	/* Note, EFI doesn't yet support 5-level paging. */
+#ifdef CONFIG_EFI
+	load_absolute_addr $ptl4, %eax
+#else
 	mov pt_root, %eax
+#endif
 	mov %eax, %cr3
 
 efer = 0xc0000080
@@ -87,11 +111,19 @@ efer = 0xc0000080
 	mov %eax, %cr0
 	ret
 
+.globl ap_start32
 ap_start32:
 	setup_segments
+
+	load_absolute_addr $smp_stacktop, %edx
 	mov $-4096, %esp
-	lock xaddl %esp, smp_stacktop
+	lock xaddl %esp, (%edx)
+
 	setup_percpu_area
 	call prepare_64
-	ljmpl $8, $ap_start64
+
+	load_absolute_addr $ap_start64, %edx
+	pushl $0x08
+	pushl %edx
+	lretl
 #endif
-- 
2.36.1.476.g0c4daa206d-goog

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

[Sean Christopherson]

From: Varad Gautam <varad.gautam@suse.com>

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.

Cc: Andrew Jones <drjones@redhat.com>
Cc: Marc Orr <marcorr@google.com>
Cc: Zixuan Wang <zxwang42@gmail.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: David Rientjes <rientjes@google.com>
Cc: Thomas.Lendacky@amd.com
Cc: Joerg Roedel <jroedel@suse.de>
Cc: Borislav Petkov <bp@alien8.de>
Signed-off-by: Varad Gautam <varad.gautam@suse.com>
[sean: halt in AP wait loop, add comment]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 lib/x86/apic.c       |  2 --
 lib/x86/asm/setup.h  |  3 +++
 lib/x86/setup.c      | 53 +++++++++++++++++++++++++++++++++++---------
 lib/x86/smp.c        |  1 +
 lib/x86/smp.h        |  2 ++
 x86/cstart.S         |  3 ---
 x86/cstart64.S       | 26 ----------------------
 x86/efi/efistart64.S |  5 -----
 8 files changed, 48 insertions(+), 47 deletions(-)

diff --git a/lib/x86/apic.c b/lib/x86/apic.c
index 5d4c776..6fa857c 100644
--- a/lib/x86/apic.c
+++ b/lib/x86/apic.c
@@ -243,8 +243,6 @@ void mask_pic_interrupts(void)
 	outb(0xff, 0xa1);
 }
 
-extern unsigned char online_cpus[(MAX_TEST_CPUS + 7) / 8];
-
 void init_apic_map(void)
 {
 	unsigned int i, j = 0;
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 2d199b4..2f60a58 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -14,6 +14,9 @@
 #include "apic.h"
 #include "apic-defs.h"
 #include "asm/setup.h"
+#include "atomic.h"
+#include "processor.h"
+#include "smp.h"
 
 extern char edata;
 
@@ -195,7 +198,22 @@ static void setup_segments64(void)
 		     "1:"
 		     :: "r" ((u64)KERNEL_DS), "i" (KERNEL_CS));
 }
+#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
+	setup_segments64();
+#endif
+}
+
+#ifdef CONFIG_EFI
 static efi_status_t setup_memory_allocator(efi_bootinfo_t *efi_bootinfo)
 {
 	int i;
@@ -292,17 +310,6 @@ static void setup_page_table(void)
 	write_cr3((ulong)&ptl4);
 }
 
-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);
-
-	setup_segments64();
-}
-
 efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
 {
 	efi_status_t status;
@@ -349,6 +356,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();
@@ -371,3 +379,26 @@ void setup_libcflat(void)
 			add_setup_arg("bootloader");
 	}
 }
+
+void save_id(void)
+{
+	set_bit(apic_id(), online_cpus);
+}
+
+void ap_start64(void)
+{
+	setup_gdt_tss();
+	reset_apic();
+	load_idt();
+	save_id();
+	enable_apic();
+	enable_x2apic();
+	sti();
+	asm volatile ("nop");
+	printf("setup: AP %d online\n", apic_id());
+	atomic_inc(&cpu_online_count);
+
+	/* Only the BSP runs the test's main(), APs are given work via IPIs. */
+	for (;;)
+		asm volatile("hlt");
+}
diff --git a/lib/x86/smp.c b/lib/x86/smp.c
index 022d627..e056181 100644
--- a/lib/x86/smp.c
+++ b/lib/x86/smp.c
@@ -37,6 +37,7 @@ extern u8 stacktop;
 
 /* The BSP is online from time zero. */
 atomic_t cpu_online_count = { .counter = 1 };
+unsigned char online_cpus[(MAX_TEST_CPUS + 7) / 8];
 
 static __attribute__((used)) void ipi(void)
 {
diff --git a/lib/x86/smp.h b/lib/x86/smp.h
index b805be5..3a5ad1b 100644
--- a/lib/x86/smp.h
+++ b/lib/x86/smp.h
@@ -5,6 +5,7 @@
 #include <asm/spinlock.h>
 #include "libcflat.h"
 #include "atomic.h"
+#include "apic-defs.h"
 
 /* Address where to store the address of realmode GDT descriptor. */
 #define REALMODE_GDT_LOWMEM (PAGE_SIZE - 2)
@@ -86,5 +87,6 @@ void smp_reset_apic(void);
 void ap_init(void);
 
 extern atomic_t cpu_online_count;
+extern unsigned char online_cpus[(MAX_TEST_CPUS + 7) / 8];
 
 #endif
diff --git a/x86/cstart.S b/x86/cstart.S
index 20fcb64..0707985 100644
--- a/x86/cstart.S
+++ b/x86/cstart.S
@@ -124,7 +124,4 @@ start32:
 	push %eax
 	call exit
 
-online_cpus:
-	.fill (max_cpus + 7) / 8, 1, 0
-
 #include "trampolines.S"
diff --git a/x86/cstart64.S b/x86/cstart64.S
index 0a561ce..e44d46c 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -1,8 +1,6 @@
 
 #include "apic-defs.h"
 
-.globl online_cpus
-
 ipi_vector = 0x20
 
 max_cpus = MAX_TEST_CPUS
@@ -105,27 +103,6 @@ gdt32:
 gdt32_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:
-	call reset_apic
-	call load_idt
-	load_tss
-	call enable_apic
-	call save_id
-	call enable_x2apic
-	sti
-	nop
-	lock incw cpu_online_count
-
-1:	hlt
-	jmp 1b
-
 start64:
 	call reset_apic
 	call load_idt
@@ -164,6 +141,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 a514612..3fc1601 100644
--- a/x86/efi/efistart64.S
+++ b/x86/efi/efistart64.S
@@ -74,8 +74,3 @@ sipi_end:
 rm_trampoline_end:
 
 #include "../trampolines.S"
-
-.code64:
-
-ap_start64:
-	jmp ap_start64
-- 
2.36.1.476.g0c4daa206d-goog

[kvm-unit-tests PATCH v4 12/13] x86: Rename ap_init() to bringup_aps()

[Sean Christopherson]

Rename the helper that wakes and waits for APs to bringup_aps(), ap_init()
is terribly confusing because it's called from the BSP, not APs.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 lib/x86/setup.c | 2 +-
 lib/x86/smp.c   | 2 +-
 lib/x86/smp.h   | 2 +-
 x86/cstart.S    | 2 +-
 x86/cstart64.S  | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index 2f60a58..d9fd9e7 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -357,7 +357,7 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
 	setup_page_table();
 	enable_apic();
 	save_id();
-	ap_init();
+	bringup_aps();
 	enable_x2apic();
 	smp_init();
 
diff --git a/lib/x86/smp.c b/lib/x86/smp.c
index e056181..dd4eb8c 100644
--- a/lib/x86/smp.c
+++ b/lib/x86/smp.c
@@ -242,7 +242,7 @@ static void setup_rm_gdt(void)
 #endif
 }
 
-void ap_init(void)
+void bringup_aps(void)
 {
 	void *rm_trampoline_dst = RM_TRAMPOLINE_ADDR;
 	size_t rm_trampoline_size = (&rm_trampoline_end - &rm_trampoline) + 1;
diff --git a/lib/x86/smp.h b/lib/x86/smp.h
index 3a5ad1b..3ddc39e 100644
--- a/lib/x86/smp.h
+++ b/lib/x86/smp.h
@@ -84,7 +84,7 @@ 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);
+void bringup_aps(void);
 
 extern atomic_t cpu_online_count;
 extern unsigned char online_cpus[(MAX_TEST_CPUS + 7) / 8];
diff --git a/x86/cstart.S b/x86/cstart.S
index 0707985..fdbe343 100644
--- a/x86/cstart.S
+++ b/x86/cstart.S
@@ -114,7 +114,7 @@ start32:
 	call save_id
 	call mask_pic_interrupts
 	call enable_apic
-	call ap_init
+	call bringup_aps
 	call enable_x2apic
 	call smp_init
         push $__environ
diff --git a/x86/cstart64.S b/x86/cstart64.S
index e44d46c..80b3552 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -118,7 +118,7 @@ start64:
 	mov %rax, __args(%rip)
 	call __setup_args
 
-	call ap_init
+	call bringup_aps
 	call enable_x2apic
 	call smp_init
 
-- 
2.36.1.476.g0c4daa206d-goog

[kvm-unit-tests PATCH v4 13/13] x86: Add ap_online() to consolidate final "AP is alive!" code

[Sean Christopherson]

Add ap_online() to consolidate the last stage of onlining an AP CPU.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 lib/x86/setup.c |  9 +--------
 lib/x86/smp.c   | 12 ++++++++++++
 lib/x86/smp.h   |  1 +
 x86/cstart.S    |  8 +++-----
 4 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index d9fd9e7..65c0fbf 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -393,12 +393,5 @@ void ap_start64(void)
 	save_id();
 	enable_apic();
 	enable_x2apic();
-	sti();
-	asm volatile ("nop");
-	printf("setup: AP %d online\n", apic_id());
-	atomic_inc(&cpu_online_count);
-
-	/* Only the BSP runs the test's main(), APs are given work via IPIs. */
-	for (;;)
-		asm volatile("hlt");
+	ap_online();
 }
diff --git a/lib/x86/smp.c b/lib/x86/smp.c
index dd4eb8c..feaab7a 100644
--- a/lib/x86/smp.c
+++ b/lib/x86/smp.c
@@ -82,6 +82,18 @@ static void setup_smp_id(void *data)
 	this_cpu_write_smp_id(apic_id());
 }
 
+void ap_online(void)
+{
+	irq_enable();
+
+	printf("setup: CPU %d online\n", apic_id());
+	atomic_inc(&cpu_online_count);
+
+	/* Only the BSP runs the test's main(), APs are given work via IPIs. */
+	for (;;)
+		asm volatile("hlt");
+}
+
 static void __on_cpu(int cpu, void (*function)(void *data), void *data, int wait)
 {
 	const u32 ipi_icr = APIC_INT_ASSERT | APIC_DEST_PHYSICAL | APIC_DM_FIXED | IPI_VECTOR;
diff --git a/lib/x86/smp.h b/lib/x86/smp.h
index 3ddc39e..08a440b 100644
--- a/lib/x86/smp.h
+++ b/lib/x86/smp.h
@@ -85,6 +85,7 @@ 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 bringup_aps(void);
+void ap_online(void);
 
 extern atomic_t cpu_online_count;
 extern unsigned char online_cpus[(MAX_TEST_CPUS + 7) / 8];
diff --git a/x86/cstart.S b/x86/cstart.S
index fdbe343..e82bed7 100644
--- a/x86/cstart.S
+++ b/x86/cstart.S
@@ -101,12 +101,10 @@ ap_start32:
 	call save_id
 	call enable_apic
 	call enable_x2apic
-	sti
-	nop
-	lock incw cpu_online_count
+	call ap_online
 
-1:	hlt
-	jmp 1b
+	/* ap_online() should never return */
+	ud2
 
 start32:
 	setup_tr_and_percpu
-- 
2.36.1.476.g0c4daa206d-goog

Re: [kvm-unit-tests PATCH v4 00/13] x86: SMP Support for x86 UEFI Tests

[Vasant Karasulli]

Hi all,
    I ran the tests on these patches and I see 8/11 subtests of
vmware_backdoors.c test fail.

But this failure is because I couldn't figure out the check parameter
/sys/module/kvm/parameters/enable_vmware_backdoor on my test machine
specified in the unittests.cfg. So this test fails also without the patch.

Rest of the tests look fine.

Thanks,
Vasant

Re: [kvm-unit-tests PATCH v4 08/13] x86: efi: Provide percpu storage

[Vasant Karasulli]

Writing to MSR_IA32_APICBASE in reset_apic() is an
intercepted operation and causes #VC exception when the test is launched as
an SEV-ES guest.

So calling reset_apic() before IDT is set up in setup_idt() and
load_idt() might cause problems. Similarly if accessing _percpu_data
array element in setup_segments64() results in a page fault,
this will lead to a double fault.

Hence move reset_apic() call and percpu data setup after
setup_idt() and load_idt().
---
 lib/x86/setup.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index 7df0256..b14e692 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -192,8 +192,6 @@ static void setup_segments64(void)
 	write_gs(KERNEL_DS);
 	write_ss(KERNEL_DS);

-	/* Setup percpu base */
-	wrmsr(MSR_GS_BASE, (u64)&__percpu_data[pre_boot_apic_id()]);

 	/*
 	 * Update the code segment by putting it on the stack before the return
@@ -322,7 +320,7 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
 		}
 		return status;
 	}
-
+
 	status = setup_rsdp(efi_bootinfo);
 	if (status != EFI_SUCCESS) {
 		printf("Cannot find RSDP in EFI system table\n");
@@ -344,14 +342,15 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
 	}

 	setup_gdt_tss();
+	setup_segments64();
+	setup_idt();
+	load_idt();
 	/*
 	 * GS.base, which points at the per-vCPU data, must be configured prior
 	 * to resetting the APIC, which sets the per-vCPU APIC ops.
 	 */
-	setup_segments64();
+	wrmsr(MSR_GS_BASE, (u64)&__percpu_data[pre_boot_apic_id()]);
 	reset_apic();
-	setup_idt();
-	load_idt();
 	mask_pic_interrupts();
 	setup_page_table();
 	enable_apic();
--
2.34.1

Re: [kvm-unit-tests PATCH v4 08/13] x86: efi: Provide percpu storage

[Sean Christopherson]

Please send a standalone patch (with a SOB), this series has already been merged.

On Mon, Aug 22, 2022, Vasant Karasulli wrote:
> Writing to MSR_IA32_APICBASE in reset_apic() is an
> intercepted operation

Is _typically_ an intercepted operation, architecturally there's nothing that
requires APICBASE to emulated.  

> and causes #VC exception when the test is launched as
> an SEV-ES guest.
>
> So calling reset_apic() before IDT is set up in setup_idt() and
> load_idt() might cause problems.

> Similarly if accessing _percpu_data array element in setup_segments64() results
> in a page fault, this will lead to a double fault.

Well, yeah, but loading the IDT isn't going to magically fix the #PF.  I suspect
you're actually referring to the emulated MMIO #NPF=>#VC when accessing the xAPIC
through MMIO?

> Hence move reset_apic() call and percpu data setup after
> setup_idt() and load_idt().
> ---
>  lib/x86/setup.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/x86/setup.c b/lib/x86/setup.c
> index 7df0256..b14e692 100644
> --- a/lib/x86/setup.c
> +++ b/lib/x86/setup.c
> @@ -192,8 +192,6 @@ static void setup_segments64(void)
>  	write_gs(KERNEL_DS);
>  	write_ss(KERNEL_DS);
> 
> -	/* Setup percpu base */
> -	wrmsr(MSR_GS_BASE, (u64)&__percpu_data[pre_boot_apic_id()]);
> 
>  	/*
>  	 * Update the code segment by putting it on the stack before the return
> @@ -322,7 +320,7 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
>  		}
>  		return status;
>  	}
> -
> +
>  	status = setup_rsdp(efi_bootinfo);
>  	if (status != EFI_SUCCESS) {
>  		printf("Cannot find RSDP in EFI system table\n");
> @@ -344,14 +342,15 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
>  	}
> 
>  	setup_gdt_tss();
> +	setup_segments64();
> +	setup_idt();
> +	load_idt();
>  	/*
>  	 * GS.base, which points at the per-vCPU data, must be configured prior
>  	 * to resetting the APIC, which sets the per-vCPU APIC ops.
>  	 */
> -	setup_segments64();
> +	wrmsr(MSR_GS_BASE, (u64)&__percpu_data[pre_boot_apic_id()]);

This absolutely needs a comment, otherwise someone will wonder why on earth GS.base
isn't configured during setup_segments64().  Easist thing is probalby to split and
reword the above comment, e.g.

	/*
	 * Load GS.base with the per-vCPU data.  This must be done after loading
	 * the IDT as reading the APIC ID may #VC when running as an SEV-ES guest.
	 */
	wrmsr(MSR_GS_BASE, (u64)&__percpu_data[pre_boot_apic_id()]);

	/*
	 * Resetting the APIC sets the per-vCPU APIC ops and so must be done
	 * after loading GS.base with the per-vCPU data.
	 */
	reset_apic();	
	
>  	reset_apic();
> -	setup_idt();
> -	load_idt();
>  	mask_pic_interrupts();
>  	setup_page_table();
>  	enable_apic();
> --
> 2.34.1
> 

Re: [kvm-unit-tests PATCH v4 08/13] x86: efi: Provide percpu storage

[Vasant Karasulli]

On Mo 22-08-22 17:30:00, Sean Christopherson wrote:
> Please send a standalone patch (with a SOB), this series has already been merged.
>

Sorry, I will send a new patch.

> On Mon, Aug 22, 2022, Vasant Karasulli wrote:
> > Writing to MSR_IA32_APICBASE in reset_apic() is an
> > intercepted operation
>
> Is _typically_ an intercepted operation, architecturally there's nothing that
> requires APICBASE to emulated.

Right, I will improve the wording.

>
> > and causes #VC exception when the test is launched as
> > an SEV-ES guest.
> >
> > So calling reset_apic() before IDT is set up in setup_idt() and
> > load_idt() might cause problems.
>
> > Similarly if accessing _percpu_data array element in setup_segments64() results
> > in a page fault, this will lead to a double fault.
>
> Well, yeah, but loading the IDT isn't going to magically fix the #PF.  I suspect
> you're actually referring to the emulated MMIO #NPF=>#VC when accessing the xAPIC
> through MMIO?

Damn, I overlooked the pre_boot_apic_id() call, thought just accessing the array
element is the culprit. :(
>
> > Hence move reset_apic() call and percpu data setup after
> > setup_idt() and load_idt().
> > ---
> >  lib/x86/setup.c | 11 +++++------
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/lib/x86/setup.c b/lib/x86/setup.c
> > index 7df0256..b14e692 100644
> > --- a/lib/x86/setup.c
> > +++ b/lib/x86/setup.c
> > @@ -192,8 +192,6 @@ static void setup_segments64(void)
> >  	write_gs(KERNEL_DS);
> >  	write_ss(KERNEL_DS);
> >
> > -	/* Setup percpu base */
> > -	wrmsr(MSR_GS_BASE, (u64)&__percpu_data[pre_boot_apic_id()]);
> >
> >  	/*
> >  	 * Update the code segment by putting it on the stack before the return
> > @@ -322,7 +320,7 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
> >  		}
> >  		return status;
> >  	}
> > -
> > +
> >  	status = setup_rsdp(efi_bootinfo);
> >  	if (status != EFI_SUCCESS) {
> >  		printf("Cannot find RSDP in EFI system table\n");
> > @@ -344,14 +342,15 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
> >  	}
> >
> >  	setup_gdt_tss();
> > +	setup_segments64();
> > +	setup_idt();
> > +	load_idt();
> >  	/*
> >  	 * GS.base, which points at the per-vCPU data, must be configured prior
> >  	 * to resetting the APIC, which sets the per-vCPU APIC ops.
> >  	 */
> > -	setup_segments64();
> > +	wrmsr(MSR_GS_BASE, (u64)&__percpu_data[pre_boot_apic_id()]);
>
> This absolutely needs a comment, otherwise someone will wonder why on earth GS.base
> isn't configured during setup_segments64().  Easist thing is probalby to split and
> reword the above comment, e.g.

Will do that. Thanks for the suggestion and the review.
>
> 	/*
> 	 * Load GS.base with the per-vCPU data.  This must be done after loading
> 	 * the IDT as reading the APIC ID may #VC when running as an SEV-ES guest.
> 	 */
> 	wrmsr(MSR_GS_BASE, (u64)&__percpu_data[pre_boot_apic_id()]);
>
> 	/*
> 	 * Resetting the APIC sets the per-vCPU APIC ops and so must be done
> 	 * after loading GS.base with the per-vCPU data.
> 	 */
> 	reset_apic();
>
> >  	reset_apic();
> > -	setup_idt();
> > -	load_idt();
> >  	mask_pic_interrupts();
> >  	setup_page_table();
> >  	enable_apic();
> > --
> > 2.34.1
> >

Thanks,
Vasant Karasulli
Kernel generalist
www.suse.com<http://www.suse.com>
[https://www.suse.com/assets/img/social-platforms-suse-logo.png]<http://www.suse.com/>
SUSE - Open Source Solutions for Enterprise Servers & Cloud<http://www.suse.com/>
Modernize your infrastructure with SUSE Linux Enterprise servers, cloud technology for IaaS, and SUSE's software-defined storage.
www.suse.com