On 3/24/25 10:56 AM, Eric Auger wrote:


On 3/21/25 1:59 AM, Donald Dutile wrote:


On 3/19/25 2:21 PM, Eric Auger wrote:
Hi Don,


On 3/19/25 5:21 PM, Donald Dutile wrote:


On 3/19/25 5:26 AM, Shameerali Kolothum Thodi wrote:
Hi Don,

Hey!

-----Original Message-----
From: Donald Dutile <ddut...@redhat.com>
Sent: Tuesday, March 18, 2025 10:12 PM
To: Shameerali Kolothum Thodi
<shameerali.kolothum.th...@huawei.com>; qemu-...@nongnu.org;
qemu-devel@nongnu.org
Cc: eric.au...@redhat.com; peter.mayd...@linaro.org; j...@nvidia.com;
nicol...@nvidia.com; berra...@redhat.com; nath...@nvidia.com;
mo...@nvidia.com; smost...@google.com; Linuxarm
<linux...@huawei.com>; Wangzhou (B) <wangzh...@hisilicon.com>;
jiangkunkun <jiangkun...@huawei.com>; Jonathan Cameron
<jonathan.came...@huawei.com>; zhangfei....@linaro.org
Subject: Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a
pxb-
pcie bus

Shameer,

Hi!

On 3/11/25 10:10 AM, Shameer Kolothum wrote:
User must associate a pxb-pcie root bus to smmuv3-accel
and that is set as the primary-bus for the smmu dev.

Signed-off-by: Shameer Kolothum
<shameerali.kolothum.th...@huawei.com>
---
     hw/arm/smmuv3-accel.c | 19 +++++++++++++++++++
     1 file changed, 19 insertions(+)

diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
index c327661636..1471b65374 100644
--- a/hw/arm/smmuv3-accel.c
+++ b/hw/arm/smmuv3-accel.c
@@ -9,6 +9,21 @@
     #include "qemu/osdep.h"

     #include "hw/arm/smmuv3-accel.h"
