Re: [PATCH v2 08/36] KVM: arm64: Introduce kvm_call_hyp_nvhe_res()

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Thu, 8 Jan 2026 09:48:39 +0000
Sascha Bischoff <Sascha.Bischoff@arm.com> wrote:

> On Wed, 2026-01-07 at 10:30 +0000, Jonathan Cameron wrote:
> > On Fri, 19 Dec 2025 15:52:38 +0000
> > Sascha Bischoff <Sascha.Bischoff@arm.com> wrote:
> >   
> > > There are times when it is desirable to have more than one return
> > > value for a hyp call. Split out kvm_call_hyp_nvhe_res from
> > > kvm_call_hyp_nvhe such that it is possible to directly provide
> > > struct
> > > arm_smccc_res from the calling code. Thereby, additional return
> > > values
> > > can be stored in res.a2, etc.
> > > 
> > > Suggested-by: Marc Zyngier <maz@kernel.org>
> > > Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>  
> > One question inline, mostly because I'm curious rather than a
> > suggestion
> > to change anything
> > 
> > Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> >   
> > > ---
> > >  arch/arm64/include/asm/kvm_host.h | 15 ++++++++++-----
> > >  1 file changed, 10 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/arch/arm64/include/asm/kvm_host.h
> > > b/arch/arm64/include/asm/kvm_host.h
> > > index b552a1e03848c..971b153b0a3fa 100644
> > > --- a/arch/arm64/include/asm/kvm_host.h
> > > +++ b/arch/arm64/include/asm/kvm_host.h
> > > @@ -1208,14 +1208,19 @@ void kvm_arm_resume_guest(struct kvm *kvm);
> > >  #define vcpu_has_run_once(vcpu)	(!!READ_ONCE((vcpu)->pid))
> > >  
> > >  #ifndef __KVM_NVHE_HYPERVISOR__
> > > -#define kvm_call_hyp_nvhe(f,
> > > ...)						\
> > > +#define kvm_call_hyp_nvhe_res(res, f,
> > > ...)				\
> > >  	({							
> > > 	\
> > > -		struct arm_smccc_res
> > > res;				\
> > > -
> > > 									\
> > > +		struct arm_smccc_res *__res =
> > > res;			\  
> > 
> > What's the purpose of the local variable? Type sanity check?
> > Feels like typecheck() would make the intent clearer.
> > Meh. Not used anywhere in arch/arm64 so maybe not.  
> 
> Far less exciting, I'm afraid.
> 
> It is because of the text substitution that happens. The res here is
> replaced with &res from kvm_call_hyp_nvhe() and we end up with &res-
> >a0. Given that -> binds more tightly than &, we end up with the wrong  
> thing, and the compiler barfs.
> 
> An alternative would be to do:
> 
> 	WARN_ON((res)->a0 != SMCCC_RET_SUCCESS);
> 
> This removes the need for the local variable. I've got no preference
> for which version we go with.

Ah fair enough.  Just leave it as it is.

Jonathan

> 
> Sascha
> 
> > 
> > 
> >   
> > >  		arm_smccc_1_1_hvc(KVM_HOST_SMCCC_FUNC(f),	
> > > 	\
> > > -				  ##__VA_ARGS__,
> > > &res);			\
> > > -		WARN_ON(res.a0 !=
> > > SMCCC_RET_SUCCESS);			\
> > > +				  ##__VA_ARGS__,
> > > __res);		\
> > > +		WARN_ON(__res->a0 !=
> > > SMCCC_RET_SUCCESS);		\
> > > +	})
> > > +
> > > +#define kvm_call_hyp_nvhe(f,
> > > ...)					\
> > > +	({							
> > > 	\
> > > +		struct arm_smccc_res
> > > res;				\
> > >  								
> > > 	\
> > > +		kvm_call_hyp_nvhe_res(&res, f,
> > > ##__VA_ARGS__);		\
> > >  		res.a1;					
> > > 		\
> > >  	})
> > >    
> >   
>