x86/hyperv: Use __naked attribute to fix stackless C function

From: Ard Biesheuvel <[email protected]>

hv_crash_c_entry() is a C function that is entered without a stack, and this is only allowed for functions that have the __naked attribute, which informs the compiler that it must not emit the usual prologue and epilogue or emit any other kind of instrumentation that relies on a stack frame.

So split up the function, and set the __naked attribute on the initial part that sets up the stack, GDT, IDT and other pieces that are needed for ordinary C execution. Given that function calls are not permitted either, use the existing long return coded in an asm() block to call the second part of the function, which is an ordinary function that is permitted to call other functions as usual.

Fixes 94212d34618c ("x86/hyperv: Implement hypervisor RAM collection into vmcore")

Signed-off-by Ard Biesheuvel <[email protected]>

---
Build tested only.

Cc Mukesh Rathor <[email protected]>

Cc "K. Y. Srinivasan" <[email protected]>

Cc Haiyang Zhang <[email protected]>

Cc Wei Liu <[email protected]>

Cc Dexuan Cui <[email protected]>

Cc Long Li <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Dave Hansen <[email protected]>

Cc "H. Peter Anvin" <[email protected]>

Cc Uros Bizjak <[email protected]>

Cc [email protected]

 arch/x86/hyperv/hv_crash.c | 80 ++++++++++----------
 1 file changed, 42 insertions(+), 38 deletions(-)

arch/x86/hyperv/hv_crash.c +37 -30

index a78e4fed5720..d77766e8d37e 100644
@@ -107,14 +107,12 @@ static void __noreturn hv_panic_timeout_reboot(void)
                cpu_relax();
 }
 
-/* This cannot be inlined as it needs stack */
-static noinline __noclone void hv_crash_restore_tss(void)
+static void hv_crash_restore_tss(void)
 {
        load_TR_desc();
 }
 
-/* This cannot be inlined as it needs stack */
-static noinline void hv_crash_clear_kernpt(void)
+static void hv_crash_clear_kernpt(void)
 {
        pgd_t *pgd;
        p4d_t *p4d;

@@ -125,6 +123,25 @@ static noinline void hv_crash_clear_kernpt(void)
        native_p4d_clear(p4d);
 }
 
+
+static void __noreturn hv_crash_handle(void)
+{
+       hv_crash_restore_tss();
+       hv_crash_clear_kernpt();
+
+       /* we are now fully in devirtualized normal kernel mode */
+       __crash_kexec(NULL);
+
+       hv_panic_timeout_reboot();
+}
+
+/*
+ * __naked functions do not permit function calls, not even to __always_inline
+ * functions that only contain asm() blocks themselves. So use a macro instead.
+ */
+#define hv_wrmsr(msr, val) \
+       asm("wrmsr" :: "c"(msr), "a"((u32)val), "d"((u32)(val >> 32)) : "memory")
+
 /*
  * This is the C entry point from the asm glue code after the disable hypercall.
  * We enter here in IA32-e long mode, ie, full 64bit mode running on kernel

@@ -133,49 +150,36 @@ static noinline void hv_crash_clear_kernpt(void)
  * available. We restore kernel GDT, and rest of the context, and continue
  * to kexec.
  */
