Re: [PATCH 2/3] tools/libxl: Correctly aligned buffer for ACPI tables

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.09.2021 18:34, Kevin Stefanov wrote:
> The pointer resulting from libxl__malloc() has no explicit alignment.
> As an implementation detail, it has 16-byte alignment.

But the buffers obtained via libxl__malloc() have no association with
guest address space layout (or at least they aren't supposed to have).
That's solely determined by mem_alloc(). I think it is a bug of
mem_alloc() to determine padding from alloc_currp; it should
rather/also maintain a current address in guest address space (e.g.
by having separate alloc_currp and alloc_currv). Not doing so leaves
us prone to encountering the same issue again down the road. For
example if higher than page alignment was requested from somewhere in
libacpi.

> --- a/tools/libs/light/libxl_x86_acpi.c
> +++ b/tools/libs/light/libxl_x86_acpi.c
> @@ -193,6 +193,7 @@ int libxl__dom_load_acpi(libxl__gc *gc,
>       * Set up allocator memory.
>       * Start next to acpi_info page to avoid fracturing e820.
>       */
> +    acpi_pages = (void *)ROUNDUP((unsigned long)acpi_pages, 
> libxl_ctxt.page_shift);
>      libxl_ctxt.alloc_base_paddr = ACPI_INFO_PHYSICAL_ADDRESS +
>          libxl_ctxt.page_size;
>      libxl_ctxt.alloc_base_vaddr = libxl_ctxt.alloc_currp =

Below from here there's also a suspicious use of 4096. From surrounding
code I would conclude libxl_ctxt.page_size is meant instead. Could you
consider taking care of this as well at this occasion (possibly in yet
another tiny patch), while playing in this area?

Jan