[kvm-unit-tests PATCH v3 00/11] SMP Support for x86 UEFI Tests

[kvm-unit-tests PATCH v3 00/11] 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.

Changes in v3:
- Unbreak i386 build, ingest seanjc's reviews from v2.

Git branch: https://github.com/varadgautam/kvm-unit-tests/commits/ap-boot-v3/
v2: https://lore.kernel.org/kvm/20220412173407.13637-1-varad.gautam@suse.com/

Varad Gautam (11):
  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: 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: 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            |  39 +++++++++--
 lib/x86/desc.h            |   3 +
 lib/x86/setup.c           |  84 ++++++++++++++++++++----
 lib/x86/smp.c             | 132 +++++++++++++++++++++++++++++++++++++-
 lib/x86/smp.h             |  10 +++
 x86/cstart.S              |  38 +----------
 x86/cstart64.S            | 120 +---------------------------------
 x86/efi/crt0-efi-x86_64.S |   3 +
 x86/efi/efistart64.S      |  81 ++++++++++++++---------
 x86/start16.S             |  27 ++++++++
 x86/start32.S             | 102 +++++++++++++++++++++++++++++
 x86/svm_tests.c           |  10 ++-
 15 files changed, 443 insertions(+), 214 deletions(-)
 create mode 100644 x86/start16.S
 create mode 100644 x86/start32.S

-- 
2.32.0

[kvm-unit-tests PATCH v3 01/11] x86: Share realmode trampoline between i386 and x86_64

[Varad Gautam]

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 start16.S to
be shared by both.

Signed-off-by: Varad Gautam <varad.gautam@suse.com>
---
 x86/cstart.S   | 20 ++++----------------
 x86/cstart64.S | 18 +++---------------
 x86/start16.S  | 27 +++++++++++++++++++++++++++
 3 files changed, 34 insertions(+), 31 deletions(-)
 create mode 100644 x86/start16.S

diff --git a/x86/cstart.S b/x86/cstart.S
index 6db6a38..06b5be6 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 "start16.S"
diff --git a/x86/cstart64.S b/x86/cstart64.S
index 7272452..cae6f51 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -156,19 +156,7 @@ 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:
+#include "start16.S"
 
 .code32
 ap_start32:
@@ -243,9 +231,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/start16.S b/x86/start16.S
new file mode 100644
index 0000000..e61b22a
--- /dev/null
+++ b/x86/start16.S
@@ -0,0 +1,27 @@
+/* Common 16-bit bootstrapping code. */
+
+.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:
-- 
2.32.0

[kvm-unit-tests PATCH v3 02/11] 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/alloc_page.h     |  3 ++
 lib/x86/setup.c      |  1 +
 lib/x86/smp.c        | 68 ++++++++++++++++++++++++++++++++++++++++++--
 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, 94 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..2c28fb4 100644
--- a/lib/x86/smp.c
+++ b/lib/x86/smp.c
@@ -6,6 +6,8 @@
 #include "apic.h"
 #include "fwcfg.h"
 #include "desc.h"
+#include "alloc_page.h"
+#include "asm/page.h"
 
 #define IPI_VECTOR 0x20
 
@@ -18,6 +20,11 @@ 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
+atomic_t cpu_online_count = { .counter = 1 };
 
 static __attribute__((used)) void ipi(void)
 {
@@ -114,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);
@@ -142,3 +147,62 @@ 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)) {
+		;
+	}
+}
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 06b5be6..b5a3a0f 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 "start16.S"
diff --git a/x86/cstart64.S b/x86/cstart64.S
index cae6f51..26c3039 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
 
@@ -228,21 +227,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 6a9b03b..c8f52cd 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.32.0

Re: [kvm-unit-tests PATCH v3 02/11] x86: Move ap_init() to smp.c

[Sean Christopherson]

