mbox series

[V15,0/7] Add architecture agnostic code to support vCPU Hotplug

Message ID 20240713182516.1457-1-salil.mehta@huawei.com (mailing list archive)
Headers show
Series Add architecture agnostic code to support vCPU Hotplug | expand

Message

Salil Mehta July 13, 2024, 6:25 p.m. UTC
[Note: References are present at the last after the revision history]

Virtual CPU hotplug support is being added across various architectures [1][3].
This series adds various code bits common across all architectures:

1. vCPU creation and Parking code refactor [Patch 1]
2. Update ACPI GED framework to support vCPU Hotplug [Patch 2,3]
3. ACPI CPUs AML code change [Patch 4,5]
4. Helper functions to support unrealization of CPU objects [Patch 6,7]

Repository:

[*] Architecture *Agnostic* Patch-set (This series)
   V14: https://github.com/salil-mehta/qemu.git virt-cpuhp-armv8/rfc-v3.arch.agnostic.v15

   NOTE: This series is meant to work in conjunction with the architecture-specific
   patch-set. For ARM, a combined patch-set (architecture agnostic + specific) was
   earlier pushed as RFC V2 [1]. Later, RFC V2 was split into the ARM Architecture
   specific patch-set RFC V3 [4] (a subset of RFC V2) and the architecture agnostic
   patch-set. Patch-set V14 is the latest version in that series. This series
   works in conjunction with RFC V4-rc2, present at the following link.

[*] ARM Architecture *Specific* Patch-set
   RFC V3 [4]: https://github.com/salil-mehta/qemu.git virt-cpuhp-armv8/rfc-v3
   RFC V4-rc2: https://github.com/salil-mehta/qemu.git virt-cpuhp-armv8/rfc-v4-rc2 (combined)


Revision History:

Patch-set V14 -> V15
1. Addressed commnet from Igor Mammedov's on [PATCH V14 4/7]
   - Removed ACPI_CPU_SCAN_METHOD
   - Introduced AML_GED_EVT_CPU_SCAN_METHOD ("\\_SB.GED.CPSCN") macro
2. Fix the stray change of "assert (" in "PATCH V14 3/7"
Link: https://lore.kernel.org/qemu-devel/20240712134201.214699-4-salil.mehta@huawei.com/

Patch-set V13 -> V14
1. Addressed Igor Mammedov's following review comments
   - Mentioned abput new external APIs in the header note of [PATCH 1/7]
   - Merged Doc [PATCH V13 8/8] with [PATCH V14 3/7]
   - Introduced GED realize function for various CPU Hotplug regions initializations
   - Added back event handler method to indirectly expose \\_SB.CPUS.CSCN to GED
     _EVT. Like for ARM, it would be through \\_SB.GED.CSCN event handler method
   - Collected the Ack given for [Patch V13 6/8]
   - Added back the gfree'ing of GDB regs in common finalize and made it conditional
   - Updated the header notes of [PATCH V13 3/8,4/8,5/8] to reflect the changes

Patch-set  V12 -> V13
1. Added Reviewed-by Tag of Harsh Prateek Bora's (IBM) [PATCH V12 1/8]
2. Moved the kvm_{create,park,unpark}_vcpu prototypes from accel/kvm/kvm-cpus.h
   to include/sysemu/kvm.h. These can later be exported through AccelOps.
Link: https://lore.kernel.org/qemu-devel/62f55169-1796-4d8e-a35d-7f003a172750@linux.ibm.com/

Patch-set  V11 -> V12
1. Addressed Harsh Prateek Bora's (IBM) comment
   - Changed @cpu to @vcpu_id in the kvm_unpark_vcpu protoype header/
2. Added Zhao Liu's (Intel) Tested-by for whole series
   - Qtest does not breaks on Intel platforms now.
3. Added Zhao Liu's (Intel) Reviewed-by for [PATCH V11 {1/8 - 3/8}]
Link: https://lore.kernel.org/qemu-devel/ZlRSPuJGBgyEUW6w@intel.com/
Link: https://lore.kernel.org/qemu-devel/a5f3d78e-cfed-441f-9c56-e3e78fa5edee@linux.ibm.com/

Patch-set  V10 -> V11
1. Addressed Nicholas Piggin's (IBM) comment
   - moved the traces in kvm_unpark_vcpu and kvm_create_vcpu at the end
   - Added the Reviewed-by Tag for [PATCH V10 1/8]
2.  Addressed Alex Bennée's (Linaro) comments
   - Added a note explaining dependency of the [PATCH V10 7/8] on Arch specific patch-set
Link: https://lore.kernel.org/qemu-devel/D1FS5GOOFWWK.2PNRIVL0V6DBL@gmail.com/ 
Link: https://lore.kernel.org/qemu-devel/87frubi402.fsf@draig.linaro.org/

Patch-set  V9 -> V10
1. Addressed Nicholas Piggin's (IBM) & Philippe Mathieu-Daudé (Linaro) comments
   - carved out kvm_unpark_vcpu and added its trace
   - Widened the scope of the kvm_unpark_vcpu so that it can be used by generic framework
     being thought out
Link: https://lore.kernel.org/qemu-devel/20240519210620.228342-1-salil.mehta@huawei.com/
Link: https://lore.kernel.org/qemu-devel/e94b0e14-efee-4050-9c9f-08382a36b63a@linaro.org/

Patch-set  V8 -> V9
1. Addressed Vishnu Pajjuri's (Ampere) comments
   - Added kvm_fd to trace in kvm_create_vcpu
   - Some clean ups: arch vcpu-id and sbd variable
   - Added the missed initialization of cpu->gdb_num_regs
2. Addressed the commnet from Zhao Liu (Intel)
   - Make initialization of CPU Hotplug state conditional (possible_cpu_arch_ids!=NULL)
Link: https://lore.kernel.org/qemu-devel/20240312020000.12992-1-salil.mehta@huawei.com/

Patch-set V7 -> V8
1. Rebased and Fixed the conflicts

Patch-set  V6 -> V7
1. Addressed Alex Bennée's comments
   - Updated the docs
2. Addressed Igor Mammedov's comments
   - Merged patches [Patch V6 3/9] & [Patch V6 7/9] with [Patch V6 4/9]
   - Updated commit-log of [Patch V6 1/9] and [Patch V6 5/9]     
3. Added Shaoqin Huang's Reviewed-by tags for whole series.
Link: https://lore.kernel.org/qemu-devel/20231013105129.25648-1-salil.mehta@huawei.com/

Patch-set  V5 -> V6
1. Addressed Gavin Shan's comments
   - Fixed the assert() ranges of address spaces
   - Rebased the patch-set to latest changes in the qemu.git
   - Added Reviewed-by tags for patches {8,9}
2. Addressed Jonathan Cameron's comments
   - Updated commit-log for [Patch V5 1/9] with mention of trace events
   - Added Reviewed-by tags for patches {1,5}
3. Added Tested-by tags from Xianglai Li
4. Fixed checkpatch.pl error "Qemu -> QEMU" in [Patch V5 1/9] 
Link: https://lore.kernel.org/qemu-devel/20231011194355.15628-1-salil.mehta@huawei.com/

Patch-set  V4 -> V5
1. Addressed Gavin Shan's comments
   - Fixed the trace events print string for kvm_{create,get,park,destroy}_vcpu
   - Added Reviewed-by tag for patch {1}
2. Added Shaoqin Huang's Reviewed-by tags for Patches {2,3}
3. Added Tested-by Tag from Vishnu Pajjuri to the patch-set
4. Dropped the ARM specific [Patch V4 10/10]
Link: https://lore.kernel.org/qemu-devel/20231009203601.17584-1-salil.mehta@huawei.com/

Patch-set  V3 -> V4
1. Addressed David Hilderbrand's comments
   - Fixed the wrong doc comment of kvm_park_vcpu API prototype
   - Added Reviewed-by tags for patches {2,4}
Link: https://lore.kernel.org/qemu-devel/20231009112812.10612-1-salil.mehta@huawei.com/

Patch-set  V2 -> V3
1. Addressed Jonathan Cameron's comments
   - Fixed 'vcpu-id' type wrongly changed from 'unsigned long' to 'integer'
   - Removed unnecessary use of variable 'vcpu_id' in kvm_park_vcpu
   - Updated [Patch V2 3/10] commit-log with details of ACPI_CPU_SCAN_METHOD macro
   - Updated [Patch V2 5/10] commit-log with details of conditional event handler method
   - Added Reviewed-by tags for patches {2,3,4,6,7}
2. Addressed Gavin Shan's comments
   - Remove unnecessary use of variable 'vcpu_id' in kvm_par_vcpu
   - Fixed return value in kvm_get_vcpu from -1 to -ENOENT
   - Reset the value of 'gdb_num_g_regs' in gdb_unregister_coprocessor_all
   - Fixed the kvm_{create,park}_vcpu prototypes docs
   - Added Reviewed-by tags for patches {2,3,4,5,6,7,9,10}
3. Addressed one earlier missed comment by Alex Bennée in RFC V1
   - Added traces instead of DPRINTF in the newly added and some existing functions
Link: https://lore.kernel.org/qemu-devel/20230930001933.2660-1-salil.mehta@huawei.com/

Patch-set V1 -> V2
1. Addressed Alex Bennée's comments
   - Refactored the kvm_create_vcpu logic to get rid of goto
   - Added the docs for kvm_{create,park}_vcpu prototypes
   - Splitted the gdbstub and AddressSpace destruction change into separate patches
   - Added Reviewed-by tags for patches {2,10}
Link: https://lore.kernel.org/qemu-devel/20230929124304.13672-1-salil.mehta@huawei.com/

References:

[1] https://lore.kernel.org/qemu-devel/20230926100436.28284-1-salil.mehta@huawei.com/
[2] https://lore.kernel.org/all/20230913163823.7880-1-james.morse@arm.com/
[3] https://lore.kernel.org/qemu-devel/cover.1695697701.git.lixianglai@loongson.cn/
[4] https://lore.kernel.org/qemu-devel/20240613233639.202896-2-salil.mehta@huawei.com/

Salil Mehta (7):
  accel/kvm: Extract common KVM vCPU {creation,parking} code
  hw/acpi: Move CPU ctrl-dev MMIO region len macro to common header file
  hw/acpi: Update ACPI GED framework to support vCPU Hotplug
  hw/acpi: Update GED _EVT method AML with CPU scan
  hw/acpi: Update CPUs AML with cpu-(ctrl)dev change
  physmem: Add helper function to destroy CPU AddressSpace
  gdbstub: Add helper function to unregister GDB register space

 accel/kvm/kvm-all.c                    | 95 +++++++++++++++++---------
 accel/kvm/kvm-cpus.h                   |  1 -
 accel/kvm/trace-events                 |  5 +-
 docs/specs/acpi_hw_reduced_hotplug.rst |  3 +-
 gdbstub/gdbstub.c                      | 13 ++++
 hw/acpi/acpi-cpu-hotplug-stub.c        |  6 ++
 hw/acpi/cpu.c                          | 18 +++--
 hw/acpi/generic_event_device.c         | 27 ++++++++
 hw/core/cpu-common.c                   |  4 +-
 hw/i386/acpi-build.c                   |  3 +-
 include/exec/cpu-common.h              |  8 +++
 include/exec/gdbstub.h                 |  6 ++
 include/hw/acpi/cpu.h                  |  7 +-
 include/hw/acpi/generic_event_device.h |  5 ++
 include/hw/core/cpu.h                  |  1 +
 include/sysemu/kvm.h                   | 25 +++++++
 system/physmem.c                       | 29 ++++++++
 17 files changed, 212 insertions(+), 44 deletions(-)

Comments

Zhao Liu July 15, 2024, 6:11 a.m. UTC | #1
Hi Salil,

I ran the unit tests again on x86 platform, and everything looks good.

Please feel free to keep my tested-by tag.

Regards,
Zhao

