Re: [PATCH V3 0/5] Arch agnostic ACPI changes to support vCPU Hotplug (on Archs like ARM)

[Miguel Luis]

Hi Salil,

> On 3 Nov 2024, at 09:24, Salil Mehta via <qemu-devel@nongnu.org> wrote:
> 
> Change Log
> ==========
> 
> Patch V2 -> V3:
> 1. Addressed left over issues of x86 suggested by Igor Mammedov (Redhat):
>   - Removed the `ACPICPUstatus::is_enabled` State as well as it was
>     breaking the x86 migration
>   - Above is in addition to `is_present` state which was removed in V2
>   - Dropped [PATCH 1/6] of V2 patch-set 
>   - Introduced hooks `CPUClass::cpu_{persistence,enabled}_status()`
> 2. Fixed the accidental CPUs AML break of x86 in V2 (sorry for this!).
> 
> Patch V1 -> V2:
> 1. Addressed Igor Mammedov's (Redhat) raised issues:
>   - Removed `ACPICPUstatus::is_present` State and its handling in the
>     ACPI APUs AML code and now all QOM vCPUs are present.
>   - Dropped the concept of `acpi_persistent` because now QOM vCPUs
>     states and the ACPI vCPU states are consistent.
>   - QOM vCPU objects are always present now (accompanying RFC V6)
>   - Added .needed() hook to the GED VMSD toconditionally migrate the
>     ACPI CPU hotplug state at the source VM.
> 2. Addressed Zhao Liu's (Intel) reported x86 DSDT Table bios-acpi-test 
> failure 
> 3. Incorporated Gustavo Romero's comments and added his Tag to [Patches 1,2,4]
> 4. Addressed Gavin Shan's comment to reduce the inline code comments
>   wherever possible.
> 
> 
> Brief Recall
> ============
> With the advent of multi-core SoCs, the concept of CPU-level hotplugging
> has become less distinct, often functioning as a power on/off event
> rather than a full hotplug operation. However, hotplug support at the
> die/NUMA/socket level remains meaningful and is architecture-specific.
> Virtual CPU hotplug can be supported regardless of architectural support
> for physical CPU hotplug, as it serves resource management needs—such as
> dynamically scaling VM compute capacity based on demand or SLAs,
> optimizing boot times, or reducing memory footprint. Although, its
> desing must be scalable and must co-exist with future die/NUMA/socket
> level hotplug features.
> 
> 
> Motivation for this patch-set
> =============================
> In architectures that support vCPU hotplug, firmware or the VMM/QEMU
> typically reflects vCPU status changes to the OS via the ACPI
> `_STA.Present` bit. However, certain CPU architectures prohibit [1]
> modifications to CPU presence after kernel boot. This restriction [2][3]
> could be due to closely integrated per-CPU components—like interrupt
> controllers or other features— that may not support reconfiguration
> post-boot and are often in an `always-on` power domain. Consequently,
> specifications for these architectures require all CPUs to remain
> present i.e.`_STA.Present=True`, once the system is initialized.
> 
> To be able to support vCPU Hotplug feature, as a workaround to the above
> limitation,  ACPI method `_STA.Enabled` Bit could be used to reflect the
> plugged and unplugged states of the QOM vCPUs.
> 
> This patch set introduces the following changes:
> 
> 1. Introduces`CPUClass::cpu_{persistence,enabled}_status` hooks: To
>   check if the CPU should always remain present even after unplug
>   operation and to fetch the vCPU `enabled` status in case it is
>   present. Later is used in context when Guest Kernel evaluates
>   ACPI `_STA` method and the status is read from the flags stored
>   in the Qemu ACPI code.
> 
> 2. Support for Migrating ACPI CPU Hotplug State: Facilitates the
>   migration of the ACPI CPU hotplug state in VMSD by conditonally
>   making it part of the GED VMSD.
> 
> 3. Updates the Qtest bios-acpi-tables/DSDT binaries for x86/{pc,q35}
> 
> This patch set is designed to work independently and is meant to ensure
> compatibility with existing hotplug support across different
> architectures. This patch-set has been tested alongside ARM-specific
> vCPU hotplug changes (included in the upcoming RFC V6 [4]), and
> hot(un)plug functionalities performed as expected which concerns this
> patch-set. Please have a look.
> 

