Re: [PATCH v5 4/5] arm64/kvm: add a userspace option to enable pointer authentication

[Amit Daniel Kachhap]

Hi,

On 2/13/19 11:05 PM, Kristina Martsenko wrote:
> On 28/01/2019 06:58, Amit Daniel Kachhap wrote:
> > This feature will allow the KVM guest to allow the handling of
> > pointer authentication instructions or to treat them as undefined
> > if not set. It uses the existing vcpu API KVM_ARM_VCPU_INIT to
> > supply this parameter instead of creating a new API.
> >
> > A new register is not created to pass this parameter via
> > SET/GET_ONE_REG interface as just a flag (KVM_ARM_VCPU_PTRAUTH)
> > supplied is enough to enable this feature.
>
> [...]
>
> > diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> > index b200c14..b6950df 100644
> > --- a/arch/arm/include/asm/kvm_host.h
> > +++ b/arch/arm/include/asm/kvm_host.h
> > @@ -346,6 +346,10 @@ static inline int kvm_arm_have_ssbd(void)
> >   static inline void kvm_vcpu_load_sysregs(struct kvm_vcpu *vcpu) {}
> >   static inline void kvm_vcpu_put_sysregs(struct kvm_vcpu *vcpu) {}
> >   static inline void kvm_arm_vcpu_ptrauth_reset(struct kvm_vcpu *vcpu) {}
> > +static inline bool kvm_arm_vcpu_ptrauth_allowed(struct kvm_vcpu *vcpu)
> > +{
> > +	return false;
> > +}
>
> It seems like this is only ever called from arm64 code, so do we need an
> arch/arm/ definition?
Yes not required. Nice catch.
> > +/**
> > + * kvm_arm_vcpu_ptrauth_allowed - checks if ptrauth feature is present in vcpu
> > + *
> > + * @vcpu: The VCPU pointer
> > + *
> > + * This function will be used to enable/disable ptrauth in guest as configured
> > + * by the KVM userspace API.
> > + */
> > +bool kvm_arm_vcpu_ptrauth_allowed(struct kvm_vcpu *vcpu)
> > +{
> > +	return test_bit(KVM_ARM_VCPU_PTRAUTH, vcpu->arch.features);
> > +}
>
> I'm not sure, but should there also be something like
>
> if (test_bit(KVM_ARM_VCPU_PTRAUTH, vcpu->arch.features) &&
>      !kvm_supports_ptrauth())
> 	return -EINVAL;
>
> in kvm_reset_vcpu?
Yes makes sense. I missed it and with Dave martin patch this may be done 
cleanly.

Thanks,
Amit D

> Thanks,
> Kristina