Re: [PATCH 15/15] x86/PVH: simplify writing of Dom0's RSDP

[Roger Pau Monné <roger@xxxxxxxxxxxxxx>]

On Mon, Oct 12, 2026 at 10:26:58AM +0200, Jan Beulich wrote:
> Now that we establish where the RSDP is to live before actually loading
> the kernel (and writing start-info there) we can simply hand the pointer
> there, eliminating the need for the (fragile) separate write to the field
> out of pvh_setup_acpi().
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Acked-by: Roger Pau Monné <roger@xxxxxxxxxxxxxx>

One nit below.

> ---
> Alternatively we could move start_info to the top-level function, have
> pvh_load_kernel() and pvh_setup_acpi() fill what need filling, and do
> the write from the top-level function. Yet it looks to me as if this was
> more churn for little extra gain.
> 
> --- a/xen/arch/x86/hvm/dom0_build.c
> +++ b/xen/arch/x86/hvm/dom0_build.c
> @@ -976,7 +976,8 @@ static int __init pvh_load_kernel(
>      const struct boot_domain *bd,
>      struct elf_binary *elf,
>      struct hvm_modlist_entry *mod,
> -    paddr_t *start_info_addr)
> +    paddr_t *start_info_addr,
> +    paddr_t rsdp)
>  {
>      struct domain *d = bd->d;
>      struct boot_module *initrd = bd->initrd;
> @@ -1110,6 +1111,7 @@ static int __init pvh_load_kernel(
>  
>      start_info.magic = XEN_HVM_START_MAGIC_VALUE;
>      start_info.flags = SIF_PRIVILEGED | SIF_INITDOMAIN;
> +    start_info.rsdp_paddr = rsdp;
>      rc = hvm_copy_to_guest_phys(last_addr, &start_info, sizeof(start_info), 
> v);
>      if ( rc )
>      {
> @@ -1587,10 +1589,7 @@ static int __init pvh_place_acpi(
>      return 0;
>  }
>  
> -static int __init pvh_setup_acpi(
> -    struct domain *d,
> -    struct acpi_info *info,
> -    paddr_t start_info)
> +static int __init pvh_setup_acpi(struct domain *d, struct acpi_info *info)
>  {
>      int rc;
>      struct acpi_table_rsdp *native_rsdp, rsdp = {
> @@ -1642,20 +1641,6 @@ static int __init pvh_setup_acpi(
>          return rc;
>      }
>  
> -    /* Copy RSDP address to start_info. */
> -    BUILD_BUG_ON(sizeof(info->rsdp) !=
> -                 sizeof_field(struct hvm_start_info, rsdp_paddr));

You add this BUILD_BUG_ON() in a previous patch and remove it here.
Maybe it's not worth adding?

Thanks, Roger.