According to the Arm architecture, SMMU-originated memory accesses,
such as fetching commands or writing events for a secure stream, must
target the Secure Physical Address (PA) space. The existing model sends
all DMA to the global address_space_memory.
This patch introduces the infrastructure to differentiate between secure
and non-secure memory accesses. A weak global symbol,
arm_secure_address_space, is added, which can be provided by the
machine model to represent the Secure PA space.
A new helper, smmu_get_address_space(), selects the target address
space based on the is_secure context. All internal DMA calls
(dma_memory_read/write) are updated to use this helper. Additionally,
the attrs.secure bit is set on transactions targeting the secure
address space.
Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
---
hw/arm/smmu-common.c | 8 ++++++++
hw/arm/virt.c | 5 +++++
include/hw/arm/smmu-common.h | 20 ++++++++++++++++++++
3 files changed, 33 insertions(+)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 62a7612184..24db448683 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -30,6 +30,14 @@
#include "hw/arm/smmu-common.h"
#include "smmu-internal.h"
+/* Global state for secure address space availability */
+bool arm_secure_as_available;
+
+void smmu_enable_secure_address_space(void)
+{
+ arm_secure_as_available = true;
+}
+
/* IOTLB Management */
static guint smmu_iotlb_key_hash(gconstpointer v)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 02209fadcf..805d9aadb7 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -92,6 +92,8 @@
#include "hw/cxl/cxl_host.h"
#include "qemu/guest-random.h"
+AddressSpace arm_secure_address_space;
+
static GlobalProperty arm_virt_compat[] = {
{ TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "48" },
};
@@ -2243,6 +2245,9 @@ static void machvirt_init(MachineState *machine)
memory_region_init(secure_sysmem, OBJECT(machine), "secure-memory",
UINT64_MAX);
memory_region_add_subregion_overlap(secure_sysmem, 0, sysmem, -1);
+ address_space_init(&arm_secure_address_space, secure_sysmem,
+ "secure-memory-space");
+ smmu_enable_secure_address_space();
}
firmware_loaded = virt_firmware_init(vms, sysmem,
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index 3df82b83eb..cd61c5e126 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -53,6 +53,26 @@ typedef enum SMMUSecurityIndex {
SMMU_SEC_IDX_NUM,
} SMMUSecurityIndex;
+extern AddressSpace __attribute__((weak)) arm_secure_address_space;
+extern bool arm_secure_as_available;
+void smmu_enable_secure_address_space(void);
+
+static inline AddressSpace *smmu_get_address_space(SMMUSecurityIndex sec_sid)
+{
+ switch (sec_sid) {
+ case SMMU_SEC_IDX_S:
+ {
+ if (arm_secure_as_available) {
+ return &arm_secure_address_space;
+ }
+ }
+ QEMU_FALLTHROUGH;
+ case SMMU_SEC_IDX_NS:
+ default:
+ return &address_space_memory;
+ }
+}
+
/*
* Page table walk error types
*/
--
2.34.1Hi Tao,
On 9/25/25 6:26 PM, Tao Tang wrote:
> According to the Arm architecture, SMMU-originated memory accesses,
> such as fetching commands or writing events for a secure stream, must
> target the Secure Physical Address (PA) space. The existing model sends
> all DMA to the global address_space_memory.
>
> This patch introduces the infrastructure to differentiate between secure
> and non-secure memory accesses. A weak global symbol,
> arm_secure_address_space, is added, which can be provided by the
> machine model to represent the Secure PA space.
>
> A new helper, smmu_get_address_space(), selects the target address
> space based on the is_secure context. All internal DMA calls
> (dma_memory_read/write) are updated to use this helper. Additionally,
> the attrs.secure bit is set on transactions targeting the secure
> address space.
The last sentence does not seem to be implemented in that patch?
>
> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
> ---
> hw/arm/smmu-common.c | 8 ++++++++
> hw/arm/virt.c | 5 +++++
> include/hw/arm/smmu-common.h | 20 ++++++++++++++++++++
> 3 files changed, 33 insertions(+)
>
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index 62a7612184..24db448683 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -30,6 +30,14 @@
> #include "hw/arm/smmu-common.h"
> #include "smmu-internal.h"
>
> +/* Global state for secure address space availability */
> +bool arm_secure_as_available;
> +
> +void smmu_enable_secure_address_space(void)
> +{
> + arm_secure_as_available = true;
> +}
> +
> /* IOTLB Management */
>
> static guint smmu_iotlb_key_hash(gconstpointer v)
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 02209fadcf..805d9aadb7 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -92,6 +92,8 @@
> #include "hw/cxl/cxl_host.h"
> #include "qemu/guest-random.h"
>
> +AddressSpace arm_secure_address_space;
> +
> static GlobalProperty arm_virt_compat[] = {
> { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "48" },
> };
> @@ -2243,6 +2245,9 @@ static void machvirt_init(MachineState *machine)
> memory_region_init(secure_sysmem, OBJECT(machine), "secure-memory",
> UINT64_MAX);
> memory_region_add_subregion_overlap(secure_sysmem, 0, sysmem, -1);
> + address_space_init(&arm_secure_address_space, secure_sysmem,
> + "secure-memory-space");
> + smmu_enable_secure_address_space();
> }
>
> firmware_loaded = virt_firmware_init(vms, sysmem,
> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
> index 3df82b83eb..cd61c5e126 100644
> --- a/include/hw/arm/smmu-common.h
> +++ b/include/hw/arm/smmu-common.h
> @@ -53,6 +53,26 @@ typedef enum SMMUSecurityIndex {
> SMMU_SEC_IDX_NUM,
> } SMMUSecurityIndex;
>
> +extern AddressSpace __attribute__((weak)) arm_secure_address_space;
> +extern bool arm_secure_as_available;
> +void smmu_enable_secure_address_space(void);
> +
> +static inline AddressSpace *smmu_get_address_space(SMMUSecurityIndex sec_sid)
> +{
> + switch (sec_sid) {
> + case SMMU_SEC_IDX_S:
> + {
> + if (arm_secure_as_available) {
> + return &arm_secure_address_space;
> + }
don't you want to return NULL or at least emit an error in case
!arm_secure_as_available. When adding Realm support this will avoid to
return NS AS.
> + }
> + QEMU_FALLTHROUGH;
> + case SMMU_SEC_IDX_NS:
> + default:
Maybe return an error here in case of other value than NS
> + return &address_space_memory;
> + }
> +}
> +
> /*
> * Page table walk error types
> */
Thanks
Eric
Hi Eric,
On 2025/9/29 15:44, Eric Auger wrote:
> Hi Tao,
>
> On 9/25/25 6:26 PM, Tao Tang wrote:
>> According to the Arm architecture, SMMU-originated memory accesses,
>> such as fetching commands or writing events for a secure stream, must
>> target the Secure Physical Address (PA) space. The existing model sends
>> all DMA to the global address_space_memory.
>>
>> This patch introduces the infrastructure to differentiate between secure
>> and non-secure memory accesses. A weak global symbol,
>> arm_secure_address_space, is added, which can be provided by the
>> machine model to represent the Secure PA space.
>>
>> A new helper, smmu_get_address_space(), selects the target address
>> space based on the is_secure context. All internal DMA calls
>> (dma_memory_read/write) are updated to use this helper. Additionally,
>> the attrs.secure bit is set on transactions targeting the secure
>> address space.
> The last sentence does not seem to be implemented in that patch?
You are right to point this out, and my apologies for the confusion. As
I was preparing the series, the patches were intertwined, and I didn't
manage their boundaries clearly. This led me to mistakenly describe a
feature in this commit message that is only implemented in a subsequent
patch #07.
I'm very sorry for the confusion and the unnecessary time this has cost
you. In all future community interactions, I will pay special attention
to ensuring each patch and its description are atomic and self-contained
to reduce the review burden for everyone. Thank you for your guidance on
this.
>> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
>> ---
>> hw/arm/smmu-common.c | 8 ++++++++
>> hw/arm/virt.c | 5 +++++
>> include/hw/arm/smmu-common.h | 20 ++++++++++++++++++++
>> 3 files changed, 33 insertions(+)
>>
>> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
>> index 62a7612184..24db448683 100644
>> --- a/hw/arm/smmu-common.c
>> +++ b/hw/arm/smmu-common.c
>> @@ -30,6 +30,14 @@
>> #include "hw/arm/smmu-common.h"
>> #include "smmu-internal.h"
>>
>> +/* Global state for secure address space availability */
>> +bool arm_secure_as_available;
>> +
>> +void smmu_enable_secure_address_space(void)
>> +{
>> + arm_secure_as_available = true;
>> +}
>> +
>> /* IOTLB Management */
>>
>> static guint smmu_iotlb_key_hash(gconstpointer v)
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 02209fadcf..805d9aadb7 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -92,6 +92,8 @@
>> #include "hw/cxl/cxl_host.h"
>> #include "qemu/guest-random.h"
>>
>> +AddressSpace arm_secure_address_space;
>> +
>> static GlobalProperty arm_virt_compat[] = {
>> { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "48" },
>> };
>> @@ -2243,6 +2245,9 @@ static void machvirt_init(MachineState *machine)
>> memory_region_init(secure_sysmem, OBJECT(machine), "secure-memory",
>> UINT64_MAX);
>> memory_region_add_subregion_overlap(secure_sysmem, 0, sysmem, -1);
>> + address_space_init(&arm_secure_address_space, secure_sysmem,
>> + "secure-memory-space");
>> + smmu_enable_secure_address_space();
>> }
>>
>> firmware_loaded = virt_firmware_init(vms, sysmem,
>> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
>> index 3df82b83eb..cd61c5e126 100644
>> --- a/include/hw/arm/smmu-common.h
>> +++ b/include/hw/arm/smmu-common.h
>> @@ -53,6 +53,26 @@ typedef enum SMMUSecurityIndex {
>> SMMU_SEC_IDX_NUM,
>> } SMMUSecurityIndex;
>>
>> +extern AddressSpace __attribute__((weak)) arm_secure_address_space;
>> +extern bool arm_secure_as_available;
>> +void smmu_enable_secure_address_space(void);
>> +
>> +static inline AddressSpace *smmu_get_address_space(SMMUSecurityIndex sec_sid)
>> +{
>> + switch (sec_sid) {
>> + case SMMU_SEC_IDX_S:
>> + {
>> + if (arm_secure_as_available) {
>> + return &arm_secure_address_space;
>> + }
> don't you want to return NULL or at least emit an error in case
> !arm_secure_as_available. When adding Realm support this will avoid to
> return NS AS.
That's a great point. Silently falling back to the non-secure address
space is indeed dangerous. I will update the logic to return NULL and
emit an error if the secure address space is requested but not available.
>> + }
>> + QEMU_FALLTHROUGH;
>> + case SMMU_SEC_IDX_NS:
>> + default:
> Maybe return an error here in case of other value than NS
Also I will change the default case to handle unexpected values by
returning NULL, which will make the code safer for future extensions
like Realm. Then a check for the NULL return value at the call sites of
smmu_get_address_space will be applied to handle the error appropriately
in v3 series.
Thanks again for your helpful feedback.
Best,
Tao
>> + return &address_space_memory;
>> + }
>> +}
>> +
>> /*
>> * Page table walk error types
>> */
> Thanks
>
> Eric
On 9/29/25 10:33 AM, Tao Tang wrote:
> Hi Eric,
>
> On 2025/9/29 15:44, Eric Auger wrote:
>> Hi Tao,
>>
>> On 9/25/25 6:26 PM, Tao Tang wrote:
>>> According to the Arm architecture, SMMU-originated memory accesses,
>>> such as fetching commands or writing events for a secure stream, must
>>> target the Secure Physical Address (PA) space. The existing model sends
>>> all DMA to the global address_space_memory.
>>>
>>> This patch introduces the infrastructure to differentiate between
>>> secure
>>> and non-secure memory accesses. A weak global symbol,
>>> arm_secure_address_space, is added, which can be provided by the
>>> machine model to represent the Secure PA space.
>>>
>>> A new helper, smmu_get_address_space(), selects the target address
>>> space based on the is_secure context. All internal DMA calls
>>> (dma_memory_read/write) are updated to use this helper. Additionally,
>>> the attrs.secure bit is set on transactions targeting the secure
>>> address space.
>> The last sentence does not seem to be implemented in that patch?
>
>
> You are right to point this out, and my apologies for the confusion.
> As I was preparing the series, the patches were intertwined, and I
> didn't manage their boundaries clearly. This led me to mistakenly
> describe a feature in this commit message that is only implemented in
> a subsequent patch #07.
>
> I'm very sorry for the confusion and the unnecessary time this has
> cost you. In all future community interactions, I will pay special
> attention to ensuring each patch and its description are atomic and
> self-contained to reduce the review burden for everyone. Thank you for
> your guidance on this.
No problem. Your commit messages are pretty well written and we all do
such kind of oversights - at least I do ;-) -
Eric
>
>>> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
>>> ---
>>> hw/arm/smmu-common.c | 8 ++++++++
>>> hw/arm/virt.c | 5 +++++
>>> include/hw/arm/smmu-common.h | 20 ++++++++++++++++++++
>>> 3 files changed, 33 insertions(+)
>>>
>>> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
>>> index 62a7612184..24db448683 100644
>>> --- a/hw/arm/smmu-common.c
>>> +++ b/hw/arm/smmu-common.c
>>> @@ -30,6 +30,14 @@
>>> #include "hw/arm/smmu-common.h"
>>> #include "smmu-internal.h"
>>> +/* Global state for secure address space availability */
>>> +bool arm_secure_as_available;
>>> +
>>> +void smmu_enable_secure_address_space(void)
>>> +{
>>> + arm_secure_as_available = true;
>>> +}
>>> +
>>> /* IOTLB Management */
>>> static guint smmu_iotlb_key_hash(gconstpointer v)
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index 02209fadcf..805d9aadb7 100644
>>> --- a/hw/arm/virt.c
>>> +++ b/hw/arm/virt.c
>>> @@ -92,6 +92,8 @@
>>> #include "hw/cxl/cxl_host.h"
>>> #include "qemu/guest-random.h"
>>> +AddressSpace arm_secure_address_space;
>>> +
>>> static GlobalProperty arm_virt_compat[] = {
>>> { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "48" },
>>> };
>>> @@ -2243,6 +2245,9 @@ static void machvirt_init(MachineState *machine)
>>> memory_region_init(secure_sysmem, OBJECT(machine),
>>> "secure-memory",
>>> UINT64_MAX);
>>> memory_region_add_subregion_overlap(secure_sysmem, 0,
>>> sysmem, -1);
>>> + address_space_init(&arm_secure_address_space, secure_sysmem,
>>> + "secure-memory-space");
>>> + smmu_enable_secure_address_space();
>>> }
>>> firmware_loaded = virt_firmware_init(vms, sysmem,
>>> diff --git a/include/hw/arm/smmu-common.h
>>> b/include/hw/arm/smmu-common.h
>>> index 3df82b83eb..cd61c5e126 100644
>>> --- a/include/hw/arm/smmu-common.h
>>> +++ b/include/hw/arm/smmu-common.h
>>> @@ -53,6 +53,26 @@ typedef enum SMMUSecurityIndex {
>>> SMMU_SEC_IDX_NUM,
>>> } SMMUSecurityIndex;
>>> +extern AddressSpace __attribute__((weak)) arm_secure_address_space;
>>> +extern bool arm_secure_as_available;
>>> +void smmu_enable_secure_address_space(void);
>>> +
>>> +static inline AddressSpace
>>> *smmu_get_address_space(SMMUSecurityIndex sec_sid)
>>> +{
>>> + switch (sec_sid) {
>>> + case SMMU_SEC_IDX_S:
>>> + {
>>> + if (arm_secure_as_available) {
>>> + return &arm_secure_address_space;
>>> + }
>> don't you want to return NULL or at least emit an error in case
>> !arm_secure_as_available. When adding Realm support this will avoid to
>> return NS AS.
>
>
> That's a great point. Silently falling back to the non-secure address
> space is indeed dangerous. I will update the logic to return NULL and
> emit an error if the secure address space is requested but not available.
>
>>> + }
>>> + QEMU_FALLTHROUGH;
>>> + case SMMU_SEC_IDX_NS:
>>> + default:
>> Maybe return an error here in case of other value than NS
>
> Also I will change the default case to handle unexpected values by
> returning NULL, which will make the code safer for future extensions
> like Realm. Then a check for the NULL return value at the call sites
> of smmu_get_address_space will be applied to handle the error
> appropriately in v3 series.
>
>
> Thanks again for your helpful feedback.
>
>
> Best,
>
> Tao
>
>
>>> + return &address_space_memory;
>>> + }
>>> +}
>>> +
>>> /*
>>> * Page table walk error types
>>> */
>> Thanks
>>
>> Eric
>
© 2016 - 2026 Red Hat, Inc.