Re: [PATCH v5 03/22] KVM: arm64: Support SDEI_VERSION hypercall

[Gavin Shan]

Hi Oliver,

On 3/23/22 2:04 AM, Oliver Upton wrote:
> On Tue, Mar 22, 2022 at 04:06:51PM +0800, Gavin Shan wrote:
> > This supports SDEI_VERSION hypercall by returning v1.1, which is
> > the specification version we're following. The vendor is set to
> > 'KVM'.
> >
> > Signed-off-by: Gavin Shan <gshan@xxxxxxxxxx>
> > ---
> >   arch/arm64/kvm/sdei.c | 10 ++++++++++
> >   1 file changed, 10 insertions(+)
> >
> > diff --git a/arch/arm64/kvm/sdei.c b/arch/arm64/kvm/sdei.c
> > index 8a9b477b8977..5a3a64cd6e84 100644
> > --- a/arch/arm64/kvm/sdei.c
> > +++ b/arch/arm64/kvm/sdei.c
> > @@ -118,6 +118,14 @@ static bool remove_all_vcpu_events(struct kvm_vcpu *vcpu,
> >   	return pending;
> >   }
> >   
> > +static unsigned long hypercall_version(struct kvm_vcpu *vcpu)
> > +{
> > +	/* v1.1 and the vendor is KVM */
> > +	return (1UL << SDEI_VERSION_MAJOR_SHIFT) |
> > +	       (1UL << SDEI_VERSION_MINOR_SHIFT) |
> > +	       0x4b564d;
>
> It looks like the SDEI specification states that the vendor-defined
> version number is 32 bits. Could we just use one of the
> ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_{0,3} values instead?
>
> ASCII 'KVM' is neat, but in reality guest software will just throw it in
> a macro regardless. Might as well use one of the values we've already
> trained it to use :-)
>
> Also, it would appear that guest discovery of SDEI relies upon KVM
> reporting a valid SDEI version. IMO, this patch should come at the very
> end when KVM actually implements SDEI.

Yeah, I was sticky to the pattern of "KVM". However, I think it's good
to reuse the existing one. Lets use ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_2
if you agree. Its first two characters are "VM" at least.

It's fine to return the version here because the SDEI capability isn't
exposed yet. It means no events have been exposed and able to be
registered. However, It's also fine to move this patch after the
following one:

[PATCH v5 16/22] KVM: arm64: Support SDEI_EVENT_{COMPLETE, COMPLETE_AND_RESUME} hypercall

Thanks,
Gavin