For arm64:
I’ve ran make check and bios-tables-test passed successfully and got 0 failed 
tests.
After boot the number of vcpus matched what was initially requested via -smp 
cpus.
I’ve also tested this patchset alongside the upcoming RFC V6-rc5 and booting, 
basic hot-plugging/unplugging of vCPUs and live migration with an asymmetrical 
number of hotplugged vCPUs looked good to me.

Please, feel free to add:
Tested-by: Miguel Luis <miguel.luis@oracle.com>

Thank you.
Miguel

> Thank you!
> 
> 
> Repository of this patch-set
> ============================
> (*) Latest Architecture Agnostic ACPI changes patch-set:
>    Repo: https://github.com/salil-mehta/qemu.git   
> virt-cpuhp-armv8/rfc-v6.arch.agnostic.acpi.v3
>    Link: 
> https://github.com/salil-mehta/qemu/commits/virt-cpuhp-armv8/rfc-v6.arch.agnostic.acpi.v3
> (*) Works with upcoming ARM architecture specific patch-set RFC V6-rc5 [4]
> 
> 
> References
> ==========
> [1] Check comment 5 in the bugzilla entry
>    Link: https://bugzilla.tianocore.org/show_bug.cgi?id=4481#c5
> [2] KVMForum 2023 Presentation: Challenges Revisited in Supporting Virt CPU 
> Hotplug on
>    architectures that don’t Support CPU Hotplug (like ARM64)
>    a. Kernel Link: 
> https://kvm-forum.qemu.org/2023/KVM-forum-cpu-hotplug_7OJ1YyJ.pdf
>    b. Qemu Link:  
> https://kvm-forum.qemu.org/2023/Challenges_Revisited_in_Supporting_Virt_CPU_Hotplug_-__ii0iNb3.pdf
> [3] KVMForum 2020 Presentation: Challenges in Supporting Virtual CPU Hotplug 
> on
>    SoC Based Systems (like ARM64)
>    Link: https://kvmforum2020.sched.com/event/eE4m
> [4] Upcoming RFC V6, Support of Virtual CPU Hotplug for ARMv8 Arch
>    Link: 
> https://github.com/salil-mehta/qemu/commits/virt-cpuhp-armv8/rfc-v6-rc5
> 
> Salil Mehta (5):
>  hw/acpi: Make CPUs ACPI `presence` conditional during vCPU hot-unplug
>  qtest: allow ACPI DSDT Table changes
>  hw/acpi: Update ACPI `_STA` method with QOM vCPU ACPI Hotplug states
>  tests/qtest/bios-tables-test: Update DSDT golden masters for
>    x86/{pc,q35}
>  hw/acpi: Update GED with vCPU Hotplug VMSD for migration
> 
> hw/acpi/cpu.c                                 |  53 ++++++++++++++++--
> hw/acpi/generic_event_device.c                |  19 +++++++
> include/hw/core/cpu.h                         |   2 +
> tests/data/acpi/x86/pc/DSDT                   | Bin 8527 -> 8561 bytes
> tests/data/acpi/x86/pc/DSDT.acpierst          | Bin 8438 -> 8472 bytes
> tests/data/acpi/x86/pc/DSDT.acpihmat          | Bin 9852 -> 9886 bytes
> tests/data/acpi/x86/pc/DSDT.bridge            | Bin 15398 -> 15432 bytes
> tests/data/acpi/x86/pc/DSDT.cphp              | Bin 8991 -> 9025 bytes
> tests/data/acpi/x86/pc/DSDT.dimmpxm           | Bin 10181 -> 10215 bytes
> tests/data/acpi/x86/pc/DSDT.hpbridge          | Bin 8478 -> 8512 bytes
> tests/data/acpi/x86/pc/DSDT.hpbrroot          | Bin 5034 -> 5068 bytes
> tests/data/acpi/x86/pc/DSDT.ipmikcs           | Bin 8599 -> 8633 bytes
> tests/data/acpi/x86/pc/DSDT.memhp             | Bin 9886 -> 9920 bytes
> tests/data/acpi/x86/pc/DSDT.nohpet            | Bin 8385 -> 8419 bytes
> tests/data/acpi/x86/pc/DSDT.numamem           | Bin 8533 -> 8567 bytes
> tests/data/acpi/x86/pc/DSDT.roothp            | Bin 12320 -> 12354 bytes
> tests/data/acpi/x86/q35/DSDT                  | Bin 8355 -> 8389 bytes
> tests/data/acpi/x86/q35/DSDT.acpierst         | Bin 8372 -> 8406 bytes
> tests/data/acpi/x86/q35/DSDT.acpihmat         | Bin 9680 -> 9714 bytes
> .../acpi/x86/q35/DSDT.acpihmat-noinitiator    | Bin 8634 -> 8668 bytes
> tests/data/acpi/x86/q35/DSDT.applesmc         | Bin 8401 -> 8435 bytes
> tests/data/acpi/x86/q35/DSDT.bridge           | Bin 11968 -> 12002 bytes
> tests/data/acpi/x86/q35/DSDT.core-count       | Bin 12913 -> 12947 bytes
> tests/data/acpi/x86/q35/DSDT.core-count2      | Bin 33770 -> 33804 bytes
> tests/data/acpi/x86/q35/DSDT.cphp             | Bin 8819 -> 8853 bytes
> tests/data/acpi/x86/q35/DSDT.cxl              | Bin 13148 -> 13182 bytes
> tests/data/acpi/x86/q35/DSDT.dimmpxm          | Bin 10009 -> 10043 bytes
> tests/data/acpi/x86/q35/DSDT.ipmibt           | Bin 8430 -> 8464 bytes
> tests/data/acpi/x86/q35/DSDT.ipmismbus        | Bin 8443 -> 8477 bytes
> tests/data/acpi/x86/q35/DSDT.ivrs             | Bin 8372 -> 8406 bytes
> tests/data/acpi/x86/q35/DSDT.memhp            | Bin 9714 -> 9748 bytes
> tests/data/acpi/x86/q35/DSDT.mmio64           | Bin 9485 -> 9519 bytes
> tests/data/acpi/x86/q35/DSDT.multi-bridge     | Bin 13208 -> 13242 bytes
> tests/data/acpi/x86/q35/DSDT.noacpihp         | Bin 8235 -> 8269 bytes
> tests/data/acpi/x86/q35/DSDT.nohpet           | Bin 8213 -> 8247 bytes
> tests/data/acpi/x86/q35/DSDT.numamem          | Bin 8361 -> 8395 bytes
> tests/data/acpi/x86/q35/DSDT.pvpanic-isa      | Bin 8456 -> 8490 bytes
> tests/data/acpi/x86/q35/DSDT.thread-count     | Bin 12913 -> 12947 bytes
> tests/data/acpi/x86/q35/DSDT.thread-count2    | Bin 33770 -> 33804 bytes
> tests/data/acpi/x86/q35/DSDT.tis.tpm12        | Bin 8961 -> 8995 bytes
> tests/data/acpi/x86/q35/DSDT.tis.tpm2         | Bin 8987 -> 9021 bytes
> tests/data/acpi/x86/q35/DSDT.type4-count      | Bin 18589 -> 18623 bytes
> tests/data/acpi/x86/q35/DSDT.viot             | Bin 14615 -> 14649 bytes
> tests/data/acpi/x86/q35/DSDT.xapic            | Bin 35718 -> 35752 bytes
> 44 files changed, 69 insertions(+), 5 deletions(-)
> 
> -- 
> 2.34.1
> 
>