Re: [PATCH for-4.19] libxl: fix population of the online vCPU bitmap for PVH

[Leigh Brown <leigh@xxxxxxxxxxxxx>]

Hi Roger,

Thanks for responding and fixing this so quickly.

On 2024-05-10 13:49, Roger Pau Monne wrote:
> libxl passes some information to libacpi to create the ACPI table for a 
> PVH
> guest, and among that information it's a bitmap of which vCPUs are 
> online
> which can be less than the maximum number of vCPUs assigned to the 
> domain.
> While the population of the bitmap is done correctly for HVM based on 
> the
> number of online vCPUs, for PVH the population of the bitmap is done 
> based on
> the number of maximum vCPUs allowed.  This leads to all local APIC 
> entries in
> the MADT being set as enabled, which contradicts the data in xenstore 
> if vCPUs
> is different than maximum vCPUs.
>
> Fix by copying the internal libxl bitmap that's populated based on the 
> vCPUs
> parameter.
>
> Reported-by: Arthur Borsboom <arthurborsboom@xxxxxxxxx>
> Link: https://gitlab.com/libvirt/libvirt/-/issues/399
> Reported-by: Leigh Brown <leigh@xxxxxxxxxxxxx>
> Fixes: 14c0d328da2b ('libxl/acpi: Build ACPI tables for HVMlite 
> guests')
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
> Note that the setup of hvm_info_table could be shared between PVH and 
> HVM, as
> the fields are very limited, see hvm_build_set_params() for the HVM 
> side.
> However this late in the release it's safer to just adjust the PVH 
> path.
> Also note the checksum is not provided when hvm_info_table is built for 
> PVH.
> This is fine so far because such checksum is only consumed by hvmloader 
> and not
> libacpi itself.
>
> It's a bugfix, so should be considered for 4.19.
> ---
>  tools/libs/light/libxl_x86_acpi.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/libs/light/libxl_x86_acpi.c 
> b/tools/libs/light/libxl_x86_acpi.c
> index 620f3c700c3e..5cf261bd6794 100644
> --- a/tools/libs/light/libxl_x86_acpi.c
> +++ b/tools/libs/light/libxl_x86_acpi.c
> @@ -89,7 +89,7 @@ static int init_acpi_config(libxl__gc *gc,
>      uint32_t domid = dom->guest_domid;
>      xc_domaininfo_t info;
>      struct hvm_info_table *hvminfo;
> -    int i, r, rc;
> +    int r, rc;
>
>      config->dsdt_anycpu = config->dsdt_15cpu = dsdt_pvh;
>      config->dsdt_anycpu_len = config->dsdt_15cpu_len = dsdt_pvh_len;
> @@ -138,8 +138,8 @@ static int init_acpi_config(libxl__gc *gc,
>          hvminfo->nr_vcpus = info.max_vcpu_id + 1;
>      }
>
> -    for (i = 0; i < hvminfo->nr_vcpus; i++)
> -        hvminfo->vcpu_online[i / 8] |= 1 << (i & 7);
> +    memcpy(hvminfo->vcpu_online, b_info->avail_vcpus.map,
> +           b_info->avail_vcpus.size);
>
>      config->hvminfo = hvminfo;
Tested-by: Leigh Brown <leigh@xxxxxxxxxxxxx>

Regards,

Leigh.