[PATCH v13 2/9] arm/arm64: KVM: Advertise KVM UID to guests via SMCCC

[Will Deacon]

On Tue, Jul 28, 2020 at 01:07:14AM +0000, Jianyong Wu wrote:
> 
> 
> > -----Original Message-----
> > From: Will Deacon <will at kernel.org>
> > Sent: Monday, July 27, 2020 7:38 PM
> > To: Jianyong Wu <Jianyong.Wu at arm.com>
> > Cc: netdev at vger.kernel.org; yangbo.lu at nxp.com; john.stultz at linaro.org;
> > tglx at linutronix.de; pbonzini at redhat.com; sean.j.christopherson at intel.com;
> > maz at kernel.org; richardcochran at gmail.com; Mark Rutland
> > <Mark.Rutland at arm.com>; Suzuki Poulose <Suzuki.Poulose at arm.com>;
> > Steven Price <Steven.Price at arm.com>; linux-kernel at vger.kernel.org; linux-
> > arm-kernel at lists.infradead.org; kvmarm at lists.cs.columbia.edu;
> > kvm at vger.kernel.org; Steve Capper <Steve.Capper at arm.com>; Kaly Xin
> > <Kaly.Xin at arm.com>; Justin He <Justin.He at arm.com>; Wei Chen
> > <Wei.Chen at arm.com>; nd <nd at arm.com>
> > Subject: Re: [PATCH v13 2/9] arm/arm64: KVM: Advertise KVM UID to guests
> > via SMCCC
> > 
> > On Mon, Jul 27, 2020 at 03:45:37AM +0000, Jianyong Wu wrote:
> > > > From: Will Deacon <will at kernel.org>
> > > >
> > > > We can advertise ourselves to guests as KVM and provide a basic
> > > > features bitmap for discoverability of future hypervisor services.
> > > >
> > > > Cc: Marc Zyngier <maz at kernel.org>
> > > > Signed-off-by: Will Deacon <will at kernel.org>
> > > > Signed-off-by: Jianyong Wu <jianyong.wu at arm.com>
> > > > ---
> > > >  arch/arm64/kvm/hypercalls.c | 29 +++++++++++++++++++----------
> > > >  1 file changed, 19 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/kvm/hypercalls.c
> > > > b/arch/arm64/kvm/hypercalls.c index 550dfa3e53cd..db6dce3d0e23
> > > > 100644
> > > > --- a/arch/arm64/kvm/hypercalls.c
> > > > +++ b/arch/arm64/kvm/hypercalls.c
> > > > @@ -12,13 +12,13 @@
> > > >  int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)  {
> > > >  	u32 func_id = smccc_get_function(vcpu);
> > > > -	long val = SMCCC_RET_NOT_SUPPORTED;
> > > > +	u32 val[4] = {SMCCC_RET_NOT_SUPPORTED};
> > >
> > > There is a risk as this u32 value will return here and a u64 value
> > > will be obtained in guest. For example, The val[0] is initialized as
> > > -1 of 0xffffffff and the guest get 0xffffffff then it will be compared
> > > with -1 of 0xffffffffffffffff Also this problem exists for the
> > > transfer of address in u64 type. So the following assignment to "val"
> > > should be split into two
> > > u32 value and assign to val[0] and val[1] respectively.
> > > WDYT?
> > 
> > Yes, I think you're right that this is a bug, but isn't the solution just to make
> > that an array of 'long'?
> > 
> > 	long val [4];
> > 
> > That will sign-extend the negative error codes as required, while leaving the
> > explicitly unsigned UID constants alone.
> 
> Ok, that's much better. I will fix it at next version.
> 
> By the way, I wonder when will you update this patch set. I see someone like me
> adopt this patch set as code base and need rebase it every time, so expect your update.

I'm not working on it, so please feel free to include it along with the
patches that add an upstream user.

Will