Re: [PATCH v4 12/25] KVM: arm64: Add infrastructure to create and track pKVM instances at EL2

Will Deacon
2022-10-19T12:45:44Z

On Tue, Oct 18, 2022 at 04:40:11PM +0000, Quentin Perret wrote:
> On Monday 17 Oct 2022 at 12:51:56 (+0100), Will Deacon wrote:
> > +static void *map_donated_memory_noclear(unsigned long host_va, size_t size)
> > +{
> > + void *va = (void *)kern_hyp_va(host_va);
> > +
> > + if (!PAGE_ALIGNED(va))
> > + return NULL;
> > +
> > + if (__pkvm_host_donate_hyp(hyp_virt_to_pfn(va),
> > + PAGE_ALIGN(size) >> PAGE_SHIFT))
> > + return NULL;
> > +
> > + return va;
> > +}
> > +
> > +static void *map_donated_memory(unsigned long host_va, size_t size)
> > +{
> > + void *va = map_donated_memory_noclear(host_va, size);
> > +
> > + if (va)
> > + memset(va, 0, size);
> > +
> > + return va;
> > +}
> > +
> > +static void __unmap_donated_memory(void *va, size_t size)
> > +{
> > + WARN_ON(__pkvm_hyp_donate_host(hyp_virt_to_pfn(va),
> > + PAGE_ALIGN(size) >> PAGE_SHIFT));
> > +}
> > +
> > +static void unmap_donated_memory(void *va, size_t size)
> > +{
> > + if (!va)
> > + return;
> > +
> > + memset(va, 0, size);
> > + __unmap_donated_memory(va, size);
> > +}
> > +
> > +static void unmap_donated_memory_noclear(void *va, size_t size)
> > +{
> > + if (!va)
> > + return;
> > +
> > + __unmap_donated_memory(va, size);
> > +}
>
> Nit: I'm not a huge fan of the naming here, these do more than just
> map/unmap random pages. This only works for host pages, the donation
> path has permission checks, etc. Maybe {admit,return}_host_memory()?

Hmm, so I made this change locally and I found return_host_memory() to be
really hard to read, particularly on error paths where we 'goto' an error
label and have calls to 'return_host_memory' before an actual 'return'
statement.

So I've left as-is, not because I'm tied to the existing names, but because
I did struggle with your suggestion. At the end of the day, these are static
functions so it's really easy to change the name later on if we can think
of something better.

Will