On Sat, Jul 13, 2024 at 07:25:09PM +0100, Salil Mehta via wrote:
> Date: Sat, 13 Jul 2024 19:25:09 +0100
> From: Salil Mehta via <qemu-devel@nongnu.org>
> Subject: [PATCH V15 0/7] Add architecture agnostic code to support vCPU
>  Hotplug
> X-Mailer: git-send-email 2.34.1
> 
> [Note: References are present at the last after the revision history]
> 
> Virtual CPU hotplug support is being added across various architectures [1][3].
> This series adds various code bits common across all architectures:
> 
> 1. vCPU creation and Parking code refactor [Patch 1]
> 2. Update ACPI GED framework to support vCPU Hotplug [Patch 2,3]
> 3. ACPI CPUs AML code change [Patch 4,5]
> 4. Helper functions to support unrealization of CPU objects [Patch 6,7]
> 
> Repository:
> 
> [*] Architecture *Agnostic* Patch-set (This series)
>    V14: https://github.com/salil-mehta/qemu.git virt-cpuhp-armv8/rfc-v3.arch.agnostic.v15
> 
>    NOTE: This series is meant to work in conjunction with the architecture-specific
>    patch-set. For ARM, a combined patch-set (architecture agnostic + specific) was
>    earlier pushed as RFC V2 [1]. Later, RFC V2 was split into the ARM Architecture
>    specific patch-set RFC V3 [4] (a subset of RFC V2) and the architecture agnostic
>    patch-set. Patch-set V14 is the latest version in that series. This series
>    works in conjunction with RFC V4-rc2, present at the following link.
> 
> [*] ARM Architecture *Specific* Patch-set
>    RFC V3 [4]: https://github.com/salil-mehta/qemu.git virt-cpuhp-armv8/rfc-v3
>    RFC V4-rc2: https://github.com/salil-mehta/qemu.git virt-cpuhp-armv8/rfc-v4-rc2 (combined)
> 
> 
> Revision History:
> 
> Patch-set V14 -> V15
> 1. Addressed commnet from Igor Mammedov's on [PATCH V14 4/7]
>    - Removed ACPI_CPU_SCAN_METHOD
>    - Introduced AML_GED_EVT_CPU_SCAN_METHOD ("\\_SB.GED.CPSCN") macro
> 2. Fix the stray change of "assert (" in "PATCH V14 3/7"
> Link: https://lore.kernel.org/qemu-devel/20240712134201.214699-4-salil.mehta@huawei.com/
> 
> Patch-set V13 -> V14
> 1. Addressed Igor Mammedov's following review comments
>    - Mentioned abput new external APIs in the header note of [PATCH 1/7]
>    - Merged Doc [PATCH V13 8/8] with [PATCH V14 3/7]
>    - Introduced GED realize function for various CPU Hotplug regions initializations
>    - Added back event handler method to indirectly expose \\_SB.CPUS.CSCN to GED
>      _EVT. Like for ARM, it would be through \\_SB.GED.CSCN event handler method
>    - Collected the Ack given for [Patch V13 6/8]
>    - Added back the gfree'ing of GDB regs in common finalize and made it conditional
>    - Updated the header notes of [PATCH V13 3/8,4/8,5/8] to reflect the changes
> 
> Patch-set  V12 -> V13
> 1. Added Reviewed-by Tag of Harsh Prateek Bora's (IBM) [PATCH V12 1/8]
> 2. Moved the kvm_{create,park,unpark}_vcpu prototypes from accel/kvm/kvm-cpus.h
>    to include/sysemu/kvm.h. These can later be exported through AccelOps.
> Link: https://lore.kernel.org/qemu-devel/62f55169-1796-4d8e-a35d-7f003a172750@linux.ibm.com/
> 
> Patch-set  V11 -> V12
> 1. Addressed Harsh Prateek Bora's (IBM) comment
>    - Changed @cpu to @vcpu_id in the kvm_unpark_vcpu protoype header/
> 2. Added Zhao Liu's (Intel) Tested-by for whole series
>    - Qtest does not breaks on Intel platforms now.
> 3. Added Zhao Liu's (Intel) Reviewed-by for [PATCH V11 {1/8 - 3/8}]
> Link: https://lore.kernel.org/qemu-devel/ZlRSPuJGBgyEUW6w@intel.com/
> Link: https://lore.kernel.org/qemu-devel/a5f3d78e-cfed-441f-9c56-e3e78fa5edee@linux.ibm.com/
> 
> Patch-set  V10 -> V11
> 1. Addressed Nicholas Piggin's (IBM) comment
>    - moved the traces in kvm_unpark_vcpu and kvm_create_vcpu at the end
>    - Added the Reviewed-by Tag for [PATCH V10 1/8]
> 2.  Addressed Alex Bennée's (Linaro) comments
>    - Added a note explaining dependency of the [PATCH V10 7/8] on Arch specific patch-set
> Link: https://lore.kernel.org/qemu-devel/D1FS5GOOFWWK.2PNRIVL0V6DBL@gmail.com/ 
> Link: https://lore.kernel.org/qemu-devel/87frubi402.fsf@draig.linaro.org/
> 
> Patch-set  V9 -> V10
> 1. Addressed Nicholas Piggin's (IBM) & Philippe Mathieu-Daudé (Linaro) comments
>    - carved out kvm_unpark_vcpu and added its trace
>    - Widened the scope of the kvm_unpark_vcpu so that it can be used by generic framework
>      being thought out
> Link: https://lore.kernel.org/qemu-devel/20240519210620.228342-1-salil.mehta@huawei.com/
> Link: https://lore.kernel.org/qemu-devel/e94b0e14-efee-4050-9c9f-08382a36b63a@linaro.org/
> 
> Patch-set  V8 -> V9
> 1. Addressed Vishnu Pajjuri's (Ampere) comments
>    - Added kvm_fd to trace in kvm_create_vcpu
>    - Some clean ups: arch vcpu-id and sbd variable
>    - Added the missed initialization of cpu->gdb_num_regs
> 2. Addressed the commnet from Zhao Liu (Intel)
>    - Make initialization of CPU Hotplug state conditional (possible_cpu_arch_ids!=NULL)
> Link: https://lore.kernel.org/qemu-devel/20240312020000.12992-1-salil.mehta@huawei.com/
> 
> Patch-set V7 -> V8
> 1. Rebased and Fixed the conflicts
> 
> Patch-set  V6 -> V7
> 1. Addressed Alex Bennée's comments
>    - Updated the docs
> 2. Addressed Igor Mammedov's comments
>    - Merged patches [Patch V6 3/9] & [Patch V6 7/9] with [Patch V6 4/9]
>    - Updated commit-log of [Patch V6 1/9] and [Patch V6 5/9]     
> 3. Added Shaoqin Huang's Reviewed-by tags for whole series.
> Link: https://lore.kernel.org/qemu-devel/20231013105129.25648-1-salil.mehta@huawei.com/
> 
> Patch-set  V5 -> V6
> 1. Addressed Gavin Shan's comments
>    - Fixed the assert() ranges of address spaces
>    - Rebased the patch-set to latest changes in the qemu.git
>    - Added Reviewed-by tags for patches {8,9}
> 2. Addressed Jonathan Cameron's comments
>    - Updated commit-log for [Patch V5 1/9] with mention of trace events
>    - Added Reviewed-by tags for patches {1,5}
> 3. Added Tested-by tags from Xianglai Li
> 4. Fixed checkpatch.pl error "Qemu -> QEMU" in [Patch V5 1/9] 
> Link: https://lore.kernel.org/qemu-devel/20231011194355.15628-1-salil.mehta@huawei.com/
> 
> Patch-set  V4 -> V5
> 1. Addressed Gavin Shan's comments
>    - Fixed the trace events print string for kvm_{create,get,park,destroy}_vcpu
>    - Added Reviewed-by tag for patch {1}
> 2. Added Shaoqin Huang's Reviewed-by tags for Patches {2,3}
> 3. Added Tested-by Tag from Vishnu Pajjuri to the patch-set
> 4. Dropped the ARM specific [Patch V4 10/10]
> Link: https://lore.kernel.org/qemu-devel/20231009203601.17584-1-salil.mehta@huawei.com/
> 
> Patch-set  V3 -> V4
> 1. Addressed David Hilderbrand's comments
>    - Fixed the wrong doc comment of kvm_park_vcpu API prototype
>    - Added Reviewed-by tags for patches {2,4}
> Link: https://lore.kernel.org/qemu-devel/20231009112812.10612-1-salil.mehta@huawei.com/
> 
> Patch-set  V2 -> V3
> 1. Addressed Jonathan Cameron's comments
>    - Fixed 'vcpu-id' type wrongly changed from 'unsigned long' to 'integer'
>    - Removed unnecessary use of variable 'vcpu_id' in kvm_park_vcpu
>    - Updated [Patch V2 3/10] commit-log with details of ACPI_CPU_SCAN_METHOD macro
>    - Updated [Patch V2 5/10] commit-log with details of conditional event handler method
>    - Added Reviewed-by tags for patches {2,3,4,6,7}
> 2. Addressed Gavin Shan's comments
>    - Remove unnecessary use of variable 'vcpu_id' in kvm_par_vcpu
>    - Fixed return value in kvm_get_vcpu from -1 to -ENOENT
>    - Reset the value of 'gdb_num_g_regs' in gdb_unregister_coprocessor_all
>    - Fixed the kvm_{create,park}_vcpu prototypes docs
>    - Added Reviewed-by tags for patches {2,3,4,5,6,7,9,10}
> 3. Addressed one earlier missed comment by Alex Bennée in RFC V1
>    - Added traces instead of DPRINTF in the newly added and some existing functions
> Link: https://lore.kernel.org/qemu-devel/20230930001933.2660-1-salil.mehta@huawei.com/
> 
> Patch-set V1 -> V2
> 1. Addressed Alex Bennée's comments
>    - Refactored the kvm_create_vcpu logic to get rid of goto
>    - Added the docs for kvm_{create,park}_vcpu prototypes
>    - Splitted the gdbstub and AddressSpace destruction change into separate patches
>    - Added Reviewed-by tags for patches {2,10}
> Link: https://lore.kernel.org/qemu-devel/20230929124304.13672-1-salil.mehta@huawei.com/
> 
> References:
> 
> [1] https://lore.kernel.org/qemu-devel/20230926100436.28284-1-salil.mehta@huawei.com/
> [2] https://lore.kernel.org/all/20230913163823.7880-1-james.morse@arm.com/
> [3] https://lore.kernel.org/qemu-devel/cover.1695697701.git.lixianglai@loongson.cn/
> [4] https://lore.kernel.org/qemu-devel/20240613233639.202896-2-salil.mehta@huawei.com/
> 
> Salil Mehta (7):
>   accel/kvm: Extract common KVM vCPU {creation,parking} code
>   hw/acpi: Move CPU ctrl-dev MMIO region len macro to common header file
>   hw/acpi: Update ACPI GED framework to support vCPU Hotplug
>   hw/acpi: Update GED _EVT method AML with CPU scan
>   hw/acpi: Update CPUs AML with cpu-(ctrl)dev change
>   physmem: Add helper function to destroy CPU AddressSpace
>   gdbstub: Add helper function to unregister GDB register space
> 
>  accel/kvm/kvm-all.c                    | 95 +++++++++++++++++---------
>  accel/kvm/kvm-cpus.h                   |  1 -
>  accel/kvm/trace-events                 |  5 +-
>  docs/specs/acpi_hw_reduced_hotplug.rst |  3 +-
>  gdbstub/gdbstub.c                      | 13 ++++
>  hw/acpi/acpi-cpu-hotplug-stub.c        |  6 ++
>  hw/acpi/cpu.c                          | 18 +++--
>  hw/acpi/generic_event_device.c         | 27 ++++++++
>  hw/core/cpu-common.c                   |  4 +-
>  hw/i386/acpi-build.c                   |  3 +-
>  include/exec/cpu-common.h              |  8 +++
>  include/exec/gdbstub.h                 |  6 ++
>  include/hw/acpi/cpu.h                  |  7 +-
>  include/hw/acpi/generic_event_device.h |  5 ++
>  include/hw/core/cpu.h                  |  1 +
>  include/sysemu/kvm.h                   | 25 +++++++
>  system/physmem.c                       | 29 ++++++++
>  17 files changed, 212 insertions(+), 44 deletions(-)
> 
> -- 
> 2.34.1
> 
>
Salil Mehta July 15, 2024, 8:45 a.m. UTC | #2
Hi Zhao,

>  From: Zhao Liu <zhao1.liu@intel.com>
>  Sent: Monday, July 15, 2024 7:11 AM
>  To: Salil Mehta <salil.mehta@huawei.com>
>  
>  Hi Salil,
>  
>  I ran the unit tests again on x86 platform, and everything looks good.
>  
>  Please feel free to keep my tested-by tag.


Many thanks for confirming this. Appreciate this.

Best Wishes
Salil.

>  
>  Regards,
>  Zhao
>  
>  On Sat, Jul 13, 2024 at 07:25:09PM +0100, Salil Mehta via wrote:
>  > Date: Sat, 13 Jul 2024 19:25:09 +0100
>  > From: Salil Mehta via <qemu-devel@nongnu.org>
>  > Subject: [PATCH V15 0/7] Add architecture agnostic code to support
>  > vCPU  Hotplug
>  > X-Mailer: git-send-email 2.34.1
>  >
>  > [Note: References are present at the last after the revision history]
>  >
>  > Virtual CPU hotplug support is being added across various architectures
>  [1][3].
>  > This series adds various code bits common across all architectures:
>  >
>  > 1. vCPU creation and Parking code refactor [Patch 1] 2. Update ACPI
>  > GED framework to support vCPU Hotplug [Patch 2,3] 3. ACPI CPUs AML
>  > code change [Patch 4,5] 4. Helper functions to support unrealization
>  > of CPU objects [Patch 6,7]
>  >
>  > Repository:
>  >
>  > [*] Architecture *Agnostic* Patch-set (This series)
>  >    V14: https://github.com/salil-mehta/qemu.git
>  > virt-cpuhp-armv8/rfc-v3.arch.agnostic.v15
>  >
>  >    NOTE: This series is meant to work in conjunction with the architecture-
>  specific
>  >    patch-set. For ARM, a combined patch-set (architecture agnostic +
>  specific) was
>  >    earlier pushed as RFC V2 [1]. Later, RFC V2 was split into the ARM
>  Architecture
>  >    specific patch-set RFC V3 [4] (a subset of RFC V2) and the architecture
>  agnostic
>  >    patch-set. Patch-set V14 is the latest version in that series. This series
>  >    works in conjunction with RFC V4-rc2, present at the following link.
>  >
>  > [*] ARM Architecture *Specific* Patch-set
>  >    RFC V3 [4]: https://github.com/salil-mehta/qemu.git virt-cpuhp-
>  armv8/rfc-v3
>  >    RFC V4-rc2: https://github.com/salil-mehta/qemu.git
>  > virt-cpuhp-armv8/rfc-v4-rc2 (combined)
>  >
>  >
>  > Revision History:
>  >
>  > Patch-set V14 -> V15
>  > 1. Addressed commnet from Igor Mammedov's on [PATCH V14 4/7]
>  >    - Removed ACPI_CPU_SCAN_METHOD
>  >    - Introduced AML_GED_EVT_CPU_SCAN_METHOD
>  ("\\_SB.GED.CPSCN") macro
>  > 2. Fix the stray change of "assert (" in "PATCH V14 3/7"
>  > Link:
>  > https://lore.kernel.org/qemu-devel/20240712134201.214699-4-salil.mehta
>  > @huawei.com/
>  >
>  > Patch-set V13 -> V14
>  > 1. Addressed Igor Mammedov's following review comments
>  >    - Mentioned abput new external APIs in the header note of [PATCH 1/7]
>  >    - Merged Doc [PATCH V13 8/8] with [PATCH V14 3/7]
>  >    - Introduced GED realize function for various CPU Hotplug regions
>  initializations
>  >    - Added back event handler method to indirectly expose
>  \\_SB.CPUS.CSCN to GED
>  >      _EVT. Like for ARM, it would be through \\_SB.GED.CSCN event handler
>  method
>  >    - Collected the Ack given for [Patch V13 6/8]
>  >    - Added back the gfree'ing of GDB regs in common finalize and made it
>  conditional
>  >    - Updated the header notes of [PATCH V13 3/8,4/8,5/8] to reflect
>  > the changes
>  >
>  > Patch-set  V12 -> V13
>  > 1. Added Reviewed-by Tag of Harsh Prateek Bora's (IBM) [PATCH V12 1/8]
>  > 2. Moved the kvm_{create,park,unpark}_vcpu prototypes from
>  accel/kvm/kvm-cpus.h
>  >    to include/sysemu/kvm.h. These can later be exported through
>  AccelOps.
>  > Link:
>  > https://lore.kernel.org/qemu-devel/62f55169-1796-4d8e-a35d-
>  7f003a17275
>  > 0@linux.ibm.com/
>  >
>  > Patch-set  V11 -> V12
>  > 1. Addressed Harsh Prateek Bora's (IBM) comment
>  >    - Changed @cpu to @vcpu_id in the kvm_unpark_vcpu protoype
>  header/
>  > 2. Added Zhao Liu's (Intel) Tested-by for whole series
>  >    - Qtest does not breaks on Intel platforms now.
>  > 3. Added Zhao Liu's (Intel) Reviewed-by for [PATCH V11 {1/8 - 3/8}]
>  > Link: https://lore.kernel.org/qemu-
>  devel/ZlRSPuJGBgyEUW6w@intel.com/
>  > Link:
>  > https://lore.kernel.org/qemu-devel/a5f3d78e-cfed-441f-9c56-
>  e3e78fa5ede
>  > e@linux.ibm.com/
>  >
>  > Patch-set  V10 -> V11
>  > 1. Addressed Nicholas Piggin's (IBM) comment
>  >    - moved the traces in kvm_unpark_vcpu and kvm_create_vcpu at the
>  end
>  >    - Added the Reviewed-by Tag for [PATCH V10 1/8] 2.  Addressed Alex
>  > Bennée's (Linaro) comments
>  >    - Added a note explaining dependency of the [PATCH V10 7/8] on Arch
>  > specific patch-set
>  > Link:
>  > https://lore.kernel.org/qemu-
>  devel/D1FS5GOOFWWK.2PNRIVL0V6DBL@gmail.co
>  > m/
>  > Link:
>  > https://lore.kernel.org/qemu-devel/87frubi402.fsf@draig.linaro.org/
>  >
>  > Patch-set  V9 -> V10
>  > 1. Addressed Nicholas Piggin's (IBM) & Philippe Mathieu-Daudé (Linaro)
>  comments
>  >    - carved out kvm_unpark_vcpu and added its trace
>  >    - Widened the scope of the kvm_unpark_vcpu so that it can be used by
>  generic framework
>  >      being thought out
>  > Link:
>  > https://lore.kernel.org/qemu-devel/20240519210620.228342-1-salil.mehta
>  > @huawei.com/
>  > Link:
>  > https://lore.kernel.org/qemu-devel/e94b0e14-efee-4050-9c9f-
>  08382a36b63
>  > a@linaro.org/
>  >
>  > Patch-set  V8 -> V9
>  > 1. Addressed Vishnu Pajjuri's (Ampere) comments
>  >    - Added kvm_fd to trace in kvm_create_vcpu
>  >    - Some clean ups: arch vcpu-id and sbd variable
>  >    - Added the missed initialization of cpu->gdb_num_regs 2. Addressed
>  > the commnet from Zhao Liu (Intel)
>  >    - Make initialization of CPU Hotplug state conditional
>  > (possible_cpu_arch_ids!=NULL)
>  > Link:
>  > https://lore.kernel.org/qemu-devel/20240312020000.12992-1-
>  salil.mehta@
>  > huawei.com/
>  >
>  > Patch-set V7 -> V8
>  > 1. Rebased and Fixed the conflicts
>  >
>  > Patch-set  V6 -> V7
>  > 1. Addressed Alex Bennée's comments
>  >    - Updated the docs
>  > 2. Addressed Igor Mammedov's comments
>  >    - Merged patches [Patch V6 3/9] & [Patch V6 7/9] with [Patch V6 4/9]
>  >    - Updated commit-log of [Patch V6 1/9] and [Patch V6 5/9]
>  > 3. Added Shaoqin Huang's Reviewed-by tags for whole series.
>  > Link:
>  > https://lore.kernel.org/qemu-devel/20231013105129.25648-1-
>  salil.mehta@
>  > huawei.com/
>  >
>  > Patch-set  V5 -> V6
>  > 1. Addressed Gavin Shan's comments
>  >    - Fixed the assert() ranges of address spaces
>  >    - Rebased the patch-set to latest changes in the qemu.git
>  >    - Added Reviewed-by tags for patches {8,9} 2. Addressed Jonathan
>  > Cameron's comments
>  >    - Updated commit-log for [Patch V5 1/9] with mention of trace events
>  >    - Added Reviewed-by tags for patches {1,5} 3. Added Tested-by tags
>  > from Xianglai Li 4. Fixed checkpatch.pl error "Qemu -> QEMU" in [Patch
>  > V5 1/9]
>  > Link:
>  > https://lore.kernel.org/qemu-devel/20231011194355.15628-1-
>  salil.mehta@
>  > huawei.com/
>  >
>  > Patch-set  V4 -> V5
>  > 1. Addressed Gavin Shan's comments
>  >    - Fixed the trace events print string for
>  kvm_{create,get,park,destroy}_vcpu
>  >    - Added Reviewed-by tag for patch {1} 2. Added Shaoqin Huang's
>  > Reviewed-by tags for Patches {2,3} 3. Added Tested-by Tag from Vishnu
>  > Pajjuri to the patch-set 4. Dropped the ARM specific [Patch V4 10/10]
>  > Link:
>  > https://lore.kernel.org/qemu-devel/20231009203601.17584-1-
>  salil.mehta@
>  > huawei.com/
>  >
>  > Patch-set  V3 -> V4
>  > 1. Addressed David Hilderbrand's comments
>  >    - Fixed the wrong doc comment of kvm_park_vcpu API prototype
>  >    - Added Reviewed-by tags for patches {2,4}
>  > Link:
>  > https://lore.kernel.org/qemu-devel/20231009112812.10612-1-
>  salil.mehta@
>  > huawei.com/
>  >
>  > Patch-set  V2 -> V3
>  > 1. Addressed Jonathan Cameron's comments
>  >    - Fixed 'vcpu-id' type wrongly changed from 'unsigned long' to 'integer'
>  >    - Removed unnecessary use of variable 'vcpu_id' in kvm_park_vcpu
>  >    - Updated [Patch V2 3/10] commit-log with details of
>  ACPI_CPU_SCAN_METHOD macro
>  >    - Updated [Patch V2 5/10] commit-log with details of conditional event
>  handler method
>  >    - Added Reviewed-by tags for patches {2,3,4,6,7} 2. Addressed Gavin
>  > Shan's comments
>  >    - Remove unnecessary use of variable 'vcpu_id' in kvm_par_vcpu
>  >    - Fixed return value in kvm_get_vcpu from -1 to -ENOENT
>  >    - Reset the value of 'gdb_num_g_regs' in
>  gdb_unregister_coprocessor_all
>  >    - Fixed the kvm_{create,park}_vcpu prototypes docs
>  >    - Added Reviewed-by tags for patches {2,3,4,5,6,7,9,10} 3.
>  > Addressed one earlier missed comment by Alex Bennée in RFC V1
>  >    - Added traces instead of DPRINTF in the newly added and some
>  > existing functions
>  > Link:
>  > https://lore.kernel.org/qemu-devel/20230930001933.2660-1-
>  salil.mehta@h
>  > uawei.com/
>  >
>  > Patch-set V1 -> V2
>  > 1. Addressed Alex Bennée's comments
>  >    - Refactored the kvm_create_vcpu logic to get rid of goto
>  >    - Added the docs for kvm_{create,park}_vcpu prototypes
>  >    - Splitted the gdbstub and AddressSpace destruction change into
>  separate patches
>  >    - Added Reviewed-by tags for patches {2,10}
>  > Link:
>  > https://lore.kernel.org/qemu-devel/20230929124304.13672-1-
>  salil.mehta@
>  > huawei.com/
>  >
>  > References:
>  >
>  > [1]
>  > https://lore.kernel.org/qemu-devel/20230926100436.28284-1-
>  salil.mehta@
>  > huawei.com/ [2]
>  > https://lore.kernel.org/all/20230913163823.7880-1-
>  james.morse@arm.com/
>  > [3]
>  > https://lore.kernel.org/qemu-devel/cover.1695697701.git.lixianglai@loo
>  > ngson.cn/ [4]
>  > https://lore.kernel.org/qemu-devel/20240613233639.202896-2-salil.mehta
>  > @huawei.com/
>  >
>  > Salil Mehta (7):
>  >   accel/kvm: Extract common KVM vCPU {creation,parking} code
>  >   hw/acpi: Move CPU ctrl-dev MMIO region len macro to common header
>  file
>  >   hw/acpi: Update ACPI GED framework to support vCPU Hotplug
>  >   hw/acpi: Update GED _EVT method AML with CPU scan
>  >   hw/acpi: Update CPUs AML with cpu-(ctrl)dev change
>  >   physmem: Add helper function to destroy CPU AddressSpace
>  >   gdbstub: Add helper function to unregister GDB register space
>  >
>  >  accel/kvm/kvm-all.c                    | 95 +++++++++++++++++---------
>  >  accel/kvm/kvm-cpus.h                   |  1 -
>  >  accel/kvm/trace-events                 |  5 +-
>  >  docs/specs/acpi_hw_reduced_hotplug.rst |  3 +-
>  >  gdbstub/gdbstub.c                      | 13 ++++
>  >  hw/acpi/acpi-cpu-hotplug-stub.c        |  6 ++
>  >  hw/acpi/cpu.c                          | 18 +++--
>  >  hw/acpi/generic_event_device.c         | 27 ++++++++
>  >  hw/core/cpu-common.c                   |  4 +-
>  >  hw/i386/acpi-build.c                   |  3 +-
>  >  include/exec/cpu-common.h              |  8 +++
>  >  include/exec/gdbstub.h                 |  6 ++
>  >  include/hw/acpi/cpu.h                  |  7 +-
>  >  include/hw/acpi/generic_event_device.h |  5 ++
>  >  include/hw/core/cpu.h                  |  1 +
>  >  include/sysemu/kvm.h                   | 25 +++++++
>  >  system/physmem.c                       | 29 ++++++++
>  >  17 files changed, 212 insertions(+), 44 deletions(-)
>  >
>  > --
>  > 2.34.1
>  >
>  >
Vishnu Pajjuri July 15, 2024, 11:03 a.m. UTC | #3
Hi Salil,

