RE: [PATCH v6 3/4] KVM: arm64: Report all the KVM/arm64-specific hypercalls

[Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>]

> -----Original Message-----
> From: Oliver Upton <oliver.upton@linux.dev>
> Sent: Friday, February 7, 2025 6:24 PM
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: kvmarm@lists.linux.dev; maz@kernel.org; catalin.marinas@arm.com;
> will@kernel.org; mark.rutland@arm.com; cohuck@redhat.com;
> eric.auger@redhat.com; sebott@redhat.com; yuzenghui
> <yuzenghui@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>; Anthony Jebson
> <anthony.jebson@huawei.com>; linux-arm-kernel@lists.infradead.org;
> Linuxarm <linuxarm@huawei.com>
> Subject: Re: [PATCH v6 3/4] KVM: arm64: Report all the KVM/arm64-specific
> hypercalls
> 
> On Fri, Feb 07, 2025 at 10:21:13AM -0800, Oliver Upton wrote:
> > On Wed, Feb 05, 2025 at 01:22:21PM +0000, Shameer Kolothum wrote:
> > > Currently ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID returns
> the
> > > bitmap corresponding to KVM_REG_ARM_VENDOR_HYP_BMAP and it
> only
> > > returns _KVM_FEATURES_FUNC_ID and _KVM_PTP_FUNC_ID. Change
> that
> > > to return all the KVM/arm64-specific hypercalls exposed by
> > > KVM/arm64 to guest operating systems.
> > >
> > > Signed-off-by: Shameer Kolothum
> <shameerali.kolothum.thodi@huawei.com>
> > > ---
> > >  arch/arm64/kvm/hypercalls.c | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
> > > index 27ce4cb44904..5cef2590ffdf 100644
> > > --- a/arch/arm64/kvm/hypercalls.c
> > > +++ b/arch/arm64/kvm/hypercalls.c
> > > @@ -359,7 +359,11 @@ int kvm_smccc_call_handler(struct kvm_vcpu
> *vcpu)
> > >  		val[3] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_3;
> > >  		break;
> > >  	case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
> > > -		val[0] = smccc_feat->vendor_hyp_bmap;
> > > +		val[0] = GENMASK(ARM_SMCCC_KVM_FUNC_PTP,
> > > +				 ARM_SMCCC_KVM_FUNC_FEATURES);
> > > +		/* Function numbers 2-63 are reserved for pKVM for now */
> > > +		val[2] =
> GENMASK((ARM_SMCCC_KVM_FUNC_DISCOVER_IMPL_CPUS - 64),
> > > +
> (ARM_SMCCC_KVM_FUNC_DISCOVER_IMPL_VER - 64));
> > >  		break;
> >
> > This isn't right, vendor_hyp_bmap is very much load bearing. We have a
> > documented UAPI that allows userspace to control the hypercalls exposed
> > to the guest.
> >
> > The idea being a user wants kernel rollback safety and doesn't expose
> > hypercalls that could potentially be revoked.
> >
> > https://docs.kernel.org/virt/kvm/arm/fw-pseudo-registers.html#bitmap-
> feature-firmware-registers
> 
> To add:
> 
> KVM cannot advertise the DISCOVER_IMPL* stuff unconditionally, since the
> expectation is that userspace implements these hypercalls. These bits
> may need to be writable from userspace but have a reset value of 0.
> 

Ok. So IIUC, vendor_hyp_bmap actually holds the information which Vendor Hyp
services  are available to the user space and can be get/set using GET/SET _ONE_REG
interfaces.

Currently this bitmap is a 64 bit one and if we have to have a one to one mapping
between these bitmap and the hypercall function numbers, then that requires
some changes.

Because function numbers 2-63 are now reserved for pKVM and the new ones
introduced in this series take 64 & 65.

May be we can have KVM_REG_ARM_VENDOR_HYP_BMAP_2  which represents
64-127?

Or can we take the next available bits(2 & 3) for KVM_REG_ARM_VENDOR_HYP_BMAP
and then map it to the function number appropriately (64 & 65) when
ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID is handled?

Thoughts?

Thanks,
Shameer