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

Re: [PATCH 3/3] tools/libxl: Only allocate 64 bytes for RSDP

  • To: Kevin Stefanov <kevin.stefanov@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Fri, 10 Sep 2021 10:36:07 +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=TZZUXn54cWZ3j2p/0IrNe7n8D3YP75eKJVHxyNlkK+s=; b=no4z6rQyjfZdy6h7ubZhD8/YANvBpdBh40P1J9dC6ZAGyA3f9n2QmvlCxuzG2KOi0BydIElnqyZ/eDnKFQr9wyr5wratX8S24S1w51YScBwK3yuiWlGJyQ0mYtdqLu8Cfio2B5vrcqQIzLt8CU1lQJohHHu2UOxehoTVBz473hiOHVuMTrjYgGu718bRi0JJ4kC4wh0IehB4AJReyPge0AAubO6cU+1HzTb8ZMidAKBIdHrpBLNVFBxFI/RRqRFHupIivBlvQ19V35XZbqkqXYrBwT3vwkRZI3DBCLQkn0Eop32HSdTdbpWBOQLHN+gF58N+DgdqLZB7RI9+DJEBQw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=TX2TvLRuNQcvTw47cIP+CpeY6PWaownceDaROxluRLJkFs1iFdQE+G+BbJ6vnKVSGHJRo+9yfvyb4vLGNB9+rSu0iQVPrJrPD/gEpBRXWkd/VzEYyPWYkSIlf19Tg03W4adpqc8+W8XLDRsJk53wHGuYWzdQlo19TmINvZS9TIGVxq/UA46azYoPckZHDuG5BcvqoWUOTnLs5fZ+NqIIsyFG28fwfm0fXI9geTJLw9wwiSX6RgE6hnJLTqiUykyYHYw0MsneFIEpD/uPc2adU25CbFCrNhfHbPJlRjTReaIHOUEq52pP0XjbPoaBcyCimjWBGhHFjqklNoHcqo7GDw==
  • 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:36:21 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 09.09.2021 18:34, Kevin Stefanov wrote:
> RSDP's size is 64 bytes and later in the function, its buffer is
> hardcoded to be 64 bytes long. Don't bother to allocate a whole page.
> Signed-off-by: Kevin Stefanov <kevin.stefanov@xxxxxxxxxx>

Purely technically and within the constraints of the present code:
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
However, ...

> --- a/tools/libs/light/libxl_x86_acpi.c
> +++ b/tools/libs/light/libxl_x86_acpi.c
> @@ -183,7 +183,7 @@ int libxl__dom_load_acpi(libxl__gc *gc,
>          goto out;
>      }
> -    config.rsdp = (unsigned long)libxl__malloc(gc, libxl_ctxt.page_size);
> +    config.rsdp = (unsigned long)libxl__malloc(gc, 64);

... this is the 4th literal 64 in the function (including one in a
comment), all of which are meant to represent the same abstract thing.
And none of which look to be really correct:
sizeof(struct acpi_20_rsdp) == 36 according to my counting. While I
don't mind using 64 (for the time being), I think this should then be
via a #define, which would be accompanied by a respective comment. Or
else via sizeof(struct acpi_20_rsdp).

But of course hard-coding the size isn't really forward compatible
anyway. It should rather be libacpi to specify what size the blob is.
And then it might be safer to stick to allocating a full page here, as
the actual size won't be known up front. Or the allocated size would
need to become an input to acpi_build_tables(), so that it wouldn't
risk overrunning the space.




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