Re: [PATCH 03/22] acpi: vmap pages in acpi_os_alloc_memory

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 20/12/2022 15:15, Jan Beulich wrote:
> On 16.12.2022 12:48, Julien Grall wrote:
> > --- a/xen/common/vmap.c
> > +++ b/xen/common/vmap.c
> > @@ -244,6 +244,11 @@ void *vmap(const mfn_t *mfn, unsigned int nr)
> >       return __vmap(mfn, 1, nr, 1, PAGE_HYPERVISOR, VMAP_DEFAULT);
> >   }
> >   
> > +void *vmap_contig_pages(mfn_t mfn, unsigned int nr_pages)
> I don't think the _pages suffix buys us much here. I also think parameter
> names would better be consistent with other functions here, in particular
> with vmap() (i.e. s/nr_pages/nr/).
I will do the renaming.

> > --- a/xen/drivers/acpi/osl.c
> > +++ b/xen/drivers/acpi/osl.c
> > @@ -221,7 +221,11 @@ void *__init acpi_os_alloc_memory(size_t sz)
> >         void *ptr;
> >   
> >   	if (system_state == SYS_STATE_early_boot)
> > -               return mfn_to_virt(mfn_x(alloc_boot_pages(PFN_UP(sz), 1)));
> > +       {
> > +               mfn_t mfn = alloc_boot_pages(PFN_UP(sz), 1);
> > +
> > +               return vmap_contig_pages(mfn, PFN_UP(sz));
> > +       }
> Multiple pages may be allocated here, yet ...
>
> > @@ -246,5 +250,10 @@ void __init acpi_os_free_memory(void *ptr)
> >         if (is_xmalloc_memory(ptr))
> >                 xfree(ptr);
> >         else if (ptr && system_state == SYS_STATE_early_boot)
> > -               init_boot_pages(__pa(ptr), __pa(ptr) + PAGE_SIZE);
> > +       {
> > +               paddr_t addr = mfn_to_maddr(vmap_to_mfn(ptr));
> > +
> > +               vunmap(ptr);
> > +               init_boot_pages(addr, addr + PAGE_SIZE);
> > +       }
> ... (as before) only one page would be freed here. With the move to
> vmap() it ought to be possible to do better now. (If you want to
> defer this to a later patch, please at least mention the aspect in
> the description.)
Good point, I will have a look to solve it in this patch.

Cheers,

--
Julien Grall