On 13-07-2024 23:55, Salil Mehta wrote:
> [Note: References are present at the last after the revision history]
>
> Virtual CPU hotplug support is being added across various architectures [1][3].
> This series adds various code bits common across all architectures:
>
> 1. vCPU creation and Parking code refactor [Patch 1]
> 2. Update ACPI GED framework to support vCPU Hotplug [Patch 2,3]
> 3. ACPI CPUs AML code change [Patch 4,5]
> 4. Helper functions to support unrealization of CPU objects [Patch 6,7]
>
> Repository:
>
> [*] Architecture *Agnostic* Patch-set (This series)
>     V14:https://github.com/salil-mehta/qemu.git  virt-cpuhp-armv8/rfc-v3.arch.agnostic.v15
>
>     NOTE: This series is meant to work in conjunction with the architecture-specific
>     patch-set. For ARM, a combined patch-set (architecture agnostic + specific) was
>     earlier pushed as RFC V2 [1]. Later, RFC V2 was split into the ARM Architecture
>     specific patch-set RFC V3 [4] (a subset of RFC V2) and the architecture agnostic
>     patch-set. Patch-set V14 is the latest version in that series. This series
>     works in conjunction with RFC V4-rc2, present at the following link.
>
> [*] ARM Architecture *Specific* Patch-set
>     RFC V3 [4]:https://github.com/salil-mehta/qemu.git  virt-cpuhp-armv8/rfc-v3
>     RFC V4-rc2:https://github.com/salil-mehta/qemu.git  virt-cpuhp-armv8/rfc-v4-rc2 (combined)
>
>
> Revision History:
>
> Patch-set V14 -> V15
> 1. Addressed commnet from Igor Mammedov's on [PATCH V14 4/7]
>     - Removed ACPI_CPU_SCAN_METHOD
>     - Introduced AML_GED_EVT_CPU_SCAN_METHOD ("\\_SB.GED.CPSCN") macro
> 2. Fix the stray change of "assert (" in "PATCH V14 3/7"
> Link:https://lore.kernel.org/qemu-devel/20240712134201.214699-4-salil.mehta@huawei.com/

I tried following test cases with rfc-v4-rc2 and kernel patches v10, and 
it looks good on Ampere platforms.

  * Regular hotplug and hot unplug tests
  * Save/restore  VM, suspend/resume VM with and with out hot-plugging
    vcpus tests
  * Live migration with and with out hot-plugging vcpus tests

Please feel free to add,
Tested-by: Vishnu Pajjuri <vishnu@os.amperecomputing.com>

_Regards_,

-Vishnu.
> Patch-set V13 -> V14
> 1. Addressed Igor Mammedov's following review comments
>     - Mentioned abput new external APIs in the header note of [PATCH 1/7]
>     - Merged Doc [PATCH V13 8/8] with [PATCH V14 3/7]
>     - Introduced GED realize function for various CPU Hotplug regions initializations
>     - Added back event handler method to indirectly expose \\_SB.CPUS.CSCN to GED
>       _EVT. Like for ARM, it would be through \\_SB.GED.CSCN event handler method
>     - Collected the Ack given for [Patch V13 6/8]
>     - Added back the gfree'ing of GDB regs in common finalize and made it conditional
>     - Updated the header notes of [PATCH V13 3/8,4/8,5/8] to reflect the changes
>
> Patch-set  V12 -> V13
> 1. Added Reviewed-by Tag of Harsh Prateek Bora's (IBM) [PATCH V12 1/8]
> 2. Moved the kvm_{create,park,unpark}_vcpu prototypes from accel/kvm/kvm-cpus.h
>     to include/sysemu/kvm.h. These can later be exported through AccelOps.
> Link:https://lore.kernel.org/qemu-devel/62f55169-1796-4d8e-a35d-7f003a172750@linux.ibm.com/
>
> Patch-set  V11 -> V12
> 1. Addressed Harsh Prateek Bora's (IBM) comment
>     - Changed @cpu to @vcpu_id in the kvm_unpark_vcpu protoype header/
> 2. Added Zhao Liu's (Intel) Tested-by for whole series
>     - Qtest does not breaks on Intel platforms now.
> 3. Added Zhao Liu's (Intel) Reviewed-by for [PATCH V11 {1/8 - 3/8}]
> Link:https://lore.kernel.org/qemu-devel/ZlRSPuJGBgyEUW6w@intel.com/
> Link:https://lore.kernel.org/qemu-devel/a5f3d78e-cfed-441f-9c56-e3e78fa5edee@linux.ibm.com/
>
> Patch-set  V10 -> V11
> 1. Addressed Nicholas Piggin's (IBM) comment
>     - moved the traces in kvm_unpark_vcpu and kvm_create_vcpu at the end
>     - Added the Reviewed-by Tag for [PATCH V10 1/8]
> 2.  Addressed Alex Bennée's (Linaro) comments
>     - Added a note explaining dependency of the [PATCH V10 7/8] on Arch specific patch-set
> Link:https://lore.kernel.org/qemu-devel/D1FS5GOOFWWK.2PNRIVL0V6DBL@gmail.com/  
> Link:https://lore.kernel.org/qemu-devel/87frubi402.fsf@draig.linaro.org/
>
> Patch-set  V9 -> V10
> 1. Addressed Nicholas Piggin's (IBM) & Philippe Mathieu-Daudé (Linaro) comments
>     - carved out kvm_unpark_vcpu and added its trace
>     - Widened the scope of the kvm_unpark_vcpu so that it can be used by generic framework
>       being thought out
> Link:https://lore.kernel.org/qemu-devel/20240519210620.228342-1-salil.mehta@huawei.com/
> Link:https://lore.kernel.org/qemu-devel/e94b0e14-efee-4050-9c9f-08382a36b63a@linaro.org/
>
> Patch-set  V8 -> V9
> 1. Addressed Vishnu Pajjuri's (Ampere) comments
>     - Added kvm_fd to trace in kvm_create_vcpu
>     - Some clean ups: arch vcpu-id and sbd variable
>     - Added the missed initialization of cpu->gdb_num_regs
> 2. Addressed the commnet from Zhao Liu (Intel)
>     - Make initialization of CPU Hotplug state conditional (possible_cpu_arch_ids!=NULL)
> Link:https://lore.kernel.org/qemu-devel/20240312020000.12992-1-salil.mehta@huawei.com/
>
> Patch-set V7 -> V8
> 1. Rebased and Fixed the conflicts
>
> Patch-set  V6 -> V7
> 1. Addressed Alex Bennée's comments
>     - Updated the docs
> 2. Addressed Igor Mammedov's comments
>     - Merged patches [Patch V6 3/9] & [Patch V6 7/9] with [Patch V6 4/9]
>     - Updated commit-log of [Patch V6 1/9] and [Patch V6 5/9]
> 3. Added Shaoqin Huang's Reviewed-by tags for whole series.
> Link:https://lore.kernel.org/qemu-devel/20231013105129.25648-1-salil.mehta@huawei.com/
>
> Patch-set  V5 -> V6
> 1. Addressed Gavin Shan's comments
>     - Fixed the assert() ranges of address spaces
>     - Rebased the patch-set to latest changes in the qemu.git
>     - Added Reviewed-by tags for patches {8,9}
> 2. Addressed Jonathan Cameron's comments
>     - Updated commit-log for [Patch V5 1/9] with mention of trace events
>     - Added Reviewed-by tags for patches {1,5}
> 3. Added Tested-by tags from Xianglai Li
> 4. Fixed checkpatch.pl error "Qemu -> QEMU" in [Patch V5 1/9]
> Link:https://lore.kernel.org/qemu-devel/20231011194355.15628-1-salil.mehta@huawei.com/
>
> Patch-set  V4 -> V5
> 1. Addressed Gavin Shan's comments
>     - Fixed the trace events print string for kvm_{create,get,park,destroy}_vcpu
>     - Added Reviewed-by tag for patch {1}
> 2. Added Shaoqin Huang's Reviewed-by tags for Patches {2,3}
> 3. Added Tested-by Tag from Vishnu Pajjuri to the patch-set
> 4. Dropped the ARM specific [Patch V4 10/10]
> Link:https://lore.kernel.org/qemu-devel/20231009203601.17584-1-salil.mehta@huawei.com/
>
> Patch-set  V3 -> V4
> 1. Addressed David Hilderbrand's comments
>     - Fixed the wrong doc comment of kvm_park_vcpu API prototype
>     - Added Reviewed-by tags for patches {2,4}
> Link:https://lore.kernel.org/qemu-devel/20231009112812.10612-1-salil.mehta@huawei.com/
>
> Patch-set  V2 -> V3
> 1. Addressed Jonathan Cameron's comments
>     - Fixed 'vcpu-id' type wrongly changed from 'unsigned long' to 'integer'
>     - Removed unnecessary use of variable 'vcpu_id' in kvm_park_vcpu
>     - Updated [Patch V2 3/10] commit-log with details of ACPI_CPU_SCAN_METHOD macro
>     - Updated [Patch V2 5/10] commit-log with details of conditional event handler method
>     - Added Reviewed-by tags for patches {2,3,4,6,7}
> 2. Addressed Gavin Shan's comments
>     - Remove unnecessary use of variable 'vcpu_id' in kvm_par_vcpu
>     - Fixed return value in kvm_get_vcpu from -1 to -ENOENT
>     - Reset the value of 'gdb_num_g_regs' in gdb_unregister_coprocessor_all
>     - Fixed the kvm_{create,park}_vcpu prototypes docs
>     - Added Reviewed-by tags for patches {2,3,4,5,6,7,9,10}
> 3. Addressed one earlier missed comment by Alex Bennée in RFC V1
>     - Added traces instead of DPRINTF in the newly added and some existing functions
> Link:https://lore.kernel.org/qemu-devel/20230930001933.2660-1-salil.mehta@huawei.com/
>
> Patch-set V1 -> V2
> 1. Addressed Alex Bennée's comments
>     - Refactored the kvm_create_vcpu logic to get rid of goto
>     - Added the docs for kvm_{create,park}_vcpu prototypes
>     - Splitted the gdbstub and AddressSpace destruction change into separate patches
>     - Added Reviewed-by tags for patches {2,10}
> Link:https://lore.kernel.org/qemu-devel/20230929124304.13672-1-salil.mehta@huawei.com/
>
> References:
>
> [1]https://lore.kernel.org/qemu-devel/20230926100436.28284-1-salil.mehta@huawei.com/
> [2]https://lore.kernel.org/all/20230913163823.7880-1-james.morse@arm.com/
> [3]https://lore.kernel.org/qemu-devel/cover.1695697701.git.lixianglai@loongson.cn/
> [4]https://lore.kernel.org/qemu-devel/20240613233639.202896-2-salil.mehta@huawei.com/
>
> Salil Mehta (7):
>    accel/kvm: Extract common KVM vCPU {creation,parking} code
>    hw/acpi: Move CPU ctrl-dev MMIO region len macro to common header file
>    hw/acpi: Update ACPI GED framework to support vCPU Hotplug
>    hw/acpi: Update GED _EVT method AML with CPU scan
>    hw/acpi: Update CPUs AML with cpu-(ctrl)dev change
>    physmem: Add helper function to destroy CPU AddressSpace
>    gdbstub: Add helper function to unregister GDB register space
>
>   accel/kvm/kvm-all.c                    | 95 +++++++++++++++++---------
>   accel/kvm/kvm-cpus.h                   |  1 -
>   accel/kvm/trace-events                 |  5 +-
>   docs/specs/acpi_hw_reduced_hotplug.rst |  3 +-
>   gdbstub/gdbstub.c                      | 13 ++++
>   hw/acpi/acpi-cpu-hotplug-stub.c        |  6 ++
>   hw/acpi/cpu.c                          | 18 +++--
>   hw/acpi/generic_event_device.c         | 27 ++++++++
>   hw/core/cpu-common.c                   |  4 +-
>   hw/i386/acpi-build.c                   |  3 +-
>   include/exec/cpu-common.h              |  8 +++
>   include/exec/gdbstub.h                 |  6 ++
>   include/hw/acpi/cpu.h                  |  7 +-
>   include/hw/acpi/generic_event_device.h |  5 ++
>   include/hw/core/cpu.h                  |  1 +
>   include/sysemu/kvm.h                   | 25 +++++++
>   system/physmem.c                       | 29 ++++++++
>   17 files changed, 212 insertions(+), 44 deletions(-)
>
Salil Mehta July 15, 2024, 11:07 a.m. UTC | #4
Hi Vishnu,

On Mon, 15 Jul 2024 at 12:04, Vishnu Pajjuri <
vishnu@amperemail.onmicrosoft.com> wrote:

> Hi Salil,
> On 13-07-2024 23:55, Salil Mehta wrote:
>
> [Note: References are present at the last after the revision history]
>
> Virtual CPU hotplug support is being added across various architectures [1][3].
> This series adds various code bits common across all architectures:
>
> 1. vCPU creation and Parking code refactor [Patch 1]
> 2. Update ACPI GED framework to support vCPU Hotplug [Patch 2,3]
> 3. ACPI CPUs AML code change [Patch 4,5]
> 4. Helper functions to support unrealization of CPU objects [Patch 6,7]
>
> Repository:
>
> [*] Architecture *Agnostic* Patch-set (This series)
>    V14: https://github.com/salil-mehta/qemu.git virt-cpuhp-armv8/rfc-v3.arch.agnostic.v15
>
>    NOTE: This series is meant to work in conjunction with the architecture-specific
>    patch-set. For ARM, a combined patch-set (architecture agnostic + specific) was
>    earlier pushed as RFC V2 [1]. Later, RFC V2 was split into the ARM Architecture
>    specific patch-set RFC V3 [4] (a subset of RFC V2) and the architecture agnostic
>    patch-set. Patch-set V14 is the latest version in that series. This series
>    works in conjunction with RFC V4-rc2, present at the following link.
>
> [*] ARM Architecture *Specific* Patch-set
>    RFC V3 [4]: https://github.com/salil-mehta/qemu.git virt-cpuhp-armv8/rfc-v3
>    RFC V4-rc2: https://github.com/salil-mehta/qemu.git virt-cpuhp-armv8/rfc-v4-rc2 (combined)
>
>
> Revision History:
>
> Patch-set V14 -> V15
> 1. Addressed commnet from Igor Mammedov's on [PATCH V14 4/7]
>    - Removed ACPI_CPU_SCAN_METHOD
>    - Introduced AML_GED_EVT_CPU_SCAN_METHOD ("\\_SB.GED.CPSCN") macro
> 2. Fix the stray change of "assert (" in "PATCH V14 3/7"
> Link: https://lore.kernel.org/qemu-devel/20240712134201.214699-4-salil.mehta@huawei.com/
>
> I tried following test cases with rfc-v4-rc2 and kernel patches v10, and
> it looks good on Ampere platforms.
>
>    - Regular hotplug and hot unplug tests
>    - Save/restore  VM, suspend/resume VM with and with out hot-plugging
>    vcpus tests
>    - Live migration with and with out hot-plugging vcpus tests
>
> Please feel free to add,
> Tested-by: Vishnu Pajjuri <vishnu@os.amperecomputing.com>
> <vishnu@os.amperecomputing.com>
>

