Re: [PATCH v2 10/17] KVM: arm64: Prevent host PC adjustments for protected vCPUs

[Fuad Tabba]

Hi Marc,

...

> So this is the sole reason why we need to access the kvm structure,
> and the cause for all of this churn. Can't this be written as some
> form of:
>
>     kvm = vcpu_get_kvm(vcpu);
>
>     [do whatever is needed]
>
>     vcpu_put_kvm(vcpu, kvm);
>
> where get/put do the pinning/unpinning as required? These helpers
> would become NOPs on VHE, for example.

I've put the get/put pair one level up instead, in __kvm_adjust_pc()
itself: pkvm_adjust_pc_get() returns the vCPU to adjust and
pkvm_adjust_pc_put() copies PC_UPDATE_REQ back, both no-ops outside
the nVHE build and outside protected mode. The kvm read goes back to
the base's kern_hyp_va(vcpu->kvm), and exception.c keeps its
signatures.

> And this stuff could simply be a similar set of helpers in
> __kvm_adjust_pc(), the original one.

That's where it lives now, and this patch no longer touches
handle___kvm_adjust_pc().

> > +    /* The host copy is authoritative, used only while pinned. */
> > +    if (hyp_pin_shared_mem(host_vcpu, host_vcpu + 1))
> > +        return;
>
> Huh. Really?

After taking a closer look I realized that we have a live bug, and
this is actually a fix. The base dereferences the host vCPU unpinned
in this handler, and the vCPU and its VM are mapped at EL2 only once
pinned, at the first KVM_RUN. KVM_SET_VCPU_EVENTS with
ext_dabt_pending gets to __kvm_adjust_pc() before that, and the
hypervisor panics on the read. Any process with /dev/kvm on a pKVM
host can trigger it, since 6.17 (efa1368ba9f4b). I'll send it as a
separate fix before sending V3 (later today).

Cheers,
/fuad