On Tue, Apr 26, 2022, Varad Gautam wrote:
> +	printf("smp: waiting for %d APs\n", _cpu_count - 1);
> +	while (_cpu_count != atomic_read(&cpu_online_count)) {

Curly braces aren't needed.  And to make this more robust, I think it makes sense
to do cpu_relax() in the loop so that it's more obvious that this is busy waiting.

Re: [kvm-unit-tests PATCH v3 02/11] x86: Move ap_init() to smp.c

[Sean Christopherson]

On Tue, Apr 26, 2022, Varad Gautam wrote:
> +	printf("smp: waiting for %d APs\n", _cpu_count - 1);

Oof, this breaks run_test.sh / runtime.bash.  runtime.bash has a godawful hack
to detect that dummy.efi ran cleanly; it looks for "enabling apic" as the last
line to detect success.  I'll add a patch to fix this by having dummy.c print an
explicit magic string, e.g. Dummy Hello World!.

Re: [kvm-unit-tests PATCH v3 02/11] x86: Move ap_init() to smp.c

[Andrew Jones]

On Wed, Jun 15, 2022 at 09:31:07PM +0000, Sean Christopherson wrote:
> On Tue, Apr 26, 2022, Varad Gautam wrote:
> > +	printf("smp: waiting for %d APs\n", _cpu_count - 1);
> 
> Oof, this breaks run_test.sh / runtime.bash.  runtime.bash has a godawful hack
> to detect that dummy.efi ran cleanly; it looks for "enabling apic" as the last
> line to detect success.  I'll add a patch to fix this by having dummy.c print an
> explicit magic string, e.g. Dummy Hello World!.
>

powerpc and s390x always exit with

 printf("\nEXIT: STATUS=%d\n", ((code) << 1) | 1);

It's because they can only exit with exit code zero, but maybe it's worth
adopting the same line and format for EFI tests?

Thanks,
drew

Re: [kvm-unit-tests PATCH v3 02/11] x86: Move ap_init() to smp.c

[Sean Christopherson]

On Thu, Jun 16, 2022, Andrew Jones wrote:
> On Wed, Jun 15, 2022 at 09:31:07PM +0000, Sean Christopherson wrote:
> > On Tue, Apr 26, 2022, Varad Gautam wrote:
> > > +	printf("smp: waiting for %d APs\n", _cpu_count - 1);
> > 
> > Oof, this breaks run_test.sh / runtime.bash.  runtime.bash has a godawful hack
> > to detect that dummy.efi ran cleanly; it looks for "enabling apic" as the last
> > line to detect success.  I'll add a patch to fix this by having dummy.c print an
> > explicit magic string, e.g. Dummy Hello World!.
> >
> 
> powerpc and s390x always exit with
> 
>  printf("\nEXIT: STATUS=%d\n", ((code) << 1) | 1);
> 
> It's because they can only exit with exit code zero, but maybe it's worth
> adopting the same line and format for EFI tests?

Honestly, I'd prefer using truly magic string in dummy.c for the probing code so
that it's super obvious that there's meaning in the string.

[kvm-unit-tests PATCH v3 03/11] 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/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 b5a3a0f..65782be 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 26c3039..9465099 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -65,7 +65,6 @@ MSR_GS_BASE = 0xc0000101
 .endm
 
 .macro load_tss
-	lidtq idt_descr
 	movq %rsp, %rdi
 	call setup_tss
 	ltr %ax
@@ -176,6 +175,7 @@ save_id:
 
 ap_start64:
 	call reset_apic
+	call load_idt
 	load_tss
 	call enable_apic
 	call save_id
@@ -189,6 +189,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.32.0

[kvm-unit-tests PATCH v3 04/11] 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_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>
---
 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 087e85c..a3e7255 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -57,22 +57,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_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..d743504 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_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.32.0

[kvm-unit-tests PATCH v3 05/11] 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 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>
---
 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 a3e7255..bb0b7b2 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -362,6 +362,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 d743504..7ac505a 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 dd2b916..367c13f 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.32.0

[kvm-unit-tests PATCH v3 06/11] x86: efi: Provide a stack within testcase memory

[Varad Gautam]

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>
---
 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.32.0

[kvm-unit-tests PATCH v3 07/11] x86: efi: Provide percpu storage

[Varad Gautam]

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>
---
 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 367c13f..c34a8bb 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 2c28fb4..90f6210 100644
--- a/lib/x86/smp.c
+++ b/lib/x86/smp.c
@@ -179,25 +179,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.32.0

[kvm-unit-tests PATCH v3 08/11] 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 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>
---
 lib/x86/smp.c        | 73 +++++++++++++++++++++++++++++++++++++++++++-
 lib/x86/smp.h        |  3 ++
 x86/efi/efistart64.S | 30 +++++++++++++++++-
 3 files changed, 104 insertions(+), 2 deletions(-)

diff --git a/lib/x86/smp.c b/lib/x86/smp.c
index 90f6210..360ae82 100644
--- a/lib/x86/smp.c
+++ b/lib/x86/smp.c
@@ -21,9 +21,13 @@ 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;
+#endif
 atomic_t cpu_online_count = { .counter = 1 };
 
 static __attribute__((used)) void ipi(void)
@@ -158,6 +162,73 @@ static void setup_rm_gdt(void)
 	 * the vector code.
 	 */
 	sgdt(rm_gdt);
+#elif defined(CONFIG_EFI)
+	/*
+	 * 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.
+	 */
+	idt_entry_t *gate_descr = (idt_entry_t *) ((u8 *)(&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
 }
 
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..c6b1a5b 100644
--- a/x86/efi/efistart64.S
+++ b/x86/efi/efistart64.S
@@ -33,16 +33,44 @@ 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:
+
+.code32
+.globl ap_start32
+ap_start32:
+	jmp ap_start32
-- 
2.32.0

[kvm-unit-tests PATCH v3 09/11] 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 9465099..3c8d78f 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -55,35 +55,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:
@@ -117,33 +95,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
@@ -156,15 +107,6 @@ gdt32_end:
 
 #include "start16.S"
 
-.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

Re: [kvm-unit-tests PATCH v3 09/11] x86: Move 32-bit bringup routines to start32.S

[Sean Christopherson]

On Tue, Apr 26, 2022, Varad Gautam wrote:
> 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 ++++++++++++++++++++++++++++++++++++++++++++++++++

start32.S is not a good name.  Yes, it's 32-bit code, but used only for 64-bit
boot.  Looking back, start16.S isn't that great either because it's not _the_
16-bit start sequence (EFI has it's own).

If we go with trampolines.S, then the intent is relatively clear and we can shove
both 16=>32 and 32=>64 trampolines in there.

[kvm-unit-tests PATCH v3 10/11] 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.

This brings the APs into 64-bit mode to loop at a known location.

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

diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index c34a8bb..7ca0fab 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 360ae82..8e5c6e8 100644
--- a/lib/x86/smp.c
+++ b/lib/x86/smp.c
@@ -27,6 +27,8 @@ extern u8 ap_rm_gdt_descr;
 #ifdef CONFIG_EFI
 extern u8 ap_rm_gdt, ap_rm_gdt_end;
 extern u8 ap_start32;
+extern u32 smp_stacktop;
+extern u8 stacktop;
 #endif
 atomic_t cpu_online_count = { .counter = 1 };
 
@@ -250,6 +252,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/x86/efi/efistart64.S b/x86/efi/efistart64.S
index c6b1a5b..d82e3fc 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:
@@ -71,6 +74,10 @@ sipi_end:
 rm_trampoline_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

Re: [kvm-unit-tests PATCH v3 10/11] x86: efi, smp: Transition APs from 32-bit to 64-bit mode

[Sean Christopherson]

On Tue, Apr 26, 2022, Varad Gautam wrote:
> 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:

Use "1:" for the label, and 1f / 1b, that way it's obvious it's a relatively transient
thing.

> +	pop %edx
> +	add $gdt_descr - prepare_64_1, %edx
> +#else
> +	mov $gdt_descr, %edx
> +#endif

Rather than have #ifdefs everyway, add a macro to hide the differences, e.g.
load_absolute_addr.

[kvm-unit-tests PATCH v3 11/11] 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/apic.c       |  2 --
 lib/x86/asm/setup.h  |  3 +++
 lib/x86/setup.c      | 52 ++++++++++++++++++++++++++++++++++----------
 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, 47 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 7ca0fab..0066e67 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,25 @@ 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);
+	while (1) {
+		;
+	}
+}
diff --git a/lib/x86/smp.c b/lib/x86/smp.c
index 8e5c6e8..17a5ade 100644
--- a/lib/x86/smp.c
+++ b/lib/x86/smp.c
@@ -31,6 +31,7 @@ extern u32 smp_stacktop;
 extern u8 stacktop;
 #endif
 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 65782be..ed18184 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 "start16.S"
diff --git a/x86/cstart64.S b/x86/cstart64.S
index 3c8d78f..53db843 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
@@ -108,27 +106,6 @@ gdt32_end:
 #include "start16.S"
 
 .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
@@ -167,6 +144,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 d82e3fc..5635204 100644
--- a/x86/efi/efistart64.S
+++ b/x86/efi/efistart64.S
@@ -76,8 +76,3 @@ rm_trampoline_end:
 .code32
 
 #include "../start32.S"
-
-.code64:
-
-ap_start64:
-	jmp ap_start64
-- 
2.32.0

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

[Sean Christopherson]

On Tue, Apr 26, 2022, Varad Gautam wrote:
> +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);
> +	while (1) {

Unnecessary curly braces.  And rather then spin, let's do "hlt", that way everything
from the sti onwards can be moved to a common helper, e.g. ap_online().

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

[Sean Christopherson]

On Tue, Apr 26, 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.
> 
> Changes in v3:
> - Unbreak i386 build, ingest seanjc's reviews from v2.

I have more comments for this version, but if it's ok with you, I'll do the fixup
myself and send v4.  I'd like to my opinionated renames, e.g. ap_init() => bringup_aps(),
as part of this series instead of having to wait for this to get merged.