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? > > Signed-off-by: Tao Tang <[email protected]> > --- > 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