Many thanks for confirming this.

Best wishes
Salil.


<vishnu@os.amperecomputing.com>
>
> *Regards*,
> -Vishnu.
>
> Patch-set V13 -> V14
> 1. Addressed Igor Mammedov's following review comments
>    - Mentioned abput new external APIs in the header note of [PATCH 1/7]
>    - Merged Doc [PATCH V13 8/8] with [PATCH V14 3/7]
>    - Introduced GED realize function for various CPU Hotplug regions initializations
>    - Added back event handler method to indirectly expose \\_SB.CPUS.CSCN to GED
>      _EVT. Like for ARM, it would be through \\_SB.GED.CSCN event handler method
>    - Collected the Ack given for [Patch V13 6/8]
>    - Added back the gfree'ing of GDB regs in common finalize and made it conditional
>    - Updated the header notes of [PATCH V13 3/8,4/8,5/8] to reflect the changes
>
> Patch-set  V12 -> V13
> 1. Added Reviewed-by Tag of Harsh Prateek Bora's (IBM) [PATCH V12 1/8]
> 2. Moved the kvm_{create,park,unpark}_vcpu prototypes from accel/kvm/kvm-cpus.h
>    to include/sysemu/kvm.h. These can later be exported through AccelOps.
> Link: https://lore.kernel.org/qemu-devel/62f55169-1796-4d8e-a35d-7f003a172750@linux.ibm.com/
>
> Patch-set  V11 -> V12
> 1. Addressed Harsh Prateek Bora's (IBM) comment
>    - Changed @cpu to @vcpu_id in the kvm_unpark_vcpu protoype header/
> 2. Added Zhao Liu's (Intel) Tested-by for whole series
>    - Qtest does not breaks on Intel platforms now.
> 3. Added Zhao Liu's (Intel) Reviewed-by for [PATCH V11 {1/8 - 3/8}]
> Link: https://lore.kernel.org/qemu-devel/ZlRSPuJGBgyEUW6w@intel.com/
> Link: https://lore.kernel.org/qemu-devel/a5f3d78e-cfed-441f-9c56-e3e78fa5edee@linux.ibm.com/
>
> Patch-set  V10 -> V11
> 1. Addressed Nicholas Piggin's (IBM) comment
>    - moved the traces in kvm_unpark_vcpu and kvm_create_vcpu at the end
>    - Added the Reviewed-by Tag for [PATCH V10 1/8]
> 2.  Addressed Alex Bennée's (Linaro) comments
>    - Added a note explaining dependency of the [PATCH V10 7/8] on Arch specific patch-set
> Link: https://lore.kernel.org/qemu-devel/D1FS5GOOFWWK.2PNRIVL0V6DBL@gmail.com/
> Link: https://lore.kernel.org/qemu-devel/87frubi402.fsf@draig.linaro.org/
>
> Patch-set  V9 -> V10
> 1. Addressed Nicholas Piggin's (IBM) & Philippe Mathieu-Daudé (Linaro) comments
>    - carved out kvm_unpark_vcpu and added its trace
>    - Widened the scope of the kvm_unpark_vcpu so that it can be used by generic framework
>      being thought out
> Link: https://lore.kernel.org/qemu-devel/20240519210620.228342-1-salil.mehta@huawei.com/
> Link: https://lore.kernel.org/qemu-devel/e94b0e14-efee-4050-9c9f-08382a36b63a@linaro.org/
>
> Patch-set  V8 -> V9
> 1. Addressed Vishnu Pajjuri's (Ampere) comments
>    - Added kvm_fd to trace in kvm_create_vcpu
>    - Some clean ups: arch vcpu-id and sbd variable
>    - Added the missed initialization of cpu->gdb_num_regs
> 2. Addressed the commnet from Zhao Liu (Intel)
>    - Make initialization of CPU Hotplug state conditional (possible_cpu_arch_ids!=NULL)
> Link: https://lore.kernel.org/qemu-devel/20240312020000.12992-1-salil.mehta@huawei.com/
>
> Patch-set V7 -> V8
> 1. Rebased and Fixed the conflicts
>
> Patch-set  V6 -> V7
> 1. Addressed Alex Bennée's comments
>    - Updated the docs
> 2. Addressed Igor Mammedov's comments
>    - Merged patches [Patch V6 3/9] & [Patch V6 7/9] with [Patch V6 4/9]
>    - Updated commit-log of [Patch V6 1/9] and [Patch V6 5/9]
> 3. Added Shaoqin Huang's Reviewed-by tags for whole series.
> Link: https://lore.kernel.org/qemu-devel/20231013105129.25648-1-salil.mehta@huawei.com/
>
> Patch-set  V5 -> V6
> 1. Addressed Gavin Shan's comments
>    - Fixed the assert() ranges of address spaces
>    - Rebased the patch-set to latest changes in the qemu.git
>    - Added Reviewed-by tags for patches {8,9}
> 2. Addressed Jonathan Cameron's comments
>    - Updated commit-log for [Patch V5 1/9] with mention of trace events
>    - Added Reviewed-by tags for patches {1,5}
> 3. Added Tested-by tags from Xianglai Li
> 4. Fixed checkpatch.pl error "Qemu -> QEMU" in [Patch V5 1/9]
> Link: https://lore.kernel.org/qemu-devel/20231011194355.15628-1-salil.mehta@huawei.com/
>
> Patch-set  V4 -> V5
> 1. Addressed Gavin Shan's comments
>    - Fixed the trace events print string for kvm_{create,get,park,destroy}_vcpu
>    - Added Reviewed-by tag for patch {1}
> 2. Added Shaoqin Huang's Reviewed-by tags for Patches {2,3}
> 3. Added Tested-by Tag from Vishnu Pajjuri to the patch-set
> 4. Dropped the ARM specific [Patch V4 10/10]
> Link: https://lore.kernel.org/qemu-devel/20231009203601.17584-1-salil.mehta@huawei.com/
>
> Patch-set  V3 -> V4
> 1. Addressed David Hilderbrand's comments
>    - Fixed the wrong doc comment of kvm_park_vcpu API prototype
>    - Added Reviewed-by tags for patches {2,4}
> Link: https://lore.kernel.org/qemu-devel/20231009112812.10612-1-salil.mehta@huawei.com/
>
> Patch-set  V2 -> V3
> 1. Addressed Jonathan Cameron's comments
>    - Fixed 'vcpu-id' type wrongly changed from 'unsigned long' to 'integer'
>    - Removed unnecessary use of variable 'vcpu_id' in kvm_park_vcpu
>    - Updated [Patch V2 3/10] commit-log with details of ACPI_CPU_SCAN_METHOD macro
>    - Updated [Patch V2 5/10] commit-log with details of conditional event handler method
>    - Added Reviewed-by tags for patches {2,3,4,6,7}
> 2. Addressed Gavin Shan's comments
>    - Remove unnecessary use of variable 'vcpu_id' in kvm_par_vcpu
>    - Fixed return value in kvm_get_vcpu from -1 to -ENOENT
>    - Reset the value of 'gdb_num_g_regs' in gdb_unregister_coprocessor_all
>    - Fixed the kvm_{create,park}_vcpu prototypes docs
>    - Added Reviewed-by tags for patches {2,3,4,5,6,7,9,10}
> 3. Addressed one earlier missed comment by Alex Bennée in RFC V1
>    - Added traces instead of DPRINTF in the newly added and some existing functions
> Link: https://lore.kernel.org/qemu-devel/20230930001933.2660-1-salil.mehta@huawei.com/
>
> Patch-set V1 -> V2
> 1. Addressed Alex Bennée's comments
>    - Refactored the kvm_create_vcpu logic to get rid of goto
>    - Added the docs for kvm_{create,park}_vcpu prototypes
>    - Splitted the gdbstub and AddressSpace destruction change into separate patches
>    - Added Reviewed-by tags for patches {2,10}
> Link: https://lore.kernel.org/qemu-devel/20230929124304.13672-1-salil.mehta@huawei.com/
>
> References:
>
> [1] https://lore.kernel.org/qemu-devel/20230926100436.28284-1-salil.mehta@huawei.com/
> [2] https://lore.kernel.org/all/20230913163823.7880-1-james.morse@arm.com/
> [3] https://lore.kernel.org/qemu-devel/cover.1695697701.git.lixianglai@loongson.cn/
> [4] https://lore.kernel.org/qemu-devel/20240613233639.202896-2-salil.mehta@huawei.com/
>
> Salil Mehta (7):
>   accel/kvm: Extract common KVM vCPU {creation,parking} code
>   hw/acpi: Move CPU ctrl-dev MMIO region len macro to common header file
>   hw/acpi: Update ACPI GED framework to support vCPU Hotplug
>   hw/acpi: Update GED _EVT method AML with CPU scan
>   hw/acpi: Update CPUs AML with cpu-(ctrl)dev change
>   physmem: Add helper function to destroy CPU AddressSpace
>   gdbstub: Add helper function to unregister GDB register space
>
>  accel/kvm/kvm-all.c                    | 95 +++++++++++++++++---------
>  accel/kvm/kvm-cpus.h                   |  1 -
>  accel/kvm/trace-events                 |  5 +-
>  docs/specs/acpi_hw_reduced_hotplug.rst |  3 +-
>  gdbstub/gdbstub.c                      | 13 ++++
>  hw/acpi/acpi-cpu-hotplug-stub.c        |  6 ++
>  hw/acpi/cpu.c                          | 18 +++--
>  hw/acpi/generic_event_device.c         | 27 ++++++++
>  hw/core/cpu-common.c                   |  4 +-
>  hw/i386/acpi-build.c                   |  3 +-
>  include/exec/cpu-common.h              |  8 +++
>  include/exec/gdbstub.h                 |  6 ++
>  include/hw/acpi/cpu.h                  |  7 +-
>  include/hw/acpi/generic_event_device.h |  5 ++
>  include/hw/core/cpu.h                  |  1 +
>  include/sysemu/kvm.h                   | 25 +++++++
>  system/physmem.c                       | 29 ++++++++
>  17 files changed, 212 insertions(+), 44 deletions(-)
>
>
>
Michael S. Tsirkin July 15, 2024, 11:13 a.m. UTC | #5
On Sat, Jul 13, 2024 at 07:25:09PM +0100, Salil Mehta wrote:
> [Note: References are present at the last after the revision history]

Igor any comments before I merge this?
Salil Mehta July 15, 2024, 11:14 a.m. UTC | #6
Hi  Igor,

We are approaching end of this Qemu cycle, I believe that’s on 17th July. If you are
satisfied with the changes. May I request your Reviewed/Acked-Bys for this series?

This series is vouched by many companies. It will be good it  to merge it in this cycle.

Best regards
Salil.

>  From: Salil Mehta <salil.mehta@huawei.com>
>  Sent: Saturday, July 13, 2024 7:25 PM
>  To: qemu-devel@nongnu.org; qemu-arm@nongnu.org
>  
>  [Note: References are present at the last after the revision history]
>  
>  Virtual CPU hotplug support is being added across various architectures
>  [1][3].
>  This series adds various code bits common across all architectures:
>  
>  1. vCPU creation and Parking code refactor [Patch 1] 2. Update ACPI GED
>  framework to support vCPU Hotplug [Patch 2,3] 3. ACPI CPUs AML code
>  change [Patch 4,5] 4. Helper functions to support unrealization of CPU
>  objects [Patch 6,7]
>  
>  Repository:
>  
>  [*] Architecture *Agnostic* Patch-set (This series)
>     V14: https://github.com/salil-mehta/qemu.git virt-cpuhp-armv8/rfc-
>  v3.arch.agnostic.v15
>  
>     NOTE: This series is meant to work in conjunction with the architecture-
>  specific
>     patch-set. For ARM, a combined patch-set (architecture agnostic +
>  specific) was
>     earlier pushed as RFC V2 [1]. Later, RFC V2 was split into the ARM
>  Architecture
>     specific patch-set RFC V3 [4] (a subset of RFC V2) and the architecture
>  agnostic
>     patch-set. Patch-set V14 is the latest version in that series. This series
>     works in conjunction with RFC V4-rc2, present at the following link.
>  
>  [*] ARM Architecture *Specific* Patch-set
>     RFC V3 [4]: https://github.com/salil-mehta/qemu.git virt-cpuhp-
>  armv8/rfc-v3
>     RFC V4-rc2: https://github.com/salil-mehta/qemu.git virt-cpuhp-
>  armv8/rfc-v4-rc2 (combined)
>  
>  
>  Revision History:
>  
>  Patch-set V14 -> V15
>  1. Addressed commnet from Igor Mammedov's on [PATCH V14 4/7]
>     - Removed ACPI_CPU_SCAN_METHOD
>     - Introduced AML_GED_EVT_CPU_SCAN_METHOD ("\\_SB.GED.CPSCN")
>  macro 2. Fix the stray change of "assert (" in "PATCH V14 3/7"
>  Link: https://lore.kernel.org/qemu-devel/20240712134201.214699-4-
>  salil.mehta@huawei.com/
>  
>  Patch-set V13 -> V14
>  1. Addressed Igor Mammedov's following review comments
>     - Mentioned abput new external APIs in the header note of [PATCH 1/7]
>     - Merged Doc [PATCH V13 8/8] with [PATCH V14 3/7]
>     - Introduced GED realize function for various CPU Hotplug regions
>  initializations
>     - Added back event handler method to indirectly expose
>  \\_SB.CPUS.CSCN to GED
>       _EVT. Like for ARM, it would be through \\_SB.GED.CSCN event handler
>  method
>     - Collected the Ack given for [Patch V13 6/8]
>     - Added back the gfree'ing of GDB regs in common finalize and made it
>  conditional
>     - Updated the header notes of [PATCH V13 3/8,4/8,5/8] to reflect the
>  changes
>  
>  Patch-set  V12 -> V13
>  1. Added Reviewed-by Tag of Harsh Prateek Bora's (IBM) [PATCH V12 1/8] 2.
>  Moved the kvm_{create,park,unpark}_vcpu prototypes from
>  accel/kvm/kvm-cpus.h
>     to include/sysemu/kvm.h. These can later be exported through AccelOps.
>  Link: https://lore.kernel.org/qemu-devel/62f55169-1796-4d8e-a35d-
>  7f003a172750@linux.ibm.com/
>  
>  Patch-set  V11 -> V12
>  1. Addressed Harsh Prateek Bora's (IBM) comment
>     - Changed @cpu to @vcpu_id in the kvm_unpark_vcpu protoype header/
>  2. Added Zhao Liu's (Intel) Tested-by for whole series
>     - Qtest does not breaks on Intel platforms now.
>  3. Added Zhao Liu's (Intel) Reviewed-by for [PATCH V11 {1/8 - 3/8}]
>  Link: https://lore.kernel.org/qemu-devel/ZlRSPuJGBgyEUW6w@intel.com/
>  Link: https://lore.kernel.org/qemu-devel/a5f3d78e-cfed-441f-9c56-
>  e3e78fa5edee@linux.ibm.com/
>  
>  Patch-set  V10 -> V11
>  1. Addressed Nicholas Piggin's (IBM) comment
>     - moved the traces in kvm_unpark_vcpu and kvm_create_vcpu at the end
>     - Added the Reviewed-by Tag for [PATCH V10 1/8] 2.  Addressed Alex
>  Bennée's (Linaro) comments
>     - Added a note explaining dependency of the [PATCH V10 7/8] on Arch
>  specific patch-set
>  Link: https://lore.kernel.org/qemu-
>  devel/D1FS5GOOFWWK.2PNRIVL0V6DBL@gmail.com/
>  Link: https://lore.kernel.org/qemu-devel/87frubi402.fsf@draig.linaro.org/
>  
>  Patch-set  V9 -> V10
>  1. Addressed Nicholas Piggin's (IBM) & Philippe Mathieu-Daudé (Linaro)
>  comments
>     - carved out kvm_unpark_vcpu and added its trace
>     - Widened the scope of the kvm_unpark_vcpu so that it can be used by
>  generic framework
>       being thought out
>  Link: https://lore.kernel.org/qemu-devel/20240519210620.228342-1-
>  salil.mehta@huawei.com/
>  Link: https://lore.kernel.org/qemu-devel/e94b0e14-efee-4050-9c9f-
>  08382a36b63a@linaro.org/
>  
>  Patch-set  V8 -> V9
>  1. Addressed Vishnu Pajjuri's (Ampere) comments
>     - Added kvm_fd to trace in kvm_create_vcpu
>     - Some clean ups: arch vcpu-id and sbd variable
>     - Added the missed initialization of cpu->gdb_num_regs 2. Addressed the
>  commnet from Zhao Liu (Intel)
>     - Make initialization of CPU Hotplug state conditional
>  (possible_cpu_arch_ids!=NULL)
>  Link: https://lore.kernel.org/qemu-devel/20240312020000.12992-1-
>  salil.mehta@huawei.com/
>  
>  Patch-set V7 -> V8
>  1. Rebased and Fixed the conflicts
>  
>  Patch-set  V6 -> V7
>  1. Addressed Alex Bennée's comments
>     - Updated the docs
>  2. Addressed Igor Mammedov's comments
>     - Merged patches [Patch V6 3/9] & [Patch V6 7/9] with [Patch V6 4/9]
>     - Updated commit-log of [Patch V6 1/9] and [Patch V6 5/9]
>  3. Added Shaoqin Huang's Reviewed-by tags for whole series.
>  Link: https://lore.kernel.org/qemu-devel/20231013105129.25648-1-
>  salil.mehta@huawei.com/
>  
>  Patch-set  V5 -> V6
>  1. Addressed Gavin Shan's comments
>     - Fixed the assert() ranges of address spaces
>     - Rebased the patch-set to latest changes in the qemu.git
>     - Added Reviewed-by tags for patches {8,9} 2. Addressed Jonathan
>  Cameron's comments
>     - Updated commit-log for [Patch V5 1/9] with mention of trace events
>     - Added Reviewed-by tags for patches {1,5} 3. Added Tested-by tags from
>  Xianglai Li 4. Fixed checkpatch.pl error "Qemu -> QEMU" in [Patch V5 1/9]
>  Link: https://lore.kernel.org/qemu-devel/20231011194355.15628-1-
>  salil.mehta@huawei.com/
>  
>  Patch-set  V4 -> V5
>  1. Addressed Gavin Shan's comments
>     - Fixed the trace events print string for
>  kvm_{create,get,park,destroy}_vcpu
>     - Added Reviewed-by tag for patch {1} 2. Added Shaoqin Huang's
>  Reviewed-by tags for Patches {2,3} 3. Added Tested-by Tag from Vishnu
>  Pajjuri to the patch-set 4. Dropped the ARM specific [Patch V4 10/10]
>  Link: https://lore.kernel.org/qemu-devel/20231009203601.17584-1-
>  salil.mehta@huawei.com/
>  
>  Patch-set  V3 -> V4
>  1. Addressed David Hilderbrand's comments
>     - Fixed the wrong doc comment of kvm_park_vcpu API prototype
>     - Added Reviewed-by tags for patches {2,4}
>  Link: https://lore.kernel.org/qemu-devel/20231009112812.10612-1-
>  salil.mehta@huawei.com/
>  
>  Patch-set  V2 -> V3
>  1. Addressed Jonathan Cameron's comments
>     - Fixed 'vcpu-id' type wrongly changed from 'unsigned long' to 'integer'
>     - Removed unnecessary use of variable 'vcpu_id' in kvm_park_vcpu
>     - Updated [Patch V2 3/10] commit-log with details of
>  ACPI_CPU_SCAN_METHOD macro
>     - Updated [Patch V2 5/10] commit-log with details of conditional event
>  handler method
>     - Added Reviewed-by tags for patches {2,3,4,6,7} 2. Addressed Gavin
>  Shan's comments
>     - Remove unnecessary use of variable 'vcpu_id' in kvm_par_vcpu
>     - Fixed return value in kvm_get_vcpu from -1 to -ENOENT
>     - Reset the value of 'gdb_num_g_regs' in gdb_unregister_coprocessor_all
>     - Fixed the kvm_{create,park}_vcpu prototypes docs
>     - Added Reviewed-by tags for patches {2,3,4,5,6,7,9,10} 3. Addressed one
>  earlier missed comment by Alex Bennée in RFC V1
>     - Added traces instead of DPRINTF in the newly added and some existing
>  functions
>  Link: https://lore.kernel.org/qemu-devel/20230930001933.2660-1-
>  salil.mehta@huawei.com/
>  
>  Patch-set V1 -> V2
>  1. Addressed Alex Bennée's comments
>     - Refactored the kvm_create_vcpu logic to get rid of goto
>     - Added the docs for kvm_{create,park}_vcpu prototypes
>     - Splitted the gdbstub and AddressSpace destruction change into separate
>  patches
>     - Added Reviewed-by tags for patches {2,10}
>  Link: https://lore.kernel.org/qemu-devel/20230929124304.13672-1-
>  salil.mehta@huawei.com/
>  
>  References:
>  
>  [1] https://lore.kernel.org/qemu-devel/20230926100436.28284-1-
>  salil.mehta@huawei.com/
>  [2] https://lore.kernel.org/all/20230913163823.7880-1-
>  james.morse@arm.com/
>  [3] https://lore.kernel.org/qemu-
>  devel/cover.1695697701.git.lixianglai@loongson.cn/
>  [4] https://lore.kernel.org/qemu-devel/20240613233639.202896-2-
>  salil.mehta@huawei.com/
>  
>  Salil Mehta (7):
>    accel/kvm: Extract common KVM vCPU {creation,parking} code
>    hw/acpi: Move CPU ctrl-dev MMIO region len macro to common header
>  file
>    hw/acpi: Update ACPI GED framework to support vCPU Hotplug
>    hw/acpi: Update GED _EVT method AML with CPU scan
>    hw/acpi: Update CPUs AML with cpu-(ctrl)dev change
>    physmem: Add helper function to destroy CPU AddressSpace
>    gdbstub: Add helper function to unregister GDB register space
>  
>   accel/kvm/kvm-all.c                    | 95 +++++++++++++++++---------
>   accel/kvm/kvm-cpus.h                   |  1 -
>   accel/kvm/trace-events                 |  5 +-
>   docs/specs/acpi_hw_reduced_hotplug.rst |  3 +-
>   gdbstub/gdbstub.c                      | 13 ++++
>   hw/acpi/acpi-cpu-hotplug-stub.c        |  6 ++
>   hw/acpi/cpu.c                          | 18 +++--
>   hw/acpi/generic_event_device.c         | 27 ++++++++
>   hw/core/cpu-common.c                   |  4 +-
>   hw/i386/acpi-build.c                   |  3 +-
>   include/exec/cpu-common.h              |  8 +++
>   include/exec/gdbstub.h                 |  6 ++
>   include/hw/acpi/cpu.h                  |  7 +-
>   include/hw/acpi/generic_event_device.h |  5 ++
>   include/hw/core/cpu.h                  |  1 +
>   include/sysemu/kvm.h                   | 25 +++++++
>   system/physmem.c                       | 29 ++++++++
>   17 files changed, 212 insertions(+), 44 deletions(-)
>  
>  --
>  2.34.1
Salil Mehta July 15, 2024, 11:27 a.m. UTC | #7
Hi Michael,

