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 <tangtao1634@phytium.com.cn>
---
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 = {
+ .name = "smmuv3_bank",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (const VMStateField[]) {
+ VMSTATE_UINT32(features, SMMUv3RegBank),
+ VMSTATE_UINT8(sid_split, SMMUv3RegBank),
+ VMSTATE_UINT32_ARRAY(cr, SMMUv3RegBank, 3),
+ VMSTATE_UINT32(cr0ack, SMMUv3RegBank),
+ VMSTATE_UINT32(irq_ctrl, SMMUv3RegBank),
+ VMSTATE_UINT32(gerror, SMMUv3RegBank),
+ VMSTATE_UINT32(gerrorn, SMMUv3RegBank),
+ VMSTATE_UINT64(gerror_irq_cfg0, SMMUv3RegBank),
+ VMSTATE_UINT32(gerror_irq_cfg1, SMMUv3RegBank),
+ VMSTATE_UINT32(gerror_irq_cfg2, SMMUv3RegBank),
+ VMSTATE_UINT64(strtab_base, SMMUv3RegBank),
+ VMSTATE_UINT32(strtab_base_cfg, SMMUv3RegBank),
+ VMSTATE_UINT64(eventq_irq_cfg0, SMMUv3RegBank),
+ VMSTATE_UINT32(eventq_irq_cfg1, SMMUv3RegBank),
+ VMSTATE_UINT32(eventq_irq_cfg2, SMMUv3RegBank),
+ VMSTATE_STRUCT(cmdq, SMMUv3RegBank, 0,
+ vmstate_smmuv3_queue, SMMUQueue),
+ VMSTATE_STRUCT(eventq, SMMUv3RegBank, 0,
+ vmstate_smmuv3_queue, SMMUQueue),
+ VMSTATE_END_OF_LIST(),
+ },
+};
+
+static bool smmuv3_secure_bank_needed(void *opaque)
+{
+ SMMUv3State *s = opaque;
+
+ return s->secure_impl && s->migrate_secure_bank;
+}
+
+static const VMStateDescription vmstate_smmuv3_bank_s = {
+ .name = "smmuv3/bank_s",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = smmuv3_secure_bank_needed,
+ .fields = (const VMStateField[]) {
+ VMSTATE_STRUCT(bank[SMMU_SEC_IDX_S], SMMUv3State, 0,
+ vmstate_smmuv3_bank, SMMUv3RegBank),
+ VMSTATE_END_OF_LIST(),
+ },
+};
+
static bool smmuv3_gbpa_needed(void *opaque)
{
SMMUv3State *s = opaque;
@@ -2469,6 +2516,25 @@ static const VMStateDescription vmstate_gbpa = {
}
};
+static bool smmuv3_gbpa_secure_needed(void *opaque)
+{
+ SMMUv3State *s = opaque;
+
+ return s->secure_impl && s->migrate_secure_bank &&
+ s->bank[SMMU_SEC_IDX_S].gbpa != SMMU_GBPA_RESET_VAL;
+}
+
+static const VMStateDescription vmstate_gbpa_secure = {
+ .name = "smmuv3/gbpa_secure",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = smmuv3_gbpa_secure_needed,
+ .fields = (const VMStateField[]) {
+ VMSTATE_UINT32(bank[SMMU_SEC_IDX_S].gbpa, SMMUv3State),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
static const VMStateDescription vmstate_smmuv3 = {
.name = "smmuv3",
.version_id = 1,
@@ -2502,6 +2568,8 @@ static const VMStateDescription vmstate_smmuv3 = {
},
.subsections = (const VMStateDescription * const []) {
&vmstate_gbpa,
+ &vmstate_smmuv3_bank_s,
+ &vmstate_gbpa_secure,
NULL
}
};
@@ -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),
};
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
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 <tangtao1634@phytium.com.cn>
> ---
> 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
> + .name = "smmuv3_bank",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (const VMStateField[]) {
> + VMSTATE_UINT32(features, SMMUv3RegBank),
> + VMSTATE_UINT8(sid_split, SMMUv3RegBank),
> + VMSTATE_UINT32_ARRAY(cr, SMMUv3RegBank, 3),
> + VMSTATE_UINT32(cr0ack, SMMUv3RegBank),
> + VMSTATE_UINT32(irq_ctrl, SMMUv3RegBank),
> + VMSTATE_UINT32(gerror, SMMUv3RegBank),
> + VMSTATE_UINT32(gerrorn, SMMUv3RegBank),
> + VMSTATE_UINT64(gerror_irq_cfg0, SMMUv3RegBank),
> + VMSTATE_UINT32(gerror_irq_cfg1, SMMUv3RegBank),
> + VMSTATE_UINT32(gerror_irq_cfg2, SMMUv3RegBank),
> + VMSTATE_UINT64(strtab_base, SMMUv3RegBank),
> + VMSTATE_UINT32(strtab_base_cfg, SMMUv3RegBank),
> + VMSTATE_UINT64(eventq_irq_cfg0, SMMUv3RegBank),
> + VMSTATE_UINT32(eventq_irq_cfg1, SMMUv3RegBank),
> + VMSTATE_UINT32(eventq_irq_cfg2, SMMUv3RegBank),
> + VMSTATE_STRUCT(cmdq, SMMUv3RegBank, 0,
> + vmstate_smmuv3_queue, SMMUQueue),
> + VMSTATE_STRUCT(eventq, SMMUv3RegBank, 0,
> + vmstate_smmuv3_queue, SMMUQueue),
> + VMSTATE_END_OF_LIST(),
> + },
> +};
> +
> +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 const VMStateDescription vmstate_smmuv3_bank_s = {
> + .name = "smmuv3/bank_s",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .needed = smmuv3_secure_bank_needed,
> + .fields = (const VMStateField[]) {
> + VMSTATE_STRUCT(bank[SMMU_SEC_IDX_S], SMMUv3State, 0,
> + vmstate_smmuv3_bank, SMMUv3RegBank),
> + VMSTATE_END_OF_LIST(),
> + },
> +};
> +
> static bool smmuv3_gbpa_needed(void *opaque)
> {
> SMMUv3State *s = opaque;
> @@ -2469,6 +2516,25 @@ static const VMStateDescription vmstate_gbpa = {
> }
> };
>
> +static bool smmuv3_gbpa_secure_needed(void *opaque)
> +{
> + SMMUv3State *s = opaque;
> +
> + return s->secure_impl && s->migrate_secure_bank &&
same
> + s->bank[SMMU_SEC_IDX_S].gbpa != SMMU_GBPA_RESET_VAL;
> +}
> +
> +static const VMStateDescription vmstate_gbpa_secure = {
> + .name = "smmuv3/gbpa_secure",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .needed = smmuv3_gbpa_secure_needed,
> + .fields = (const VMStateField[]) {
> + VMSTATE_UINT32(bank[SMMU_SEC_IDX_S].gbpa, SMMUv3State),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> static const VMStateDescription vmstate_smmuv3 = {
> .name = "smmuv3",
> .version_id = 1,
> @@ -2502,6 +2568,8 @@ static const VMStateDescription vmstate_smmuv3 = {
> },
> .subsections = (const VMStateDescription * const []) {
> &vmstate_gbpa,
> + &vmstate_smmuv3_bank_s,
> + &vmstate_gbpa_secure,
> NULL
> }
> };
> @@ -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?
> };
>
> 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
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 <tangtao1634@phytium.com.cn>
>> ---
>> 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
© 2016 - 2026 Red Hat, Inc.