Re: [PATCH v2 09/11] KVM: arm64: Type-check hypercall arguments at the caller

[Marc Zyngier <maz@kernel.org>]

On Mon, 03 Aug 2026 13:42:18 +0100,
Fuad Tabba <fuad.tabba@linux.dev> wrote:
> 
> kvm_call_hyp_nvhe() reduces its target to an SMCCC function number, so
> the compiler never sees a callable: arguments that are wrong in number,
> type or order are silently marshalled into registers. The kvm_call_hyp()
> wrappers only catch this on the VHE branch, and the pKVM-only hypercalls
> have no such branch.
> 
> Declare each hypercall's signature once in kvm_hcall.h and generate a
> typed stub from it, in the mold of the syscall wrappers. Make
> kvm_call_hyp_nvhe() resolve to the stub so every caller is checked
> against the declared signature; a stale or mistyped call now fails to
> compile. The stubs inline to the same SMCCC call the untyped macro used
> to make: the compiled callers are unchanged, apart from hypercall
> returns now being tested at their declared width.
> 
> The stage-2 protection arguments are declared u64 rather than
> enum kvm_pgtable_prot, as kvm_pgtable.h includes linux/kvm_host.h and
> the enum cannot be completed here.
> 
> Assisted-by: Antigravity:gemini-3.1-pro
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
> ---
>  arch/arm64/include/asm/kvm_hcall.h | 168 ++++++++++++++++++++++++++++-
>  arch/arm64/kvm/hyp_trace.c         |   2 +-
>  2 files changed, 168 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_hcall.h b/arch/arm64/include/asm/kvm_hcall.h
> index d925b2c28a3d8..1c9c182ddcd1b 100644
> --- a/arch/arm64/include/asm/kvm_hcall.h
> +++ b/arch/arm64/include/asm/kvm_hcall.h
> @@ -9,6 +9,7 @@
>  #ifndef __ARM64_KVM_HCALL_H__
>  #define __ARM64_KVM_HCALL_H__
>  
> +#include <linux/args.h>
>  #include <linux/arm-smccc.h>
>  #include <linux/bug.h>
>  #include <linux/errno.h>
> @@ -16,12 +17,37 @@
>  
>  #include <asm/barrier.h>
>  #include <asm/kvm_asm.h>
> +#include <asm/spectre.h>
>  #include <asm/virt.h>
>  
>  typedef u16 pkvm_handle_t;
>  
> +struct kvm;
> +struct kvm_s2_mmu;
> +struct kvm_vcpu;
> +struct vgic_v3_cpu_if;
> +struct vgic_v5_cpu_if;
> +
> +/*
> + * Hypercall signatures are declared as (type, name) argument pairs.
> + * __KVM_HCALL_MAP() applies a macro to each pair, in the mold of __MAP()
> + * in <linux/syscalls.h>. The ladder is indexed by list entries, two per
> + * argument; __KVM_HCALL_MAP_N() takes that count explicitly.
> + */
> +#define __KVM_HCALL_MAP2(m, t, a, ...) m(t, a)
> +#define __KVM_HCALL_MAP4(m, t, a, ...) m(t, a), __KVM_HCALL_MAP2(m, __VA_ARGS__)
> +#define __KVM_HCALL_MAP6(m, t, a, ...) m(t, a), __KVM_HCALL_MAP4(m, __VA_ARGS__)
> +#define __KVM_HCALL_MAP8(m, t, a, ...) m(t, a), __KVM_HCALL_MAP6(m, __VA_ARGS__)
> +#define __KVM_HCALL_MAP10(m, t, a, ...) m(t, a), __KVM_HCALL_MAP8(m, __VA_ARGS__)
> +#define __KVM_HCALL_MAP12(m, t, a, ...) m(t, a), __KVM_HCALL_MAP10(m, __VA_ARGS__)
> +#define __KVM_HCALL_MAP_N(n, m, ...) CONCATENATE(__KVM_HCALL_MAP, n)(m, __VA_ARGS__)
> +#define __KVM_HCALL_MAP(m, ...) __KVM_HCALL_MAP_N(COUNT_ARGS(__VA_ARGS__), m, __VA_ARGS__)
> +
> +#define __KVM_HCALL_DECL(t, a)	t a
> +#define __KVM_HCALL_ARGS(t, a)	a
> +
>  #ifndef __KVM_NVHE_HYPERVISOR__
> -#define kvm_call_hyp_nvhe(f, ...)					\
> +#define __kvm_call_hyp_nvhe(f, ...)					\
>  	({								\
>  		struct arm_smccc_res res;				\
>  									\
> @@ -33,6 +59,43 @@ typedef u16 pkvm_handle_t;
>  		res.a1;							\
>  	})
>  
> +/*
> + * Generate a typed stub for each declared hypercall. kvm_call_hyp_nvhe()
> + * resolves to the stub, so a call with the wrong argument count or types
> + * fails to compile instead of being silently truncated to an SMCCC function
> + * number and a pile of registers. The stub inlines to the same SMCCC call
> + * the untyped macro used to make.
> + */
> +#define DECLARE_KVM_HOST_HCALL(ret, name, ...)				\
> +	static __always_inline						\
> +	ret nvhe_hvc_##name(__KVM_HCALL_MAP(__KVM_HCALL_DECL, __VA_ARGS__)) \
> +	{								\
> +		return (ret)__kvm_call_hyp_nvhe(name,			\
> +			__KVM_HCALL_MAP(__KVM_HCALL_ARGS, __VA_ARGS__));\
> +	}
> +
> +#define DECLARE_KVM_HOST_HCALL_VOID(name, ...)				\
> +	static __always_inline						\
> +	void nvhe_hvc_##name(__KVM_HCALL_MAP(__KVM_HCALL_DECL, __VA_ARGS__)) \
> +	{								\
> +		__kvm_call_hyp_nvhe(name,				\
> +			__KVM_HCALL_MAP(__KVM_HCALL_ARGS, __VA_ARGS__));\
> +	}

