Re: [PATCH v7 7/10] KVM: arm/arm64: context-switch ptrauth registers

[Amit Daniel Kachhap]

On 3/26/19 11:31 PM, Kristina Martsenko wrote:
> On 26/03/2019 04:03, Amit Daniel Kachhap wrote:
> > Hi,
> >
> > On 3/26/19 1:34 AM, Kristina Martsenko wrote:
> > > On 19/03/2019 08:30, Amit Daniel Kachhap wrote:
> > > > From: Mark Rutland <mark.rutland@xxxxxxx>
> > > >
> > > > When pointer authentication is supported, a guest may wish to use it.
> > > > This patch adds the necessary KVM infrastructure for this to work, with
> > > > a semi-lazy context switch of the pointer auth state.
> > >
> > > [...]
> > >
> > > > +ÂÂÂ if (test_bit(KVM_ARM_VCPU_PTRAUTH_ADDRESS, vcpu->arch.features) ||
> > > > +ÂÂÂÂÂÂÂ test_bit(KVM_ARM_VCPU_PTRAUTH_GENERIC, vcpu->arch.features)) {
> > > > +ÂÂÂÂÂÂÂ /* Verify that KVM startup matches the conditions for ptrauth */
> > > > +ÂÂÂÂÂÂÂ if (WARN_ON(!vcpu_has_ptrauth(vcpu)))
> > > > +ÂÂÂÂÂÂÂÂÂÂÂ return -EINVAL;
> > > > +ÂÂÂ }
> > >
> > > I think this now needs to have "goto out;" instead of "return -EINVAL;",
> > > since 5.1-rcX contains commit e761a927bc9a ("KVM: arm/arm64: Reset the
> > > VCPU without preemption and vcpu state loaded") which changed some of
> > > this code.
> > ok missed the changes for this commit.
>
> One more thing - I think the WARN_ON() here should be removed. Otherwise
> if panic_on_warn is set then userspace can panic the kernel. I think
> WARN_ON is only for internal kernel errors (see comment in
> include/asm-generic/bug.h).

The documentation makes sense so in this case a pr_err like message 
will suffice. Btw there is one WARN in the function kvm_set_ipa_limit in 
the same file.

Thanks,
Amit D
> Thanks,
> Kristina