lkml.org 
[lkml]   [2024]   [Feb]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[tip: x86/boot] x86/boot/64: Simplify global variable accesses in GDT/IDT programming
    The following commit has been merged into the x86/boot branch of tip:

    Commit-ID: 5da793671957e8e99fa74423fab2737bf8c772a8
    Gitweb: https://git.kernel.org/tip/5da793671957e8e99fa74423fab2737bf8c772a8
    Author: Ard Biesheuvel <ardb@kernel.org>
    AuthorDate: Wed, 21 Feb 2024 12:35:08 +01:00
    Committer: Ingo Molnar <mingo@kernel.org>
    CommitterDate: Mon, 26 Feb 2024 12:58:11 +01:00

    x86/boot/64: Simplify global variable accesses in GDT/IDT programming

    There are two code paths in the startup code to program an IDT: one that
    runs from the 1:1 mapping and one that runs from the virtual kernel
    mapping. Currently, these are strictly separate because fixup_pointer()
    is used on the 1:1 path, which will produce the wrong value when used
    while executing from the virtual kernel mapping.

    Switch to RIP_REL_REF() so that the two code paths can be merged. Also,
    move the GDT and IDT descriptors to the stack so that they can be
    referenced directly, rather than via RIP_REL_REF().

    Rename startup_64_setup_env() to startup_64_setup_gdt_idt() while at it,
    to make the call from assembler self-documenting.

    Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
    Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
    Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
    Signed-off-by: Ingo Molnar <mingo@kernel.org>
    Link: https://lore.kernel.org/r/20240221113506.2565718-19-ardb+git@google.com
    ---
    arch/x86/include/asm/setup.h | 2 +-
    arch/x86/kernel/head64.c | 75 ++++++++++++++---------------------
    arch/x86/kernel/head_64.S | 4 +--
    3 files changed, 32 insertions(+), 49 deletions(-)

    diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
    index 5c83729..e61e68d 100644
    --- a/arch/x86/include/asm/setup.h
    +++ b/arch/x86/include/asm/setup.h
    @@ -48,7 +48,7 @@ extern unsigned long saved_video_mode;
    extern void reserve_standard_io_resources(void);
    extern void i386_reserve_resources(void);
    extern unsigned long __startup_64(unsigned long physaddr, struct boot_params *bp);
    -extern void startup_64_setup_env(unsigned long physbase);
    +extern void startup_64_setup_gdt_idt(void);
    extern void early_setup_idt(void);
    extern void __init do_early_exception(struct pt_regs *regs, int trapnr);

    diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
    index dc09560..1d6865e 100644
    --- a/arch/x86/kernel/head64.c
    +++ b/arch/x86/kernel/head64.c
    @@ -22,6 +22,7 @@
    #include <linux/cc_platform.h>
    #include <linux/pgtable.h>

    +#include <asm/asm.h>
    #include <asm/processor.h>
    #include <asm/proto.h>
    #include <asm/smp.h>
    @@ -76,15 +77,6 @@ static struct desc_struct startup_gdt[GDT_ENTRIES] __initdata = {
    [GDT_ENTRY_KERNEL_DS] = GDT_ENTRY_INIT(DESC_DATA64, 0, 0xfffff),
    };

    -/*
    - * Address needs to be set at runtime because it references the startup_gdt
    - * while the kernel still uses a direct mapping.
    - */
    -static struct desc_ptr startup_gdt_descr __initdata = {
    - .size = sizeof(startup_gdt)-1,
    - .address = 0,
    -};
    -
    static void __head *fixup_pointer(void *ptr, unsigned long physaddr)
    {
    return ptr - (void *)_text + (void *)physaddr;
    @@ -569,62 +561,52 @@ void __init __noreturn x86_64_start_reservations(char *real_mode_data)
    */
    static gate_desc bringup_idt_table[NUM_EXCEPTION_VECTORS] __page_aligned_data;

    -static struct desc_ptr bringup_idt_descr = {
    - .size = (NUM_EXCEPTION_VECTORS * sizeof(gate_desc)) - 1,
    - .address = 0, /* Set at runtime */
    -};
    -
    -static void set_bringup_idt_handler(gate_desc *idt, int n, void *handler)
    +/* This may run while still in the direct mapping */
    +static void __head startup_64_load_idt(void *vc_handler)
    {
    -#ifdef CONFIG_AMD_MEM_ENCRYPT
    + struct desc_ptr desc = {
    + .address = (unsigned long)&RIP_REL_REF(bringup_idt_table),
    + .size = sizeof(bringup_idt_table) - 1,
    + };
    struct idt_data data;
    - gate_desc desc;
    -
    - init_idt_data(&data, n, handler);
    - idt_init_desc(&desc, &data);
    - native_write_idt_entry(idt, n, &desc);
    -#endif
    -}
    + gate_desc idt_desc;

    -/* This runs while still in the direct mapping */
    -static void __head startup_64_load_idt(unsigned long physbase)
    -{
    - struct desc_ptr *desc = fixup_pointer(&bringup_idt_descr, physbase);
    - gate_desc *idt = fixup_pointer(bringup_idt_table, physbase);
    -
    -
    - if (IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT)) {
    - void *handler;
    -
    - /* VMM Communication Exception */
    - handler = fixup_pointer(vc_no_ghcb, physbase);
    - set_bringup_idt_handler(idt, X86_TRAP_VC, handler);
    + /* @vc_handler is set only for a VMM Communication Exception */
    + if (vc_handler) {
    + init_idt_data(&data, X86_TRAP_VC, vc_handler);
    + idt_init_desc(&idt_desc, &data);
    + native_write_idt_entry((gate_desc *)desc.address, X86_TRAP_VC, &idt_desc);
    }

    - desc->address = (unsigned long)idt;
    - native_load_idt(desc);
    + native_load_idt(&desc);
    }

    /* This is used when running on kernel addresses */
    void early_setup_idt(void)
    {
    - /* VMM Communication Exception */
    + void *handler = NULL;
    +
    if (IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT)) {
    setup_ghcb();
    - set_bringup_idt_handler(bringup_idt_table, X86_TRAP_VC, vc_boot_ghcb);
    + handler = vc_boot_ghcb;
    }

    - bringup_idt_descr.address = (unsigned long)bringup_idt_table;
    - native_load_idt(&bringup_idt_descr);
    + startup_64_load_idt(handler);
    }

    /*
    * Setup boot CPU state needed before kernel switches to virtual addresses.
    */
    -void __head startup_64_setup_env(unsigned long physbase)
    +void __head startup_64_setup_gdt_idt(void)
    {
    + void *handler = NULL;
    +
    + struct desc_ptr startup_gdt_descr = {
    + .address = (unsigned long)&RIP_REL_REF(startup_gdt),
    + .size = sizeof(startup_gdt) - 1,
    + };
    +
    /* Load GDT */
    - startup_gdt_descr.address = (unsigned long)fixup_pointer(startup_gdt, physbase);
    native_load_gdt(&startup_gdt_descr);

    /* New GDT is live - reload data segment registers */
    @@ -632,5 +614,8 @@ void __head startup_64_setup_env(unsigned long physbase)
    "movl %%eax, %%ss\n"
    "movl %%eax, %%es\n" : : "a"(__KERNEL_DS) : "memory");

    - startup_64_load_idt(physbase);
    + if (IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT))
    + handler = &RIP_REL_REF(vc_no_ghcb);
    +
    + startup_64_load_idt(handler);
    }
    diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
    index bfbac50..e09cf0b 100644
    --- a/arch/x86/kernel/head_64.S
    +++ b/arch/x86/kernel/head_64.S
    @@ -68,8 +68,6 @@ SYM_CODE_START_NOALIGN(startup_64)
    /* Set up the stack for verify_cpu() */
    leaq (__end_init_task - PTREGS_SIZE)(%rip), %rsp

    - leaq _text(%rip), %rdi
    -
    /* Setup GSBASE to allow stack canary access for C code */
    movl $MSR_GS_BASE, %ecx
    leaq INIT_PER_CPU_VAR(fixed_percpu_data)(%rip), %rdx
    @@ -77,7 +75,7 @@ SYM_CODE_START_NOALIGN(startup_64)
    shrq $32, %rdx
    wrmsr

    - call startup_64_setup_env
    + call startup_64_setup_gdt_idt

    /* Now switch to __KERNEL_CS so IRET works reliably */
    pushq $__KERNEL_CS
    \
     
     \ /
      Last update: 2024-05-27 15:21    [W:2.880 / U:0.484 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site