Re: [PATCH v2 10/11] KVM: arm64: nVHE: Check hypercall handlers against the declared ABI

[Marc Zyngier <maz@kernel.org>]

On Mon, 03 Aug 2026 13:42:19 +0100,
Fuad Tabba <fuad.tabba@linux.dev> wrote:
> 
> Each hypercall handler unmarshals its arguments from the host context
> with hand-written DECLARE_REG() casts that nothing ties to what the
> caller passed: a handler can disagree with its caller in argument type,
> count or register index without a diagnostic.
> 
> Generate the unmarshalling instead. DEFINE_KVM_HOST_HCALL() expands to
> the handle_<name>() glue, modelled on the syscall wrappers, and checks
> the handler's parameter list against the signature declared in
> kvm_hcall.h, so both ends of every hypercall are now compiled against
> the same declaration. Handler bodies keep their logic and lose the
> DECLARE_REG() and return-register boilerplate. The compiled handlers
> are instruction-for-instruction identical, apart from flush_hyp_vcpu()
> and sync_hyp_vcpu() now being inlined into their only caller.
> 
> Assisted-by: Antigravity:gemini-3.1-pro
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
> ---
>  arch/arm64/include/asm/kvm_hcall.h |  19 +-
>  arch/arm64/kvm/hyp/nvhe/hyp-main.c | 421 ++++++++++++++---------------
>  2 files changed, 214 insertions(+), 226 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_hcall.h b/arch/arm64/include/asm/kvm_hcall.h
> index 1c9c182ddcd1b..39374bae4bea4 100644
> --- a/arch/arm64/include/asm/kvm_hcall.h
> +++ b/arch/arm64/include/asm/kvm_hcall.h
> @@ -44,6 +44,8 @@ struct vgic_v5_cpu_if;
>  #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_LONG(t, a)	unsigned long a
> +#define __KVM_HCALL_CAST(t, a)	(__force t) a
>  #define __KVM_HCALL_ARGS(t, a)	a
>  
>  #ifndef __KVM_NVHE_HYPERVISOR__
> @@ -127,10 +129,19 @@ struct vgic_v5_cpu_if;
>  #define kvm_call_hyp_ret(f, ...) f(__VA_ARGS__)
>  #define kvm_call_hyp_nvhe(f, ...) f(__VA_ARGS__)
>  
> -#define DECLARE_KVM_HOST_HCALL(ret, name, ...)
> -#define DECLARE_KVM_HOST_HCALL_VOID(name, ...)
> -#define DECLARE_KVM_HOST_HCALL0(ret, name)
> -#define DECLARE_KVM_HOST_HCALL0_VOID(name)
> +/*
> + * At EL2 each declaration emits the canonical signature of the hypercall,
> + * which DEFINE_KVM_HOST_HCALL() in hyp-main.c checks the handler
> + * definition against.
> + */
> +#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. */
> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> index 23cb4313c60a2..6986ce55fd9ef 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> @@ -24,6 +24,63 @@
>  
>  DEFINE_PER_CPU(struct kvm_nvhe_init_params, kvm_init_params);
>  
> +/*
> + * Define a hypercall handler: handle_<name> unmarshals the arguments from
> + * the host context and hands them, correctly typed, to the body that
> + * follows the macro. The parameter list is type-checked against the
> + * signature declared in <asm/kvm_hcall.h>, so the handler cannot drift
> + * from what the typed caller stubs marshal in. Modelled on the syscall
> + * wrappers.
> + */
> +/* Truncate the fixed list of argument registers to the declared signature. */
> +#define KVM_HOST_HCALL_REGS(...)					\
> +	__KVM_HCALL_MAP_N(COUNT_ARGS(__VA_ARGS__), __KVM_HCALL_ARGS	\
> +		,, cpu_reg(host_ctxt, 1),, cpu_reg(host_ctxt, 2)	\
> +		,, cpu_reg(host_ctxt, 3),, cpu_reg(host_ctxt, 4)	\
> +		,, cpu_reg(host_ctxt, 5),, cpu_reg(host_ctxt, 6))
> +
> +#define DEFINE_KVM_HOST_HCALL(ret, name, ...)				\
> +	static kvm_host_hcall_sig_##name __do_##name;			\
> +	static __always_inline						\
> +	ret __se_##name(__KVM_HCALL_MAP(__KVM_HCALL_LONG, __VA_ARGS__))	\
> +	{								\
> +		return __do_##name(__KVM_HCALL_MAP(__KVM_HCALL_CAST, __VA_ARGS__)); \
> +	}								\
> +	static void handle_##name(struct kvm_cpu_context *host_ctxt)	\
> +	{								\
> +		cpu_reg(host_ctxt, 1) = __se_##name(KVM_HOST_HCALL_REGS(__VA_ARGS__)); \
> +	}								\
> +	static ret __do_##name(__KVM_HCALL_MAP(__KVM_HCALL_DECL, __VA_ARGS__))
> +
> +#define DEFINE_KVM_HOST_HCALL_VOID(name, ...)				\
> +	static kvm_host_hcall_sig_##name __do_##name;			\
> +	static __always_inline						\
> +	void __se_##name(__KVM_HCALL_MAP(__KVM_HCALL_LONG, __VA_ARGS__)) \
> +	{								\
> +		__do_##name(__KVM_HCALL_MAP(__KVM_HCALL_CAST, __VA_ARGS__)); \
> +	}								\
> +	static void handle_##name(struct kvm_cpu_context *host_ctxt)	\
> +	{								\
> +		__se_##name(KVM_HOST_HCALL_REGS(__VA_ARGS__));		\
> +	}								\
> +	static void __do_##name(__KVM_HCALL_MAP(__KVM_HCALL_DECL, __VA_ARGS__))
> +

