On Thu, Feb 26, 2026 at 11:48 AM Ard Biesheuvel <[email protected]> wrote: > > Hi Uros, > > On Thu, 26 Feb 2026, at 11:35, Uros Bizjak wrote: > > On Thu, Feb 26, 2026 at 10:51 AM Ard Biesheuvel <[email protected]> wrote: > >> > >> 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(-) > >> > >> diff --git a/arch/x86/hyperv/hv_crash.c b/arch/x86/hyperv/hv_crash.c > >> index a78e4fed5720..d77766e8d37e 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,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" > > > > You can use %a1 instead of %c1(%%rip). > > > > Nice. > > >> + "pushq %q0 \n\t" > >> + "lretq \n\t" > > > > No need for terminating \n\t after the last insn in the asm template. > > > >> + :: "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 ...
Oh, you just need a temporary here... the original is OK. Indeed, "+r" will write back the value to the memory location, and this is not what we want here. Uros.
