From: Peter Zijlstra <[email protected]> Sent: Wednesday, April 30, 2025 
4:08 AM
> 
> Instead of using an indirect call to the hypercall page, use a direct
> call instead. This avoids all CFI problems, including the one where
> the hypercall page doesn't have IBT on.
> 
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> ---
>  arch/x86/hyperv/hv_init.c |   60 
> +++++++++++++++++++++++-----------------------
>  1 file changed, 30 insertions(+), 30 deletions(-)
> 
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -37,23 +37,41 @@
>  void *hv_hypercall_pg;
> 
>  #ifdef CONFIG_X86_64
> +static u64 __hv_hyperfail(u64 control, u64 param1, u64 param2)
> +{
> +     return U64_MAX;
> +}
> +
> +DEFINE_STATIC_CALL(__hv_hypercall, __hv_hyperfail);
> +
>  u64 hv_std_hypercall(u64 control, u64 param1, u64 param2)
>  {
>       u64 hv_status;
> 
> -     if (!hv_hypercall_pg)
> -             return U64_MAX;
> -
>       register u64 __r8 asm("r8") = param2;
> -     asm volatile (CALL_NOSPEC
> +     asm volatile ("call " STATIC_CALL_TRAMP_STR(__hv_hypercall)
>                     : "=a" (hv_status), ASM_CALL_CONSTRAINT,
>                       "+c" (control), "+d" (param1), "+r" (__r8)
> -                   : THUNK_TARGET(hv_hypercall_pg)
> -                   : "cc", "memory", "r9", "r10", "r11");
> +                   : : "cc", "memory", "r9", "r10", "r11");
> 
>       return hv_status;
>  }
> +
> +typedef u64 (*hv_hypercall_f)(u64 control, u64 param1, u64 param2);
> +
> +static inline void hv_set_hypercall_pg(void *ptr)
> +{
> +     hv_hypercall_pg = ptr;
> +
> +     if (!ptr)
> +             ptr = &__hv_hyperfail;
> +     static_call_update(__hv_hypercall, (hv_hypercall_f)ptr);
> +}
>  #else
> +static inline void hv_set_hypercall_pg(void *ptr)
> +{
> +     hv_hypercall_pg = ptr;
> +}
>  EXPORT_SYMBOL_GPL(hv_hypercall_pg);
>  #endif
> 
> @@ -348,7 +366,7 @@ static int hv_suspend(void)
>        * pointer is restored on resume.
>        */
>       hv_hypercall_pg_saved = hv_hypercall_pg;
> -     hv_hypercall_pg = NULL;
> +     hv_set_hypercall_pg(NULL);
> 
>       /* Disable the hypercall page in the hypervisor */
>       rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> @@ -374,7 +392,7 @@ static void hv_resume(void)
>               vmalloc_to_pfn(hv_hypercall_pg_saved);
>       wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> 
> -     hv_hypercall_pg = hv_hypercall_pg_saved;
> +     hv_set_hypercall_pg(hv_hypercall_pg_saved);
>       hv_hypercall_pg_saved = NULL;
> 
>       /*
> @@ -528,8 +546,8 @@ void __init hyperv_init(void)
>       if (hv_isolation_type_tdx() && !ms_hyperv.paravisor_present)
>               goto skip_hypercall_pg_init;
> 
> -     hv_hypercall_pg = __vmalloc_node_range(PAGE_SIZE, 1, VMALLOC_START,
> -                     VMALLOC_END, GFP_KERNEL, PAGE_KERNEL_ROX,
> +     hv_hypercall_pg = __vmalloc_node_range(PAGE_SIZE, 1, MODULES_VADDR,
> +                     MODULES_END, GFP_KERNEL, PAGE_KERNEL_ROX,

Curiosity question (which I forgot ask about in v1):  Is this change so that the
hypercall page kernel address is "close enough" for the direct call to work from
built-in code and from module code?  Or is there some other reason?

>                       VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
>                       __builtin_return_address(0));
>       if (hv_hypercall_pg == NULL)
> @@ -567,27 +585,9 @@ void __init hyperv_init(void)
>               wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
>       }
> 
> -skip_hypercall_pg_init:
> -     /*
> -      * Some versions of Hyper-V that provide IBT in guest VMs have a bug
> -      * in that there's no ENDBR64 instruction at the entry to the
> -      * hypercall page. Because hypercalls are invoked via an indirect call
> -      * to the hypercall page, all hypercall attempts fail when IBT is
> -      * enabled, and Linux panics. For such buggy versions, disable IBT.
> -      *
> -      * Fixed versions of Hyper-V always provide ENDBR64 on the hypercall
> -      * page, so if future Linux kernel versions enable IBT for 32-bit
> -      * builds, additional hypercall page hackery will be required here
> -      * to provide an ENDBR32.
> -      */
> -#ifdef CONFIG_X86_KERNEL_IBT
> -     if (cpu_feature_enabled(X86_FEATURE_IBT) &&
> -         *(u32 *)hv_hypercall_pg != gen_endbr()) {
> -             setup_clear_cpu_cap(X86_FEATURE_IBT);
> -             pr_warn("Disabling IBT because of Hyper-V bug\n");
> -     }
> -#endif

Nit: With this IBT code removed, the #include <asm/ibt.h> at the top
of this source code file should be removed.

> +     hv_set_hypercall_pg(hv_hypercall_pg);
> 
> +skip_hypercall_pg_init:
>       /*
>        * hyperv_init() is called before LAPIC is initialized: see
>        * apic_intr_mode_init() -> x86_platform.apic_post_init() and
> 
> 

The nit notwithstanding,

Reviewed-by: Michael Kelley <[email protected]>

Reply via email to