[RFC v3 08/21] hw/arm/smmuv3: Add separate address space for secure SMMU accesses

Tao Tang posted 21 patches 4 months ago
Maintainers: Eric Auger <eric.auger@redhat.com>, Peter Maydell <peter.maydell@linaro.org>
[RFC v3 08/21] hw/arm/smmuv3: Add separate address space for secure SMMU accesses
Posted by Tao Tang 4 months ago
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 non-secure address_space_memory.

This patch introduces the infrastructure to differentiate between secure
and non-secure memory accesses. Firstly, SMMU_SEC_SID_S is added in
SMMUSecSID enum to represent the secure context. Then 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 SEC_SID. All internal DMA calls
(dma_memory_read/write) will be updated to use this helper in follow-up
patches.

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 | 27 +++++++++++++++++++++++++++
 3 files changed, 40 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 175023897a..83dc62a095 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" },
 };
@@ -2257,6 +2259,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 b0dae18a62..d54558f94b 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -43,9 +43,36 @@
 /* StreamID Security state */
 typedef enum SMMUSecSID {
     SMMU_SEC_SID_NS = 0,
+    SMMU_SEC_SID_S,
     SMMU_SEC_SID_NUM,
 } SMMUSecSID;
 
+extern AddressSpace __attribute__((weak)) arm_secure_address_space;
+extern bool arm_secure_as_available;
+void smmu_enable_secure_address_space(void);
+
+/*
+ * Return the address space corresponding to the SEC_SID.
+ * If SEC_SID is Secure, but secure address space is not available,
+ * return NULL and print a warning message.
+ */
+static inline AddressSpace *smmu_get_address_space(SMMUSecSID sec_sid)
+{
+    switch (sec_sid) {
+    case SMMU_SEC_SID_NS:
+        return &address_space_memory;
+    case SMMU_SEC_SID_S:
+        if (!arm_secure_as_available || arm_secure_address_space.root == NULL) {
+            printf("Secure address space requested but not available");
+            return NULL;
+        }
+        return &arm_secure_address_space;
+    default:
+        printf("Unknown SEC_SID value %d", sec_sid);
+        return NULL;
+    }
+}
+
 /*
  * Page table walk error types
  */
-- 
2.34.1
Re: [RFC v3 08/21] hw/arm/smmuv3: Add separate address space for secure SMMU accesses
Posted by Pierrick Bouvier 2 months ago
Hi Tao,

On 10/12/25 8:06 AM, 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 non-secure address_space_memory.
> 
> This patch introduces the infrastructure to differentiate between secure
> and non-secure memory accesses. Firstly, SMMU_SEC_SID_S is added in
> SMMUSecSID enum to represent the secure context. Then 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 SEC_SID. All internal DMA calls
> (dma_memory_read/write) will be updated to use this helper in follow-up
> patches.
> 
> 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 | 27 +++++++++++++++++++++++++++
>   3 files changed, 40 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 175023897a..83dc62a095 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" },
>   };
> @@ -2257,6 +2259,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 b0dae18a62..d54558f94b 100644
> --- a/include/hw/arm/smmu-common.h
> +++ b/include/hw/arm/smmu-common.h
> @@ -43,9 +43,36 @@
>   /* StreamID Security state */
>   typedef enum SMMUSecSID {
>       SMMU_SEC_SID_NS = 0,
> +    SMMU_SEC_SID_S,
>       SMMU_SEC_SID_NUM,
>   } SMMUSecSID;
>   
> +extern AddressSpace __attribute__((weak)) arm_secure_address_space;
> +extern bool arm_secure_as_available;
> +void smmu_enable_secure_address_space(void);
> +
> +/*
> + * Return the address space corresponding to the SEC_SID.
> + * If SEC_SID is Secure, but secure address space is not available,
> + * return NULL and print a warning message.
> + */
> +static inline AddressSpace *smmu_get_address_space(SMMUSecSID sec_sid)
> +{
> +    switch (sec_sid) {
> +    case SMMU_SEC_SID_NS:
> +        return &address_space_memory;
> +    case SMMU_SEC_SID_S:
> +        if (!arm_secure_as_available || arm_secure_address_space.root == NULL) {
> +            printf("Secure address space requested but not available");
> +            return NULL;
> +        }
> +        return &arm_secure_address_space;
> +    default:
> +        printf("Unknown SEC_SID value %d", sec_sid);
> +        return NULL;
> +    }
> +}
> +
>   /*
>    * Page table walk error types
>    */

