[RFC v4 07/31] hw/arm/smmu-common: Add security-aware address space selector

Tao Tang posted 31 patches 1 month, 2 weeks ago
Maintainers: Eric Auger <eric.auger@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
[RFC v4 07/31] hw/arm/smmu-common: Add security-aware address space selector
Posted by Tao Tang 1 month, 2 weeks ago
Introduce SMMU_SEC_SID_S to represent SEC_SID == 1, meaning Secure. And
then provide smmu_get_address_space, a SMMU instance-based address space
selector. The helper now returns the per-device memory/secure-memory
AddressSpace and reports missing spaces.

Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
---
 hw/arm/smmu-common.c         | 17 +++++++++++++++++
 include/hw/arm/smmu-common.h |  3 +++
 2 files changed, 20 insertions(+)

diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 58c4452b1f5..3baba2a4c8e 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -30,6 +30,23 @@
 #include "hw/arm/smmu-common.h"
 #include "smmu-internal.h"
 
+AddressSpace *smmu_get_address_space(SMMUState *s, SMMUSecSID sec_sid)
+{
+    switch (sec_sid) {
+    case SMMU_SEC_SID_NS:
+        return &s->memory_as;
+    case SMMU_SEC_SID_S:
+        if (!s->secure_memory || s->secure_memory_as.root == NULL) {
+            warn_report("Secure address space requested but not available");
+            return NULL;
+        }
+        return &s->secure_memory_as;
+    default:
+        warn_report("Unknown SEC_SID value %d", sec_sid);
+        return NULL;
+    }
+}
+
 /* IOTLB Management */
 
 static guint smmu_iotlb_key_hash(gconstpointer v)
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index ae1489717fe..b3ca55effc5 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -43,6 +43,7 @@
 /* StreamID Security state */
 typedef enum SMMUSecSID {
     SMMU_SEC_SID_NS = 0,
+    SMMU_SEC_SID_S,
     SMMU_SEC_SID_NUM,
 } SMMUSecSID;
 
@@ -189,6 +190,8 @@ struct SMMUBaseClass {
 #define TYPE_ARM_SMMU "arm-smmu"
 OBJECT_DECLARE_TYPE(SMMUState, SMMUBaseClass, ARM_SMMU)
 
+AddressSpace *smmu_get_address_space(SMMUState *s, SMMUSecSID sec_sid);
+
 /* Return the SMMUPciBus handle associated to a PCI bus number */
 SMMUPciBus *smmu_find_smmu_pcibus(SMMUState *s, uint8_t bus_num);
 
-- 
2.34.1
Re: [RFC v4 07/31] hw/arm/smmu-common: Add security-aware address space selector
Posted by Pierrick Bouvier 1 month, 2 weeks ago
On 2/21/26 2:02 AM, Tao Tang wrote:
> Introduce SMMU_SEC_SID_S to represent SEC_SID == 1, meaning Secure. And
> then provide smmu_get_address_space, a SMMU instance-based address space
> selector. The helper now returns the per-device memory/secure-memory
> AddressSpace and reports missing spaces.
> 
> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
> ---
>   hw/arm/smmu-common.c         | 17 +++++++++++++++++
>   include/hw/arm/smmu-common.h |  3 +++
>   2 files changed, 20 insertions(+)
> 
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index 58c4452b1f5..3baba2a4c8e 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -30,6 +30,23 @@
>   #include "hw/arm/smmu-common.h"
>   #include "smmu-internal.h"
>   
> +AddressSpace *smmu_get_address_space(SMMUState *s, SMMUSecSID sec_sid)
> +{
> +    switch (sec_sid) {
> +    case SMMU_SEC_SID_NS:
> +        return &s->memory_as;
> +    case SMMU_SEC_SID_S:
> +        if (!s->secure_memory || s->secure_memory_as.root == NULL) {
> +            warn_report("Secure address space requested but not available");
> +            return NULL;
> +        }

Thinking about this one, I don't see any case where we would like to 
silently return here.

Instead, could we check that directly in smmuv3_realize?
If arm-smmuv3.secure-impl is enabled, then we should have
s->secure_memory set accordingly.

As well, we can decide to enable secure-impl by default if secure-memory 
is available. This way, it should do the right thing out of the box, and 
assert with a good message if the machine configuration lacks something.
And user is still free to deactivate it with -global 
arm-smmuv3.secure-impl=off if there is any reason to do so.

Do you see any reason to not enable secure-impl by default if 
secure-memory is available?

Regards,
Pierrick