Re: [kvm-unit-tests PATCH v2 6/6] x86 UEFI: Convert x86 test cases to PIC

Krish Sadhukhan
2021-08-24T22:13:03Z

On 8/19/21 4:34 AM, Varad Gautam wrote:
> From: Zixuan Wang <zixuanwang@google.com>
>
> UEFI loads EFI applications to dynamic runtime addresses, so it requires
> all applications to be compiled as PIC (position independent code). PIC
> does not allow the usage of compile time absolute address.
>
> This commit converts multiple x86 test cases to PIC so they can compile
> and run in UEFI:
>
> - x86/cet.efi
>
> - x86/emulator.c: x86/emulator.c depends on lib/x86/usermode.c. But
> usermode.c contains non-PIC inline assembly code and thus blocks the
> compilation with GNU-EFI. This commit converts lib/x86/usermode.c and
> x86/emulator.c to PIC, so x86/emulator.c can compile and run in UEFI.
>
> - x86/vmware_backdoors.c: it depends on lib/x86/usermode.c and now works
> without modifications
>
> - x86/eventinj.c
>
> - x86/smap.c
>
> - x86/access.c
>
> - x86/umip.c

I am wondering if these changes can be part of patch# 1 instead of being
a separate patch.

>
> Signed-off-by: Zixuan Wang <zixuanwang@google.com>
> ---
> lib/x86/usermode.c | 3 ++-
> x86/Makefile.common | 7 ++++---
> x86/Makefile.x86_64 | 5 +++--
> x86/access.c | 6 +++---
> x86/cet.c | 8 +++++---
> x86/emulator.c | 5 +++--
> x86/eventinj.c | 6 ++++--
> x86/smap.c | 8 ++++----
> x86/umip.c | 10 +++++++---
> 9 files changed, 35 insertions(+), 23 deletions(-)
>
> diff --git a/lib/x86/usermode.c b/lib/x86/usermode.c
> index f032523..c550545 100644
> --- a/lib/x86/usermode.c
> +++ b/lib/x86/usermode.c
> @@ -58,7 +58,8 @@ uint64_t run_in_user(usermode_func func, unsigned int fault_vector,
> "pushq %[user_stack_top]\n\t"
> "pushfq\n\t"
> "pushq %[user_cs]\n\t"
> - "pushq $user_mode\n\t"
> + "lea user_mode(%%rip), %%rdx\n\t"
> + "pushq %%rdx\n\t"
> "iretq\n"
>
> "user_mode:\n\t"
> diff --git a/x86/Makefile.common b/x86/Makefile.common
> index ca33e8e..a91fd4c 100644
> --- a/x86/Makefile.common
> +++ b/x86/Makefile.common
> @@ -61,8 +61,8 @@ FLATLIBS = lib/libcflat.a
> -j .reloc -j .init --target efi-app-x86_64 $*.so $@
> @chmod a-x $@
>
> -tests-flatonly = $(TEST_DIR)/realmode.$(out) $(TEST_DIR)/eventinj.$(out) \
> - $(TEST_DIR)/smap.$(out) $(TEST_DIR)/umip.$(out)
> +tests-flatonly = $(TEST_DIR)/realmode.$(out) \
> + $(TEST_DIR)/smap.$(out)
>
> tests-common = $(TEST_DIR)/vmexit.$(out) $(TEST_DIR)/tsc.$(out) \
> $(TEST_DIR)/smptest.$(out) $(TEST_DIR)/msr.$(out) \
> @@ -72,7 +72,8 @@ tests-common = $(TEST_DIR)/vmexit.$(out) $(TEST_DIR)/tsc.$(out) \
> $(TEST_DIR)/tsc_adjust.$(out) $(TEST_DIR)/asyncpf.$(out) \
> $(TEST_DIR)/init.$(out) $(TEST_DIR)/hyperv_synic.$(out) \
> $(TEST_DIR)/hyperv_stimer.$(out) $(TEST_DIR)/hyperv_connections.$(out) \
> - $(TEST_DIR)/tsx-ctrl.$(out)
> + $(TEST_DIR)/tsx-ctrl.$(out) \
> + $(TEST_DIR)/eventinj.$(out) $(TEST_DIR)/umip.$(out)
>
> ifneq ($(CONFIG_EFI),y)
> tests-common += $(tests-flatonly)
> diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64
> index f6c7bd7..e8843aa 100644
> --- a/x86/Makefile.x86_64
> +++ b/x86/Makefile.x86_64
> @@ -18,9 +18,8 @@ cflatobjs += lib/x86/intel-iommu.o
> cflatobjs += lib/x86/usermode.o
>
> # Tests that have relocation / PIC problems and need more attention for EFI.
> -tests_flatonly = $(TEST_DIR)/access.$(out) $(TEST_DIR)/emulator.$(out) \
> +tests_flatonly = $(TEST_DIR)/access.$(out) \
> $(TEST_DIR)/svm.$(out) $(TEST_DIR)/vmx.$(out) \
> - $(TEST_DIR)/vmware_backdoors.$(out)
>
> tests = $(TEST_DIR)/apic.$(out) $(TEST_DIR)/idt_test.$(out) \
> $(TEST_DIR)/xsave.$(out) $(TEST_DIR)/rmap_chain.$(out) \
> @@ -33,6 +32,8 @@ tests += $(TEST_DIR)/intel-iommu.$(out)
> tests += $(TEST_DIR)/rdpru.$(out)
> tests += $(TEST_DIR)/pks.$(out)
> tests += $(TEST_DIR)/pmu_lbr.$(out)
> +tests += $(TEST_DIR)/emulator.$(out)
> +tests += $(TEST_DIR)/vmware_backdoors.$(out)
>
> ifneq ($(fcf_protection_full),)
> tests_flatonly += $(TEST_DIR)/cet.$(out)
> diff --git a/x86/access.c b/x86/access.c
> index 4725bbd..d0c84ca 100644
> --- a/x86/access.c
> +++ b/x86/access.c
> @@ -700,7 +700,7 @@ static int ac_test_do_access(ac_test_t *at)
>
> if (F(AC_ACCESS_TWICE)) {
> asm volatile (
> - "mov $fixed2, %%rsi \n\t"
> + "lea fixed2(%%rip), %%rsi \n\t"
> "mov (%[addr]), %[reg] \n\t"
> "fixed2:"
> : [reg]"=r"(r), [fault]"=a"(fault), "=b"(e)
> @@ -710,7 +710,7 @@ static int ac_test_do_access(ac_test_t *at)
> fault = 0;
> }
>
> - asm volatile ("mov $fixed1, %%rsi \n\t"
> + asm volatile ("lea fixed1(%%rip), %%rsi \n\t"
> "mov %%rsp, %%rdx \n\t"
> "cmp $0, %[user] \n\t"
> "jz do_access \n\t"
> @@ -719,7 +719,7 @@ static int ac_test_do_access(ac_test_t *at)
> "pushq %[user_stack_top] \n\t"
> "pushfq \n\t"
> "pushq %[user_cs] \n\t"
> - "pushq $do_access \n\t"
> + "lea do_access(%%rip), %%rsi; pushq %%rsi; lea fixed1(%%rip), %%rsi\n\t"
> "iretq \n"
> "do_access: \n\t"
> "cmp $0, %[fetch] \n\t"
> diff --git a/x86/cet.c b/x86/cet.c
> index a21577a..a4b79cb 100644
> --- a/x86/cet.c
> +++ b/x86/cet.c
> @@ -52,7 +52,7 @@ static u64 cet_ibt_func(void)
> printf("No endbr64 instruction at jmp target, this triggers #CP...\n");
> asm volatile ("movq $2, %rcx\n"
> "dec %rcx\n"
> - "leaq 2f, %rax\n"
> + "leaq 2f(%rip), %rax\n"
> "jmp *%rax \n"
> "2:\n"
> "dec %rcx\n");
> @@ -67,7 +67,8 @@ void test_func(void) {
> "pushq %[user_stack_top]\n\t"
> "pushfq\n\t"
> "pushq %[user_cs]\n\t"
> - "pushq $user_mode\n\t"
> + "lea user_mode(%%rip), %%rax\n\t"
> + "pushq %%rax\n\t"
> "iretq\n"
>
> "user_mode:\n\t"
> @@ -77,7 +78,8 @@ void test_func(void) {
> [user_ds]"i"(USER_DS),
> [user_cs]"i"(USER_CS),
> [user_stack_top]"r"(user_stack +
> - sizeof(user_stack)));
> + sizeof(user_stack))
> + : "rax");
> }
>
> #define SAVE_REGS() \
> diff --git a/x86/emulator.c b/x86/emulator.c
> index 9fda1a0..4d2de24 100644
> --- a/x86/emulator.c
> +++ b/x86/emulator.c
> @@ -262,12 +262,13 @@ static void test_pop(void *mem)
>
> asm volatile("mov %%rsp, %[tmp] \n\t"
> "mov %[stack_top], %%rsp \n\t"
> - "push $1f \n\t"
> + "lea 1f(%%rip), %%rax \n\t"
> + "push %%rax \n\t"
> "ret \n\t"
> "2: jmp 2b \n\t"
> "1: mov %[tmp], %%rsp"
> : [tmp]"=&r"(tmp) : [stack_top]"r"(stack_top)
> - : "memory");
> + : "memory", "rax");
> report(1, "ret");
>
> stack_top[-1] = 0x778899;
> diff --git a/x86/eventinj.c b/x86/eventinj.c
> index 46593c9..0cd68e8 100644
> --- a/x86/eventinj.c
> +++ b/x86/eventinj.c
> @@ -155,9 +155,11 @@ asm("do_iret:"
> "pushf"W" \n\t"
> "mov %cs, %ecx \n\t"
> "push"W" %"R "cx \n\t"
> - "push"W" $2f \n\t"
> + "lea"W" 2f(%"R "ip), %"R "bx \n\t"
> + "push"W" %"R "bx \n\t"
>
> - "cmpb $0, no_test_device\n\t" // see if need to flush
> + "mov no_test_device(%"R "ip), %bl \n\t"
> + "cmpb $0, %bl\n\t" // see if need to flush
> "jnz 1f\n\t"
> "outl %eax, $0xe4 \n\t" // flush page
> "1: \n\t"
> diff --git a/x86/smap.c b/x86/smap.c
> index ac2c8d5..b3ee16f 100644
> --- a/x86/smap.c
> +++ b/x86/smap.c
> @@ -161,10 +161,10 @@ int main(int ac, char **av)
> test = -1;
> asm("or $(" xstr(USER_BASE) "), %"R "sp \n"
> "push $44 \n "
> - "decl test\n"
> + "decl test(%"R "ip)\n"
> "and $~(" xstr(USER_BASE) "), %"R "sp \n"
> "pop %"R "ax\n"
> - "movl %eax, test");
> + "movl %eax, test(%"R "ip)");
> report(pf_count == 0 && test == 44,
> "write to user stack with AC=1");
>
> @@ -173,10 +173,10 @@ int main(int ac, char **av)
> test = -1;
> asm("or $(" xstr(USER_BASE) "), %"R "sp \n"
> "push $45 \n "
> - "decl test\n"
> + "decl test(%"R "ip)\n"
> "and $~(" xstr(USER_BASE) "), %"R "sp \n"
> "pop %"R "ax\n"
> - "movl %eax, test");
> + "movl %eax, test(%"R "ip)");
> report(pf_count == 1 && test == 45 && save == -1,
> "write to user stack with AC=0");
>
> diff --git a/x86/umip.c b/x86/umip.c
> index c5700b3..8b4e798 100644
> --- a/x86/umip.c
> +++ b/x86/umip.c
> @@ -23,7 +23,10 @@ static void gp_handler(struct ex_regs *regs)
>
> #define GP_ASM(stmt, in, clobber) \
> asm volatile ( \
> - "mov" W " $1f, %[expected_rip]\n\t" \
> + "push" W " %%" R "ax\n\t" \
> + "lea 1f(%%" R "ip), %%" R "ax\n\t" \
> + "mov %%" R "ax, %[expected_rip]\n\t" \
> + "pop" W " %%" R "ax\n\t" \
> "movl $2f-1f, %[skip_count]\n\t" \
> "1: " stmt "\n\t" \
> "2: " \
> @@ -130,7 +133,8 @@ static int do_ring3(void (*fn)(const char *), const char *arg)
> "push" W " %%" R "dx \n\t"
> "pushf" W "\n\t"
> "push" W " %[user_cs] \n\t"
> - "push" W " $1f \n\t"
> + "lea 1f(%%" R "ip), %%" R "dx \n\t"
> + "push" W " %%" R "dx \n\t"
> "iret" W "\n"
> "1: \n\t"
> "push %%" R "cx\n\t" /* save kernel SP */
> @@ -144,7 +148,7 @@ static int do_ring3(void (*fn)(const char *), const char *arg)
> #endif
>
> "pop %%" R "cx\n\t"
> - "mov $1f, %%" R "dx\n\t"
> + "lea 1f(%%" R "ip), %%" R "dx\n\t"
> "int %[kernel_entry_vector]\n\t"
> ".section .text.entry \n\t"
> "kernel_entry: \n\t"