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

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

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