Re: [PATCH v2 04/18] KVM: arm64: Add a heap allocator for the pKVM hyp

[Vincent Donnefort <vdonnefort@google.com>]

On Wed, Jul 15, 2026 at 11:59:26AM +0100, Fuad Tabba wrote:
> On Tue, 14 Jul 2026 at 19:35, Fuad Tabba <fuad.tabba@linux.dev> wrote:
> >
> > On Mon, 6 Jul 2026 at 18:54, Vincent Donnefort <vdonnefort@google.com> wrote:
> >
> > ...
> >
> > > +static u32 chunk_hash_compute(const struct chunk_hdr *chunk)
> > > +{
> > > +       u32 hash = 0;
> > > +
> > > +       BUILD_BUG_ON(sizeof(*chunk) != 16);
> > > +
> > > +       hash ^= hash_64(*(const u64 *)chunk, 32);
> > > +       hash ^= hash_32(chunk->__unmapped, 32);
> >
> > Could you state the threat model in the commit msg? The VA range is
> > hyp-private and the hash is unkeyed (anything that can write EL2
> > memory recomputes it), so this only catches _accidental_ corruption,
> > i.e. a debug aid.
> >
> > Since that's the goal, should it be gated on CONFIG_NVHE_EL2_DEBUG
> > like the other checks here? As written it's O(n) hashing per
> > hyp_alloc(), in production, for a check that only fires on a hyp bug.
> > And a plain XOR/additive checksum would be cheaper and just as good
> > for accidental corruption.
> 
> Thinking about it, I was wrong to say that it would be just as good,
> slightly cheaper, but it would detect things like swaps.
> 
> Still, I think this should be gated.

After a longer offline discussion, we concluded this doesn't cost anything while
the weak protection can still be effective against attackers who may not always
control the entire bit-width.

I'll add some details in the commit message.

> 
> Cheers,
> /fuad
> 
> >
> > > +       return hash;
> > > +}
> > > +
> >
> > ...
> >
> >
> > > diff --git a/arch/arm64/kvm/hyp/nvhe/setup.c b/arch/arm64/kvm/hyp/nvhe/setup.c
> > > index 75b00c323310..1c10cf78c869 100644
> > > --- a/arch/arm64/kvm/hyp/nvhe/setup.c
> > > +++ b/arch/arm64/kvm/hyp/nvhe/setup.c
> > > @@ -10,6 +10,7 @@
> > >  #include <asm/kvm_pgtable.h>
> > >  #include <asm/kvm_pkvm.h>
> > >
> > > +#include <nvhe/alloc.h>
> > >  #include <nvhe/early_alloc.h>
> > >  #include <nvhe/ffa.h>
> > >  #include <nvhe/gfp.h>
> > > @@ -363,6 +364,10 @@ int __pkvm_init(phys_addr_t phys, unsigned long size, unsigned long *per_cpu_bas
> > >         if (ret)
> > >                 return ret;
> > >
> > > +       ret = hyp_alloc_init(SZ_128M);
> >
> > Where does 128M come from? A comment would help.

This is arbitrary, big enough to allow us to allocate plenty of VMs with tracing
enable. But I will add something.

> >
> > Cheers,
> > /fuad
> >
> >
> >
> > > +       if (ret)
> > > +               return ret;
> > > +
> > >         update_nvhe_init_params();
> > >
> > >         /* Jump in the idmap page to switch to the new page-tables */
> > > --
> > > 2.55.0.rc2.803.g1fd1e6609c-goog
> > >