search for: verify_cpu

On Tue, Jul 14, 2020 at 02:09:12PM +0200, Joerg Roedel wrote: > From: Joerg Roedel <jroedel at suse.de> > > The APs are not ready to handle exceptions when verify_cpu() is called > in secondary_startup_64. Eek, no. MSR_IA32_MISC_ENABLE_XD_DISABLE needs to be cleared very early during CPU startup; this can't just be skipped. Also, is UNWIND_HINT_EMPTY needed for the new target? -Kees > > Signed-off-by: Joerg Roedel <jroedel at suse.de> &gt...

On Tue, Jul 14, 2020 at 02:09:12PM +0200, Joerg Roedel wrote: > From: Joerg Roedel <jroedel at suse.de> > > The APs are not ready to handle exceptions when verify_cpu() is called > in secondary_startup_64. Eek, no. MSR_IA32_MISC_ENABLE_XD_DISABLE needs to be cleared very early during CPU startup; this can't just be skipped. Also, is UNWIND_HINT_EMPTY needed for the new target? -Kees > > Signed-off-by: Joerg Roedel <jroedel at suse.de> &gt...

From: Joerg Roedel <jroedel at suse.de> The APs are not ready to handle exceptions when verify_cpu() is called in secondary_startup_64. Signed-off-by: Joerg Roedel <jroedel at suse.de> --- arch/x86/include/asm/realmode.h | 1 + arch/x86/kernel/head_64.S | 12 ++++++++++++ arch/x86/realmode/init.c | 6 ++++++ 3 files changed, 19 insertions(+) diff --git a/arch/x86/include/...

