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

[Jan Beulich <jbeulich@xxxxxxxx>]

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/).

> --- 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.)

Jan