On 26.08.25 16:42, Mykola Kvach wrote:
Hello Mykola, Volodymyr
Hi Volodymyr,
On Sat, Aug 23, 2025 at 8:55 PM Volodymyr Babchuk
<volodymyr_babc...@epam.com> wrote:
Hi,
Mykola Kvach <xakep.ama...@gmail.com> writes:
From: Oleksandr Tyshchenko <oleksandr_tyshche...@epam.com>
This is done using generic iommu_suspend/resume functions that cause
IOMMU driver specific suspend/resume handlers to be called for enabled
IOMMU (if one has suspend/resume driver handlers implemented).
These handlers work only when IPMMU-VMSA is used; otherwise,
we return an error during system suspend requests.
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshche...@epam.com>
Signed-off-by: Mykola Kvach <mykola_kv...@epam.com>
Tested-by: Oleksandr Andrushchenko <oleksandr_andrushche...@epam.com>
I think, the Tested-by here should be dropped. A lot of time has passed
since Oleksandr provided the tag, and the code has changed a bit (I am
afraid, the tag is now stale).
---
xen/arch/arm/suspend.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/xen/arch/arm/suspend.c b/xen/arch/arm/suspend.c
index b5398e5ca6..cb86426ebd 100644
--- a/xen/arch/arm/suspend.c
+++ b/xen/arch/arm/suspend.c
@@ -4,6 +4,7 @@
#include <asm/suspend.h>
#include <xen/console.h>
#include <xen/cpu.h>
+#include <xen/iommu.h>
#include <xen/llc-coloring.h>
#include <xen/sched.h>
@@ -49,6 +50,13 @@ static long system_suspend(void *data)
time_suspend();
+ status = iommu_suspend();
+ if ( status )
+ {
+ system_state = SYS_STATE_resume;
+ goto resume_time;
+ }
+
local_irq_save(flags);
status = gic_suspend();
if ( status )
@@ -105,6 +113,10 @@ static long system_suspend(void *data)
resume_irqs:
local_irq_restore(flags);
+
+ iommu_resume();
+
+ resume_time:
time_resume();
resume_nonboot_cpus:
@@ -140,6 +152,15 @@ int host_system_suspend(void)
return -ENOSYS;
}
+ /* TODO: drop check once suspend/resume support for SMMU is implemented */
+#ifndef CONFIG_IPMMU_VMSA
The original patch did not seem to have this check...
This check is meaningless, as you can have CONFIG_IPMMU_VMSA enabled
along with CONFIG_ARM_SMMU on a system with SMMU. I think it is enough
that iommu_suspend() will fail during system_suspend(), isn't it?
Not exactly. In the case of SMMU, we don’t have valid iommu_suspend/iommu_resume
handlers. So it’s possible to have CONFIG_ARM_SMMU enabled and iommu_enabled
set, but without the appropriate suspend handlers. This could lead to
a crash during
system_suspend().
... I also have doubts whether this check is needed (at least in its
current shape). Xen has 2 SMMU drivers at the moment. Lets imagine that
S2R for SMMUv3 is implemented, so we will need to extend #ifndef above
to cover CONFIG_ARM_SMMU_V3 as well, right (otherwise an attempt to
suspend SMMUv2-powered system will lead to crash)? Or there is a plan to
implement S2R for both SMMU implementations?
***
If we care for possible crash because iommu_suspend is NULL for
SMMUv2/SMMUv3, maybe we should consider adding stub callbacks to the
both SMMU drivers, just returning -ENOSYS?
Let's see what other people's opinions are.
+ if ( iommu_enabled )
+ {
+ dprintk(XENLOG_ERR, "IOMMU is enabled, suspend not supported yet\n");
+ return -ENOSYS;
+ }
+#endif
+
/*
* system_suspend should be called when Dom0 finalizes the suspend
* procedure from its boot core (VCPU#0). However, Dom0's VCPU#0 could
--
WBR, Volodymyr
Best regards,
Mykola
