[PATCH v2 3/4] KVM: arm64: Factor out pKVM hyp vcpu creation to separate function

[Fuad Tabba]

Hi Quentin,

On Fri, 28 Feb 2025 at 19:44, Quentin Perret <qperret at google.com> wrote:
>
> On Wednesday 26 Feb 2025 at 21:55:19 (+0000), Fuad Tabba wrote:
> >  static int __pkvm_create_hyp_vm(struct kvm *host_kvm)
> >  {
> > -     size_t pgd_sz, hyp_vm_sz, hyp_vcpu_sz;
> > +     size_t pgd_sz, hyp_vm_sz;
> >       struct kvm_vcpu *host_vcpu;
> > -     pkvm_handle_t handle;
> >       void *pgd, *hyp_vm;
> >       unsigned long idx;
> >       int ret;
> > @@ -161,33 +178,12 @@ static int __pkvm_create_hyp_vm(struct kvm *host_kvm)
> >       if (ret < 0)
> >               goto free_vm;
> >
> > -     handle = ret;
> > +     WRITE_ONCE(host_kvm->arch.pkvm.handle, ret);
>
> What's the reason to make this a WRITE_ONCE? Does it mean we should
> update the readers to be READ_ONCE()?

I don't remember the original reason, to be honest. In this case, it
was to make it consistent with downstream code in Android. That said,
I plan on revising all of these soon and fixing this (and related
code) in light of Will's comment regarding potential specter gadgets:

https://lore.kernel.org/all/20250218092705.GA17030@willie-the-truck/

Cheers,
/fuad

> > -     host_kvm->arch.pkvm.handle = handle;
> > -
> > -     /* Donate memory for the vcpus at hyp and initialize it. */
> > -     hyp_vcpu_sz = PAGE_ALIGN(PKVM_HYP_VCPU_SIZE);
> >       kvm_for_each_vcpu(idx, host_vcpu, host_kvm) {
> > -             void *hyp_vcpu;
> > -
> > -             /* Indexing of the vcpus to be sequential starting at 0. */
> > -             if (WARN_ON(host_vcpu->vcpu_idx != idx)) {
> > -                     ret = -EINVAL;
> > -                     goto destroy_vm;
> > -             }
> > -
> > -             hyp_vcpu = alloc_pages_exact(hyp_vcpu_sz, GFP_KERNEL_ACCOUNT);
> > -             if (!hyp_vcpu) {
> > -                     ret = -ENOMEM;
> > -                     goto destroy_vm;
> > -             }
> > -
> > -             ret = kvm_call_hyp_nvhe(__pkvm_init_vcpu, handle, host_vcpu,
> > -                                     hyp_vcpu);
> > -             if (ret) {
> > -                     free_pages_exact(hyp_vcpu, hyp_vcpu_sz);
> > +             ret = __pkvm_create_hyp_vcpu(host_vcpu);
> > +             if (ret)
> >                       goto destroy_vm;
> > -             }
> >       }
> >
> >       return 0;
> > --
> > 2.48.1.711.g2feabab25a-goog
> >