I ran into the same issue, when adding Granule Protection Check to the 
SMMU, for RME support. It requires access to secure memory, where 
Granule Protection Table is kept, and thus, access secure address space.

After talking with Richard and Philippe, I have been suggested a better 
way. Similar to how arm cpus handle this, boards (virt & sbsa-ref) are 
simply passing pointers to MemoryRegion for global and secure memory.
Then, the SMMU can create its own address spaces, based on those regions.

It's clean, does not require any weak variable, and mimic what is 
already done for cpus. Please see the two patches attached.

First one define properties, and pass memory regions from boards to 
SMMU. Second one replace global address spaces with SMMU ones.

I'll send patch 1 as it's own series, and you can take inspiration from 
patch 2 for this series. SMMU unit tests will need to be modified to be 
passed the memory regions also.

Regards,
Pierrick
Re: [RFC v3 08/21] hw/arm/smmuv3: Add separate address space for secure SMMU accesses
Posted by Pierrick Bouvier 2 months ago
On 12/11/25 2:12 PM, Pierrick Bouvier wrote:
> Hi Tao,
> 
> On 10/12/25 8:06 AM, 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 non-secure address_space_memory.
>>
>> This patch introduces the infrastructure to differentiate between secure
>> and non-secure memory accesses. Firstly, SMMU_SEC_SID_S is added in
>> SMMUSecSID enum to represent the secure context. Then 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 SEC_SID. All internal DMA calls
>> (dma_memory_read/write) will be updated to use this helper in follow-up
>> patches.
>>
>> 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 | 27 +++++++++++++++++++++++++++
>>    3 files changed, 40 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 175023897a..83dc62a095 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" },
>>    };
>> @@ -2257,6 +2259,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 b0dae18a62..d54558f94b 100644
>> --- a/include/hw/arm/smmu-common.h
>> +++ b/include/hw/arm/smmu-common.h
>> @@ -43,9 +43,36 @@
>>    /* StreamID Security state */
>>    typedef enum SMMUSecSID {
>>        SMMU_SEC_SID_NS = 0,
>> +    SMMU_SEC_SID_S,
>>        SMMU_SEC_SID_NUM,
>>    } SMMUSecSID;
>>    
>> +extern AddressSpace __attribute__((weak)) arm_secure_address_space;
>> +extern bool arm_secure_as_available;
>> +void smmu_enable_secure_address_space(void);
>> +
>> +/*
>> + * Return the address space corresponding to the SEC_SID.
>> + * If SEC_SID is Secure, but secure address space is not available,
>> + * return NULL and print a warning message.
>> + */
>> +static inline AddressSpace *smmu_get_address_space(SMMUSecSID sec_sid)
>> +{
>> +    switch (sec_sid) {
>> +    case SMMU_SEC_SID_NS:
>> +        return &address_space_memory;
>> +    case SMMU_SEC_SID_S:
>> +        if (!arm_secure_as_available || arm_secure_address_space.root == NULL) {
>> +            printf("Secure address space requested but not available");
>> +            return NULL;
>> +        }
>> +        return &arm_secure_address_space;
>> +    default:
>> +        printf("Unknown SEC_SID value %d", sec_sid);
>> +        return NULL;
>> +    }
>> +}
>> +
>>    /*
>>     * Page table walk error types
>>     */
> 
> I ran into the same issue, when adding Granule Protection Check to the
> SMMU, for RME support. It requires access to secure memory, where
> Granule Protection Table is kept, and thus, access secure address space.
> 
> After talking with Richard and Philippe, I have been suggested a better
> way. Similar to how arm cpus handle this, boards (virt & sbsa-ref) are
> simply passing pointers to MemoryRegion for global and secure memory.
> Then, the SMMU can create its own address spaces, based on those regions.
> 
> It's clean, does not require any weak variable, and mimic what is
> already done for cpus. Please see the two patches attached.
> 
> First one define properties, and pass memory regions from boards to
> SMMU. Second one replace global address spaces with SMMU ones.
> 
> I'll send patch 1 as it's own series, and you can take inspiration from
> patch 2 for this series. SMMU unit tests will need to be modified to be
> passed the memory regions also.
> 
> Regards,
> Pierrick