I've also dropped this with the following hacks. The set_cpu_reg()
stuff isn't brilliant, and requires a single word type. But I find the
overall scheme less invasive.

WDYT?

	M.

diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index f8ea1661b3166..0d5fa6432461d 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -39,6 +39,12 @@ DEFINE_PER_CPU(struct kvm_nvhe_init_params, kvm_init_params);
 		,, cpu_reg(host_ctxt, 3),, cpu_reg(host_ctxt, 4)	\
 		,, cpu_reg(host_ctxt, 5),, cpu_reg(host_ctxt, 6))
 
+#define set_cpu_reg_ulong(ctxt, r, v)	{ cpu_reg(ctxt, r) = v; }
+#define set_cpu_reg_u64(ctxt, r, v)	{ cpu_reg(ctxt, r) = v; }
+#define set_cpu_reg_int(ctxt, r, v)	{ cpu_reg(ctxt, r) = v; }
+#define set_cpu_reg_void(ctxt, r, v)	{ v; }
+#define set_cpu_reg(ctxt, r, t, v)	set_cpu_reg_##t(ctxt, r, v)
+
 #define DEFINE_KVM_HOST_HCALL(ret, name, ...)				\
 	static kvm_host_hcall_sig_##name __do_##name;			\
 	static __always_inline						\
