Re: [PATCH v6 00/13] Add initial Xen Suspend-to-RAM support on ARM64

[Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>]

Hi Mykola,

Mykola Kvach <xakep.amatop@xxxxxxxxx> writes:

> From: Mykola Kvach <mykola_kvach@xxxxxxxx>
>
> This is part 2 of version 5 of the ARM Xen system suspend/resume patch
> series, based on earlier work by Mirela Simonovic and Mykyta Poturai.
>
> The first part is here:
> https://marc.info/?l=xen-devel&m=175659181415965&w=2
>
> This version is ported to Xen master (4.21-unstable) and includes
> extensive improvements based on reviewer feedback. The patch series
> restructures code to improve robustness, maintainability, and implements
> system Suspend-to-RAM support on ARM64 hardware domains.
>
> At a high-level, this patch series provides:
>  - Support for Host system suspend/resume via PSCI SYSTEM_SUSPEND
> (ARM64)

I am wondering if you had to split this into 3 patches. Looks like patches
8 and 9 are useless without patch 10. They just add bunch of dead
code. Maybe it is better to squash them into one patch? I may be wrong
here, so maybe other reviewers/maintainers will correct me.


>  - Suspend/resume infrastructure for CPU context, timers, GICv2/GICv3 and 
> IPMMU-VMSA
>  - Proper error propagation and recovery throughout the suspend/resume flow
>
> Key updates in this series:
>  - Introduced architecture-specific suspend/resume infrastructure (new 
> `suspend.c`, `suspend.h`, low-level context save/restore in `head.S`)
>  - Integrated GICv2/GICv3 suspend and resume, including memory-backed context 
> save/restore with error handling
>  - Added time and IRQ suspend/resume hooks, ensuring correct timer/interrupt 
> state across suspend cycles
>  - Implemented proper PSCI SYSTEM_SUSPEND invocation and version checks
>  - Improved state management and recovery in error cases during suspend/resume
>  - Added support for IPMMU-VMSA context save/restore
>  - Added support for GICv3 eSPI registers context save/restore
>
> ---
> TODOs:
>  - Test system suspend with llc_coloring_enabled set and verify functionality
>  - Implement SMMUv3 suspend/resume handlers
>  - Enable "xl suspend" support on ARM
>  - Properly disable Xen timer watchdog from relevant services (only init.d 
> left)
>  - Add suspend/resume CI test for ARM (QEMU if feasible)
>  - Investigate feasibility and need for implementing system suspend on ARM32
> ---
>
> Changelog for v6:
>  - Add suspend/resume support for GICv3 eSPI registers (to be applied after 
> the
>    main eSPI series).
>  - Drop redundant iommu_enabled check from host system suspend.
>  - Switch from continue_hypercall_on_cpu to a dedicated tasklet for system
>    suspend, avoiding user register modification and decoupling guest/system
>    suspend status.
>  - Refactor IOMMU register context code.
>  - Improve IRQ handling: call handler->disable(), move system state checks, 
> and
>    skip IRQ release during suspend inside release_irq().
>  - Remove redundant GICv3 save/restore state logic now handled during vCPU
>    context switch.
>  - Clarify and unify error/warning messages, comments, and documentation.
>  - Correct loops for saving/restoring priorities and merge loops where 
> possible.
>  - Add explicit error for unimplemented ITS suspend support.
>  - Add missing GICD_CTLR_DS bit definition and clarify GICR_WAKER comments.
>  - Cleanup active and enable registers before restoring.
>  - Minor comment improvements and code cleanups.
>
> Changes introduced in V5:
>  - Add support for IPMMU-VMSA context save/restore
>  - Add support for GICv3 context save/restore
>  - Select HAS_SYSTEM_SUSPEND in ARM_64 instead of ARM
>  - Check llc_coloring_enabled instead of LLC_COLORING during the selection
>    of HAS_SYSTEM_SUSPEND config
>  - Call host_system_suspend from guest PSCI system suspend instead of
>    arch_domain_shutdown, reducing the complexity of the new code
>
> Changes introduced in V4:
>  - Remove the prior tasklet-based workaround in favor of a more
>    straightforward and safer solution.
>  - Rework the approach by adding explicit system state checks around
>    request_irq and release_irq calls; skip these calls during suspend
>    and resume states to avoid unsafe memory operations when IRQs are
>    disabled.
>  - Prevent reinitialization of local IRQ descriptors on system resume.
>  - Restore the state of local IRQs during system resume for secondary CPUs.
>  - Drop code for saving and restoring VCPU context (see part 1 of the patch
>    series for details).
>  - Remove IOMMU suspend and resume calls until these features are implemented.
>  - Move system suspend logic to arch_domain_shutdown, invoked from
>    domain_shutdown.
>  - Add console_end_sync to the resume path after system suspend.
>  - Drop unnecessary DAIF masking; interrupts are already masked on resume.
>  - Remove leftover TLB flush instructions; flushing is handled in enable_mmu.
>  - Avoid setting x19 in hyp_resume as it is not required.
>  - Replace prepare_secondary_mm with set_init_ttbr, and call it from 
> system_suspend.
>  - Produce a build-time error for ARM32 when CONFIG_SYSTEM_SUSPEND is enabled.
>  - Use register_t instead of uint64_t in the cpu_context structure.
>  - Apply minor fixes such as renaming functions, updating comments, and
>    modifying commit messages to accurately reflect the changes introduced
>    by this patch series.
>
> For earlier changelogs, please refer to the previous cover letters.
>
> Previous versions:
>   V1: https://marc.info/?l=xen-devel&m=154202231501850&w=2
>   V2: https://marc.info/?l=xen-devel&m=166514782207736&w=2
>   V3: https://lists.xen.org/archives/html/xen-devel/2025-03/msg00168.html
>
> Mirela Simonovic (6):
>   xen/arm: Add suspend and resume timer helpers
>   xen/arm: gic-v2: Implement GIC suspend/resume functions
>   xen/arm: Implement PSCI SYSTEM_SUSPEND call (host interface)
>   xen/arm: Resume memory management on Xen resume
>   xen/arm: Save/restore context on suspend/resume
>   xen/arm: Add support for system suspend triggered by hardware domain
>
> Mykola Kvach (5):
>   xen/arm: gic-v3: Implement GICv3 suspend/resume functions
>   xen/arm: Don't release IRQs on suspend
>   xen/arm: irq: avoid local IRQ descriptors reinit on system resume
>   xen/arm: irq: Restore state of local IRQs during system resume
>   xen/arm: gic-v3: Add suspend/resume support for eSPI registers
>
> Oleksandr Tyshchenko (2):
>   iommu/ipmmu-vmsa: Implement suspend/resume callbacks
>   xen/arm: Suspend/resume IOMMU on Xen suspend/resume
>
>  xen/arch/arm/Kconfig                     |   1 +
>  xen/arch/arm/Makefile                    |   1 +
>  xen/arch/arm/arm64/head.S                | 112 +++++++++
>  xen/arch/arm/gic-v2.c                    | 143 +++++++++++
>  xen/arch/arm/gic-v3-lpi.c                |   3 +
>  xen/arch/arm/gic-v3.c                    | 288 +++++++++++++++++++++++
>  xen/arch/arm/gic.c                       |  32 +++
>  xen/arch/arm/include/asm/gic.h           |  12 +
>  xen/arch/arm/include/asm/gic_v3_defs.h   |   1 +
>  xen/arch/arm/include/asm/mm.h            |   2 +
>  xen/arch/arm/include/asm/psci.h          |   1 +
>  xen/arch/arm/include/asm/suspend.h       |  46 ++++
>  xen/arch/arm/include/asm/time.h          |   5 +
>  xen/arch/arm/irq.c                       |  46 ++++
>  xen/arch/arm/mmu/smpboot.c               |   2 +-
>  xen/arch/arm/psci.c                      |  23 +-
>  xen/arch/arm/suspend.c                   | 175 ++++++++++++++
>  xen/arch/arm/tee/ffa_notif.c             |   2 +-
>  xen/arch/arm/time.c                      |  49 +++-
>  xen/arch/arm/vpsci.c                     |   9 +-
>  xen/common/domain.c                      |   4 +
>  xen/drivers/passthrough/arm/ipmmu-vmsa.c | 257 ++++++++++++++++++++
>  xen/drivers/passthrough/arm/smmu-v3.c    |  10 +
>  xen/drivers/passthrough/arm/smmu.c       |  10 +
>  24 files changed, 1220 insertions(+), 14 deletions(-)
>  create mode 100644 xen/arch/arm/include/asm/suspend.h
>  create mode 100644 xen/arch/arm/suspend.c

-- 
WBR, Volodymyr