Sent patch 1 here:
https://lore.kernel.org/qemu-devel/20251211221715.2206662-1-pierrick.bouvier@linaro.org/T/#u
Re: [RFC v3 08/21] hw/arm/smmuv3: Add separate address space for secure SMMU accesses
Posted by Eric Auger 2 months, 1 week ago
Hi Tao,

On 10/12/25 5:06 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 non-secure address_space_memory.
>
> This patch introduces the infrastructure to differentiate between secure
> and non-secure memory accesses. Firstly, SMMU_SEC_SID_S is added in
> SMMUSecSID enum to represent the secure context. Then 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 SEC_SID. All internal DMA calls
> (dma_memory_read/write) will be updated to use this helper in follow-up
> patches.
>
> 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 | 27 +++++++++++++++++++++++++++
>  3 files changed, 40 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;
don't you need to initialize it?

why is it local to the SMMU. To me the secure address space sounds
global like address_space_memory usable by other IPs than the SMMU and
the CPUs.
> +
> +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 175023897a..83dc62a095 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" },
>  };
> @@ -2257,6 +2259,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");
besides using dynamic allocation like in cpu_address_space_init() would
allow to get rid of arm_secure_as_available
> +        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 b0dae18a62..d54558f94b 100644
> --- a/include/hw/arm/smmu-common.h
> +++ b/include/hw/arm/smmu-common.h
> @@ -43,9 +43,36 @@
>  /* StreamID Security state */
>  typedef enum SMMUSecSID {
>      SMMU_SEC_SID_NS = 0,
> +    SMMU_SEC_SID_S,
>      SMMU_SEC_SID_NUM,
>  } SMMUSecSID;
>  
> +extern AddressSpace __attribute__((weak)) arm_secure_address_space;
> +extern bool arm_secure_as_available;
> +void smmu_enable_secure_address_space(void);
> +
> +/*
> + * Return the address space corresponding to the SEC_SID.
> + * If SEC_SID is Secure, but secure address space is not available,
> + * return NULL and print a warning message.
> + */
> +static inline AddressSpace *smmu_get_address_space(SMMUSecSID sec_sid)
> +{
> +    switch (sec_sid) {
> +    case SMMU_SEC_SID_NS:
> +        return &address_space_memory;
> +    case SMMU_SEC_SID_S:
> +        if (!arm_secure_as_available || arm_secure_address_space.root == NULL) {
> +            printf("Secure address space requested but not available");
> +            return NULL;
> +        }
> +        return &arm_secure_address_space;
> +    default:
> +        printf("Unknown SEC_SID value %d", sec_sid);
> +        return NULL;
> +    }
> +}
> +
>  /*
>   * Page table walk error types
>   */
Thanks

Eric


Re: [RFC v3 08/21] hw/arm/smmuv3: Add separate address space for secure SMMU accesses
Posted by Tao Tang 2 months, 1 week ago
Hi Eric,

On 2025/12/2 21:53, Eric Auger wrote:
> Hi Tao,
>
> On 10/12/25 5:06 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 non-secure address_space_memory.
>>
>> This patch introduces the infrastructure to differentiate between secure
>> and non-secure memory accesses. Firstly, SMMU_SEC_SID_S is added in
>> SMMUSecSID enum to represent the secure context. Then 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 SEC_SID. All internal DMA calls
>> (dma_memory_read/write) will be updated to use this helper in follow-up
>> patches.
>>
>> 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 | 27 +++++++++++++++++++++++++++
>>   3 files changed, 40 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;
> don't you need to initialize it?
>
> why is it local to the SMMU. To me the secure address space sounds
> global like address_space_memory usable by other IPs than the SMMU and
> the CPUs.
>> +
>> +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 175023897a..83dc62a095 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" },
>>   };
>> @@ -2257,6 +2259,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");
> besides using dynamic allocation like in cpu_address_space_init() would
> allow to get rid of arm_secure_as_available


Thanks for the feedback.

I also think the extra arm_secure_as_available flag is unnecessary after 
read the cpu_address_space_init code.

For the next version I plan to:

- Drop the arm_secure_as_available concept entirely.

- Make the secure address space dynamically allocated in the machine 
code : secure_address_space = g_new0(AddressSpace, 1);

- Have smmu_get_address_space() simply check whether the secure 
AddressSpace * is non-NULL, instead of relying on a separate 
availability flag.

Thanks again for the review and suggestions. Best regards, Tao