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 <[email protected]>
---
  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


Reply via email to