+#include "hw/pci/pci_bridge.h"
+
+static int smmuv3_accel_pxb_pcie_bus(Object *obj, void *opaque)
+{
+    DeviceState *d = opaque;
+
+    if (object_dynamic_cast(obj, "pxb-pcie-bus")) {
+        PCIBus *bus = PCI_HOST_BRIDGE(obj->parent)->bus;
+        if (d->parent_bus && !strcmp(bus->qbus.name, d->parent_bus-
name)) {
+            object_property_set_link(OBJECT(d), "primary-bus",
OBJECT(bus),
+                                     &error_abort);
+        }
+    }
+    return 0;
+}

     static void smmu_accel_realize(DeviceState *d, Error **errp)
     {
@@ -17,6 +32,9 @@ static void smmu_accel_realize(DeviceState *d,
Error
**errp)
         SysBusDevice *dev = SYS_BUS_DEVICE(d);
         Error *local_err = NULL;

+    object_child_foreach_recursive(object_get_root(),
+                                   smmuv3_accel_pxb_pcie_bus, d);
+
         object_property_set_bool(OBJECT(dev), "accel", true,
&error_abort);
         c->parent_realize(d, &local_err);
         if (local_err) {
@@ -33,6 +51,7 @@ static void smmuv3_accel_class_init(ObjectClass
*klass, void *data)
         device_class_set_parent_realize(dc, smmu_accel_realize,
                                         &c->parent_realize);
         dc->hotpluggable = false;
+    dc->bus_type = TYPE_PCIE_BUS;
     }

     static const TypeInfo smmuv3_accel_type_info = {

I am not seeing the need for a pxb-pcie bus(switch) introduced for
each
'accel'.
Isn't the IORT able to define different SMMUs for different RIDs?
if so,
itsn't that sufficient
to associate (define) an SMMU<->RID association without introducing a
pxb-pcie?
and again, I'm not sure how that improves/enables the device<->SMMU
associativity?

Thanks for taking a look at the series. As discussed elsewhere in
this thread(with
Eric), normally in physical world (or atleast in the most common
cases) SMMUv3
is attached to PCIe Root Complex and if you take a look at the IORT
spec, it describes
association of ID mappings between a RC node and SMMUV3 node.

And if my understanding is correct, in Qemu, only pxb-pcie allows you
to add
extra root complexes even though it is still plugged to
parent(pcie.0). ie, for all
devices downstream it acts as a root complex but still plugged into a
parent pcie.0.
This allows us to add/describe multiple "smmuv3-accel" each
associated with a RC.

I find the qemu statements a bit unclear here as well.
I looked at the hot plug statement(s) in docs/pcie.txt, as I figured
that's where dynamic
IORT changes would be needed as well.  There, it says you can hot-add
PCIe devices to RPs,
one has to define/add RP's to the machine model for that plug-in.

Using libvirt, it could auto-add the needed RPs to do dynmaic smmuv3
additions,
I am not sure I understand your statement here. we don't want "dynamic"
SMMUv3 instantiation. SMMUv3 is a platform device which is supposed to
be coldplugged on a pre-existing PCIe hierarchy. The SMMUv3 device is
not something that is meant to be hotplugged or hotunplugged.
To me we hijack the bus= property to provide information about the IORT
IDMAP

Dynamic in the sense that if one adds smmuv3 for multiple devices,
libvirt will dynamically figure out how to instantiate one, two,
three... smmu's
in the machine at cold boot.
If you want a machine to be able to hot-plug a device that would
require another smmu,
than the config, and smmu, would have to be explicilty stated; as is
done today for
hot-plug PCIe if the simple machine that libvirt would make is not
sufficient to
hot-add a PCIe device.

Hum this will need to be discussed with libvirt guys but I am not sure
they will be inclined to support such kind of policy, esp because vIOMMU
is a pretty marginal use case as of now. They do automatic instantiation
for pcie, usb controllers but I am not sure they will take care of the
vIOMMU tbh

Eric

A discussion w/libvirt developers would be prudent.
I don't think it's that complicated and lots of parallels to device-assigned pcie 
devices & virtio-devices,
but for possibly different reasons: for pci(e) assigned devices, need to add 
(hw-centric) RP's and pcie bus's;
virtio devices can share a (n emulated) PCI.

for smmu: devices assigned can be attached to an smmu, which libvirt can have 
accel=auto added to it, on
a separate smmu than those added to virtio devices(sharing that smmu).  Each 
assigned device can have a
unique smmu-id, like assigned PCI(e) devices have unique pci-id (pcie is 
one-device/one-bus, of course)
but the assigned devices may actually use the same smmu (physically, even 
though virtually defined as separate).
If we end up with too many smmu's b/c we have successfully exploited their 
advanced features
for assigned devices in guests, I'll consider that a win! ;-)
Seriously, I'm sure we can figure out how to improve the libvirt smmu/iommu 
assignment of (assigned) devices to (virtual) smmu's/iommu's
with a bit more hw-tree lookup code.

I look forward to the discussion with the libvirt developers.


Thanks

Eric
if I understand how libvirt does that today for pcie devices now (/me
looks at danpb for feedback).

Having said that,  current code only allows pxb-pcie root complexes
avoiding
the pcie.0. The idea behind this was, user can use pcie.0 with a non
accel SMMUv3
for any emulated devices avoiding the performance bottlenecks we are
discussing for emulated dev+smmuv3-accel cases. But based on the
feedback from
Eric and Daniel I will relax that restriction and will allow
association with pcie.0.

So, I think this isn't a restriction that this smmuv3 feature should
enforce;
lack of a proper RP or pxb-pcie will yield an invalid config
issue/error, and
the machine definition will be modified to meet the needs for IORT.

Thanks,
Shameer








to root complexes.
Feel free to enlighten me where I may have mis-read/interpreted the
IORT
& SMMUv3 specs.

Thanks,
- Don








Reply via email to