@@ -48,39 +54,18 @@ DEFINE_PER_CPU(struct kvm_nvhe_init_params, kvm_init_params);
 	}								\
 	static void handle_##name(struct kvm_cpu_context *host_ctxt)	\
 	{								\
-		cpu_reg(host_ctxt, 1) = __se_##name(KVM_HOST_HCALL_REGS(__VA_ARGS__)); \
+		set_cpu_reg(host_ctxt, 1, ret, __se_##name(KVM_HOST_HCALL_REGS(__VA_ARGS__))); \
 	}								\
 	static ret __do_##name(__KVM_HCALL_MAP(__KVM_HCALL_DECL, __VA_ARGS__))
 
-#define DEFINE_KVM_HOST_HCALL_VOID(name, ...)				\
-	static kvm_host_hcall_sig_##name __do_##name;			\
-	static __always_inline						\
-	void __se_##name(__KVM_HCALL_MAP(__KVM_HCALL_LONG, __VA_ARGS__)) \
-	{								\
-		__do_##name(__KVM_HCALL_MAP(__KVM_HCALL_CAST, __VA_ARGS__)); \
-	}								\
-	static void handle_##name(struct kvm_cpu_context *host_ctxt)	\
-	{								\
-		__se_##name(KVM_HOST_HCALL_REGS(__VA_ARGS__));		\
-	}								\
-	static void __do_##name(__KVM_HCALL_MAP(__KVM_HCALL_DECL, __VA_ARGS__))
-
 #define DEFINE_KVM_HOST_HCALL0(ret, name)				\
 	static kvm_host_hcall_sig_##name __do_##name;			\
 	static void handle_##name(struct kvm_cpu_context *host_ctxt)	\
 	{								\
-		cpu_reg(host_ctxt, 1) = __do_##name();			\
+		set_cpu_reg(host_ctxt, 1, ret, __do_##name());		\
 	}								\
 	static ret __do_##name(void)
 
-#define DEFINE_KVM_HOST_HCALL0_VOID(name)				\
-	static kvm_host_hcall_sig_##name __do_##name;			\
-	static void handle_##name(struct kvm_cpu_context *host_ctxt)	\
-	{								\
-		__do_##name();						\
-	}								\
-	static void __do_##name(void)
-
 /* Number of implemented GICv3 LRs. Used by flush_hyp_vcpu(). */
 unsigned int hyp_gicv3_nr_lr;
 
@@ -242,7 +227,7 @@ static void sync_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
 		host_cpu_if->vgic_lr[i] = hyp_cpu_if->vgic_lr[i];
 }
 
-DEFINE_KVM_HOST_HCALL_VOID(__pkvm_vcpu_load,
+DEFINE_KVM_HOST_HCALL(void, __pkvm_vcpu_load,
 	pkvm_handle_t, handle, unsigned int, vcpu_idx, u64, hcr_el2)
 {
 	struct pkvm_hyp_vcpu *hyp_vcpu;
@@ -261,7 +246,7 @@ DEFINE_KVM_HOST_HCALL_VOID(__pkvm_vcpu_load,
 	}
 }
 
-DEFINE_KVM_HOST_HCALL0_VOID(__pkvm_vcpu_put)
+DEFINE_KVM_HOST_HCALL0(void, __pkvm_vcpu_put)
 {
 	struct pkvm_hyp_vcpu *hyp_vcpu = pkvm_get_loaded_hyp_vcpu();
 
@@ -421,42 +406,42 @@ DEFINE_KVM_HOST_HCALL(int, __pkvm_host_mkyoung_guest,
 	return __pkvm_host_mkyoung_guest(gfn, hyp_vcpu);
 }
 
-DEFINE_KVM_HOST_HCALL_VOID(__kvm_adjust_pc,
+DEFINE_KVM_HOST_HCALL(void, __kvm_adjust_pc,
 	struct kvm_vcpu __kern *, vcpu)
 {
 	__kvm_adjust_pc(kern_hyp_va_host(vcpu));
 }
 
-DEFINE_KVM_HOST_HCALL0_VOID(__kvm_flush_vm_context)
+DEFINE_KVM_HOST_HCALL0(void, __kvm_flush_vm_context)
 {
 	__kvm_flush_vm_context();
 }
 
-DEFINE_KVM_HOST_HCALL_VOID(__kvm_tlb_flush_vmid_ipa,
+DEFINE_KVM_HOST_HCALL(void, __kvm_tlb_flush_vmid_ipa,
 	struct kvm_s2_mmu __kern *, mmu, phys_addr_t, ipa, int, level)
 {
 	__kvm_tlb_flush_vmid_ipa(kern_hyp_va_host(mmu), ipa, level);
 }
 
-DEFINE_KVM_HOST_HCALL_VOID(__kvm_tlb_flush_vmid_ipa_nsh,
+DEFINE_KVM_HOST_HCALL(void, __kvm_tlb_flush_vmid_ipa_nsh,
 	struct kvm_s2_mmu __kern *, mmu, phys_addr_t, ipa, int, level)
 {
 	__kvm_tlb_flush_vmid_ipa_nsh(kern_hyp_va_host(mmu), ipa, level);
 }
 
-DEFINE_KVM_HOST_HCALL_VOID(__kvm_tlb_flush_vmid_range,
+DEFINE_KVM_HOST_HCALL(void, __kvm_tlb_flush_vmid_range,
 	struct kvm_s2_mmu __kern *, mmu, phys_addr_t, start, unsigned long, pages)
 {
 	__kvm_tlb_flush_vmid_range(kern_hyp_va_host(mmu), start, pages);
 }
 
-DEFINE_KVM_HOST_HCALL_VOID(__kvm_tlb_flush_vmid,
+DEFINE_KVM_HOST_HCALL(void, __kvm_tlb_flush_vmid,
 	struct kvm_s2_mmu __kern *, mmu)
 {
 	__kvm_tlb_flush_vmid(kern_hyp_va_host(mmu));
 }
 
-DEFINE_KVM_HOST_HCALL_VOID(__pkvm_tlb_flush_vmid,
+DEFINE_KVM_HOST_HCALL(void, __pkvm_tlb_flush_vmid,
 	pkvm_handle_t, handle)
 {
 	struct pkvm_hyp_vm *hyp_vm = get_np_pkvm_hyp_vm(handle);
@@ -468,19 +453,19 @@ DEFINE_KVM_HOST_HCALL_VOID(__pkvm_tlb_flush_vmid,
 	put_pkvm_hyp_vm(hyp_vm);
 }
 
-DEFINE_KVM_HOST_HCALL_VOID(__kvm_flush_cpu_context,
+DEFINE_KVM_HOST_HCALL(void, __kvm_flush_cpu_context,
 	struct kvm_s2_mmu __kern *, mmu)
 {
 	__kvm_flush_cpu_context(kern_hyp_va_host(mmu));
 }
 
-DEFINE_KVM_HOST_HCALL_VOID(__kvm_timer_set_cntvoff,
+DEFINE_KVM_HOST_HCALL(void, __kvm_timer_set_cntvoff,
 	u64, cntvoff)
 {
 	__kvm_timer_set_cntvoff(cntvoff);
 }
 
-DEFINE_KVM_HOST_HCALL0_VOID(__kvm_enable_ssbs)
+DEFINE_KVM_HOST_HCALL0(void, __kvm_enable_ssbs)
 {
 	u64 tmp;
 
@@ -494,18 +479,18 @@ DEFINE_KVM_HOST_HCALL0(u64, __vgic_v3_get_gic_config)
 	return __vgic_v3_get_gic_config();
 }
 
-DEFINE_KVM_HOST_HCALL0_VOID(__vgic_v3_init_lrs)
+DEFINE_KVM_HOST_HCALL0(void, __vgic_v3_init_lrs)
 {
 	__vgic_v3_init_lrs();
 }
 
-DEFINE_KVM_HOST_HCALL_VOID(__vgic_v3_save_aprs,
+DEFINE_KVM_HOST_HCALL(void, __vgic_v3_save_aprs,
 	struct vgic_v3_cpu_if __kern *, cpu_if)
 {
 	__vgic_v3_save_aprs(kern_hyp_va_host(cpu_if));
 }
 
-DEFINE_KVM_HOST_HCALL_VOID(__vgic_v3_restore_vmcr_aprs,
+DEFINE_KVM_HOST_HCALL(void, __vgic_v3_restore_vmcr_aprs,
 	struct vgic_v3_cpu_if __kern *, cpu_if)
 {
 	__vgic_v3_restore_vmcr_aprs(kern_hyp_va_host(cpu_if));
@@ -541,7 +526,7 @@ DEFINE_KVM_HOST_HCALL(int, __pkvm_host_unshare_hyp,
 	return __pkvm_host_unshare_hyp(pfn);
 }
 
-DEFINE_KVM_HOST_HCALL(unsigned long, __pkvm_create_private_mapping,
+DEFINE_KVM_HOST_HCALL(ulong, __pkvm_create_private_mapping,
 	phys_addr_t, phys, size_t, size, u64, prot)
 {
 	/*
@@ -554,7 +539,7 @@ DEFINE_KVM_HOST_HCALL(unsigned long, __pkvm_create_private_mapping,
 	 * Instead pass the allocation address as the return value (or return
 	 * ERR_PTR() on failure).
 	 */
-	unsigned long haddr;
+	ulong haddr;
 	int err = __pkvm_create_private_mapping(phys, size, prot, &haddr);
 
 	if (err)
@@ -573,7 +558,7 @@ DEFINE_KVM_HOST_HCALL0(int, __pkvm_reserve_vm)
 	return __pkvm_reserve_vm();
 }
 
-DEFINE_KVM_HOST_HCALL_VOID(__pkvm_unreserve_vm,
+DEFINE_KVM_HOST_HCALL(void, __pkvm_unreserve_vm,
 	pkvm_handle_t, handle)
 {
 	__pkvm_unreserve_vm(handle);
@@ -630,7 +615,7 @@ DEFINE_KVM_HOST_HCALL(int, __tracing_load,
 	return __tracing_load(desc_hva, desc_size);
 }
 
-DEFINE_KVM_HOST_HCALL0_VOID(__tracing_unload)
+DEFINE_KVM_HOST_HCALL0(void, __tracing_unload)
 {
 	__tracing_unload();
 }
@@ -647,7 +632,7 @@ DEFINE_KVM_HOST_HCALL(int, __tracing_swap_reader,
 	return __tracing_swap_reader(cpu);
 }
 
-DEFINE_KVM_HOST_HCALL_VOID(__tracing_update_clock,
+DEFINE_KVM_HOST_HCALL(void, __tracing_update_clock,
 	u32, mult, u32, shift, u64, epoch_ns, u64, epoch_cyc)
 {
 	__tracing_update_clock(mult, shift, epoch_ns, epoch_cyc);
@@ -665,19 +650,19 @@ DEFINE_KVM_HOST_HCALL(int, __tracing_enable_event,
 	return __tracing_enable_event(id, enable);
 }
 
-DEFINE_KVM_HOST_HCALL_VOID(__tracing_write_event,
+DEFINE_KVM_HOST_HCALL(void, __tracing_write_event,
 	u64, id)
 {
 	trace_selftest(id);
 }
 
-DEFINE_KVM_HOST_HCALL_VOID(__vgic_v5_save_apr,
+DEFINE_KVM_HOST_HCALL(void, __vgic_v5_save_apr,
 	struct vgic_v5_cpu_if __kern *, cpu_if)
 {
 	__vgic_v5_save_apr(kern_hyp_va_host(cpu_if));
 }
 
-DEFINE_KVM_HOST_HCALL_VOID(__vgic_v5_restore_vmcr_apr,
+DEFINE_KVM_HOST_HCALL(void, __vgic_v5_restore_vmcr_apr,
 	struct vgic_v5_cpu_if __kern *, cpu_if)
 {
 	__vgic_v5_restore_vmcr_apr(kern_hyp_va_host(cpu_if));

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