Re: [PATCH v2 14/18] KVM: arm64: Use noclear for PGD in __pkvm_init_vm error path

[Vincent Donnefort <vdonnefort@google.com>]

On Wed, Jul 15, 2026 at 01:09:12PM +0100, Fuad Tabba wrote:
> On Mon, 6 Jul 2026 at 18:54, Vincent Donnefort <vdonnefort@google.com> wrote:
> >
> > In the error path of __pkvm_init_vm(), use unmap_donated_memory_noclear()
> > instead of the clearing variant to release the donated stage-2 PGD back
> > to the host.
> >
> > This intends to eliminate the clearing variant of
> > unmap_donated_memory(), as zeroing the PGD memory before returning it to
> > the host is unnecessary in this failure path.
> >
> > Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
> >
> > diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> > index d9ab58226889..0c82f1ddb93d 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> > @@ -861,7 +861,7 @@ int __pkvm_init_vm(struct kvm *host_kvm, unsigned long vm_hva,
> >         kvm_guest_destroy_stage2(hyp_vm);
> >  err_remove_mappings:
> >         unmap_donated_memory(hyp_vm, vm_size);
> > -       unmap_donated_memory(pgd, pgd_size);
> > +       unmap_donated_memory_noclear(pgd, pgd_size);
> 
> 
> We spoke about this in person before you sent the series and I thought
> it was ok. Sashiko's review made me doubt at first, since the freed
> blocks carry EL2 list_head pointers that the noclear variant hands
> back to the host.
> 
> You mentioned that the mapping isn't secret (later when we discussed
> Sashiko's review in person), and I checked: kvm_compute_layout() runs
> at EL1 on the host and rolls the tag itself. The host already knows
> the PA of every page it donated, so it can compute those VAs anyway.
> So I don't think this is a real leak.
> 
> However, this relies on the pool and hyp_vm both being donated
> linear-map memory here. Once hyp_vm moves to hyp_alloc() in the
> private range, the free_area anchor becomes a private-range VA, and I
> haven't checked whether that's host-derivable.
> 
> Given it's the init error path and not hot, clearing costs basically
> nothing, so I'd lean towards just keeping unmap_donated_memory()
> rather than reasoning about derivability.
> 
> What do you think?

Happy to not take any risk and to drop it. It was just an attempt to simplify
the code anyway.

> 
> 
> 
> /fuad
> 
> >  err_unpin_kvm:
> >         hyp_unpin_shared_mem(host_kvm, host_kvm + 1);
> >         return ret;
> > --
> > 2.55.0.rc2.803.g1fd1e6609c-goog
> >