Re: [PATCH v2] x86/hyperv: Use __naked attribute to fix stackless C function

[Mukesh R]

On 2/27/26 15:03, Mukesh R wrote:

On 2/27/26 14:40, Ard Biesheuvel wrote:

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.

Thank you for the patch. I'll start a build on the side and test it
out and let you know.

Well, never that simple. I am able to generate cores, both before and
after the patch, but the crash command hangs on the vmcore (with correct
vmlinux). But, since the kexec happened, I think it is fair to say that
the patch works. With that:

Reviewed-by: Mukesh R <mrat...@linux.microsoft.com>

However, I did notice a pre-exising cut-n-paste oopsie:

  asm volatile("movq %0, %%cr2" : : "r"(hv_crash_ctxt.cr4)); <== cr2, not cr4


So, if you happen to do another churn, feel free to fix it. Otherwise,
no worries, I'll submit another patch.

Thanks for all your help.
-Mukesh

Thanks,
-Mukesh

Cc: Mukesh Rathor <mrat...@linux.microsoft.com>
Cc: Wei Liu <wei....@kernel.org>
Cc: Uros Bizjak <ubiz...@gmail.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Cc: linux-hyp...@vger.kernel.org
Fixes: 94212d34618c ("x86/hyperv: Implement hypervisor RAM collection into 
vmcore")
Signed-off-by: Ard Biesheuvel <a...@kernel.org>
---
v2: apply some asm tweaks suggested by Uros and Andrew

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

diff --git a/arch/x86/hyperv/hv_crash.c b/arch/x86/hyperv/hv_crash.c
index 92da1b4f2e73..1c0965eb346e 100644
--- a/arch/x86/hyperv/hv_crash.c
+++ b/arch/x86/hyperv/hv_crash.c
@@ -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,35 @@ 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 %0, %%ss" : : "m"(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 %0, %%ds" : : "m"(hv_crash_ctxt.ds));
+    asm volatile("movw %0, %%es" : : "m"(hv_crash_ctxt.es));
+    asm volatile("movw %0, %%fs" : : "m"(hv_crash_ctxt.fs));
+    asm volatile("movw %0, %%gs" : : "m"(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"
+             "pushq %q1\n\t"
+             "lretq"
+             :: "r"(hv_crash_ctxt.cs), "r"(hv_crash_handle));
  }
  /* Tell gcc we are using lretq long jump in the above function intentionally 
*/
  STACK_FRAME_NON_STANDARD(hv_crash_c_entry);