>  From: Michael S. Tsirkin <mst@redhat.com>
>  Sent: Monday, July 15, 2024 12:13 PM
>  To: Salil Mehta <salil.mehta@huawei.com>
>  
>  On Sat, Jul 13, 2024 at 07:25:09PM +0100, Salil Mehta wrote:
>  > [Note: References are present at the last after the revision history]
>  
>  Igor any comments before I merge this?

Hi Michael,

Assuming there are no last-minute surprises and If you decide to merge this
series, could I kindly request that you collect all the Tags (XXX-Bys) including
the Igor's pending Reviewed/Acked-By Tag for the entire series, so that I won't
have to churn out another version (V16)?

Many thanks!

Best regards
Salil


>  
>  --
>  MST
>
Michael S. Tsirkin July 15, 2024, 11:33 a.m. UTC | #8
On Mon, Jul 15, 2024 at 11:27:57AM +0000, Salil Mehta wrote:
> Hi Michael,
> 
> >  From: Michael S. Tsirkin <mst@redhat.com>
> >  Sent: Monday, July 15, 2024 12:13 PM
> >  To: Salil Mehta <salil.mehta@huawei.com>
> >  
> >  On Sat, Jul 13, 2024 at 07:25:09PM +0100, Salil Mehta wrote:
> >  > [Note: References are present at the last after the revision history]
> >  
> >  Igor any comments before I merge this?
> 
> Hi Michael,
> 
> Assuming there are no last-minute surprises and If you decide to merge this
> series, could I kindly request that you collect all the Tags (XXX-Bys) including
> the Igor's pending Reviewed/Acked-By Tag for the entire series, so that I won't
> have to churn out another version (V16)?
> 
> Many thanks!
> 
> Best regards
> Salil


Yes, there's no need to resend just to add acks, I collect them
automatically.

> 
> >  
> >  --
> >  MST
> >
Salil Mehta July 15, 2024, 11:35 a.m. UTC | #9
>  From: Michael S. Tsirkin <mst@redhat.com>
>  Sent: Monday, July 15, 2024 12:33 PM
>  To: Salil Mehta <salil.mehta@huawei.com>
>  
>  On Mon, Jul 15, 2024 at 11:27:57AM +0000, Salil Mehta wrote:
>  > Hi Michael,
>  >
>  > >  From: Michael S. Tsirkin <mst@redhat.com>
>  > >  Sent: Monday, July 15, 2024 12:13 PM
>  > >  To: Salil Mehta <salil.mehta@huawei.com>
>  > >
>  > >  On Sat, Jul 13, 2024 at 07:25:09PM +0100, Salil Mehta wrote:
>  > >  > [Note: References are present at the last after the revision
>  > > history]
>  > >
>  > >  Igor any comments before I merge this?
>  >
>  > Hi Michael,
>  >
>  > Assuming there are no last-minute surprises and If you decide to merge
>  > this series, could I kindly request that you collect all the Tags
>  > (XXX-Bys) including the Igor's pending Reviewed/Acked-By Tag for the
>  > entire series, so that I won't have to churn out another version (V16)?
>  >
>  > Many thanks!
>  >
>  > Best regards
>  > Salil
>  
>  
>  Yes, there's no need to resend just to add acks, I collect them automatically.


Brilliant. Many thanks!


Best regards
Salil.

>  > >  --
>  > >  MST
>  > >
>
Igor Mammedov July 15, 2024, 1:54 p.m. UTC | #10
On Sat, 13 Jul 2024 19:25:09 +0100
Salil Mehta <salil.mehta@huawei.com> wrote:

> [Note: References are present at the last after the revision history]
> 
> Virtual CPU hotplug support is being added across various architectures [1][3].
> This series adds various code bits common across all architectures:
> 
> 1. vCPU creation and Parking code refactor [Patch 1]
> 2. Update ACPI GED framework to support vCPU Hotplug [Patch 2,3]
> 3. ACPI CPUs AML code change [Patch 4,5]
> 4. Helper functions to support unrealization of CPU objects [Patch 6,7]

with patch 1 and 3 fixed should be good to go.

Salil,
Can you remind me what happened to migration part of this?
Ideally it should be a part of of this series as it should be common
for everything that uses GED and should be a conditional part
of GED's VMSTATE.

If this series is just a common base and no actual hotplug
on top of it is merged in this release (provided patch 13 is fixed),
I'm fine with migration bits being a separate series on top.

However if some machine would be introducing cpu hotplug in
the same release, then the migration part should be merged before
it or be a part that cpu hotplug series. 
 
> Repository:
> 
> [*] Architecture *Agnostic* Patch-set (This series)
>    V14: https://github.com/salil-mehta/qemu.git virt-cpuhp-armv8/rfc-v3.arch.agnostic.v15
> 
>    NOTE: This series is meant to work in conjunction with the architecture-specific
>    patch-set. For ARM, a combined patch-set (architecture agnostic + specific) was
>    earlier pushed as RFC V2 [1]. Later, RFC V2 was split into the ARM Architecture
>    specific patch-set RFC V3 [4] (a subset of RFC V2) and the architecture agnostic
>    patch-set. Patch-set V14 is the latest version in that series. This series
>    works in conjunction with RFC V4-rc2, present at the following link.
> 
> [*] ARM Architecture *Specific* Patch-set
>    RFC V3 [4]: https://github.com/salil-mehta/qemu.git virt-cpuhp-armv8/rfc-v3
>    RFC V4-rc2: https://github.com/salil-mehta/qemu.git virt-cpuhp-armv8/rfc-v4-rc2 (combined)
> 
> 
> Revision History:
> 
> Patch-set V14 -> V15
> 1. Addressed commnet from Igor Mammedov's on [PATCH V14 4/7]
>    - Removed ACPI_CPU_SCAN_METHOD
>    - Introduced AML_GED_EVT_CPU_SCAN_METHOD ("\\_SB.GED.CPSCN") macro
> 2. Fix the stray change of "assert (" in "PATCH V14 3/7"
> Link: https://lore.kernel.org/qemu-devel/20240712134201.214699-4-salil.mehta@huawei.com/
> 
> Patch-set V13 -> V14
> 1. Addressed Igor Mammedov's following review comments
>    - Mentioned abput new external APIs in the header note of [PATCH 1/7]
>    - Merged Doc [PATCH V13 8/8] with [PATCH V14 3/7]
>    - Introduced GED realize function for various CPU Hotplug regions initializations
>    - Added back event handler method to indirectly expose \\_SB.CPUS.CSCN to GED
>      _EVT. Like for ARM, it would be through \\_SB.GED.CSCN event handler method
>    - Collected the Ack given for [Patch V13 6/8]
>    - Added back the gfree'ing of GDB regs in common finalize and made it conditional
>    - Updated the header notes of [PATCH V13 3/8,4/8,5/8] to reflect the changes
> 
> Patch-set  V12 -> V13
> 1. Added Reviewed-by Tag of Harsh Prateek Bora's (IBM) [PATCH V12 1/8]
> 2. Moved the kvm_{create,park,unpark}_vcpu prototypes from accel/kvm/kvm-cpus.h
>    to include/sysemu/kvm.h. These can later be exported through AccelOps.
> Link: https://lore.kernel.org/qemu-devel/62f55169-1796-4d8e-a35d-7f003a172750@linux.ibm.com/
> 
> Patch-set  V11 -> V12
> 1. Addressed Harsh Prateek Bora's (IBM) comment
>    - Changed @cpu to @vcpu_id in the kvm_unpark_vcpu protoype header/
> 2. Added Zhao Liu's (Intel) Tested-by for whole series
>    - Qtest does not breaks on Intel platforms now.
> 3. Added Zhao Liu's (Intel) Reviewed-by for [PATCH V11 {1/8 - 3/8}]
> Link: https://lore.kernel.org/qemu-devel/ZlRSPuJGBgyEUW6w@intel.com/
> Link: https://lore.kernel.org/qemu-devel/a5f3d78e-cfed-441f-9c56-e3e78fa5edee@linux.ibm.com/
> 
> Patch-set  V10 -> V11
> 1. Addressed Nicholas Piggin's (IBM) comment
>    - moved the traces in kvm_unpark_vcpu and kvm_create_vcpu at the end
>    - Added the Reviewed-by Tag for [PATCH V10 1/8]
> 2.  Addressed Alex Bennée's (Linaro) comments
>    - Added a note explaining dependency of the [PATCH V10 7/8] on Arch specific patch-set
> Link: https://lore.kernel.org/qemu-devel/D1FS5GOOFWWK.2PNRIVL0V6DBL@gmail.com/ 
> Link: https://lore.kernel.org/qemu-devel/87frubi402.fsf@draig.linaro.org/
> 
> Patch-set  V9 -> V10
> 1. Addressed Nicholas Piggin's (IBM) & Philippe Mathieu-Daudé (Linaro) comments
>    - carved out kvm_unpark_vcpu and added its trace
>    - Widened the scope of the kvm_unpark_vcpu so that it can be used by generic framework
>      being thought out
> Link: https://lore.kernel.org/qemu-devel/20240519210620.228342-1-salil.mehta@huawei.com/
> Link: https://lore.kernel.org/qemu-devel/e94b0e14-efee-4050-9c9f-08382a36b63a@linaro.org/
> 
> Patch-set  V8 -> V9
> 1. Addressed Vishnu Pajjuri's (Ampere) comments
>    - Added kvm_fd to trace in kvm_create_vcpu
>    - Some clean ups: arch vcpu-id and sbd variable
>    - Added the missed initialization of cpu->gdb_num_regs
> 2. Addressed the commnet from Zhao Liu (Intel)
>    - Make initialization of CPU Hotplug state conditional (possible_cpu_arch_ids!=NULL)
> Link: https://lore.kernel.org/qemu-devel/20240312020000.12992-1-salil.mehta@huawei.com/
> 
> Patch-set V7 -> V8
> 1. Rebased and Fixed the conflicts
> 
> Patch-set  V6 -> V7
> 1. Addressed Alex Bennée's comments
>    - Updated the docs
> 2. Addressed Igor Mammedov's comments
>    - Merged patches [Patch V6 3/9] & [Patch V6 7/9] with [Patch V6 4/9]
>    - Updated commit-log of [Patch V6 1/9] and [Patch V6 5/9]     
> 3. Added Shaoqin Huang's Reviewed-by tags for whole series.
> Link: https://lore.kernel.org/qemu-devel/20231013105129.25648-1-salil.mehta@huawei.com/
> 
> Patch-set  V5 -> V6
> 1. Addressed Gavin Shan's comments
>    - Fixed the assert() ranges of address spaces
>    - Rebased the patch-set to latest changes in the qemu.git
>    - Added Reviewed-by tags for patches {8,9}
> 2. Addressed Jonathan Cameron's comments
>    - Updated commit-log for [Patch V5 1/9] with mention of trace events
>    - Added Reviewed-by tags for patches {1,5}
> 3. Added Tested-by tags from Xianglai Li
> 4. Fixed checkpatch.pl error "Qemu -> QEMU" in [Patch V5 1/9] 
> Link: https://lore.kernel.org/qemu-devel/20231011194355.15628-1-salil.mehta@huawei.com/
> 
> Patch-set  V4 -> V5
> 1. Addressed Gavin Shan's comments
>    - Fixed the trace events print string for kvm_{create,get,park,destroy}_vcpu
>    - Added Reviewed-by tag for patch {1}
> 2. Added Shaoqin Huang's Reviewed-by tags for Patches {2,3}
> 3. Added Tested-by Tag from Vishnu Pajjuri to the patch-set
> 4. Dropped the ARM specific [Patch V4 10/10]
> Link: https://lore.kernel.org/qemu-devel/20231009203601.17584-1-salil.mehta@huawei.com/
> 
> Patch-set  V3 -> V4
> 1. Addressed David Hilderbrand's comments
>    - Fixed the wrong doc comment of kvm_park_vcpu API prototype
>    - Added Reviewed-by tags for patches {2,4}
> Link: https://lore.kernel.org/qemu-devel/20231009112812.10612-1-salil.mehta@huawei.com/
> 
> Patch-set  V2 -> V3
> 1. Addressed Jonathan Cameron's comments
>    - Fixed 'vcpu-id' type wrongly changed from 'unsigned long' to 'integer'
>    - Removed unnecessary use of variable 'vcpu_id' in kvm_park_vcpu
>    - Updated [Patch V2 3/10] commit-log with details of ACPI_CPU_SCAN_METHOD macro
>    - Updated [Patch V2 5/10] commit-log with details of conditional event handler method
>    - Added Reviewed-by tags for patches {2,3,4,6,7}
> 2. Addressed Gavin Shan's comments
>    - Remove unnecessary use of variable 'vcpu_id' in kvm_par_vcpu
>    - Fixed return value in kvm_get_vcpu from -1 to -ENOENT
>    - Reset the value of 'gdb_num_g_regs' in gdb_unregister_coprocessor_all
>    - Fixed the kvm_{create,park}_vcpu prototypes docs
>    - Added Reviewed-by tags for patches {2,3,4,5,6,7,9,10}
> 3. Addressed one earlier missed comment by Alex Bennée in RFC V1
>    - Added traces instead of DPRINTF in the newly added and some existing functions
> Link: https://lore.kernel.org/qemu-devel/20230930001933.2660-1-salil.mehta@huawei.com/
> 
> Patch-set V1 -> V2
> 1. Addressed Alex Bennée's comments
>    - Refactored the kvm_create_vcpu logic to get rid of goto
>    - Added the docs for kvm_{create,park}_vcpu prototypes
>    - Splitted the gdbstub and AddressSpace destruction change into separate patches
>    - Added Reviewed-by tags for patches {2,10}
> Link: https://lore.kernel.org/qemu-devel/20230929124304.13672-1-salil.mehta@huawei.com/
> 
> References:
> 
> [1] https://lore.kernel.org/qemu-devel/20230926100436.28284-1-salil.mehta@huawei.com/
> [2] https://lore.kernel.org/all/20230913163823.7880-1-james.morse@arm.com/
> [3] https://lore.kernel.org/qemu-devel/cover.1695697701.git.lixianglai@loongson.cn/
> [4] https://lore.kernel.org/qemu-devel/20240613233639.202896-2-salil.mehta@huawei.com/
> 
> Salil Mehta (7):
>   accel/kvm: Extract common KVM vCPU {creation,parking} code
>   hw/acpi: Move CPU ctrl-dev MMIO region len macro to common header file
>   hw/acpi: Update ACPI GED framework to support vCPU Hotplug
>   hw/acpi: Update GED _EVT method AML with CPU scan
>   hw/acpi: Update CPUs AML with cpu-(ctrl)dev change
>   physmem: Add helper function to destroy CPU AddressSpace
>   gdbstub: Add helper function to unregister GDB register space
> 
>  accel/kvm/kvm-all.c                    | 95 +++++++++++++++++---------
>  accel/kvm/kvm-cpus.h                   |  1 -
>  accel/kvm/trace-events                 |  5 +-
>  docs/specs/acpi_hw_reduced_hotplug.rst |  3 +-
>  gdbstub/gdbstub.c                      | 13 ++++
>  hw/acpi/acpi-cpu-hotplug-stub.c        |  6 ++
>  hw/acpi/cpu.c                          | 18 +++--
>  hw/acpi/generic_event_device.c         | 27 ++++++++
>  hw/core/cpu-common.c                   |  4 +-
>  hw/i386/acpi-build.c                   |  3 +-
>  include/exec/cpu-common.h              |  8 +++
>  include/exec/gdbstub.h                 |  6 ++
>  include/hw/acpi/cpu.h                  |  7 +-
>  include/hw/acpi/generic_event_device.h |  5 ++
>  include/hw/core/cpu.h                  |  1 +
>  include/sysemu/kvm.h                   | 25 +++++++
>  system/physmem.c                       | 29 ++++++++
>  17 files changed, 212 insertions(+), 44 deletions(-)
>
Igor Mammedov July 15, 2024, 1:55 p.m. UTC | #11
On Mon, 15 Jul 2024 11:27:57 +0000
Salil Mehta <salil.mehta@huawei.com> wrote:

