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

[Oliver Upton <oliver.upton@linux.dev>]

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.

-- 
Thanks,
Oliver