On 1/5/26 12:10 PM, Philippe Mathieu-Daudé wrote:
On 5/1/26 21:05, Philippe Mathieu-Daudé wrote:
On 17/12/25 00:57, Pierrick Bouvier wrote:
This will be used to access non-secure and secure memory. Secure support
and Granule Protection Check (for RME) for SMMU need to access secure
memory.

As well, it allows to remove usage of global address_space_memory,
allowing different SMMU instances to have a specific view of memory.

User creatable SMMU are handled as well for virt machine,
by setting the memory properties when device is plugged in.

Signed-off-by: Pierrick Bouvier <[email protected]>
---
   include/hw/arm/smmu-common.h |  4 ++++
   include/hw/arm/virt.h        |  2 ++
   hw/arm/sbsa-ref.c            | 16 ++++++++++++----
   hw/arm/smmu-common.c         | 25 +++++++++++++++++++++++++
   hw/arm/virt.c                | 13 +++++++++++--
   5 files changed, 54 insertions(+), 6 deletions(-)


diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 62a76121841..9a67ce857fe 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -944,6 +944,13 @@ static void smmu_base_realize(DeviceState *dev,
Error **errp)
           return;
       }
+    g_assert(s->memory);
+    address_space_init(&s->memory_as, s->memory, "smmu-memory-view");
+    if (s->secure_memory) {
+        address_space_init(&s->secure_memory_as, s->secure_memory,
+                           "smmu-secure-memory-view");

Preferrably: "smmu-normal-view" and "smmu-secure-view" (IMO 'memory'
is more confusing than helping).


A bit of bike shedding, but I'm not sure what "normal" is supposed to mean. Basically, it's the view of main memory, as opposed to secure view. Especially, "normal" is used to access realm memory also, so I would prefer to have something hinting it's global vs secure memory, thus the names chosen.

Else, are we sure the SMMU implementations will behave correctly?

Alternatively, use AddressSpace pointers, then:

          } else {

              s->secure_memory_as = s->memory_as;

+    }
+
       /*
        * We only allow default PCIe Root Complex(pcie.0) or pxb-pcie
based extra
        * root complexes to be associated with SMMU.
@@ -1008,10 +1015,28 @@ static void smmu_base_class_init(ObjectClass
*klass, const void *data)
       rc->phases.exit = smmu_base_reset_exit;
   }
+static void smmu_base_instance_init(Object *obj)
+{
+    SMMUState *s = ARM_SMMU(obj);
+
+    object_property_add_link(obj, "memory",
+                             TYPE_MEMORY_REGION,
+                             (Object **)&s->memory,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_STRONG);
+
+    object_property_add_link(obj, "secure-memory",
+                             TYPE_MEMORY_REGION,
+                             (Object **)&s->secure_memory,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_STRONG);

Why can't we use device_class_set_props(&static_properties)
in smmu_base_class_init()?

+}
+
   static const TypeInfo smmu_base_info = {
       .name          = TYPE_ARM_SMMU,
       .parent        = TYPE_SYS_BUS_DEVICE,
       .instance_size = sizeof(SMMUState),
+    .instance_init = smmu_base_instance_init,
       .class_data    = NULL,
       .class_size    = sizeof(SMMUBaseClass),
       .class_init    = smmu_base_class_init,

Anyhow this is functional and I suppose this can be improved on top, so:

Reviewed-by: Philippe Mathieu-Daudé <[email protected]>




Reply via email to