-static asmlinkage void __noreturn hv_crash_c_entry(void)
+static void __naked hv_crash_c_entry(void)
 {
-       struct hv_crash_ctxt *ctxt = &hv_crash_ctxt;
-
        /* first thing, restore kernel gdt */
-       native_load_gdt(&ctxt->gdtr);
+       asm volatile("lgdt %0" : : "m" (hv_crash_ctxt.gdtr));
 
-       asm volatile("movw %%ax, %%ss" : : "a"(ctxt->ss));
-       asm volatile("movq %0, %%rsp" : : "m"(ctxt->rsp));
+       asm volatile("movw %%ax, %%ss" : : "a"(hv_crash_ctxt.ss));
+       asm volatile("movq %0, %%rsp" : : "m"(hv_crash_ctxt.rsp));
 
-       asm volatile("movw %%ax, %%ds" : : "a"(ctxt->ds));
-       asm volatile("movw %%ax, %%es" : : "a"(ctxt->es));
-       asm volatile("movw %%ax, %%fs" : : "a"(ctxt->fs));
-       asm volatile("movw %%ax, %%gs" : : "a"(ctxt->gs));
+       asm volatile("movw %%ax, %%ds" : : "a"(hv_crash_ctxt.ds));
+       asm volatile("movw %%ax, %%es" : : "a"(hv_crash_ctxt.es));
+       asm volatile("movw %%ax, %%fs" : : "a"(hv_crash_ctxt.fs));
+       asm volatile("movw %%ax, %%gs" : : "a"(hv_crash_ctxt.gs));
 
-       native_wrmsrq(MSR_IA32_CR_PAT, ctxt->pat);
-       asm volatile("movq %0, %%cr0" : : "r"(ctxt->cr0));
+       hv_wrmsr(MSR_IA32_CR_PAT, hv_crash_ctxt.pat);
+       asm volatile("movq %0, %%cr0" : : "r"(hv_crash_ctxt.cr0));
 
-       asm volatile("movq %0, %%cr8" : : "r"(ctxt->cr8));
-       asm volatile("movq %0, %%cr4" : : "r"(ctxt->cr4));
-       asm volatile("movq %0, %%cr2" : : "r"(ctxt->cr4));
+       asm volatile("movq %0, %%cr8" : : "r"(hv_crash_ctxt.cr8));
+       asm volatile("movq %0, %%cr4" : : "r"(hv_crash_ctxt.cr4));
+       asm volatile("movq %0, %%cr2" : : "r"(hv_crash_ctxt.cr4));
 
-       native_load_idt(&ctxt->idtr);
-       native_wrmsrq(MSR_GS_BASE, ctxt->gsbase);
-       native_wrmsrq(MSR_EFER, ctxt->efer);
+       asm volatile("lidt %0" : : "m" (hv_crash_ctxt.idtr));
+       hv_wrmsr(MSR_GS_BASE, hv_crash_ctxt.gsbase);
+       hv_wrmsr(MSR_EFER, hv_crash_ctxt.efer);
 
        /* restore the original kernel CS now via far return */
-       asm volatile("movzwq %0, %%rax\n\t"
-                    "pushq %%rax\n\t"
-                    "pushq $1f\n\t"
-                    "lretq\n\t"
-                    "1:nop\n\t" : : "m"(ctxt->cs) : "rax");
-
-       /* We are in asmlinkage without stack frame, hence make C function
-        * calls which will buy stack frames.

Quoted 144 lines · Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>

hv_crash_c_entry() is a C function that is entered without a stack, and this is only allowed for functions that have the __naked attribute, which informs the compiler that it must not emit the usual prologue and epilogue or emit any other kind of instrumentation that relies on a stack frame.

So split up the function, and set the __naked attribute on the initial part that sets up the stack, GDT, IDT and other pieces that are needed for ordinary C execution. Given that function calls are not permitted either, use the existing long return coded in an asm() block to call the second part of the function, which is an ordinary function that is permitted to call other functions as usual.

Fixes 94212d34618c ("x86/hyperv: Implement hypervisor RAM collection into vmcore")

Signed-off-by Ard Biesheuvel <[email protected]>

---
Build tested only.

Cc Mukesh Rathor <[email protected]>

Cc "K. Y. Srinivasan" <[email protected]>

Cc Haiyang Zhang <[email protected]>

Cc Wei Liu <[email protected]>

Cc Dexuan Cui <[email protected]>

Cc Long Li <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Dave Hansen <[email protected]>

Cc "H. Peter Anvin" <[email protected]>

Cc Uros Bizjak <[email protected]>

Cc [email protected]

 arch/x86/hyperv/hv_crash.c | 80 ++++++++++----------
 1 file changed, 42 insertions(+), 38 deletions(-)

arch/x86/hyperv/hv_crash.c +37 -34

index a78e4fed5720..d77766e8d37e 100644
@@ -107,14 +107,12 @@ static void __noreturn hv_panic_timeout_reboot(void)
                cpu_relax();
 }
 
-/* This cannot be inlined as it needs stack */
-static noinline __noclone void hv_crash_restore_tss(void)
+static void hv_crash_restore_tss(void)
 {
        load_TR_desc();
 }
 
-/* This cannot be inlined as it needs stack */
-static noinline void hv_crash_clear_kernpt(void)
+static void hv_crash_clear_kernpt(void)
 {
        pgd_t *pgd;
        p4d_t *p4d;

@@ -125,6 +123,25 @@ static noinline void hv_crash_clear_kernpt(void)
        native_p4d_clear(p4d);
 }
 
+
+static void __noreturn hv_crash_handle(void)
+{
+       hv_crash_restore_tss();
+       hv_crash_clear_kernpt();
+
+       /* we are now fully in devirtualized normal kernel mode */
+       __crash_kexec(NULL);
+
+       hv_panic_timeout_reboot();
+}
+
+/*
+ * __naked functions do not permit function calls, not even to __always_inline
+ * functions that only contain asm() blocks themselves. So use a macro instead.
+ */
+#define hv_wrmsr(msr, val) \
+       asm("wrmsr" :: "c"(msr), "a"((u32)val), "d"((u32)(val >> 32)) : "memory")
+
 /*
  * This is the C entry point from the asm glue code after the disable hypercall.
  * We enter here in IA32-e long mode, ie, full 64bit mode running on kernel

@@ -133,49 +150,36 @@ static noinline void hv_crash_clear_kernpt(void)
  * available. We restore kernel GDT, and rest of the context, and continue
  * to kexec.
  */
-static asmlinkage void __noreturn hv_crash_c_entry(void)
+static void __naked hv_crash_c_entry(void)
 {
-       struct hv_crash_ctxt *ctxt = &hv_crash_ctxt;
-
        /* first thing, restore kernel gdt */
-       native_load_gdt(&ctxt->gdtr);
+       asm volatile("lgdt %0" : : "m" (hv_crash_ctxt.gdtr));
 
-       asm volatile("movw %%ax, %%ss" : : "a"(ctxt->ss));
-       asm volatile("movq %0, %%rsp" : : "m"(ctxt->rsp));
+       asm volatile("movw %%ax, %%ss" : : "a"(hv_crash_ctxt.ss));
+       asm volatile("movq %0, %%rsp" : : "m"(hv_crash_ctxt.rsp));
 
-       asm volatile("movw %%ax, %%ds" : : "a"(ctxt->ds));
-       asm volatile("movw %%ax, %%es" : : "a"(ctxt->es));
-       asm volatile("movw %%ax, %%fs" : : "a"(ctxt->fs));
-       asm volatile("movw %%ax, %%gs" : : "a"(ctxt->gs));
+       asm volatile("movw %%ax, %%ds" : : "a"(hv_crash_ctxt.ds));
+       asm volatile("movw %%ax, %%es" : : "a"(hv_crash_ctxt.es));
+       asm volatile("movw %%ax, %%fs" : : "a"(hv_crash_ctxt.fs));
+       asm volatile("movw %%ax, %%gs" : : "a"(hv_crash_ctxt.gs));
 
-       native_wrmsrq(MSR_IA32_CR_PAT, ctxt->pat);
-       asm volatile("movq %0, %%cr0" : : "r"(ctxt->cr0));
+       hv_wrmsr(MSR_IA32_CR_PAT, hv_crash_ctxt.pat);
+       asm volatile("movq %0, %%cr0" : : "r"(hv_crash_ctxt.cr0));
 
-       asm volatile("movq %0, %%cr8" : : "r"(ctxt->cr8));
-       asm volatile("movq %0, %%cr4" : : "r"(ctxt->cr4));
-       asm volatile("movq %0, %%cr2" : : "r"(ctxt->cr4));
+       asm volatile("movq %0, %%cr8" : : "r"(hv_crash_ctxt.cr8));
+       asm volatile("movq %0, %%cr4" : : "r"(hv_crash_ctxt.cr4));
+       asm volatile("movq %0, %%cr2" : : "r"(hv_crash_ctxt.cr4));
 
-       native_load_idt(&ctxt->idtr);
-       native_wrmsrq(MSR_GS_BASE, ctxt->gsbase);
-       native_wrmsrq(MSR_EFER, ctxt->efer);
+       asm volatile("lidt %0" : : "m" (hv_crash_ctxt.idtr));
+       hv_wrmsr(MSR_GS_BASE, hv_crash_ctxt.gsbase);
+       hv_wrmsr(MSR_EFER, hv_crash_ctxt.efer);
 
        /* restore the original kernel CS now via far return */
-       asm volatile("movzwq %0, %%rax\n\t"
-                    "pushq %%rax\n\t"
-                    "pushq $1f\n\t"
-                    "lretq\n\t"
-                    "1:nop\n\t" : : "m"(ctxt->cs) : "rax");
-
-       /* We are in asmlinkage without stack frame, hence make C function
-        * calls which will buy stack frames.
-        */
-       hv_crash_restore_tss();
-       hv_crash_clear_kernpt();
-

arch/x86/hyperv/hv_crash.c static noinline void hv_crash_clear_kernpt(void) · line 179

Quoted 6 lines

arch/x86/hyperv/hv_crash.c +2 -4

-       /* we are now fully in devirtualized normal kernel mode */
-       __crash_kexec(NULL);
-
-       hv_panic_timeout_reboot();
+       asm volatile("pushq     %q0             \n\t"
+                    "leaq      %c1(%%rip), %q0 \n\t"

You can use %a1 instead of %c1(%%rip).

arch/x86/hyperv/hv_crash.c static noinline void hv_crash_clear_kernpt(void) · line 181

Quoted 2 lines

arch/x86/hyperv/hv_crash.c +2 -0

+                    "pushq     %q0             \n\t"
+                    "lretq                     \n\t"

No need for terminating \n\t after the last insn in the asm template.

arch/x86/hyperv/hv_crash.c static noinline void hv_crash_clear_kernpt(void) · line 182

Quoted 1 line

arch/x86/hyperv/hv_crash.c +1 -0

+                    :: "a"(hv_crash_ctxt.cs), "i"(hv_crash_handle));

Pedantically, you need ': "+a"(...) : "i"(...)' here.

Hi Uros,

Quoted 145 lines · Uros Bizjak

Quoted 144 lines · Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>

hv_crash_c_entry() is a C function that is entered without a stack, and this is only allowed for functions that have the __naked attribute, which informs the compiler that it must not emit the usual prologue and epilogue or emit any other kind of instrumentation that relies on a stack frame.

So split up the function, and set the __naked attribute on the initial part that sets up the stack, GDT, IDT and other pieces that are needed for ordinary C execution. Given that function calls are not permitted either, use the existing long return coded in an asm() block to call the second part of the function, which is an ordinary function that is permitted to call other functions as usual.

Fixes 94212d34618c ("x86/hyperv: Implement hypervisor RAM collection into vmcore")

Signed-off-by Ard Biesheuvel <[email protected]>

---
Build tested only.

Cc Mukesh Rathor <[email protected]>

Cc "K. Y. Srinivasan" <[email protected]>

Cc Haiyang Zhang <[email protected]>

Cc Wei Liu <[email protected]>

Cc Dexuan Cui <[email protected]>

Cc Long Li <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Dave Hansen <[email protected]>

Cc "H. Peter Anvin" <[email protected]>

Cc Uros Bizjak <[email protected]>

Cc [email protected]

 arch/x86/hyperv/hv_crash.c | 80 ++++++++++----------
 1 file changed, 42 insertions(+), 38 deletions(-)

arch/x86/hyperv/hv_crash.c +37 -34

index a78e4fed5720..d77766e8d37e 100644
@@ -107,14 +107,12 @@ static void __noreturn hv_panic_timeout_reboot(void)
                cpu_relax();
 }
 
-/* This cannot be inlined as it needs stack */
-static noinline __noclone void hv_crash_restore_tss(void)
+static void hv_crash_restore_tss(void)
 {
        load_TR_desc();
 }
 
-/* This cannot be inlined as it needs stack */
-static noinline void hv_crash_clear_kernpt(void)
+static void hv_crash_clear_kernpt(void)
 {
        pgd_t *pgd;
        p4d_t *p4d;

@@ -125,6 +123,25 @@ static noinline void hv_crash_clear_kernpt(void)
        native_p4d_clear(p4d);
 }
 
+
+static void __noreturn hv_crash_handle(void)
+{
+       hv_crash_restore_tss();
+       hv_crash_clear_kernpt();
+
+       /* we are now fully in devirtualized normal kernel mode */
+       __crash_kexec(NULL);
+
+       hv_panic_timeout_reboot();
+}
+
+/*
+ * __naked functions do not permit function calls, not even to __always_inline
+ * functions that only contain asm() blocks themselves. So use a macro instead.
+ */
+#define hv_wrmsr(msr, val) \
+       asm("wrmsr" :: "c"(msr), "a"((u32)val), "d"((u32)(val >> 32)) : "memory")
+
 /*
  * This is the C entry point from the asm glue code after the disable hypercall.
  * We enter here in IA32-e long mode, ie, full 64bit mode running on kernel

@@ -133,49 +150,36 @@ static noinline void hv_crash_clear_kernpt(void)
  * available. We restore kernel GDT, and rest of the context, and continue
  * to kexec.
  */
-static asmlinkage void __noreturn hv_crash_c_entry(void)
+static void __naked hv_crash_c_entry(void)
 {
-       struct hv_crash_ctxt *ctxt = &hv_crash_ctxt;
-
        /* first thing, restore kernel gdt */
-       native_load_gdt(&ctxt->gdtr);
+       asm volatile("lgdt %0" : : "m" (hv_crash_ctxt.gdtr));
 
-       asm volatile("movw %%ax, %%ss" : : "a"(ctxt->ss));
-       asm volatile("movq %0, %%rsp" : : "m"(ctxt->rsp));
+       asm volatile("movw %%ax, %%ss" : : "a"(hv_crash_ctxt.ss));
+       asm volatile("movq %0, %%rsp" : : "m"(hv_crash_ctxt.rsp));
 
-       asm volatile("movw %%ax, %%ds" : : "a"(ctxt->ds));
-       asm volatile("movw %%ax, %%es" : : "a"(ctxt->es));
-       asm volatile("movw %%ax, %%fs" : : "a"(ctxt->fs));
-       asm volatile("movw %%ax, %%gs" : : "a"(ctxt->gs));
+       asm volatile("movw %%ax, %%ds" : : "a"(hv_crash_ctxt.ds));
+       asm volatile("movw %%ax, %%es" : : "a"(hv_crash_ctxt.es));
+       asm volatile("movw %%ax, %%fs" : : "a"(hv_crash_ctxt.fs));
+       asm volatile("movw %%ax, %%gs" : : "a"(hv_crash_ctxt.gs));
 
-       native_wrmsrq(MSR_IA32_CR_PAT, ctxt->pat);
-       asm volatile("movq %0, %%cr0" : : "r"(ctxt->cr0));
+       hv_wrmsr(MSR_IA32_CR_PAT, hv_crash_ctxt.pat);
+       asm volatile("movq %0, %%cr0" : : "r"(hv_crash_ctxt.cr0));
 
-       asm volatile("movq %0, %%cr8" : : "r"(ctxt->cr8));
-       asm volatile("movq %0, %%cr4" : : "r"(ctxt->cr4));
-       asm volatile("movq %0, %%cr2" : : "r"(ctxt->cr4));
+       asm volatile("movq %0, %%cr8" : : "r"(hv_crash_ctxt.cr8));
+       asm volatile("movq %0, %%cr4" : : "r"(hv_crash_ctxt.cr4));
+       asm volatile("movq %0, %%cr2" : : "r"(hv_crash_ctxt.cr4));
 
-       native_load_idt(&ctxt->idtr);
-       native_wrmsrq(MSR_GS_BASE, ctxt->gsbase);
-       native_wrmsrq(MSR_EFER, ctxt->efer);
+       asm volatile("lidt %0" : : "m" (hv_crash_ctxt.idtr));
+       hv_wrmsr(MSR_GS_BASE, hv_crash_ctxt.gsbase);
+       hv_wrmsr(MSR_EFER, hv_crash_ctxt.efer);
 
        /* restore the original kernel CS now via far return */
-       asm volatile("movzwq %0, %%rax\n\t"
-                    "pushq %%rax\n\t"
-                    "pushq $1f\n\t"
-                    "lretq\n\t"
-                    "1:nop\n\t" : : "m"(ctxt->cs) : "rax");
-
-       /* We are in asmlinkage without stack frame, hence make C function
-        * calls which will buy stack frames.
-        */
-       hv_crash_restore_tss();
-       hv_crash_clear_kernpt();
-

Quoted 9 lines

arch/x86/hyperv/hv_crash.c static noinline void hv_crash_clear_kernpt(void) · line 179

Quoted 6 lines

arch/x86/hyperv/hv_crash.c +2 -4

-       /* we are now fully in devirtualized normal kernel mode */
-       __crash_kexec(NULL);
-
-       hv_panic_timeout_reboot();
+       asm volatile("pushq     %q0             \n\t"
+                    "leaq      %c1(%%rip), %q0 \n\t"

You can use %a1 instead of %c1(%%rip).

Nice.

Quoted 5 lines

arch/x86/hyperv/hv_crash.c static noinline void hv_crash_clear_kernpt(void) · line 181

Quoted 2 lines

arch/x86/hyperv/hv_crash.c +2 -0

+                    "pushq     %q0             \n\t"
+                    "lretq                     \n\t"

No need for terminating \n\t after the last insn in the asm template.

Quoted 4 lines

arch/x86/hyperv/hv_crash.c static noinline void hv_crash_clear_kernpt(void) · line 182

Quoted 1 line

arch/x86/hyperv/hv_crash.c +1 -0

+                    :: "a"(hv_crash_ctxt.cs), "i"(hv_crash_handle));

Pedantically, you need ': "+a"(...) : "i"(...)' here.

Right, so the compiler knows that the register will be updated by the asm() block. But what is preventing it from writing back this value to hv_crash_ctxt.cs? The generated code doesn't seem to do so, but the semantics of "+r" suggest otherwise AIUI.

The code following the asm() block is unreachable anyway, so it doesn't really matter either way in practice. Just curious ...

(quoted diff) +0 -4

@@ -133,49 +150,36 @@ static noinline void hv_crash_clear_kernpt(void)   * available. We restore kernel GDT, and rest of the context, and continue
  * to kexec.
  */
-static asmlinkage void __noreturn hv_crash_c_entry(void) +static void
 __naked hv_crash_c_entry(void)  {
- struct hv_crash_ctxt *ctxt = &hv_crash_ctxt; -  	/* first thing, restore kernel gdt */
- native_load_gdt(&ctxt->gdtr); + asm volatile("lgdt %0" : : "m"
 (hv_crash_ctxt.gdtr));  
- asm volatile("movw %%ax, %%ss" : : "a"(ctxt->ss)); - asm

volatile("movq %0, %%rsp" : : "m"(ctxt->rsp)); + asm volatile("movw %%ax, %%ss" : : "a"(hv_crash_ctxt.ss)); + asm volatile("movq %0, %%rsp" : : "m"(hv_crash_ctxt.rsp));

- asm volatile("movw %%ax, %%ds" : : "a"(ctxt->ds)); - asm volatile("movw %%ax, %%es" : : "a"(ctxt->es)); - asm volatile("movw %%ax, %%fs" : : "a"(ctxt->fs)); - asm volatile("movw %%ax, %%gs" : :
"a"(ctxt->gs)); + asm volatile("movw %%ax, %%ds" : :
"a"(hv_crash_ctxt.ds)); + asm volatile("movw %%ax, %%es" : :
"a"(hv_crash_ctxt.es)); + asm volatile("movw %%ax, %%fs" : :
"a"(hv_crash_ctxt.fs)); + asm volatile("movw %%ax, %%gs" : :
"a"(hv_crash_ctxt.gs));
- native_wrmsrq(MSR_IA32_CR_PAT, ctxt->pat); - asm volatile("movq %0, %%cr0" : : "r"(ctxt->cr0)); + hv_wrmsr(MSR_IA32_CR_PAT, hv_crash_ctxt.pat); + asm volatile("movq %0, %%cr0" : :
"r"(hv_crash_ctxt.cr0));
- asm volatile("movq %0, %%cr8" : : "r"(ctxt->cr8)); - asm volatile("movq %0, %%cr4" : : "r"(ctxt->cr4)); - asm volatile("movq %0, %%cr2" : : "r"(ctxt->cr4)); + asm volatile("movq %0, %%cr8" : :
"r"(hv_crash_ctxt.cr8)); + asm volatile("movq %0, %%cr4" : :
"r"(hv_crash_ctxt.cr4)); + asm volatile("movq %0, %%cr2" : :
"r"(hv_crash_ctxt.cr4));
- native_load_idt(&ctxt->idtr); - native_wrmsrq(MSR_GS_BASE, ctxt->gsbase); - native_wrmsrq(MSR_EFER, ctxt->efer); + asm volatile("lidt %0" : : "m" (hv_crash_ctxt.idtr)); + hv_wrmsr(MSR_GS_BASE, hv_crash_ctxt.gsbase); + hv_wrmsr(MSR_EFER, hv_crash_ctxt.efer);
/* restore the original kernel CS now via far return */
- asm volatile("movzwq %0, %%rax\n\t" - "pushq %%rax\n\t" - "pushq

$1f\n\t" - "lretq\n\t" - "1:nop\n\t" : : "m"(ctxt->cs) : "rax"); - - /* We are in asmlinkage without stack frame, hence make C function - * calls which will buy stack frames. - */ - hv_crash_restore_tss(); - hv_crash_clear_kernpt(); - - /* we are now fully in devirtualized normal kernel mode */ - __crash_kexec(NULL); - - hv_panic_timeout_reboot(); + asm volatile("pushq %q0 \n\t" + "leaq %c1(%%rip), %q0 \n\t" + "pushq %q0 \n\t" + "lretq \n\t" + ::
"a"(hv_crash_ctxt.cs), "i"(hv_crash_handle));

Quoted 9 lines

(quoted diff) +0 -4

@@ -133,49 +150,36 @@ static noinline void hv_crash_clear_kernpt(void)   * available. We restore kernel GDT, and rest of the context, and continue
  * to kexec.
  */
-static asmlinkage void __noreturn hv_crash_c_entry(void) +static void
 __naked hv_crash_c_entry(void)  {
- struct hv_crash_ctxt *ctxt = &hv_crash_ctxt; -  	/* first thing, restore kernel gdt */
- native_load_gdt(&ctxt->gdtr); + asm volatile("lgdt %0" : : "m"
 (hv_crash_ctxt.gdtr));  
- asm volatile("movw %%ax, %%ss" : : "a"(ctxt->ss)); - asm

Quoted 3 lines

volatile("movq %0, %%rsp" : : "m"(ctxt->rsp)); + asm volatile("movw %%ax, %%ss" : : "a"(hv_crash_ctxt.ss)); + asm volatile("movq %0, %%rsp" : : "m"(hv_crash_ctxt.rsp));

I know this is pre-existing, but the asm here is poor.

All segment registers loads can have a memory operand, rather than forcing through %eax, which in turn reduces the setup logic the compiler needs to emit.

Something like this:

    "movl %0, %%ss" : : "m"(hv_crash_ctxt.ss)

ought to do.

As Uros notes, "a" is clobbered here but the compiler is not informed. But, it's not necessary.

As a naked function you could even use 3x asm() statements, but you can get the compiler to sort out the function reference automatically with:

    asm volatile ("push %q0\n\t"
                  "push %q1\n\t"
                  "lretq"
                  :: "r"(hv_crash_ctxt.cs), "r"(hv_crash_handle));

Quoted 18 lines · Andrew Cooper

Quoted 9 lines

(quoted diff) +0 -4

@@ -133,49 +150,36 @@ static noinline void hv_crash_clear_kernpt(void)   * available. We restore kernel GDT, and rest of the context, and continue
  * to kexec.
  */
-static asmlinkage void __noreturn hv_crash_c_entry(void) +static void
 __naked hv_crash_c_entry(void)  {
- struct hv_crash_ctxt *ctxt = &hv_crash_ctxt; -  	/* first thing, restore kernel gdt */
- native_load_gdt(&ctxt->gdtr); + asm volatile("lgdt %0" : : "m"
 (hv_crash_ctxt.gdtr));  
- asm volatile("movw %%ax, %%ss" : : "a"(ctxt->ss)); - asm

Quoted 3 lines

volatile("movq %0, %%rsp" : : "m"(ctxt->rsp)); + asm volatile("movw %%ax, %%ss" : : "a"(hv_crash_ctxt.ss)); + asm volatile("movq %0, %%rsp" : : "m"(hv_crash_ctxt.rsp));

I know this is pre-existing, but the asm here is poor.

All segment registers loads can have a memory operand, rather than forcing through %eax, which in turn reduces the setup logic the compiler needs to emit.

Quoted 6 lines

Something like this:

    "movl %0, %%ss" : : "m"(hv_crash_ctxt.ss)

ought to do.

'movw' seems to work, yes.

Quoted 22 lines · Ard Biesheuvel

Quoted 20 lines · Andrew Cooper

Quoted 9 lines

(quoted diff) +0 -4

@@ -133,49 +150,36 @@ static noinline void hv_crash_clear_kernpt(void)   * available. We restore kernel GDT, and rest of the context, and continue
  * to kexec.
  */
-static asmlinkage void __noreturn hv_crash_c_entry(void) +static void
 __naked hv_crash_c_entry(void)  {
- struct hv_crash_ctxt *ctxt = &hv_crash_ctxt; -  	/* first thing, restore kernel gdt */
- native_load_gdt(&ctxt->gdtr); + asm volatile("lgdt %0" : : "m"
 (hv_crash_ctxt.gdtr));  
- asm volatile("movw %%ax, %%ss" : : "a"(ctxt->ss)); - asm

Quoted 3 lines

volatile("movq %0, %%rsp" : : "m"(ctxt->rsp)); + asm volatile("movw %%ax, %%ss" : : "a"(hv_crash_ctxt.ss)); + asm volatile("movq %0, %%rsp" : : "m"(hv_crash_ctxt.rsp));

I know this is pre-existing, but the asm here is poor.

All segment registers loads can have a memory operand, rather than forcing through %eax, which in turn reduces the setup logic the compiler needs to emit.

Something like this:

Quoted 5 lines

Quoted 4 lines

    "movl %0, %%ss" : : "m"(hv_crash_ctxt.ss)

ought to do.

'movw' seems to work, yes.

movw works, but is sub-optimal.

Quoted 22 lines · Andrew Cooper

Quoted 21 lines · Ard Biesheuvel

Quoted 20 lines · Andrew Cooper

Quoted 9 lines

(quoted diff) +0 -4

@@ -133,49 +150,36 @@ static noinline void hv_crash_clear_kernpt(void)   * available. We restore kernel GDT, and rest of the context, and continue
  * to kexec.
  */
-static asmlinkage void __noreturn hv_crash_c_entry(void) +static void
 __naked hv_crash_c_entry(void)  {
- struct hv_crash_ctxt *ctxt = &hv_crash_ctxt; -  	/* first thing, restore kernel gdt */
- native_load_gdt(&ctxt->gdtr); + asm volatile("lgdt %0" : : "m"
 (hv_crash_ctxt.gdtr));  
- asm volatile("movw %%ax, %%ss" : : "a"(ctxt->ss)); - asm

Quoted 3 lines

volatile("movq %0, %%rsp" : : "m"(ctxt->rsp)); + asm volatile("movw %%ax, %%ss" : : "a"(hv_crash_ctxt.ss)); + asm volatile("movq %0, %%rsp" : : "m"(hv_crash_ctxt.rsp));

I know this is pre-existing, but the asm here is poor.

All segment registers loads can have a memory operand, rather than forcing through %eax, which in turn reduces the setup logic the compiler needs to emit.

Something like this:

Quoted 7 lines

Quoted 5 lines

Quoted 4 lines

    "movl %0, %%ss" : : "m"(hv_crash_ctxt.ss)

ought to do.

'movw' seems to work, yes.

movw works, but is sub-optimal.

Can you give an asm example where movl with a segment register is accepted by the assembler? I only managed that with movw, hence my comment.

From: Ard Biesheuvel <[email protected]>

hv_crash_c_entry() is a C function that is entered without a stack, and this is only allowed for functions that have the __naked attribute, which informs the compiler that it must not emit the usual prologue and epilogue or emit any other kind of instrumentation that relies on a stack frame.

So split up the function, and set the __naked attribute on the initial part that sets up the stack, GDT, IDT and other pieces that are needed for ordinary C execution. Given that function calls are not permitted either, use the existing long return coded in an asm() block to call the second part of the function, which is an ordinary function that is permitted to call other functions as usual.

Fixes 94212d34618c ("x86/hyperv: Implement hypervisor RAM collection into vmcore")

Signed-off-by Ard Biesheuvel <[email protected]>