> Hi Michael,
> 
> >  From: Michael S. Tsirkin <mst@redhat.com>
> >  Sent: Monday, July 15, 2024 12:13 PM
> >  To: Salil Mehta <salil.mehta@huawei.com>
> >  
> >  On Sat, Jul 13, 2024 at 07:25:09PM +0100, Salil Mehta wrote:  
> >  > [Note: References are present at the last after the revision history]  
> >  
> >  Igor any comments before I merge this?  
> 
> Hi Michael,
> 
> Assuming there are no last-minute surprises and If you decide to merge this
> series, could I kindly request that you collect all the Tags (XXX-Bys) including
> the Igor's pending Reviewed/Acked-By Tag for the entire series, so that I won't
> have to churn out another version (V16)?

v16 might be necessary, see cover letter.

> 
> Many thanks!
> 
> Best regards
> Salil
> 
> 
> >  
> >  --
> >  MST
> >    
>
Salil Mehta July 15, 2024, 2:14 p.m. UTC | #12
Hi Igor,

>  From: Igor Mammedov <imammedo@redhat.com>
>  Sent: Monday, July 15, 2024 2:55 PM
>  To: Salil Mehta <salil.mehta@huawei.com>
>  
>  On Sat, 13 Jul 2024 19:25:09 +0100
>  Salil Mehta <salil.mehta@huawei.com> wrote:
>  
>  > [Note: References are present at the last after the revision history]
>  >
>  > Virtual CPU hotplug support is being added across various architectures
>  [1][3].
>  > This series adds various code bits common across all architectures:
>  >
>  > 1. vCPU creation and Parking code refactor [Patch 1] 2. Update ACPI
>  > GED framework to support vCPU Hotplug [Patch 2,3] 3. ACPI CPUs AML
>  > code change [Patch 4,5] 4. Helper functions to support unrealization
>  > of CPU objects [Patch 6,7]
>  
>  with patch 1 and 3 fixed should be good to go.
>  
>  Salil,
>  Can you remind me what happened to migration part of this?
>  Ideally it should be a part of of this series as it should be common for
>  everything that uses GED and should be a conditional part of GED's
>  VMSTATE.
>  
>  If this series is just a common base and no actual hotplug on top of it is
>  merged in this release (provided patch 13 is fixed), I'm fine with migration
>  bits being a separate series on top.
>  
>  However if some machine would be introducing cpu hotplug in the same
>  release, then the migration part should be merged before it or be a part
>  that cpu hotplug series.

We have tested Live/Pseudo Migration and it seem to work with the changes
part of the architecture specific patch-set.

Ampere: https://lore.kernel.org/all/e17e28ac-28c7-496f-b212-2c9b552dbf63@amperemail.onmicrosoft.com/
Oracle: https://lore.kernel.org/all/46D74D30-EE54-4AD2-8F0E-BA5627FAA63E@oracle.com/


For ARM, please check below patch part of RFC V3 for changes related to migration:
https://lore.kernel.org/qemu-devel/20240613233639.202896-15-salil.mehta@huawei.com/


Thanks
Salil.

>  
>  > Repository:
>  >
>  > [*] Architecture *Agnostic* Patch-set (This series)
>  >    V14: https://github.com/salil-mehta/qemu.git
>  > virt-cpuhp-armv8/rfc-v3.arch.agnostic.v15
>  >
>  >    NOTE: This series is meant to work in conjunction with the architecture-
>  specific
>  >    patch-set. For ARM, a combined patch-set (architecture agnostic +
>  specific) was
>  >    earlier pushed as RFC V2 [1]. Later, RFC V2 was split into the ARM
>  Architecture
>  >    specific patch-set RFC V3 [4] (a subset of RFC V2) and the architecture
>  agnostic
>  >    patch-set. Patch-set V14 is the latest version in that series. This series
>  >    works in conjunction with RFC V4-rc2, present at the following link.
>  >
>  > [*] ARM Architecture *Specific* Patch-set
>  >    RFC V3 [4]: https://github.com/salil-mehta/qemu.git virt-cpuhp-
>  armv8/rfc-v3
>  >    RFC V4-rc2: https://github.com/salil-mehta/qemu.git
>  > virt-cpuhp-armv8/rfc-v4-rc2 (combined)
>  >
>  >
>  > Revision History:
>  >
>  > Patch-set V14 -> V15
>  > 1. Addressed commnet from Igor Mammedov's on [PATCH V14 4/7]
>  >    - Removed ACPI_CPU_SCAN_METHOD
>  >    - Introduced AML_GED_EVT_CPU_SCAN_METHOD
>  ("\\_SB.GED.CPSCN") macro
>  > 2. Fix the stray change of "assert (" in "PATCH V14 3/7"
>  > Link:
>  > https://lore.kernel.org/qemu-devel/20240712134201.214699-4-salil.mehta
>  > @huawei.com/
>  >
>  > Patch-set V13 -> V14
>  > 1. Addressed Igor Mammedov's following review comments
>  >    - Mentioned abput new external APIs in the header note of [PATCH 1/7]
>  >    - Merged Doc [PATCH V13 8/8] with [PATCH V14 3/7]
>  >    - Introduced GED realize function for various CPU Hotplug regions
>  initializations
>  >    - Added back event handler method to indirectly expose
>  \\_SB.CPUS.CSCN to GED
>  >      _EVT. Like for ARM, it would be through \\_SB.GED.CSCN event handler
>  method
>  >    - Collected the Ack given for [Patch V13 6/8]
>  >    - Added back the gfree'ing of GDB regs in common finalize and made it
>  conditional
>  >    - Updated the header notes of [PATCH V13 3/8,4/8,5/8] to reflect
>  > the changes
>  >
>  > Patch-set  V12 -> V13
>  > 1. Added Reviewed-by Tag of Harsh Prateek Bora's (IBM) [PATCH V12 1/8]
>  > 2. Moved the kvm_{create,park,unpark}_vcpu prototypes from
>  accel/kvm/kvm-cpus.h
>  >    to include/sysemu/kvm.h. These can later be exported through
>  AccelOps.
>  > Link:
>  > https://lore.kernel.org/qemu-devel/62f55169-1796-4d8e-a35d-
>  7f003a17275
>  > 0@linux.ibm.com/
>  >
>  > Patch-set  V11 -> V12
>  > 1. Addressed Harsh Prateek Bora's (IBM) comment
>  >    - Changed @cpu to @vcpu_id in the kvm_unpark_vcpu protoype
>  header/
>  > 2. Added Zhao Liu's (Intel) Tested-by for whole series
>  >    - Qtest does not breaks on Intel platforms now.
>  > 3. Added Zhao Liu's (Intel) Reviewed-by for [PATCH V11 {1/8 - 3/8}]
>  > Link: https://lore.kernel.org/qemu-
>  devel/ZlRSPuJGBgyEUW6w@intel.com/
>  > Link:
>  > https://lore.kernel.org/qemu-devel/a5f3d78e-cfed-441f-9c56-
>  e3e78fa5ede
>  > e@linux.ibm.com/
>  >
>  > Patch-set  V10 -> V11
>  > 1. Addressed Nicholas Piggin's (IBM) comment
>  >    - moved the traces in kvm_unpark_vcpu and kvm_create_vcpu at the
>  end
>  >    - Added the Reviewed-by Tag for [PATCH V10 1/8] 2.  Addressed Alex
>  > Bennée's (Linaro) comments
>  >    - Added a note explaining dependency of the [PATCH V10 7/8] on Arch
>  > specific patch-set
>  > Link:
>  > https://lore.kernel.org/qemu-
>  devel/D1FS5GOOFWWK.2PNRIVL0V6DBL@gmail.co
>  > m/
>  > Link:
>  > https://lore.kernel.org/qemu-devel/87frubi402.fsf@draig.linaro.org/
>  >
>  > Patch-set  V9 -> V10
>  > 1. Addressed Nicholas Piggin's (IBM) & Philippe Mathieu-Daudé (Linaro)
>  comments
>  >    - carved out kvm_unpark_vcpu and added its trace
>  >    - Widened the scope of the kvm_unpark_vcpu so that it can be used by
>  generic framework
>  >      being thought out
>  > Link:
>  > https://lore.kernel.org/qemu-devel/20240519210620.228342-1-salil.mehta
>  > @huawei.com/
>  > Link:
>  > https://lore.kernel.org/qemu-devel/e94b0e14-efee-4050-9c9f-
>  08382a36b63
>  > a@linaro.org/
>  >
>  > Patch-set  V8 -> V9
>  > 1. Addressed Vishnu Pajjuri's (Ampere) comments
>  >    - Added kvm_fd to trace in kvm_create_vcpu
>  >    - Some clean ups: arch vcpu-id and sbd variable
>  >    - Added the missed initialization of cpu->gdb_num_regs 2. Addressed
>  > the commnet from Zhao Liu (Intel)
>  >    - Make initialization of CPU Hotplug state conditional
>  > (possible_cpu_arch_ids!=NULL)
>  > Link:
>  > https://lore.kernel.org/qemu-devel/20240312020000.12992-1-
>  salil.mehta@
>  > huawei.com/
>  >
>  > Patch-set V7 -> V8
>  > 1. Rebased and Fixed the conflicts
>  >
>  > Patch-set  V6 -> V7
>  > 1. Addressed Alex Bennée's comments
>  >    - Updated the docs
>  > 2. Addressed Igor Mammedov's comments
>  >    - Merged patches [Patch V6 3/9] & [Patch V6 7/9] with [Patch V6 4/9]
>  >    - Updated commit-log of [Patch V6 1/9] and [Patch V6 5/9]
>  > 3. Added Shaoqin Huang's Reviewed-by tags for whole series.
>  > Link:
>  > https://lore.kernel.org/qemu-devel/20231013105129.25648-1-
>  salil.mehta@
>  > huawei.com/
>  >
>  > Patch-set  V5 -> V6
>  > 1. Addressed Gavin Shan's comments
>  >    - Fixed the assert() ranges of address spaces
>  >    - Rebased the patch-set to latest changes in the qemu.git
>  >    - Added Reviewed-by tags for patches {8,9} 2. Addressed Jonathan
>  > Cameron's comments
>  >    - Updated commit-log for [Patch V5 1/9] with mention of trace events
>  >    - Added Reviewed-by tags for patches {1,5} 3. Added Tested-by tags
>  > from Xianglai Li 4. Fixed checkpatch.pl error "Qemu -> QEMU" in [Patch
>  > V5 1/9]
>  > Link:
>  > https://lore.kernel.org/qemu-devel/20231011194355.15628-1-
>  salil.mehta@
>  > huawei.com/
>  >
>  > Patch-set  V4 -> V5
>  > 1. Addressed Gavin Shan's comments
>  >    - Fixed the trace events print string for
>  kvm_{create,get,park,destroy}_vcpu
>  >    - Added Reviewed-by tag for patch {1} 2. Added Shaoqin Huang's
>  > Reviewed-by tags for Patches {2,3} 3. Added Tested-by Tag from Vishnu
>  > Pajjuri to the patch-set 4. Dropped the ARM specific [Patch V4 10/10]
>  > Link:
>  > https://lore.kernel.org/qemu-devel/20231009203601.17584-1-
>  salil.mehta@
>  > huawei.com/
>  >
>  > Patch-set  V3 -> V4
>  > 1. Addressed David Hilderbrand's comments
>  >    - Fixed the wrong doc comment of kvm_park_vcpu API prototype
>  >    - Added Reviewed-by tags for patches {2,4}
>  > Link:
>  > https://lore.kernel.org/qemu-devel/20231009112812.10612-1-
>  salil.mehta@
>  > huawei.com/
>  >
>  > Patch-set  V2 -> V3
>  > 1. Addressed Jonathan Cameron's comments
>  >    - Fixed 'vcpu-id' type wrongly changed from 'unsigned long' to 'integer'
>  >    - Removed unnecessary use of variable 'vcpu_id' in kvm_park_vcpu
>  >    - Updated [Patch V2 3/10] commit-log with details of
>  ACPI_CPU_SCAN_METHOD macro
>  >    - Updated [Patch V2 5/10] commit-log with details of conditional event
>  handler method
>  >    - Added Reviewed-by tags for patches {2,3,4,6,7} 2. Addressed Gavin
>  > Shan's comments
>  >    - Remove unnecessary use of variable 'vcpu_id' in kvm_par_vcpu
>  >    - Fixed return value in kvm_get_vcpu from -1 to -ENOENT
>  >    - Reset the value of 'gdb_num_g_regs' in
>  gdb_unregister_coprocessor_all
>  >    - Fixed the kvm_{create,park}_vcpu prototypes docs
>  >    - Added Reviewed-by tags for patches {2,3,4,5,6,7,9,10} 3.
>  > Addressed one earlier missed comment by Alex Bennée in RFC V1
>  >    - Added traces instead of DPRINTF in the newly added and some
>  > existing functions
>  > Link:
>  > https://lore.kernel.org/qemu-devel/20230930001933.2660-1-
>  salil.mehta@h
>  > uawei.com/
>  >
>  > Patch-set V1 -> V2
>  > 1. Addressed Alex Bennée's comments
>  >    - Refactored the kvm_create_vcpu logic to get rid of goto
>  >    - Added the docs for kvm_{create,park}_vcpu prototypes
>  >    - Splitted the gdbstub and AddressSpace destruction change into
>  separate patches
>  >    - Added Reviewed-by tags for patches {2,10}
>  > Link:
>  > https://lore.kernel.org/qemu-devel/20230929124304.13672-1-
>  salil.mehta@
>  > huawei.com/
>  >
>  > References:
>  >
>  > [1]
>  > https://lore.kernel.org/qemu-devel/20230926100436.28284-1-
>  salil.mehta@
>  > huawei.com/ [2]
>  > https://lore.kernel.org/all/20230913163823.7880-1-
>  james.morse@arm.com/
>  > [3]
>  > https://lore.kernel.org/qemu-devel/cover.1695697701.git.lixianglai@loo
>  > ngson.cn/ [4]
>  > https://lore.kernel.org/qemu-devel/20240613233639.202896-2-salil.mehta
>  > @huawei.com/
>  >
>  > Salil Mehta (7):
>  >   accel/kvm: Extract common KVM vCPU {creation,parking} code
>  >   hw/acpi: Move CPU ctrl-dev MMIO region len macro to common header
>  file
>  >   hw/acpi: Update ACPI GED framework to support vCPU Hotplug
>  >   hw/acpi: Update GED _EVT method AML with CPU scan
>  >   hw/acpi: Update CPUs AML with cpu-(ctrl)dev change
>  >   physmem: Add helper function to destroy CPU AddressSpace
>  >   gdbstub: Add helper function to unregister GDB register space
>  >
>  >  accel/kvm/kvm-all.c                    | 95 +++++++++++++++++---------
>  >  accel/kvm/kvm-cpus.h                   |  1 -
>  >  accel/kvm/trace-events                 |  5 +-
>  >  docs/specs/acpi_hw_reduced_hotplug.rst |  3 +-
>  >  gdbstub/gdbstub.c                      | 13 ++++
>  >  hw/acpi/acpi-cpu-hotplug-stub.c        |  6 ++
>  >  hw/acpi/cpu.c                          | 18 +++--
>  >  hw/acpi/generic_event_device.c         | 27 ++++++++
>  >  hw/core/cpu-common.c                   |  4 +-
>  >  hw/i386/acpi-build.c                   |  3 +-
>  >  include/exec/cpu-common.h              |  8 +++
>  >  include/exec/gdbstub.h                 |  6 ++
>  >  include/hw/acpi/cpu.h                  |  7 +-
>  >  include/hw/acpi/generic_event_device.h |  5 ++
>  >  include/hw/core/cpu.h                  |  1 +
>  >  include/sysemu/kvm.h                   | 25 +++++++
>  >  system/physmem.c                       | 29 ++++++++
>  >  17 files changed, 212 insertions(+), 44 deletions(-)
>  >
>
Salil Mehta July 15, 2024, 2:19 p.m. UTC | #13
>  From: qemu-arm-bounces+salil.mehta=huawei.com@nongnu.org <qemu-
>  arm-bounces+salil.mehta=huawei.com@nongnu.org> On Behalf Of Salil
>  Mehta via
>  Sent: Monday, July 15, 2024 3:14 PM
>  To: Igor Mammedov <imammedo@redhat.com>
>  
>  Hi Igor,
>  
>  >  From: Igor Mammedov <imammedo@redhat.com>
>  >  Sent: Monday, July 15, 2024 2:55 PM
>  >  To: Salil Mehta <salil.mehta@huawei.com>
>  >
>  >  On Sat, 13 Jul 2024 19:25:09 +0100
>  >  Salil Mehta <salil.mehta@huawei.com> wrote:
>  >
>  >  > [Note: References are present at the last after the revision
>  > history]  >  > Virtual CPU hotplug support is being added across
>  > various architectures  [1][3].
>  >  > This series adds various code bits common across all architectures:
>  >  >
>  >  > 1. vCPU creation and Parking code refactor [Patch 1] 2. Update ACPI
>  > > GED framework to support vCPU Hotplug [Patch 2,3] 3. ACPI CPUs AML
>  > > code change [Patch 4,5] 4. Helper functions to support unrealization
>  > > of CPU objects [Patch 6,7]
>  >
>  >  with patch 1 and 3 fixed should be good to go.
>  >
>  >  Salil,
>  >  Can you remind me what happened to migration part of this?
>  >  Ideally it should be a part of of this series as it should be common
>  > for  everything that uses GED and should be a conditional part of
>  > GED's  VMSTATE.
>  >
>  >  If this series is just a common base and no actual hotplug on top of
>  > it is  merged in this release (provided patch 13 is fixed), I'm fine
>  > with migration  bits being a separate series on top.
>  >
>  >  However if some machine would be introducing cpu hotplug in the same
>  > release, then the migration part should be merged before it or be a
>  > part  that cpu hotplug series.
>  
>  We have tested Live/Pseudo Migration and it seem to work with the
>  changes part of the architecture specific patch-set.
>  
>  Ampere: https://lore.kernel.org/all/e17e28ac-28c7-496f-b212-
>  2c9b552dbf63@amperemail.onmicrosoft.com/
>  Oracle: https://lore.kernel.org/all/46D74D30-EE54-4AD2-8F0E-
>  BA5627FAA63E@oracle.com/
>  
>  
>  For ARM, please check below patch part of RFC V3 for changes related to
>  migration:
>  https://lore.kernel.org/qemu-devel/20240613233639.202896-15-
>  salil.mehta@huawei.com/