I'll bite! ;-) void is a type, not the absence of a type. And
returning void is perfectly valid.

So with this in mind, I got rid of the *_VOID() helpers altogether,
see below.

Thanks,

	M.

diff --git a/arch/arm64/include/asm/kvm_hcall.h b/arch/arm64/include/asm/kvm_hcall.h
index 3958396a83345..0b1b10d4212da 100644
--- a/arch/arm64/include/asm/kvm_hcall.h
+++ b/arch/arm64/include/asm/kvm_hcall.h
@@ -88,26 +88,12 @@ struct vgic_v5_cpu_if;
 			__KVM_HCALL_MAP(__KVM_HCALL_ARGS, __VA_ARGS__));\
 	}
 
-#define DECLARE_KVM_HOST_HCALL_VOID(name, ...)				\
-	static __always_inline						\
-	void nvhe_hvc_##name(__KVM_HCALL_MAP(__KVM_HCALL_DECL, __VA_ARGS__)) \
-	{								\
-		__kvm_call_hyp_nvhe(name,				\
-			__KVM_HCALL_MAP(__KVM_HCALL_ARGS, __VA_ARGS__));\
-	}
-
 #define DECLARE_KVM_HOST_HCALL0(ret, name)				\
 	static __always_inline ret nvhe_hvc_##name(void)		\
 	{								\
 		return (ret)__kvm_call_hyp_nvhe(name);			\
 	}
 
-#define DECLARE_KVM_HOST_HCALL0_VOID(name)				\
-	static __always_inline void nvhe_hvc_##name(void)		\
-	{								\
-		__kvm_call_hyp_nvhe(name);				\
-	}
-
 #define kvm_call_hyp_nvhe(f, ...)	nvhe_hvc_##f(__VA_ARGS__)
 
 /*
@@ -148,12 +134,8 @@ struct vgic_v5_cpu_if;
  */
 #define DECLARE_KVM_HOST_HCALL(ret, name, ...)				\
 	typedef ret kvm_host_hcall_sig_##name(__KVM_HCALL_MAP(__KVM_HCALL_DECL, __VA_ARGS__));