...> accessed during boot on those VMs. Oh, hrm, yes, that's true. If other x86 maintainers are comfortable with this, then okay. My sense is that changing the early CPU startup paths will cause trouble down the line. > The alternative is to set up exception handling prior to calling > verify_cpu, including segments, stack and IDT. Given that verify_cpu() > does not add much value to SEV-ES guests, I'd like to avoid adding this > complexity. So, going back to the requirements here ... what things in verify_cpu() can cause exceptions? AFAICT, cpuid is safely handled (i.e. it is de...

...> accessed during boot on those VMs. Oh, hrm, yes, that's true. If other x86 maintainers are comfortable with this, then okay. My sense is that changing the early CPU startup paths will cause trouble down the line. > The alternative is to set up exception handling prior to calling > verify_cpu, including segments, stack and IDT. Given that verify_cpu() > does not add much value to SEV-ES guests, I'd like to avoid adding this > complexity. So, going back to the requirements here ... what things in verify_cpu() can cause exceptions? AFAICT, cpuid is safely handled (i.e. it is de...

From: Joerg Roedel <jroedel at suse.de> The APs are not ready to handle exceptions when verify_cpu() is called in secondary_startup_64. Signed-off-by: Joerg Roedel <jroedel at suse.de> --- arch/x86/include/asm/realmode.h | 1 + arch/x86/kernel/head_64.S | 1 + arch/x86/realmode/init.c | 6 ++++++ 3 files changed, 8 insertions(+) diff --git a/arch/x86/include/asm/realmode.h...

...; exceptions. Aaah. I see. Thanks for the details there. So ... can you add a bunch more comments about why/when the new entry path is being used? I really don't want to accidentally discover some unrelated refactoring down the road (in months, years, unrelated to SEV, etc) starts to also skip verify_cpu() on Intel systems. There had been a lot of BIOSes that set this MSR to disable NX, and I don't want to repeat that pain: Linux must never start an Intel CPU with that MSR set. :P -- Kees Cook

...> during CPU startup; this can't just be skipped. That MSR is Intel-only, right? The boot-path installed here is only used for SEV-ES guests, running on AMD systems, so this MSR is not even accessed during boot on those VMs. The alternative is to set up exception handling prior to calling verify_cpu, including segments, stack and IDT. Given that verify_cpu() does not add much value to SEV-ES guests, I'd like to avoid adding this complexity. > Also, is UNWIND_HINT_EMPTY needed for the new target? Yes, I think it is, will add it in the next version. Regards, Joerg

...ption must be avoided until the kernel is ready to handle them. With the current patches this happens when the AP runs in 64bit long-mode and loaded TSS and IDT. Therefore a slightly different AP boot-path is needed for SEV-ES guests. > So, going back to the requirements here ... what things in verify_cpu() > can cause exceptions? AFAICT, cpuid is safely handled (i.e. it is > detected and only run in a way to avoid exceptions and the MSR > reads/writes are similarly bound by CPU family/id range checks). I must > be missing something. :) It is actually the CPUID instructions that cause #...

...+++++++++++++ 1 file changed, 26 insertions(+) diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S index 4bbc770af632..5a3cde971cb7 100644 --- a/arch/x86/kernel/head_64.S +++ b/arch/x86/kernel/head_64.S @@ -72,6 +72,20 @@ SYM_CODE_START_NOALIGN(startup_64) /* Set up the stack for verify_cpu(), similar to initial_stack below */ leaq (__end_init_task - SIZEOF_PTREGS)(%rip), %rsp + /* Setup boot GDT descriptor and load boot GDT */ + leaq boot_gdt(%rip), %rax + movq %rax, boot_gdt_base(%rip) + lgdt boot_gdt_descr(%rip) + + /* GDT loaded - switch to __KERNEL_CS so IRET works reliably *...

...+++++++++++++ 1 file changed, 36 insertions(+) diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S index 4bbc770af632..11a28c1fb51f 100644 --- a/arch/x86/kernel/head_64.S +++ b/arch/x86/kernel/head_64.S @@ -72,6 +72,26 @@ SYM_CODE_START_NOALIGN(startup_64) /* Set up the stack for verify_cpu(), similar to initial_stack below */ leaq (__end_init_task - SIZEOF_PTREGS)(%rip), %rsp + /* Setup boot GDT descriptor and load boot GDT */ + leaq boot_gdt(%rip), %rax + movq %rax, boot_gdt_base(%rip) + lgdt boot_gdt_descr(%rip) + + /* New GDT is live - reload data segment registers */ + movl $...

On 2/11/20 5:52 AM, Joerg Roedel wrote: > Implement a handler for #VC exceptions caused by RDMSR/WRMSR > instructions. As a general comment on all of these event handlers: Why do we bother having the hypercalls in the interrupt handler as opposed to just calling them directly. What you have is: wrmsr() -> #VC exception hcall() But we could make our rd/wrmsr() wrappers just do:

On 2/11/20 5:52 AM, Joerg Roedel wrote: > Implement a handler for #VC exceptions caused by RDMSR/WRMSR > instructions. As a general comment on all of these event handlers: Why do we bother having the hypercalls in the interrupt handler as opposed to just calling them directly. What you have is: wrmsr() -> #VC exception hcall() But we could make our rd/wrmsr() wrappers just do:

...fixup_pointer(void *ptr, unsigned long physaddr) diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S index 4bbc770af632..62513dd1e0e4 100644 --- a/arch/x86/kernel/head_64.S +++ b/arch/x86/kernel/head_64.S @@ -72,6 +72,26 @@ SYM_CODE_START_NOALIGN(startup_64) /* Set up the stack for verify_cpu(), similar to initial_stack below */ leaq (__end_init_task - SIZEOF_PTREGS)(%rip), %rsp + /* Setup boot GDT descriptor and load boot GDT */ + leaq boot_gdt(%rip), %rax + movq %rax, boot_gdt_descr+2(%rip) + lgdt boot_gdt_descr(%rip) + + /* New GDT is live - reload data segment registers */ + mov...

...is on the list for future optimizations (besides > caching CPUID results). My idea is to use alternatives patching for > this. But the exception handling is needed anyway because #VC > exceptions happen very early already, basically the first thing after > setting up a stack is calling verify_cpu(), which uses CPUID. Ahh, bummer. How does a guest know that it's running under SEV-ES? What's the enumeration mechanism if CPUID doesn't "work"? > The other reason is that things like MMIO and IOIO instructions can't be > easily patched by alternatives. Those wou...

On Tue, Apr 28, 2020 at 05:16:41PM +0200, Joerg Roedel wrote: > @@ -480,6 +500,22 @@ SYM_DATA_LOCAL(early_gdt_descr_base, .quad INIT_PER_CPU_VAR(gdt_page)) > SYM_DATA(phys_base, .quad 0x0) > EXPORT_SYMBOL(phys_base) > > +/* Boot GDT used when kernel addresses are not mapped yet */ > +SYM_DATA_LOCAL(boot_gdt_descr, .word boot_gdt_end - boot_gdt) >

On Tue, Apr 28, 2020 at 05:16:41PM +0200, Joerg Roedel wrote: > @@ -480,6 +500,22 @@ SYM_DATA_LOCAL(early_gdt_descr_base, .quad INIT_PER_CPU_VAR(gdt_page)) > SYM_DATA(phys_base, .quad 0x0) > EXPORT_SYMBOL(phys_base) > > +/* Boot GDT used when kernel addresses are not mapped yet */ > +SYM_DATA_LOCAL(boot_gdt_descr, .word boot_gdt_end - boot_gdt) >

...000000001000071: 0f 22 e1 mov %rcx,%cr4 0x0000000001000074: 48 03 05 95 ff 20 01 add 0x120ff95(%rip),%rax # 0x2210010 the unconditional JMP is there and it hasn't been patched out yet. If you really need to test CPUID flags, you need to do something similar to what verify_cpu does that early. And looking at that thing: * verify_cpu, returns the status of longmode and SSE in register %eax. * 0: Success 1: Failure you could return the FSGSBASE CPUID bit there too and act accordingly. Hmm. -- Regards/Gruss, Boris. https://people.kernel.org/...

...000000001000071: 0f 22 e1 mov %rcx,%cr4 0x0000000001000074: 48 03 05 95 ff 20 01 add 0x120ff95(%rip),%rax # 0x2210010 the unconditional JMP is there and it hasn't been patched out yet. If you really need to test CPUID flags, you need to do something similar to what verify_cpu does that early. And looking at that thing: * verify_cpu, returns the status of longmode and SSE in register %eax. * 0: Success 1: Failure you could return the FSGSBASE CPUID bit there too and act accordingly. Hmm. -- Regards/Gruss, Boris. https://people.kernel.org/...

From: Joerg Roedel <jroedel at suse.de> Setup an early handler for #VC exceptions. There is no GHCB mapped yet, so just re-use the vc_no_ghcb_handler. It can only handle CPUID exit-codes, but that should be enough to get the kernel through verify_cpu() and __startup_64() until it runs on virtual addresses. Signed-off-by: Joerg Roedel <jroedel at suse.de> Link: https://lore.kernel.org/r/20200724160336.5435-42-joro at 8bytes.org --- arch/x86/include/asm/setup.h | 1 + arch/x86/include/asm/sev-es.h | 3 +++ arch/x86/kernel/head64.c...