Hi Eric,

On 2025/9/29 23:47, Eric Auger wrote:
Hi Tao,

On 9/26/25 5:30 AM, Tao Tang wrote:
My apologies, resending patches 13-14/14 to fix a threading mistake from
my previous attempt.

Introduce a generic vmstate_smmuv3_bank that serializes a single SMMUv3
bank (registers and queues). Add a 'smmuv3/bank_s' subsection guarded by
secure_impl and a new 'migrate-secure-bank' property; when enabled, the S
bank is migrated. Add a 'smmuv3/gbpa_secure' subsection which is only sent
when GBPA differs from its reset value.

This keeps the existing migration stream unchanged by default and remains
backward compatible: older QEMUs can ignore unknown subsections, and with
'migrate-secure-bank' defaulting to off, the stream is identical to before.

This also prepares for future RME extensions (Realm/Root) by reusing the
bank subsection pattern.

Usage:
   -global arm-smmuv3,secure-impl=on,migrate-secure-bank=on

Signed-off-by: Tao Tang <[email protected]>
---
  hw/arm/smmuv3.c         | 70 +++++++++++++++++++++++++++++++++++++++++
  include/hw/arm/smmuv3.h |  1 +
  2 files changed, 71 insertions(+)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 80fbc25cf5..2a1e80d179 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -2450,6 +2450,53 @@ static const VMStateDescription vmstate_smmuv3_queue = {
      },
  };

+static const VMStateDescription vmstate_smmuv3_bank = {
I would name this vmstate_smmuv3_secure_bank


Thank you for the excellent feedback on the migration implementation. Your points are spot on.


My original thought for using the generic vmstate_smmuv3_bank was to potentially reuse it for future Realm state migration. However, I agree with you that naming it vmstate_smmuv3_secure_bank for now is a better choice for clarity and precision. I will make that change.


+
+static bool smmuv3_secure_bank_needed(void *opaque)
+{
+    SMMUv3State *s = opaque;
+
+    return s->secure_impl && s->migrate_secure_bank;
why is it needed to check s->migrate_secure_bank?

+static bool smmuv3_gbpa_secure_needed(void *opaque)
+{
+    SMMUv3State *s = opaque;
+
+    return s->secure_impl && s->migrate_secure_bank &&
same
@@ -2521,6 +2589,8 @@ static const Property smmuv3_properties[] = {
       * Defaults to false (0)
       */
      DEFINE_PROP_BOOL("secure-impl", SMMUv3State, secure_impl, false),
+    DEFINE_PROP_BOOL("migrate-secure-bank", SMMUv3State,
+                     migrate_secure_bank, false),
I don't get why you need another migrate_secure_bank prop. You need to
migrate the subsection if secure_impl is implemented, don't you?


You are absolutely right. My intention with the migrate_secure_bank property was likely to maintain as much flexibility as possible.

However, I completely agree with your logic now. Forcing the migration of the secure state whenever secure-impl is enabled is the only correct approach to prevent inconsistent states and ensure robustness. I will remove the migrate_secure_bank property and tie the migration directly to secure-impl.

Thanks for helping me correct this design flaw.

Best,
Tao


  };

  static void smmuv3_instance_init(Object *obj)
diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
index 572f15251e..5ffb609fa2 100644
--- a/include/hw/arm/smmuv3.h
+++ b/include/hw/arm/smmuv3.h
@@ -71,6 +71,7 @@ struct SMMUv3State {
      QemuMutex mutex;
      char *stage;
      bool secure_impl;
+    bool migrate_secure_bank;
  };

  typedef enum {
--
2.34.1

Eric


Reply via email to