[PATCH v1] KVM: arm64: Tidying up PAuth code in KVM

[Fuad Tabba]

Hi Marc,

On Mon, 22 Jul 2024 at 16:09, Marc Zyngier <maz at kernel.org> wrote:
>
> Hi Fuad,
>
> On Mon, 22 Jul 2024 13:37:40 +0100,
> Fuad Tabba <tabba at google.com> wrote:
> >
> > Tidy up some of the PAuth trapping code to clear up some comments
> > and avoid clang/checkpatch warnings. Also, do not bother setting
> > the PAuth HCR_EL2 bits for protected VMs in pKVM, since that is
> > handled by the hypervisor.
> >
> > Fixes: 814ad8f96e92 ("KVM: arm64: Drop trapping of PAuth instructions/keys")
>
> nit: AFAICT, this doesn't really fix anything. It has no material
> impact on the guest or the hypervisor.

Got it.

> > Signed-off-by: Fuad Tabba <tabba at google.com>
> > ---
> >  arch/arm64/include/asm/kvm_ptrauth.h    | 2 +-
> >  arch/arm64/kvm/arm.c                    | 7 ++++---
> >  arch/arm64/kvm/hyp/include/hyp/switch.h | 1 -
> >  arch/arm64/kvm/hyp/nvhe/switch.c        | 5 ++---
> >  4 files changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/kvm_ptrauth.h b/arch/arm64/include/asm/kvm_ptrauth.h
> > index d81bac256abc..6199c9f7ec6e 100644
> > --- a/arch/arm64/include/asm/kvm_ptrauth.h
> > +++ b/arch/arm64/include/asm/kvm_ptrauth.h
> > @@ -104,7 +104,7 @@ alternative_else_nop_endif
> >
> >  #define __ptrauth_save_key(ctxt, key)                                        \
> >       do {                                                            \
> > -             u64 __val;                                              \
> > +             u64 __val;                                              \
> >               __val = read_sysreg_s(SYS_ ## key ## KEYLO_EL1);        \
> >               ctxt_sys_reg(ctxt, key ## KEYLO_EL1) = __val;           \
> >               __val = read_sysreg_s(SYS_ ## key ## KEYHI_EL1);        \
> > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> > index 59716789fe0f..6516348024ba 100644
> > --- a/arch/arm64/kvm/arm.c
> > +++ b/arch/arm64/kvm/arm.c
> > @@ -510,10 +510,10 @@ void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu)
> >
> >  static void vcpu_set_pauth_traps(struct kvm_vcpu *vcpu)
> >  {
> > -     if (vcpu_has_ptrauth(vcpu)) {
> > +     if (vcpu_has_ptrauth(vcpu) && !vcpu_is_protected(vcpu)) {
>
> I think this isn't quite correct.
>
> Non-protected VMs in protected mode are still subjected to pKVM's own
> handling of the HCR_EL2 configuration, and the whole thing should be
> skipped altogether in that case, irrespective of the pauth status of
> the vcpu.
>
> What pKVM should evaluate is that status and decide for itself whether
> it must enable it or not. You can then hoist the check for protected
> mode early and skip the whole function unconditionally, irrespective
> of the protected status of the vcpu.

What pKVM does right now is use the host value of hcr_el2 as a base,
and ensure that the HCR_GUEST_FLAGS are also set. That said, it
doesn't do that for MDCR_EL2 nor for CPTR_EL2. Thinking about it, I
think it makes more sense for HCR_EL2 in pKVM to be initialized
completely at EL2, and hoist the check, like you're suggesting.

I'll fix that (in this patch and in pKVM), and resend.

Thanks!
/fuad

> Thanks,
>
>         M.
>
> --
> Without deviation from the norm, progress is not possible.