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

[Eric Auger <eric.auger@redhat.com>]

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