RE: [PATCH v5 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: Monday, January 27, 2025 5:06 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 v5 3/4] KVM: arm64: Report all the KVM/arm64-specific
> hypercalls
> 
> Hi Shameer,
> 
> On Fri, Jan 24, 2025 at 03:17:31PM +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..6132cb542200 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_MMIO_GUARD,
> > +				 ARM_SMCCC_KVM_FUNC_FEATURES);
> > +		/* Function numbers 8-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. The pKVM carveout exists for some KVM-internal
> bookkeeping to (hopefully) avoid breaking guest ABI between what's in the
> downstream android kernel and what eventually gets accepted upstream.
> 
> The purpose of this hypercall is for the guest to discover what interfaces are
> actually implemented by the hypervisor, and we definitely do not
> implement these upstream at the moment.

Ok. I had some confusion on pKVM ones as I could see the kvm_arm_hyp_service_available()
being called with those in arm-pkvm-guest.c. So that explains it.
.
I will remove the pKVM ones and just add support to report the ones introduced
in this series.

Thanks,
Shameer