-#define DECLARE_KVM_HOST_HCALL_VOID(name, ...)				\
-	typedef void kvm_host_hcall_sig_##name(__KVM_HCALL_MAP(__KVM_HCALL_DECL, __VA_ARGS__));
 #define DECLARE_KVM_HOST_HCALL0(ret, name)				\
 	typedef ret kvm_host_hcall_sig_##name(void);
-#define DECLARE_KVM_HOST_HCALL0_VOID(name)				\
-	typedef void kvm_host_hcall_sig_##name(void);
 #endif /* __KVM_NVHE_HYPERVISOR__ */
 
 /* Hypercalls that are unavailable once pKVM has finalised. */
@@ -164,52 +146,52 @@ DECLARE_KVM_HOST_HCALL(unsigned long, __pkvm_create_private_mapping,
 	phys_addr_t, phys, size_t, size, u64, prot)
 DECLARE_KVM_HOST_HCALL(int, __pkvm_cpu_set_vector,
 	enum arm64_hyp_spectre_vector, slot)
-DECLARE_KVM_HOST_HCALL0_VOID(__kvm_enable_ssbs)
-DECLARE_KVM_HOST_HCALL0_VOID(__vgic_v3_init_lrs)
+DECLARE_KVM_HOST_HCALL0(void, __kvm_enable_ssbs)
+DECLARE_KVM_HOST_HCALL0(void, __vgic_v3_init_lrs)
 DECLARE_KVM_HOST_HCALL0(u64, __vgic_v3_get_gic_config)
 
 DECLARE_KVM_HOST_HCALL0(int, __pkvm_prot_finalize)
 
 /* Hypercalls that are always available and common to [nh]VHE/pKVM. */
