diff mbox series

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

Message ID 20210909163441.44418-4-kevin.stefanov@citrix.com (mailing list archive)
State New, archived
Headers show
Series Fix alignment of FACS in guests | expand

Commit Message

Kevin Stefanov Sept. 9, 2021, 4:34 p.m. UTC
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@citrix.com>
---
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Jackson <iwj@xenproject.org>
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libs/light/libxl_x86_acpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jan Beulich Sept. 10, 2021, 8:36 a.m. UTC | #1
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@citrix.com>

Purely technically and within the constraints of the present code:
Reviewed-by: Jan Beulich <jbeulich@suse.com>
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.

Jan
diff mbox series

Patch

diff --git a/tools/libs/light/libxl_x86_acpi.c b/tools/libs/light/libxl_x86_acpi.c
index 0a82e7cacd..2aea1eca31 100644
--- 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);
     config.infop = (unsigned long)libxl__malloc(gc, libxl_ctxt.page_size);
     /* Pages to hold ACPI tables */
     acpi_pages =  libxl__malloc(gc, (NUM_ACPI_PAGES + 1) *