RE: [PATCH v2 12/13] x86_64,hyperv: Use direct call to hypercall-page

Michael Kelley
2025-05-01T02:36:29Z

From: Peter Zijlstra <peterz@infradead.org> 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) <peterz@infradead.org>
> ---
> 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 <mhklinux@outlook.com>