On 14/09/2021 17:43, Kevin Stefanov wrote:
> The memory allocator currently calculates alignment in libxl's virtual
> address space, rather than guest physical address space. This results
> in the FACS being commonly misaligned.
>
> Furthermore, the allocator has several other bugs.
>
> The opencoded align-up calculation is currently susceptible to a bug
> that occurs in the corner case that the buffer is already aligned to
> begin with. In that case, an align-sized memory hole is introduced.
>
> The while loop is dead logic because its effects are entirely and
> unconditionally overwritten immediately after it.
>
> Rework the memory allocator to align in guest physical address space
> instead of libxl's virtual memory and improve the calculation, drop
> errant extra page in allocated buffer for ACPI tables, and give some
> of the variables better names/types.
>
> Fixes: 14c0d328da2b ("libxl/acpi: Build ACPI tables for HVMlite guests")
> Signed-off-by: Kevin Stefanov <[email protected]>
> ---
> CC: Andrew Cooper <[email protected]>
> CC: Ian Jackson <[email protected]>
> CC: Wei Liu <[email protected]>
> CC: Anthony PERARD <[email protected]>
> CC: Jan Beulich <[email protected]>
>
> v2: Rewrite completely, to align in guest physical address spaceIt appears that patchew isn't running properly right now, but ... > diff --git a/tools/libs/light/libxl_x86_acpi.c > b/tools/libs/light/libxl_x86_acpi.c > index 3eca1c7a9f..8b6dee2e05 100644 > --- a/tools/libs/light/libxl_x86_acpi.c > +++ b/tools/libs/light/libxl_x86_acpi.c > @@ -208,10 +201,8 @@ int libxl__dom_load_acpi(libxl__gc *gc, > } > > /* Calculate how many pages are needed for the tables. */ > - acpi_pages_num = > - ((libxl_ctxt.alloc_currp - (unsigned long)acpi_pages) > - >> libxl_ctxt.page_shift) + > - ((libxl_ctxt.alloc_currp & page_mask) ? 1 : 0); > + acpi_pages_num = (ALIGN(libxl_ctxt.guest_curr, libxl_ctxt.page_size) - > + libxl_ctxt.guest_start) >> libxl_ctxt.page_shift; ... this hunk means page_mask lost its only user, and is now a written-but-not-used variable, which some compilers will warn about. The fix is easy, and just to drop that local variable too, which is another 2 lines deleted in the resulting patch. ~Andrew
