[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

  • To: Kevin Stefanov <kevin.stefanov@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Fri, 10 Sep 2021 10:19:58 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=H2X3bXUxHHTgI4mmQ1aDzuY4bYr7VKDx8vAHGZJEbFQ=; b=AC5BghWwKQolaLgRHV0O3cVsCtTPlL/E3/tBFIaXV8o6h+Kh7qfSK9nZ1cazAuXfCECG9U+kIhDsz9gb5eTrtYCNKGV/m293aJVoOXuCDYv2NiYjB/j70rtjH9QZS/B2KCth2aSQqF+O2Iayetz8mKM2Ket3at7oXR9mMWMqZFIfHS2LcDzAkvoPbjoi83t3Vty4U12INZibzZHFudsRmd7NhtoFd+A9SRFi6G2ZMtWQFu+Upv7obu5JEOFPTKBkB1tSBTVIuy9RWgd3zN7KmZdToWeNdTGN3S/1NjAyxqy4c2TBLDznHA68trcOhPNRA/RrRe9cs/yk9ZdztaLpoA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=dnwBOsXM2KRsYVKEt6xjgNk/krZltEBkQo8J4CkrLcttkjN6CWR/ROQkjDZBi7kMi+8snFUqELuB+lYt+7yWtIHRO2EeVIy+DdocaWA86c2/m3lO0TrY6cPGrUQKpIli8o6gTX6MlUhDySRpzUcBbRMGfzFGj2DVN5LVpLPqHHnKXxAcAyFd2IvzH/LZ3PT8KbO1RU3bCr90cTTM0orsJCnS89XtEMB8/da52vauaItE21MjpLKXWi37kALZo0r3JN9UXvW2V/VjOCrn4Is8wF5BdO08GRACs9Hoogg1dBCVF2oFdA/W9056Ps71DWTknI7oO5WhkebIZJJCh6I+eg==
  • Authentication-results: lists.xenproject.org; dkim=none (message not signed) header.d=none;lists.xenproject.org; dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Fri, 10 Sep 2021 08:20:07 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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

> --- 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?




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.