-DECLARE_KVM_HOST_HCALL_VOID(__kvm_adjust_pc,
+DECLARE_KVM_HOST_HCALL(void, __kvm_adjust_pc,
 	struct kvm_vcpu __kern *, vcpu)
 DECLARE_KVM_HOST_HCALL(int, __kvm_vcpu_run,
 	struct kvm_vcpu __kern *, vcpu)
-DECLARE_KVM_HOST_HCALL0_VOID(__kvm_flush_vm_context)
-DECLARE_KVM_HOST_HCALL_VOID(__kvm_tlb_flush_vmid_ipa,
+DECLARE_KVM_HOST_HCALL0(void, __kvm_flush_vm_context)
+DECLARE_KVM_HOST_HCALL(void, __kvm_tlb_flush_vmid_ipa,
 	struct kvm_s2_mmu __kern *, mmu, phys_addr_t, ipa, int, level)
-DECLARE_KVM_HOST_HCALL_VOID(__kvm_tlb_flush_vmid_ipa_nsh,
+DECLARE_KVM_HOST_HCALL(void, __kvm_tlb_flush_vmid_ipa_nsh,
 	struct kvm_s2_mmu __kern *, mmu, phys_addr_t, ipa, int, level)
-DECLARE_KVM_HOST_HCALL_VOID(__kvm_tlb_flush_vmid,
+DECLARE_KVM_HOST_HCALL(void, __kvm_tlb_flush_vmid,
 	struct kvm_s2_mmu __kern *, mmu)
-DECLARE_KVM_HOST_HCALL_VOID(__kvm_tlb_flush_vmid_range,
+DECLARE_KVM_HOST_HCALL(void, __kvm_tlb_flush_vmid_range,
 	struct kvm_s2_mmu __kern *, mmu, phys_addr_t, start, unsigned long, pages)
-DECLARE_KVM_HOST_HCALL_VOID(__kvm_flush_cpu_context,
+DECLARE_KVM_HOST_HCALL(void, __kvm_flush_cpu_context,
 	struct kvm_s2_mmu __kern *, mmu)
-DECLARE_KVM_HOST_HCALL_VOID(__kvm_timer_set_cntvoff,
+DECLARE_KVM_HOST_HCALL(void, __kvm_timer_set_cntvoff,
 	u64, cntvoff)
 DECLARE_KVM_HOST_HCALL(int, __tracing_load,
 	void __kern *, desc_hva, size_t, desc_size)
-DECLARE_KVM_HOST_HCALL0_VOID(__tracing_unload)
+DECLARE_KVM_HOST_HCALL0(void, __tracing_unload)
 DECLARE_KVM_HOST_HCALL(int, __tracing_enable,
 	bool, enable)
 DECLARE_KVM_HOST_HCALL(int, __tracing_swap_reader,
 	unsigned int, cpu)
-DECLARE_KVM_HOST_HCALL_VOID(__tracing_update_clock,
+DECLARE_KVM_HOST_HCALL(void, __tracing_update_clock,
 	u32, mult, u32, shift, u64, epoch_ns, u64, epoch_cyc)
 DECLARE_KVM_HOST_HCALL(int, __tracing_reset,
 	unsigned int, cpu)
 DECLARE_KVM_HOST_HCALL(int, __tracing_enable_event,
 	unsigned short, id, bool, enable)
-DECLARE_KVM_HOST_HCALL_VOID(__tracing_write_event,
+DECLARE_KVM_HOST_HCALL(void, __tracing_write_event,
 	u64, id)
-DECLARE_KVM_HOST_HCALL_VOID(__vgic_v3_save_aprs,
+DECLARE_KVM_HOST_HCALL(void, __vgic_v3_save_aprs,
 	struct vgic_v3_cpu_if __kern *, cpu_if)
-DECLARE_KVM_HOST_HCALL_VOID(__vgic_v3_restore_vmcr_aprs,
+DECLARE_KVM_HOST_HCALL(void, __vgic_v3_restore_vmcr_aprs,
 	struct vgic_v3_cpu_if __kern *, cpu_if)
-DECLARE_KVM_HOST_HCALL_VOID(__vgic_v5_save_apr,
+DECLARE_KVM_HOST_HCALL(void, __vgic_v5_save_apr,
 	struct vgic_v5_cpu_if __kern *, cpu_if)
-DECLARE_KVM_HOST_HCALL_VOID(__vgic_v5_restore_vmcr_apr,
+DECLARE_KVM_HOST_HCALL(void, __vgic_v5_restore_vmcr_apr,
 	struct vgic_v5_cpu_if __kern *, cpu_if)
 
 /* Hypercalls that are available only when pKVM has finalised. */
@@ -232,7 +214,7 @@ DECLARE_KVM_HOST_HCALL(int, __pkvm_host_test_clear_young_guest,
 DECLARE_KVM_HOST_HCALL(int, __pkvm_host_mkyoung_guest,
 	u64, gfn)
 DECLARE_KVM_HOST_HCALL0(int, __pkvm_reserve_vm)
-DECLARE_KVM_HOST_HCALL_VOID(__pkvm_unreserve_vm,
+DECLARE_KVM_HOST_HCALL(void, __pkvm_unreserve_vm,
 	pkvm_handle_t, handle)
 DECLARE_KVM_HOST_HCALL(int, __pkvm_init_vm,
 	struct kvm __kern *, host_kvm, void __kern *, vm_hva,
@@ -249,10 +231,10 @@ DECLARE_KVM_HOST_HCALL(int, __pkvm_start_teardown_vm,
 	pkvm_handle_t, handle)
 DECLARE_KVM_HOST_HCALL(int, __pkvm_finalize_teardown_vm,
 	pkvm_handle_t, handle)
-DECLARE_KVM_HOST_HCALL_VOID(__pkvm_vcpu_load,
+DECLARE_KVM_HOST_HCALL(void, __pkvm_vcpu_load,
 	pkvm_handle_t, handle, unsigned int, vcpu_idx, u64, hcr_el2)
-DECLARE_KVM_HOST_HCALL0_VOID(__pkvm_vcpu_put)
-DECLARE_KVM_HOST_HCALL_VOID(__pkvm_tlb_flush_vmid,
+DECLARE_KVM_HOST_HCALL0(void, __pkvm_vcpu_put)
+DECLARE_KVM_HOST_HCALL(void, __pkvm_tlb_flush_vmid,
 	pkvm_handle_t, handle)
 
 #endif /* __ARM64_KVM_HCALL_H__ */

-- 
Without deviation from the norm, progress is not possible.