Do you wish to move below change into this path-set and make it common
to all instead?


diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index 63226b0040..e92ce07955 100644
--- a/hw/acpi/generic_event_device.c
+++ b/hw/acpi/generic_event_device.c
@@ -333,6 +333,16 @@ static const VMStateDescription vmstate_memhp_state = {
     }
 };
 
+static const VMStateDescription vmstate_cpuhp_state = {
+    .name = "acpi-ged/cpuhp",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_CPU_HOTPLUG(cpuhp_state, AcpiGedState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_ged_state = {
     .name = "acpi-ged-state",
     .version_id = 1,
@@ -381,6 +391,7 @@ static const VMStateDescription vmstate_acpi_ged = {
     },
     .subsections = (const VMStateDescription * const []) {
         &vmstate_memhp_state,
+        &vmstate_cpuhp_state,
         &vmstate_ghes_state,
         NULL
     }

Maybe I can add a separate patch for this in the end? Please confirm.

Thanks
Salil.
Igor Mammedov July 15, 2024, 3:11 p.m. UTC | #14
On Mon, 15 Jul 2024 14:19:12 +0000
Salil Mehta <salil.mehta@huawei.com> wrote:

> >  From: qemu-arm-bounces+salil.mehta=huawei.com@nongnu.org <qemu-
> >  arm-bounces+salil.mehta=huawei.com@nongnu.org> On Behalf Of Salil
> >  Mehta via
> >  Sent: Monday, July 15, 2024 3:14 PM
> >  To: Igor Mammedov <imammedo@redhat.com>
> >  
> >  Hi Igor,
> >    
> >  >  From: Igor Mammedov <imammedo@redhat.com>
> >  >  Sent: Monday, July 15, 2024 2:55 PM
> >  >  To: Salil Mehta <salil.mehta@huawei.com>
> >  >
> >  >  On Sat, 13 Jul 2024 19:25:09 +0100
> >  >  Salil Mehta <salil.mehta@huawei.com> wrote:
> >  >  
> >  >  > [Note: References are present at the last after the revision  
> >  > history]  >  > Virtual CPU hotplug support is being added across
> >  > various architectures  [1][3].  
> >  >  > This series adds various code bits common across all architectures:
> >  >  >
> >  >  > 1. vCPU creation and Parking code refactor [Patch 1] 2. Update ACPI
> >  > > GED framework to support vCPU Hotplug [Patch 2,3] 3. ACPI CPUs AML
> >  > > code change [Patch 4,5] 4. Helper functions to support unrealization
> >  > > of CPU objects [Patch 6,7]  
> >  >
> >  >  with patch 1 and 3 fixed should be good to go.
> >  >
> >  >  Salil,
> >  >  Can you remind me what happened to migration part of this?
> >  >  Ideally it should be a part of of this series as it should be common
> >  > for  everything that uses GED and should be a conditional part of
> >  > GED's  VMSTATE.
> >  >
> >  >  If this series is just a common base and no actual hotplug on top of
> >  > it is  merged in this release (provided patch 13 is fixed), I'm fine
> >  > with migration  bits being a separate series on top.
> >  >
> >  >  However if some machine would be introducing cpu hotplug in the same
> >  > release, then the migration part should be merged before it or be a
> >  > part  that cpu hotplug series.  
> >  
> >  We have tested Live/Pseudo Migration and it seem to work with the
> >  changes part of the architecture specific patch-set.

have you tested, migration from new QEMU to an older one (that doesn't have cpuhotplug builtin)?

> >  
> >  Ampere: https://lore.kernel.org/all/e17e28ac-28c7-496f-b212-
> >  2c9b552dbf63@amperemail.onmicrosoft.com/
> >  Oracle: https://lore.kernel.org/all/46D74D30-EE54-4AD2-8F0E-
> >  BA5627FAA63E@oracle.com/
> >  
> >  
> >  For ARM, please check below patch part of RFC V3 for changes related to
> >  migration:
> >  https://lore.kernel.org/qemu-devel/20240613233639.202896-15-
> >  salil.mehta@huawei.com/  
> 
> 
> Do you wish to move below change into this path-set and make it common
> to all instead?

it would be the best to include this with here.

> 
> 
> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> index 63226b0040..e92ce07955 100644
> --- a/hw/acpi/generic_event_device.c
> +++ b/hw/acpi/generic_event_device.c
> @@ -333,6 +333,16 @@ static const VMStateDescription vmstate_memhp_state = {
>      }
>  };
>  
> +static const VMStateDescription vmstate_cpuhp_state = {
> +    .name = "acpi-ged/cpuhp",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_CPU_HOTPLUG(cpuhp_state, AcpiGedState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static const VMStateDescription vmstate_ged_state = {
>      .name = "acpi-ged-state",
>      .version_id = 1,
> @@ -381,6 +391,7 @@ static const VMStateDescription vmstate_acpi_ged = {
>      },
>      .subsections = (const VMStateDescription * const []) {
>          &vmstate_memhp_state,
> +        &vmstate_cpuhp_state,

I'm not migration guru but I believe this should be conditional
to avoid breaking cross-version migration.
See 679dd1a957d '.needed = vmstate_test_use_cpuhp. part

CCing Peter

>          &vmstate_ghes_state,
>          NULL
>      }
> 
> Maybe I can add a separate patch for this in the end? Please confirm.
> 
> Thanks
> Salil.
Salil Mehta July 16, 2024, 3:38 a.m. UTC | #15
Hi Igor,

On 15/07/2024 15:11, Igor Mammedov wrote:
> On Mon, 15 Jul 2024 14:19:12 +0000
> Salil Mehta <salil.mehta@huawei.com> wrote:
> 
>>>   From: qemu-arm-bounces+salil.mehta=huawei.com@nongnu.org <qemu-
>>>   arm-bounces+salil.mehta=huawei.com@nongnu.org> On Behalf Of Salil
>>>   Mehta via
>>>   Sent: Monday, July 15, 2024 3:14 PM
>>>   To: Igor Mammedov <imammedo@redhat.com>
>>>   
>>>   Hi Igor,
>>>     
>>>   >  From: Igor Mammedov <imammedo@redhat.com>
>>>   >  Sent: Monday, July 15, 2024 2:55 PM
>>>   >  To: Salil Mehta <salil.mehta@huawei.com>
>>>   >
>>>   >  On Sat, 13 Jul 2024 19:25:09 +0100
>>>   >  Salil Mehta <salil.mehta@huawei.com> wrote:
>>>   >
>>>   >  > [Note: References are present at the last after the revision
>>>   > history]  >  > Virtual CPU hotplug support is being added across
>>>   > various architectures  [1][3].
>>>   >  > This series adds various code bits common across all architectures:
>>>   >  >
>>>   >  > 1. vCPU creation and Parking code refactor [Patch 1] 2. Update ACPI
>>>   > > GED framework to support vCPU Hotplug [Patch 2,3] 3. ACPI CPUs AML
>>>   > > code change [Patch 4,5] 4. Helper functions to support unrealization
>>>   > > of CPU objects [Patch 6,7]
>>>   >
>>>   >  with patch 1 and 3 fixed should be good to go.
>>>   >
>>>   >  Salil,
>>>   >  Can you remind me what happened to migration part of this?
>>>   >  Ideally it should be a part of of this series as it should be common
>>>   > for  everything that uses GED and should be a conditional part of
>>>   > GED's  VMSTATE.
>>>   >
>>>   >  If this series is just a common base and no actual hotplug on top of
>>>   > it is  merged in this release (provided patch 13 is fixed), I'm fine
>>>   > with migration  bits being a separate series on top.
>>>   >
>>>   >  However if some machine would be introducing cpu hotplug in the same
>>>   > release, then the migration part should be merged before it or be a
>>>   > part  that cpu hotplug series.
>>>   
>>>   We have tested Live/Pseudo Migration and it seem to work with the
>>>   changes part of the architecture specific patch-set.
> 
> have you tested, migration from new QEMU to an older one (that doesn't have cpuhotplug builtin)?


Just curious, how can we detect at source Qemu what version of the Qemu
destination is running. We require some sort of compatibility check but
then this is a problem not specific to CPU Hotplug?

We  are not initializing CPU Hotplug VMSD in this patch-set. I was
wondering then how can a new machine attempt to migrate VMSD state from 
new Qemu to older Qemu.

ARM vCPU Hotplug patches will be on top of this later in next Qemu cycle.


> 
>>>   
>>>   Ampere: https://lore.kernel.org/all/e17e28ac-28c7-496f-b212-
>>>   2c9b552dbf63@amperemail.onmicrosoft.com/
>>>   Oracle: https://lore.kernel.org/all/46D74D30-EE54-4AD2-8F0E-
>>>   BA5627FAA63E@oracle.com/
>>>   
>>>   
>>>   For ARM, please check below patch part of RFC V3 for changes related to
>>>   migration:
>>>   https://lore.kernel.org/qemu-devel/20240613233639.202896-15-
>>>   salil.mehta@huawei.com/
>>
>>
>> Do you wish to move below change into this path-set and make it common
>> to all instead?
> 
> it would be the best to include this with here.
> 
>>
>>
>> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
>> index 63226b0040..e92ce07955 100644
>> --- a/hw/acpi/generic_event_device.c
>> +++ b/hw/acpi/generic_event_device.c
>> @@ -333,6 +333,16 @@ static const VMStateDescription vmstate_memhp_state = {
>>       }
>>   };
>>   
>> +static const VMStateDescription vmstate_cpuhp_state = {
>> +    .name = "acpi-ged/cpuhp",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .fields      = (VMStateField[]) {
>> +        VMSTATE_CPU_HOTPLUG(cpuhp_state, AcpiGedState),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>>   static const VMStateDescription vmstate_ged_state = {
>>       .name = "acpi-ged-state",
>>       .version_id = 1,
>> @@ -381,6 +391,7 @@ static const VMStateDescription vmstate_acpi_ged = {
>>       },
>>       .subsections = (const VMStateDescription * const []) {
>>           &vmstate_memhp_state,
>> +        &vmstate_cpuhp_state,
> 
> I'm not migration guru but I believe this should be conditional
> to avoid breaking cross-version migration.
> See 679dd1a957d '.needed = vmstate_test_use_cpuhp. part


Sure, thanks for this. As I can see, the needed() function is used at
the source to decide if the state corresponding to a particular device
can be forwarded to the destination QEMU/VM. But how can this be used
to check for cross-version migration?

BTW, I've prepared V16. May I request a quick peek at:

https://github.com/salil-mehta/qemu/commits/virt-cpuhp-armv8/rfc-v3.arch.agnostic.v16/


Above does not have the suggested migration change yet. I can add it as
a separate path


Best regards,
Salil

> 
> CCing Peter
> 
>>           &vmstate_ghes_state,
>>           NULL
>>       }
>>
>> Maybe I can add a separate patch for this in the end? Please confirm.
>>
>> Thanks
>> Salil.
>
Igor Mammedov July 16, 2024, 9:52 a.m. UTC | #16
On Tue, 16 Jul 2024 03:38:29 +0000
Salil Mehta <salil.mehta@opnsrc.net> wrote:

> Hi Igor,
> 
> On 15/07/2024 15:11, Igor Mammedov wrote:
> > On Mon, 15 Jul 2024 14:19:12 +0000
> > Salil Mehta <salil.mehta@huawei.com> wrote:
> >   
> >>>   From: qemu-arm-bounces+salil.mehta=huawei.com@nongnu.org <qemu-
> >>>   arm-bounces+salil.mehta=huawei.com@nongnu.org> On Behalf Of Salil
> >>>   Mehta via
> >>>   Sent: Monday, July 15, 2024 3:14 PM
> >>>   To: Igor Mammedov <imammedo@redhat.com>
> >>>   
> >>>   Hi Igor,
> >>>       
> >>>   >  From: Igor Mammedov <imammedo@redhat.com>
> >>>   >  Sent: Monday, July 15, 2024 2:55 PM
> >>>   >  To: Salil Mehta <salil.mehta@huawei.com>
> >>>   >
> >>>   >  On Sat, 13 Jul 2024 19:25:09 +0100
> >>>   >  Salil Mehta <salil.mehta@huawei.com> wrote:
> >>>   >  
> >>>   >  > [Note: References are present at the last after the revision  
> >>>   > history]  >  > Virtual CPU hotplug support is being added across
> >>>   > various architectures  [1][3].  
> >>>   >  > This series adds various code bits common across all architectures:
> >>>   >  >
> >>>   >  > 1. vCPU creation and Parking code refactor [Patch 1] 2. Update ACPI
> >>>   > > GED framework to support vCPU Hotplug [Patch 2,3] 3. ACPI CPUs AML
> >>>   > > code change [Patch 4,5] 4. Helper functions to support unrealization
> >>>   > > of CPU objects [Patch 6,7]  
> >>>   >
> >>>   >  with patch 1 and 3 fixed should be good to go.
> >>>   >
> >>>   >  Salil,
> >>>   >  Can you remind me what happened to migration part of this?
> >>>   >  Ideally it should be a part of of this series as it should be common
> >>>   > for  everything that uses GED and should be a conditional part of
> >>>   > GED's  VMSTATE.
> >>>   >
> >>>   >  If this series is just a common base and no actual hotplug on top of
> >>>   > it is  merged in this release (provided patch 13 is fixed), I'm fine
> >>>   > with migration  bits being a separate series on top.
> >>>   >
> >>>   >  However if some machine would be introducing cpu hotplug in the same
> >>>   > release, then the migration part should be merged before it or be a
> >>>   > part  that cpu hotplug series.  
> >>>   
> >>>   We have tested Live/Pseudo Migration and it seem to work with the
> >>>   changes part of the architecture specific patch-set.  
> > 
> > have you tested, migration from new QEMU to an older one (that doesn't have cpuhotplug builtin)?  
> 
> 
> Just curious, how can we detect at source Qemu what version of the Qemu
> destination is running. We require some sort of compatibility check but
> then this is a problem not specific to CPU Hotplug?

it's usually managed by version machine types + compat settings for
machine/device.

> We  are not initializing CPU Hotplug VMSD in this patch-set. I was
> wondering then how can a new machine attempt to migrate VMSD state from 
> new Qemu to older Qemu.

If I'm not mistaken without VMSD it shouldn't explode, since CPUHP
code shouldn't create memory-regions that are migrated.
(If I recall correctly, mmio regions aren't going into migration stream)

> ARM vCPU Hotplug patches will be on top of this later in next Qemu cycle.
then it's fine to introduce VMSD later on, just make sure others
who adding cpu hotplug elsewhere also aware of it and pickup the same patch.

> 
> 
> >   
> >>>   
> >>>   Ampere: https://lore.kernel.org/all/e17e28ac-28c7-496f-b212-
> >>>   2c9b552dbf63@amperemail.onmicrosoft.com/
> >>>   Oracle: https://lore.kernel.org/all/46D74D30-EE54-4AD2-8F0E-
> >>>   BA5627FAA63E@oracle.com/
> >>>   
> >>>   
> >>>   For ARM, please check below patch part of RFC V3 for changes related to
> >>>   migration:
> >>>   https://lore.kernel.org/qemu-devel/20240613233639.202896-15-
> >>>   salil.mehta@huawei.com/  
> >>
> >>
> >> Do you wish to move below change into this path-set and make it common
> >> to all instead?  
> > 
> > it would be the best to include this with here.
> >   
> >>
> >>
> >> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> >> index 63226b0040..e92ce07955 100644
> >> --- a/hw/acpi/generic_event_device.c
> >> +++ b/hw/acpi/generic_event_device.c
> >> @@ -333,6 +333,16 @@ static const VMStateDescription vmstate_memhp_state = {
> >>       }
> >>   };
> >>   
> >> +static const VMStateDescription vmstate_cpuhp_state = {
> >> +    .name = "acpi-ged/cpuhp",
> >> +    .version_id = 1,
> >> +    .minimum_version_id = 1,
> >> +    .fields      = (VMStateField[]) {
> >> +        VMSTATE_CPU_HOTPLUG(cpuhp_state, AcpiGedState),
> >> +        VMSTATE_END_OF_LIST()
> >> +    }
> >> +};
> >> +
> >>   static const VMStateDescription vmstate_ged_state = {
> >>       .name = "acpi-ged-state",
> >>       .version_id = 1,
> >> @@ -381,6 +391,7 @@ static const VMStateDescription vmstate_acpi_ged = {
> >>       },
> >>       .subsections = (const VMStateDescription * const []) {
> >>           &vmstate_memhp_state,
> >> +        &vmstate_cpuhp_state,  
> > 
> > I'm not migration guru but I believe this should be conditional
> > to avoid breaking cross-version migration.
> > See 679dd1a957d '.needed = vmstate_test_use_cpuhp. part  
> 
> 
> Sure, thanks for this. As I can see, the needed() function is used at
> the source to decide if the state corresponding to a particular device
> can be forwarded to the destination QEMU/VM. But how can this be used
> to check for cross-version migration?

what I'd do is to make sure that older machine types to not have
cpu hotplug enabled in supported events, and only machine that
has full support for hotplug enabled the bit. And then
machine_init depending on that would manage actual 'ged-event'
property.

Then later VMSD.needed would check ged-event to decide
if section should be used or omitted.


> 
> BTW, I've prepared V16. May I request a quick peek at:
> 
> https://github.com/salil-mehta/qemu/commits/virt-cpuhp-armv8/rfc-v3.arch.agnostic.v16/

looked at
   hw/acpi: Update ACPI GED framework to support vCPU Hotplug

I get that ged_event loop in realize was copy-pasted from _EVT handler,
but that looks a bit complicated (though I won't object, it's matter of taste)

I'd prefer simpler condition than for() {} loop, and just use simpler 'if'

if (enabled_events & ACPI_GED_CPU_HOTPLUG_EVT) {
    init cpu hp code
}

PS:
if you keep for loop, I'd replace  error_report() + abort() with 'error_abort'

> 
> 
> Above does not have the suggested migration change yet. I can add it as
> a separate path
> 
> 
> Best regards,
> Salil
> 
> > 
> > CCing Peter
> >   
> >>           &vmstate_ghes_state,
> >>           NULL
> >>       }
> >>
> >> Maybe I can add a separate patch for this in the end? Please confirm.
> >>
> >> Thanks
> >> Salil.  
> >   
>
Salil Mehta July 16, 2024, 11:43 a.m. UTC | #17
Hi Igor,

>  From: Igor Mammedov <imammedo@redhat.com>
>  Sent: Tuesday, July 16, 2024 10:52 AM
>  To: Salil Mehta <salil.mehta@opnsrc.net>
>  
>  On Tue, 16 Jul 2024 03:38:29 +0000
>  Salil Mehta <salil.mehta@opnsrc.net> wrote:
>  
>  > Hi Igor,
>  >
>  > On 15/07/2024 15:11, Igor Mammedov wrote:
>  > > On Mon, 15 Jul 2024 14:19:12 +0000
>  > > Salil Mehta <salil.mehta@huawei.com> wrote:
>  > >
>  > >>>   From: qemu-arm-bounces+salil.mehta=huawei.com@nongnu.org  <qemu-
>  > >>>   arm-bounces+salil.mehta=huawei.com@nongnu.org> On Behalf Of  Salil
>  > >>>   Mehta via
>  > >>>   Sent: Monday, July 15, 2024 3:14 PM
>  > >>>   To: Igor Mammedov <imammedo@redhat.com>
>  > >>>
>  > >>>   Hi Igor,
>  > >>>
>  > >>>   >  From: Igor Mammedov <imammedo@redhat.com>
>  > >>>   >  Sent: Monday, July 15, 2024 2:55 PM
>  > >>>   >  To: Salil Mehta <salil.mehta@huawei.com>
>  > >>>   >
>  > >>>   >  On Sat, 13 Jul 2024 19:25:09 +0100
>  > >>>   >  Salil Mehta <salil.mehta@huawei.com> wrote:
>  > >>>   >
>  > >>>   >  > [Note: References are present at the last after the revision
>  > >>>   > history]  >  > Virtual CPU hotplug support is being added across
>  > >>>   > various architectures  [1][3].
>  > >>>   >  > This series adds various code bits common across all architectures:
>  > >>>   >  >
>  > >>>   >  > 1. vCPU creation and Parking code refactor [Patch 1] 2. Update ACPI
>  > >>>   > > GED framework to support vCPU Hotplug [Patch 2,3] 3. ACPI CPUs AML
>  > >>>   > > code change [Patch 4,5] 4. Helper functions to support unrealization
>  > >>>   > > of CPU objects [Patch 6,7]
>  > >>>   >
>  > >>>   >  with patch 1 and 3 fixed should be good to go.
>  > >>>   >
>  > >>>   >  Salil,
>  > >>>   >  Can you remind me what happened to migration part of this?
>  > >>>   >  Ideally it should be a part of of this series as it should be common
>  > >>>   > for  everything that uses GED and should be a conditional part of
>  > >>>   > GED's  VMSTATE.
>  > >>>   >
>  > >>>   >  If this series is just a common base and no actual hotplug on top of
>  > >>>   > it is  merged in this release (provided patch 13 is fixed), I'm fine
>  > >>>   > with migration  bits being a separate series on top.
>  > >>>   >
>  > >>>   >  However if some machine would be introducing cpu hotplug in the same
>  > >>>   > release, then the migration part should be merged before it or be a
>  > >>>   > part  that cpu hotplug series.
>  > >>>
>  > >>>   We have tested Live/Pseudo Migration and it seem to work with the
>  > >>>   changes part of the architecture specific patch-set.
>  > >
>  > > have you tested, migration from new QEMU to an older one (that doesn't have cpuhotplug builtin)?
>  >
>  >
>  > Just curious, how can we detect at source Qemu what version of the
>  > Qemu destination is running. We require some sort of compatibility
>  > check but then this is a problem not specific to CPU Hotplug?
>  
>  it's usually managed by version machine types + compat settings for
>  machine/device.

Ok. it looks to be a static checking at the source. I'm sure there must be
a way to dynamically do the same by negotiating the features i.e. only
enabling the common subset at the destination. I quickly skimmed the
migration code and I cannot find any thing like this being done as of now.
And this problem looks to be a pandoras box to me. 

>  
>  > We  are not initializing CPU Hotplug VMSD in this patch-set. I was
>  > wondering then how can a new machine attempt to migrate VMSD state
>  > from new Qemu to older Qemu.
>  
>  If I'm not mistaken without VMSD it shouldn't explode, since CPUHP code
>  shouldn't create memory-regions that are migrated.
>  (If I recall correctly, mmio regions aren't going into migration stream)

Correct.

>  
>  > ARM vCPU Hotplug patches will be on top of this later in next Qemu cycle.
>  then it's fine to introduce VMSD later on, just make sure others who adding
>  cpu hotplug elsewhere also aware of it and pickup the same patch.


Yes, thanks.


>  > >>>
>  > >>>   Ampere: https://lore.kernel.org/all/e17e28ac-28c7-496f-b212-
>  > >>>   2c9b552dbf63@amperemail.onmicrosoft.com/
>  > >>>   Oracle: https://lore.kernel.org/all/46D74D30-EE54-4AD2-8F0E-
>  > >>>   BA5627FAA63E@oracle.com/
>  > >>>
>  > >>>
>  > >>>   For ARM, please check below patch part of RFC V3 for changes related to
>  > >>>   migration:
>  > >>>   https://lore.kernel.org/qemu-devel/20240613233639.202896-15-
>  > >>>   salil.mehta@huawei.com/
>  > >>
>  > >>
>  > >> Do you wish to move below change into this path-set and make it
>  > >> common to all instead?
>  > >
>  > > it would be the best to include this with here.
>  > >
>  > >>
>  > >>
>  > >> diff --git a/hw/acpi/generic_event_device.c
>  > >> b/hw/acpi/generic_event_device.c index 63226b0040..e92ce07955
>  > >> 100644
>  > >> --- a/hw/acpi/generic_event_device.c
>  > >> +++ b/hw/acpi/generic_event_device.c
>  > >> @@ -333,6 +333,16 @@ static const VMStateDescription
>  vmstate_memhp_state = {
>  > >>       }
>  > >>   };
>  > >>
>  > >> +static const VMStateDescription vmstate_cpuhp_state = {
>  > >> +    .name = "acpi-ged/cpuhp",
>  > >> +    .version_id = 1,
>  > >> +    .minimum_version_id = 1,
>  > >> +    .fields      = (VMStateField[]) {
>  > >> +        VMSTATE_CPU_HOTPLUG(cpuhp_state, AcpiGedState),
>  > >> +        VMSTATE_END_OF_LIST()
>  > >> +    }
>  > >> +};
>  > >> +
>  > >>   static const VMStateDescription vmstate_ged_state = {
>  > >>       .name = "acpi-ged-state",
>  > >>       .version_id = 1,
>  > >> @@ -381,6 +391,7 @@ static const VMStateDescription
>  vmstate_acpi_ged = {
>  > >>       },
>  > >>       .subsections = (const VMStateDescription * const []) {
>  > >>           &vmstate_memhp_state,
>  > >> +        &vmstate_cpuhp_state,
>  > >
>  > > I'm not migration guru but I believe this should be conditional to
>  > > avoid breaking cross-version migration.
>  > > See 679dd1a957d '.needed = vmstate_test_use_cpuhp. part
>  >
>  >
>  > Sure, thanks for this. As I can see, the needed() function is used at
>  > the source to decide if the state corresponding to a particular device
>  > can be forwarded to the destination QEMU/VM. But how can this be used
>  > to check for cross-version migration?
>  
>  what I'd do is to make sure that older machine types to not have cpu
>  hotplug enabled in supported events, and only machine that has full
>  support for hotplug enabled the bit. And then machine_init depending on
>  that would manage actual 'ged-event'
>  property.
>  
>  Then later VMSD.needed would check ged-event to decide if section should
>  be used or omitted.

I understand this part, as discussed above this is not relevant to this patch-set
as we are not adding CPU Hotplug specific VMSD. But yes, I take your point and
will add above suggested change in the following patches in the next Qemu Cycle.


>  > BTW, I've prepared V16. May I request a quick peek at:
>  >
>  > https://github.com/salil-mehta/qemu/commits/virt-cpuhp-armv8/rfc-
>  v3.ar
>  > ch.agnostic.v16/
>  
>  looked at
>     hw/acpi: Update ACPI GED framework to support vCPU Hotplug
>  
>  I get that ged_event loop in realize was copy-pasted from _EVT handler, but
>  that looks a bit complicated (though I won't object, it's matter of taste)

Yes, I did that intentionally to increase the code reuse which usually is encouraged.
This reduces the amount of re-testing required. But yes, there is a con side as
well to this approach if not done properly i.e. any issues in the older code also
get propagated in the newer code.

Generally we bank upon FFS function to get the next bit and use bit shifting to
deal with this kind of logic efficiently. 

>  
>  I'd prefer simpler condition than for() {} loop, and just use simpler 'if'
>  
>  if (enabled_events & ACPI_GED_CPU_HOTPLUG_EVT) {
>      init cpu hp code
>  }
>  
>  PS:
>  if you keep for loop, I'd replace  error_report() + abort() with 'error_abort'

Maybe I should have looked a this earlier. I've  missed to address this in V16.
Do you mean error_setg(&error_abort,....) kind of logic? it has been discouraged
in favor of assert() I think?

Can we live with this for now or do you want me to send V17 for this change?


Thanks
Salil.
Igor Mammedov July 16, 2024, 3:21 p.m. UTC | #18
On Tue, 16 Jul 2024 11:43:00 +0000
Salil Mehta <salil.mehta@huawei.com> wrote:

> Hi Igor,
> 
> >  From: Igor Mammedov <imammedo@redhat.com>
> >  Sent: Tuesday, July 16, 2024 10:52 AM
> >  To: Salil Mehta <salil.mehta@opnsrc.net>
> >  
> >  On Tue, 16 Jul 2024 03:38:29 +0000
> >  Salil Mehta <salil.mehta@opnsrc.net> wrote:
> >    
> >  > Hi Igor,
> >  >
> >  > On 15/07/2024 15:11, Igor Mammedov wrote:  
> >  > > On Mon, 15 Jul 2024 14:19:12 +0000
> >  > > Salil Mehta <salil.mehta@huawei.com> wrote:
> >  > >  
> >  > >>>   From: qemu-arm-bounces+salil.mehta=huawei.com@nongnu.org  <qemu-
> >  > >>>   arm-bounces+salil.mehta=huawei.com@nongnu.org> On Behalf Of  Salil
> >  > >>>   Mehta via
> >  > >>>   Sent: Monday, July 15, 2024 3:14 PM
> >  > >>>   To: Igor Mammedov <imammedo@redhat.com>
> >  > >>>
> >  > >>>   Hi Igor,
> >  > >>>  
> >  > >>>   >  From: Igor Mammedov <imammedo@redhat.com>
> >  > >>>   >  Sent: Monday, July 15, 2024 2:55 PM
> >  > >>>   >  To: Salil Mehta <salil.mehta@huawei.com>
> >  > >>>   >
> >  > >>>   >  On Sat, 13 Jul 2024 19:25:09 +0100
> >  > >>>   >  Salil Mehta <salil.mehta@huawei.com> wrote:
> >  > >>>   >  
> >  > >>>   >  > [Note: References are present at the last after the revision  
> >  > >>>   > history]  >  > Virtual CPU hotplug support is being added across
> >  > >>>   > various architectures  [1][3].  
> >  > >>>   >  > This series adds various code bits common across all architectures:
> >  > >>>   >  >
> >  > >>>   >  > 1. vCPU creation and Parking code refactor [Patch 1] 2. Update ACPI
> >  > >>>   > > GED framework to support vCPU Hotplug [Patch 2,3] 3. ACPI CPUs AML
> >  > >>>   > > code change [Patch 4,5] 4. Helper functions to support unrealization
> >  > >>>   > > of CPU objects [Patch 6,7]  
> >  > >>>   >
> >  > >>>   >  with patch 1 and 3 fixed should be good to go.
> >  > >>>   >
> >  > >>>   >  Salil,
> >  > >>>   >  Can you remind me what happened to migration part of this?
> >  > >>>   >  Ideally it should be a part of of this series as it should be common
> >  > >>>   > for  everything that uses GED and should be a conditional part of
> >  > >>>   > GED's  VMSTATE.
> >  > >>>   >
> >  > >>>   >  If this series is just a common base and no actual hotplug on top of
> >  > >>>   > it is  merged in this release (provided patch 13 is fixed), I'm fine
> >  > >>>   > with migration  bits being a separate series on top.
> >  > >>>   >
> >  > >>>   >  However if some machine would be introducing cpu hotplug in the same
> >  > >>>   > release, then the migration part should be merged before it or be a
> >  > >>>   > part  that cpu hotplug series.  
> >  > >>>
> >  > >>>   We have tested Live/Pseudo Migration and it seem to work with the
> >  > >>>   changes part of the architecture specific patch-set.  
> >  > >
> >  > > have you tested, migration from new QEMU to an older one (that doesn't have cpuhotplug builtin)?  
> >  >
> >  >
> >  > Just curious, how can we detect at source Qemu what version of the
> >  > Qemu destination is running. We require some sort of compatibility
> >  > check but then this is a problem not specific to CPU Hotplug?  
> >  
> >  it's usually managed by version machine types + compat settings for
> >  machine/device.  
> 
> Ok. it looks to be a static checking at the source. I'm sure there must be
> a way to dynamically do the same by negotiating the features i.e. only
> enabling the common subset at the destination. I quickly skimmed the
> migration code and I cannot find any thing like this being done as of now.
> And this problem looks to be a pandoras box to me. 
no dynamic negotiating as far as I'm aware.

We've managed to survive so far with static compat knobs
(with an occasional disaster along the way)

...
> 
> Thanks
> Salil.
>