Hi Eric,
On 2026/8/31 16:51, Eric Auger wrote:
Hi Tao,
On 8/13/26 6:26 PM, Tao Tang wrote:
Add a secure-impl device property and advertise it through
S_IDR1.SECURE_IMPL. Reject secure-impl=on unless Secure memory is available
and stage-1 translation is supported.
please document why the auto mode is needed (dependency on virt secure
option)
May make sense to document it in system/arm/virt.rst too as it has a
dependency on the secure setup of the machine
I'll document auto in the commit message and virt.rst.
Usage:
-M virt,secure=on,iommu=smmuv3 \
-global arm-smmuv3.secure-impl=on
does it work with arm-smmuv3 device too?
Yes, it also works with user-created arm-smmuv3 devices. It'll be like:
-M virt,secure=on
-device arm-smmuv3,primary-bus=pcie.0,secure-impl=on
Add the smmuv3/bank_s migration subsection for the secure register bank.
Serialize secure bank state including GBPA, IRQ config, stream table and
queue state.
Signed-off-by: Tao Tang <[email protected]>
Reviewed-by: Pierrick Bouvier <[email protected]>
---
hw/arm/smmuv3.c | 93 +++++++++++++++++++++++++++++++++++++++++
include/hw/arm/smmuv3.h | 2 +
2 files changed, 95 insertions(+)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 7e7376c65e2..8e1de94fd17 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -348,6 +348,8 @@ static void smmuv3_init_id_regs(SMMUv3State *s)
sbank->idr[1] = FIELD_DP32(sbank->idr[1], S_IDR1, S_SIDSIZE,
SMMU_IDR1_SIDSIZE);
+ sbank->idr[1] = FIELD_DP32(sbank->idr[1], S_IDR1, SECURE_IMPL,
+ s->secure_impl == ON_OFF_AUTO_ON);
smmuv3_accel_idr_override(s);
}
@@ -2752,6 +2754,37 @@ static bool smmu_validate_property(SMMUv3State *s, Error **errp)
return true;
}
+static bool smmuv3_resolve_secure_impl(SMMUv3State *s, Error **errp)
+{
+ SMMUState *bs = ARM_SMMU(s);
+ bool secure_as_available = bs->secure_memory &&
+ bs->secure_memory_as.root != NULL;
+
+ if (s->secure_impl == ON_OFF_AUTO_AUTO) {
+ s->secure_impl = secure_as_available ? ON_OFF_AUTO_ON
+ : ON_OFF_AUTO_OFF;
+ }
+
+ if (s->secure_impl == ON_OFF_AUTO_ON && !secure_as_available) {
+ error_setg(errp,
+ "secure-impl=on requires a secure-memory address space");
+ return false;
+ }
+
+ /*
+ * When SECURE_IMPL == 1, stage 1 must be supported according to
+ * (IHI 0070G.b) 6.3.53 SMMU_S_IDR1, Page 442.
+ */
+ if (s->secure_impl == ON_OFF_AUTO_ON &&
+ s->stage && !strcmp(s->stage, "2")) {
we already have smmu_validate_property(). Would be better located there
if possible. Otherwise please justify it.
Please split this patch into 2. The migration support on one end, which
only depends on OnOffAuto secure_impl;
and the rest
Yes moving this checking code into smmu_validate_property is a better
choice. I'll handle it in V6. Also this patch will be splitted as your
description.
Thanks to everyone for your time and effort in reviewing this series.
I’ll send v6 in a few days.
Best Regards,
Tao
