Re: [kvm-unit-tests PATCH v1 05/12] lib/alloc_page: fix and improve the page allocator

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Mon, 28 Dec 2020 11:34:38 -0800
Sean Christopherson <seanjc@google.com> wrote:

> On Wed, Dec 16, 2020, Claudio Imbrenda wrote:
> >  /*
> > - * Allocates and reserves the specified memory range if possible.
> > - * Returns NULL in case of failure.
> > + * Allocates and reserves the specified physical memory range if
> > possible.
> > + * If the specified range cannot be reserved in its entirety, no
> > action is
> > + * performed and false is returned.
> > + *
> > + * Returns true in case of success, false otherwise.
> >   */
> > -void *alloc_pages_special(uintptr_t addr, size_t npages);
> > +bool alloc_pages_special(phys_addr_t addr, size_t npages);  
> 
> The boolean return is a bit awkward as kernel programmers will likely

do you prefer int, with 0 for success and -1 for failure?
that's surely not a problem

> expect a non-zero return to mean failure.  But, since there are no
> users, can we simply drop the entire *_pages_special() API?
> Allocating a specific PFN that isn't MMIO seems doomed to fail
> anyways; I'm having a hard time envisioning a test that would be able
> to use such an API without being horribly fragile.

I can. s390x can use this for some tests, where we need to allocate
memory at within or outside of specific areas, which might only be
known at run time (so we can't use the memory areas)

the alternative would be to allocate all the memory, take what is
needed, and then free the rest.... not very elegant

> >  
> >  /*
> >   * Frees a reserved memory range that had been reserved with
> > @@ -91,6 +110,6 @@ void *alloc_pages_special(uintptr_t addr, size_t
> > npages);
> >   * exactly, it can also be a subset, in which case only the
> > specified
> >   * pages will be freed and unreserved.
> >   */
> > -void free_pages_special(uintptr_t addr, size_t npages);
> > +void free_pages_special(phys_addr_t addr, size_t npages);