First instroduce a manifest constant, to avoid open-coding 64 in several
places. Then use this constant to bound the allocation.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Similarly bounding the info "page" allocation would be nice, but would
require knowing libacpi's struct acpi_info size here.

--- a/tools/libs/light/libxl_x86_acpi.c
+++ b/tools/libs/light/libxl_x86_acpi.c
@@ -20,6 +20,8 @@
 
  /* Number of pages holding ACPI tables */
 #define NUM_ACPI_PAGES 16
+/* Hard-coded size of RSDP */
+#define RSDP_LEN 64
 #define ALIGN(p, a) (((p) + ((a) - 1)) & ~((a) - 1))
 
 struct libxl_acpi_ctxt {
@@ -177,7 +179,7 @@ int libxl__dom_load_acpi(libxl__gc *gc,
     }
 
     /* These are all copied into guest memory, so use zero-ed memory. */
-    config.rsdp = (unsigned long)libxl__zalloc(gc, libxl_ctxt.page_size);
+    config.rsdp = (unsigned long)libxl__zalloc(gc, RSDP_LEN);
     config.infop = (unsigned long)libxl__zalloc(gc, libxl_ctxt.page_size);
     /* Pages to hold ACPI tables */
     libxl_ctxt.buf = libxl__zalloc(gc, NUM_ACPI_PAGES *
@@ -204,18 +206,18 @@ int libxl__dom_load_acpi(libxl__gc *gc,
                       libxl_ctxt.guest_start) >> libxl_ctxt.page_shift;
 
     dom->acpi_modules[0].data = (void *)config.rsdp;
-    dom->acpi_modules[0].length = 64;
+    dom->acpi_modules[0].length = RSDP_LEN;
     /*
      * Some Linux versions cannot properly process hvm_start_info.rsdp_paddr
      * and so we need to put RSDP in location that can be discovered by ACPI's
-     * standard search method, in R-O BIOS memory (we chose last 64 bytes)
+     * standard search method, in R-O BIOS memory (we chose last RSDP_LEN bytes)
      */
     if (strcmp(xc_dom_guest_os(dom), "linux") ||
         xc_dom_feature_get(dom, XENFEAT_linux_rsdp_unrestricted))
         dom->acpi_modules[0].guest_addr_out = ACPI_INFO_PHYSICAL_ADDRESS +
             (1 + acpi_pages_num) * libxl_ctxt.page_size;
     else
-        dom->acpi_modules[0].guest_addr_out = 0x100000 - 64;
+        dom->acpi_modules[0].guest_addr_out = 0x100000 - RSDP_LEN;
 
     dom->acpi_modules[1].data = (void *)config.infop;
     dom->acpi_modules[1].length = libxl_ctxt.page_size;

On Mon, Nov 25, 2024 at 04:15:49PM +0100, Jan Beulich wrote:
> First instroduce a manifest constant, to avoid open-coding 64 in several
> places. Then use this constant to bound the allocation.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Hopefully, `struct acpi_20_rsdp` isn't going to be bigger that 64, but
it would probably not work well anyway seen how `config.rsdp` is used
here.

Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>

> ---
> Similarly bounding the info "page" allocation would be nice, but would
> require knowing libacpi's struct acpi_info size here.

Or register the allocation size in `config`, so acpi_build_tables() can
check if there's enough space. Something like `config.info_size`.

Thanks,

-- 

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

On 25.11.2024 18:04, Anthony PERARD wrote:
> On Mon, Nov 25, 2024 at 04:15:49PM +0100, Jan Beulich wrote:
>> First instroduce a manifest constant, to avoid open-coding 64 in several
>> places. Then use this constant to bound the allocation.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Hopefully, `struct acpi_20_rsdp` isn't going to be bigger that 64, but
> it would probably not work well anyway seen how `config.rsdp` is used
> here.
> 
> Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>

Thanks.

>> ---
>> Similarly bounding the info "page" allocation would be nice, but would
>> require knowing libacpi's struct acpi_info size here.
> 
> Or register the allocation size in `config`, so acpi_build_tables() can
> check if there's enough space. Something like `config.info_size`.

That would feel kind of backwards. It should be libacpi to specify the
size it needs, yet that won't work as libacpi is the consumer of the
config struct. We could of course add acpi_get_info_size() to libacpi,
for libxl to use. 

Jan