| Message ID | 20240510124913.49945-1-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [for-4.19] libxl: fix population of the online vCPU bitmap for PVH | expand |
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@gmail.com> > Link: https://gitlab.com/libvirt/libvirt/-/issues/399 > Reported-by: Leigh Brown <leigh@solinno.co.uk> > Fixes: 14c0d328da2b ('libxl/acpi: Build ACPI tables for HVMlite > guests') > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > --- > 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@solinno.co.uk> Regards, Leigh.
On 10/05/2024 1:49 pm, 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@gmail.com> > Link: https://gitlab.com/libvirt/libvirt/-/issues/399 > Reported-by: Leigh Brown <leigh@solinno.co.uk> > Fixes: 14c0d328da2b ('libxl/acpi: Build ACPI tables for HVMlite guests') > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > --- > 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. Bugfixes are still completely fair game at this point. Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
On Fri, 2024-05-10 at 16:20 +0100, Andrew Cooper wrote: > On 10/05/2024 1:49 pm, 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@gmail.com> > > Link: https://gitlab.com/libvirt/libvirt/-/issues/399 > > Reported-by: Leigh Brown <leigh@solinno.co.uk> > > Fixes: 14c0d328da2b ('libxl/acpi: Build ACPI tables for HVMlite > > guests') > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > --- > > 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. > > Bugfixes are still completely fair game at this point. Considering that this is bugfixes I will be happy to have that in 4.19 release: Release-acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> ~ Oleksii > > Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
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;
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@gmail.com> Link: https://gitlab.com/libvirt/libvirt/-/issues/399 Reported-by: Leigh Brown <leigh@solinno.co.uk> Fixes: 14c0d328da2b ('libxl/acpi: Build ACPI tables for HVMlite guests') Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- 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(-)