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.thodi@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 = {
-- 
2.34.1

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.thodi@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?

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

Thanks,
- Don

Hi Don,

> -----Original Message-----
> From: Donald Dutile <ddutile@redhat.com>
> Sent: Tuesday, March 18, 2025 10:12 PM
> To: Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
> qemu-devel@nongnu.org
> Cc: eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
> nicolinc@nvidia.com; berrange@redhat.com; nathanc@nvidia.com;
> mochs@nvidia.com; smostafa@google.com; Linuxarm
> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>; zhangfei.gao@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.thodi@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.

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.

Thanks,
Shameer








 

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

On Wed, Mar 19, 2025 at 09:26:29AM +0000, Shameerali Kolothum Thodi wrote:
> 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.

Just want a clarification here..

If VM has a passthrough device only:
 attach it to PCIE.0 <=> vSMMU0 (accel=on)
If VM has an emulated device and a passthrough device:
 attach the emulated device to PCIE.0 <=> vSMMU bypass (or accel=off?)
 attach the passthrough device to pxb-pcie <=> vSMMU0 (accel=on)
?

Thanks
Nicolin

> -----Original Message-----
> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: Thursday, March 20, 2025 5:03 PM
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: Donald Dutile <ddutile@redhat.com>; qemu-arm@nongnu.org; qemu-
> devel@nongnu.org; eric.auger@redhat.com; peter.maydell@linaro.org;
> jgg@nvidia.com; berrange@redhat.com; nathanc@nvidia.com;
> mochs@nvidia.com; smostafa@google.com; Linuxarm
> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
> Subject: Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a pxb-
> pcie bus
> 
> On Wed, Mar 19, 2025 at 09:26:29AM +0000, Shameerali Kolothum Thodi
> wrote:
> > 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.
> 
> Just want a clarification here..
> 
> If VM has a passthrough device only:
>  attach it to PCIE.0 <=> vSMMU0 (accel=on)

Yes. Basically support accel=on to pcie.0 as well.

> If VM has an emulated device and a passthrough device:
>  attach the emulated device to PCIE.0 <=> vSMMU bypass (or accel=off?)
>  attach the passthrough device to pxb-pcie <=> vSMMU0 (accel=on)

This can be other way around as well:
ie, 
pass-through to pcie.0(accel=on) and emulated to any other pxb-pcie with accel = off.

I think the way bus numbers are allocated in Qemu for pcie.0 and pxb-pcie allows
us to support this in IORT ID maps.

Thanks,
Shameer

On Mon, Mar 24, 2025 at 08:19:27AM +0000, Shameerali Kolothum Thodi wrote:
> > If VM has an emulated device and a passthrough device:
> >  attach the emulated device to PCIE.0 <=> vSMMU bypass (or accel=off?)
> >  attach the passthrough device to pxb-pcie <=> vSMMU0 (accel=on)
> 
> This can be other way around as well:
> ie, 
> pass-through to pcie.0(accel=on) and emulated to any other pxb-pcie with accel = off.
> 
> I think the way bus numbers are allocated in Qemu for pcie.0 and pxb-pcie allows
> us to support this in IORT ID maps.

That sounds fine. The reason why I picked pcie.0 for emulated
devices, simply for keeping the design of pxb-pcie for multi-
vSMMU cases.

I think libvirt could still choose to keep it simple, although
on the QEMU side we have to keep the flexibility.

Thanks
Nicolin

Hi Shameer,

On 3/24/25 9:19 AM, Shameerali Kolothum Thodi wrote:
>
>> -----Original Message-----
>> From: Nicolin Chen <nicolinc@nvidia.com>
>> Sent: Thursday, March 20, 2025 5:03 PM
>> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
>> Cc: Donald Dutile <ddutile@redhat.com>; qemu-arm@nongnu.org; qemu-
>> devel@nongnu.org; eric.auger@redhat.com; peter.maydell@linaro.org;
>> jgg@nvidia.com; berrange@redhat.com; nathanc@nvidia.com;
>> mochs@nvidia.com; smostafa@google.com; Linuxarm
>> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
>> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
>> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
>> Subject: Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a pxb-
>> pcie bus
>>
>> On Wed, Mar 19, 2025 at 09:26:29AM +0000, Shameerali Kolothum Thodi
>> wrote:
>>> 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.
>>
>> Just want a clarification here..
>>
>> If VM has a passthrough device only:
>>  attach it to PCIE.0 <=> vSMMU0 (accel=on)
> Yes. Basically support accel=on to pcie.0 as well.

agreed we shall be able to instantiate the accelerated SMMU on pcie.0 too.
>
>> If VM has an emulated device and a passthrough device:
>>  attach the emulated device to PCIE.0 <=> vSMMU bypass (or accel=off?)
>>  attach the passthrough device to pxb-pcie <=> vSMMU0 (accel=on)
> This can be other way around as well:
> ie, 
> pass-through to pcie.0(accel=on) and emulated to any other pxb-pcie with accel = off.
+1
>
> I think the way bus numbers are allocated in Qemu for pcie.0 and pxb-pcie allows
> us to support this in IORT ID maps.
One trouble we may get into is possible bus reordering by the guest. I
don't know the details but I remember that in certain conditions the
guest can reorder the bus numbers.

Besides what I don't get in the above discussion, related to whether the
accelerated mode can also sipport emulated devices, is that if you use
the originally suggested hierarchy (pxb-pcie + root port + VFIO device)
you eventually get on guest side 2 devices protected by the SMMU
instance: the root port and the VFIO device. They end up in different
iommu groups. So there is already a mix of emulated and VFIO device.

Thanks

Eric
>
> Thanks,
> Shameer
>

On Mon, Mar 24, 2025 at 02:13:20PM +0100, Eric Auger wrote:
> >> If VM has an emulated device and a passthrough device:
> >>  attach the emulated device to PCIE.0 <=> vSMMU bypass (or accel=off?)
> >>  attach the passthrough device to pxb-pcie <=> vSMMU0 (accel=on)
> > This can be other way around as well:
> > ie, 
> > pass-through to pcie.0(accel=on) and emulated to any other pxb-pcie with accel = off.
> +1
> >
> > I think the way bus numbers are allocated in Qemu for pcie.0 and pxb-pcie allows
> > us to support this in IORT ID maps.
> One trouble we may get into is possible bus reordering by the guest. I
> don't know the details but I remember that in certain conditions the
> guest can reorder the bus numbers.

Hmm, that sounds troublesome. IORT mappings are done using the bus
number, which is fixed to a vSMMU. Can we disable that reordering?

> Besides what I don't get in the above discussion, related to whether the
> accelerated mode can also sipport emulated devices, is that if you use
> the originally suggested hierarchy (pxb-pcie + root port + VFIO device)
> you eventually get on guest side 2 devices protected by the SMMU
> instance: the root port and the VFIO device. They end up in different
> iommu groups. So there is already a mix of emulated and VFIO device.

Strictly speaking, yes, that's a mix. Maybe we should say emulated
endpoints and passthrough endpoints?

Thanks
Nicolin

> -----Original Message-----
> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: Monday, March 24, 2025 4:02 PM
> To: Eric Auger <eric.auger@redhat.com>
> Cc: Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>; Donald Dutile
> <ddutile@redhat.com>; qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> peter.maydell@linaro.org; jgg@nvidia.com; berrange@redhat.com;
> nathanc@nvidia.com; mochs@nvidia.com; smostafa@google.com; Linuxarm
> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
> Subject: Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a pxb-
> pcie bus
> 
> On Mon, Mar 24, 2025 at 02:13:20PM +0100, Eric Auger wrote:
> > >> If VM has an emulated device and a passthrough device:
> > >>  attach the emulated device to PCIE.0 <=> vSMMU bypass (or
> accel=off?)
> > >>  attach the passthrough device to pxb-pcie <=> vSMMU0 (accel=on)
> > > This can be other way around as well:
> > > ie,
> > > pass-through to pcie.0(accel=on) and emulated to any other pxb-pcie
> with accel = off.
> > +1
> > >
> > > I think the way bus numbers are allocated in Qemu for pcie.0 and pxb-
> pcie allows
> > > us to support this in IORT ID maps.
> > One trouble we may get into is possible bus reordering by the guest. I
> > don't know the details but I remember that in certain conditions the
> > guest can reorder the bus numbers.
> 
> Hmm, that sounds troublesome. IORT mappings are done using the bus
> number, which is fixed to a vSMMU. Can we disable that reordering?

DSM 5# is actually a way to do that. But I don't think we need that as host
kernel also will have the same issues with IORT if re enumeration happens.
I think the iommu_fwspec mechanism is to take care of this. I need to double
check though.
 
Thanks,
Shameer

Hi Eric,

> -----Original Message-----
> From: qemu-devel-
> bounces+shameerali.kolothum.thodi=huawei.com@nongnu.org <qemu-
> devel-bounces+shameerali.kolothum.thodi=huawei.com@nongnu.org> On
> Behalf Of Eric Auger
> Sent: Monday, March 24, 2025 1:13 PM
> To: Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>; Nicolin Chen
> <nicolinc@nvidia.com>
> Cc: Donald Dutile <ddutile@redhat.com>; qemu-arm@nongnu.org; qemu-
> devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com;
> berrange@redhat.com; nathanc@nvidia.com; mochs@nvidia.com;
> smostafa@google.com; Linuxarm <linuxarm@huawei.com>; Wangzhou (B)
> <wangzhou1@hisilicon.com>; jiangkunkun <jiangkunkun@huawei.com>;
> Jonathan Cameron <jonathan.cameron@huawei.com>;
> zhangfei.gao@linaro.org
> Subject: Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a pxb-
> pcie bus
> 
> Hi Shameer,
> 
> On 3/24/25 9:19 AM, Shameerali Kolothum Thodi wrote:
> >
> >> -----Original Message-----
> >> From: Nicolin Chen <nicolinc@nvidia.com>
> >> Sent: Thursday, March 20, 2025 5:03 PM
> >> To: Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>
> >> Cc: Donald Dutile <ddutile@redhat.com>; qemu-arm@nongnu.org;
> qemu-
> >> devel@nongnu.org; eric.auger@redhat.com; peter.maydell@linaro.org;
> >> jgg@nvidia.com; berrange@redhat.com; nathanc@nvidia.com;
> >> mochs@nvidia.com; smostafa@google.com; Linuxarm
> >> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> >> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
> >> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
> >> Subject: Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a
> pxb-
> >> pcie bus
> >>
> >> On Wed, Mar 19, 2025 at 09:26:29AM +0000, Shameerali Kolothum Thodi
> >> wrote:
> >>> 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.
> >>
> >> Just want a clarification here..
> >>
> >> If VM has a passthrough device only:
> >>  attach it to PCIE.0 <=> vSMMU0 (accel=on)
> > Yes. Basically support accel=on to pcie.0 as well.
> 
> agreed we shall be able to instantiate the accelerated SMMU on pcie.0 too.
> >
> >> If VM has an emulated device and a passthrough device:
> >>  attach the emulated device to PCIE.0 <=> vSMMU bypass (or accel=off?)
> >>  attach the passthrough device to pxb-pcie <=> vSMMU0 (accel=on)
> > This can be other way around as well:
> > ie,
> > pass-through to pcie.0(accel=on) and emulated to any other pxb-pcie with
> accel = off.
> +1
> >
> > I think the way bus numbers are allocated in Qemu for pcie.0 and pxb-
> pcie allows
> > us to support this in IORT ID maps.
> One trouble we may get into is possible bus reordering by the guest. I
> don't know the details but I remember that in certain conditions the
> guest can reorder the bus numbers.

Yeah, Guest kernel can re-enumerate PCIe. I will check.
 
> Besides what I don't get in the above discussion, related to whether the
> accelerated mode can also sipport emulated devices, is that if you use
> the originally suggested hierarchy (pxb-pcie + root port + VFIO device)
> you eventually get on guest side 2 devices protected by the SMMU
> instance: the root port and the VFIO device. They end up in different
> iommu groups. So there is already a mix of emulated and VFIO device.

True. But I guess the root port associated activity(invalidations etc) will be
very minimal(or nil?) compared to a virtio device.

Thanks,
Shameer

On 3/24/25 2:55 PM, Shameerali Kolothum Thodi wrote:
> Hi Eric,
>
>> -----Original Message-----
>> From: qemu-devel-
>> bounces+shameerali.kolothum.thodi=huawei.com@nongnu.org <qemu-
>> devel-bounces+shameerali.kolothum.thodi=huawei.com@nongnu.org> On
>> Behalf Of Eric Auger
>> Sent: Monday, March 24, 2025 1:13 PM
>> To: Shameerali Kolothum Thodi
>> <shameerali.kolothum.thodi@huawei.com>; Nicolin Chen
>> <nicolinc@nvidia.com>
>> Cc: Donald Dutile <ddutile@redhat.com>; qemu-arm@nongnu.org; qemu-
>> devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com;
>> berrange@redhat.com; nathanc@nvidia.com; mochs@nvidia.com;
>> smostafa@google.com; Linuxarm <linuxarm@huawei.com>; Wangzhou (B)
>> <wangzhou1@hisilicon.com>; jiangkunkun <jiangkunkun@huawei.com>;
>> Jonathan Cameron <jonathan.cameron@huawei.com>;
>> zhangfei.gao@linaro.org
>> Subject: Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a pxb-
>> pcie bus
>>
>> Hi Shameer,
>>
>> On 3/24/25 9:19 AM, Shameerali Kolothum Thodi wrote:
>>>> -----Original Message-----
>>>> From: Nicolin Chen <nicolinc@nvidia.com>
>>>> Sent: Thursday, March 20, 2025 5:03 PM
>>>> To: Shameerali Kolothum Thodi
>> <shameerali.kolothum.thodi@huawei.com>
>>>> Cc: Donald Dutile <ddutile@redhat.com>; qemu-arm@nongnu.org;
>> qemu-
>>>> devel@nongnu.org; eric.auger@redhat.com; peter.maydell@linaro.org;
>>>> jgg@nvidia.com; berrange@redhat.com; nathanc@nvidia.com;
>>>> mochs@nvidia.com; smostafa@google.com; Linuxarm
>>>> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
>>>> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
>>>> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
>>>> Subject: Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a
>> pxb-
>>>> pcie bus
>>>>
>>>> On Wed, Mar 19, 2025 at 09:26:29AM +0000, Shameerali Kolothum Thodi
>>>> wrote:
>>>>> 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.
>>>>
>>>> Just want a clarification here..
>>>>
>>>> If VM has a passthrough device only:
>>>>  attach it to PCIE.0 <=> vSMMU0 (accel=on)
>>> Yes. Basically support accel=on to pcie.0 as well.
>> agreed we shall be able to instantiate the accelerated SMMU on pcie.0 too.
>>>> If VM has an emulated device and a passthrough device:
>>>>  attach the emulated device to PCIE.0 <=> vSMMU bypass (or accel=off?)
>>>>  attach the passthrough device to pxb-pcie <=> vSMMU0 (accel=on)
>>> This can be other way around as well:
>>> ie,
>>> pass-through to pcie.0(accel=on) and emulated to any other pxb-pcie with
>> accel = off.
>> +1
>>> I think the way bus numbers are allocated in Qemu for pcie.0 and pxb-
>> pcie allows
>>> us to support this in IORT ID maps.
>> One trouble we may get into is possible bus reordering by the guest. I
>> don't know the details but I remember that in certain conditions the
>> guest can reorder the bus numbers.
> Yeah, Guest kernel can re-enumerate PCIe. I will check.
>  
>> Besides what I don't get in the above discussion, related to whether the
>> accelerated mode can also sipport emulated devices, is that if you use
>> the originally suggested hierarchy (pxb-pcie + root port + VFIO device)
>> you eventually get on guest side 2 devices protected by the SMMU
>> instance: the root port and the VFIO device. They end up in different
>> iommu groups. So there is already a mix of emulated and VFIO device.
> True. But I guess the root port associated activity(invalidations etc) will be
> very minimal(or nil?) compared to a virtio device.
Agreed. I just meant discriminating between devices that can bring
trouble and others may require some caution

Eric
>
> Thanks,
> Shameer
>
>
>

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

>> -----Original Message-----
>> From: Donald Dutile <ddutile@redhat.com>
>> Sent: Tuesday, March 18, 2025 10:12 PM
>> To: Shameerali Kolothum Thodi
>> <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
>> qemu-devel@nongnu.org
>> Cc: eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
>> nicolinc@nvidia.com; berrange@redhat.com; nathanc@nvidia.com;
>> mochs@nvidia.com; smostafa@google.com; Linuxarm
>> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
>> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
>> <jonathan.cameron@huawei.com>; zhangfei.gao@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.thodi@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,
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
>>
> 

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 <ddutile@redhat.com>
>>> Sent: Tuesday, March 18, 2025 10:12 PM
>>> To: Shameerali Kolothum Thodi
>>> <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
>>> qemu-devel@nongnu.org
>>> Cc: eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
>>> nicolinc@nvidia.com; berrange@redhat.com; nathanc@nvidia.com;
>>> mochs@nvidia.com; smostafa@google.com; Linuxarm
>>> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
>>> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
>>> <jonathan.cameron@huawei.com>; zhangfei.gao@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.thodi@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

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
>>>
>>
>

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 <ddutile@redhat.com>
>>>> Sent: Tuesday, March 18, 2025 10:12 PM
>>>> To: Shameerali Kolothum Thodi
>>>> <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
>>>> qemu-devel@nongnu.org
>>>> Cc: eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
>>>> nicolinc@nvidia.com; berrange@redhat.com; nathanc@nvidia.com;
>>>> mochs@nvidia.com; smostafa@google.com; Linuxarm
>>>> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
>>>> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
>>>> <jonathan.cameron@huawei.com>; zhangfei.gao@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.thodi@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.

> 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
>>>>
>>>
>>
> 

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 <ddutile@redhat.com>
>>>>> Sent: Tuesday, March 18, 2025 10:12 PM
>>>>> To: Shameerali Kolothum Thodi
>>>>> <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
>>>>> qemu-devel@nongnu.org
>>>>> Cc: eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
>>>>> nicolinc@nvidia.com; berrange@redhat.com; nathanc@nvidia.com;
>>>>> mochs@nvidia.com; smostafa@google.com; Linuxarm
>>>>> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
>>>>> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
>>>>> <jonathan.cameron@huawei.com>; zhangfei.gao@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.thodi@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
>
>> 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
>>>>>
>>>>
>>>
>>
>

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 <ddutile@redhat.com>
>>>>>> Sent: Tuesday, March 18, 2025 10:12 PM
>>>>>> To: Shameerali Kolothum Thodi
>>>>>> <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
>>>>>> qemu-devel@nongnu.org
>>>>>> Cc: eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
>>>>>> nicolinc@nvidia.com; berrange@redhat.com; nathanc@nvidia.com;
>>>>>> mochs@nvidia.com; smostafa@google.com; Linuxarm
>>>>>> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
>>>>>> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
>>>>>> <jonathan.cameron@huawei.com>; zhangfei.gao@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.thodi@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
>>>>>>
>>>>>
>>>>
>>>
>>
> 

On Mon, Mar 24, 2025 at 03:56:12PM +0100, 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 <ddutile@redhat.com>
> >>>>> Sent: Tuesday, March 18, 2025 10:12 PM
> >>>>> To: Shameerali Kolothum Thodi
> >>>>> <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
> >>>>> qemu-devel@nongnu.org
> >>>>> Cc: eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
> >>>>> nicolinc@nvidia.com; berrange@redhat.com; nathanc@nvidia.com;
> >>>>> mochs@nvidia.com; smostafa@google.com; Linuxarm
> >>>>> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> >>>>> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
> >>>>> <jonathan.cameron@huawei.com>; zhangfei.gao@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.thodi@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

Honestly I've lost track of what's going on this thread design-wise.

As general precedence though, the PCI(e) hierarchies libvirt auto-creates
are very flat - no PXBs for example, no association of host/guest NUMA,
etc. Libvirt does as little as possible in order to get PCI devices
working and anything even slightly "fancy" is left upto the mgmt app
to define. IIRC we have a few scenarios where IOMMUs get auto-added,
but mostly we expect the mgmt app to define them.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Hi Shameer,


On 3/11/25 3:10 PM, 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.
why do we require a pxb-pcie root bus? why can't pci.0 root bus be used
for simpler use cases (ie. I just want to passthough a NIC in
accelerated mode). Or may pci.0 is also called a pax-pcie root bus?

Besides, why do we put the constraint to plug on a root bus. I know that
at this point we always plug to pci.0 but with the new -device option it
would be possible to plug it anywhere in the pcie hierarchy. At SOC
level can't an SMMU be plugged anywhere protecting just a few RIDs?
>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@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);
if you want to stop the recursive search I think you need to return
something != 0 here.

I don't really understand why we don't simply set the primary-bus to
<bus> where -device arm-smmuv3-accel, bus=<bus>? or maybe enforce that
this bus is an actual root bus if we really need that?
> +        }
> +    }
> +    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;
shouldn't it below to 3/20? It is not really related to primary_bus
setting? Thanks Eric
>  }
>  
>  static const TypeInfo smmuv3_accel_type_info = {

Hi Eric,

> -----Original Message-----
> From: Eric Auger <eric.auger@redhat.com>
> Sent: Wednesday, March 12, 2025 4:08 PM
> To: Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
> qemu-devel@nongnu.org
> Cc: peter.maydell@linaro.org; jgg@nvidia.com; nicolinc@nvidia.com;
> ddutile@redhat.com; berrange@redhat.com; nathanc@nvidia.com;
> mochs@nvidia.com; smostafa@google.com; Linuxarm
> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
> Subject: Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a pxb-
> pcie bus
> 
> Hi Shameer,
> 
> 
> On 3/11/25 3:10 PM, 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.
> why do we require a pxb-pcie root bus? why can't pci.0 root bus be used
> for simpler use cases (ie. I just want to passthough a NIC in
> accelerated mode). Or may pci.0 is also called a pax-pcie root bus?

The idea was since pcie.0 is the default RC with virt, leave that to cases where
we want to attach any emulated devices and use pxb-pcie based RCs for vfio-pci.

> 
> Besides, why do we put the constraint to plug on a root bus. I know that
> at this point we always plug to pci.0 but with the new -device option it
> would be possible to plug it anywhere in the pcie hierarchy. At SOC
> level can't an SMMU be plugged anywhere protecting just a few RIDs?

In my understanding normally(or atleast in the most common cases) it is attached 
to root complexes. Also IORT mappings are at the root complex level, right?

To 
> > Signed-off-by: Shameer Kolothum
> <shameerali.kolothum.thodi@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);
> if you want to stop the recursive search I think you need to return
> something != 0 here.

Ok.
 
> I don't really understand why we don't simply set the primary-bus to
> <bus> where -device arm-smmuv3-accel, bus=<bus>? or maybe enforce that
> this bus is an actual root bus if we really need that?

The primary-bus here is actually the property of the parent SMMU device.
This one now has,

-device arm-smmuv3-accel, bus format.


> > +        }
> > +    }
> > +    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;
> shouldn't it below to 3/20? It is not really related to primary_bus
> setting?

This is to set the bus property of this smmuv3-accel device. As mentioned 
above primary-bus is the property of parent TYPE_ARM_SMMU device.

Thanks,
Shameer

On 3/12/25 5:34 PM, Shameerali Kolothum Thodi wrote:
> Hi Eric,
>
>> -----Original Message-----
>> From: Eric Auger <eric.auger@redhat.com>
>> Sent: Wednesday, March 12, 2025 4:08 PM
>> To: Shameerali Kolothum Thodi
>> <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
>> qemu-devel@nongnu.org
>> Cc: peter.maydell@linaro.org; jgg@nvidia.com; nicolinc@nvidia.com;
>> ddutile@redhat.com; berrange@redhat.com; nathanc@nvidia.com;
>> mochs@nvidia.com; smostafa@google.com; Linuxarm
>> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
>> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
>> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
>> Subject: Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a pxb-
>> pcie bus
>>
>> Hi Shameer,
>>
>>
>> On 3/11/25 3:10 PM, 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.
>> why do we require a pxb-pcie root bus? why can't pci.0 root bus be used
>> for simpler use cases (ie. I just want to passthough a NIC in
>> accelerated mode). Or may pci.0 is also called a pax-pcie root bus?
> The idea was since pcie.0 is the default RC with virt, leave that to cases where
> we want to attach any emulated devices and use pxb-pcie based RCs for vfio-pci.
yes but for simpler use case you may not want the extra pain to
instantiate a pxb-pcie device. Actually libvirt does not instantiate it
by default.
>
>> Besides, why do we put the constraint to plug on a root bus. I know that
>> at this point we always plug to pci.0 but with the new -device option it
>> would be possible to plug it anywhere in the pcie hierarchy. At SOC
>> level can't an SMMU be plugged anywhere protecting just a few RIDs?
> In my understanding normally(or atleast in the most common cases) it is attached 
> to root complexes. Also IORT mappings are at the root complex level, right?
Yes I do agree the IORT describes ID mappings between RC and SMMU but
the actual ID mappings allow you to be much more precise and state that
a given SMMU only translates few RIDs within that RID space. If you
force the device bus to be a root bus you can't model that anymore.

Eric
>
> To 
>>> Signed-off-by: Shameer Kolothum
>> <shameerali.kolothum.thodi@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);
>> if you want to stop the recursive search I think you need to return
>> something != 0 here.
> Ok.
>  
>> I don't really understand why we don't simply set the primary-bus to
>> <bus> where -device arm-smmuv3-accel, bus=<bus>? or maybe enforce that
>> this bus is an actual root bus if we really need that?
> The primary-bus here is actually the property of the parent SMMU device.
> This one now has,
>
> -device arm-smmuv3-accel, bus format.
>
>
>>> +        }
>>> +    }
>>> +    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;
>> shouldn't it below to 3/20? It is not really related to primary_bus
>> setting?
> This is to set the bus property of this smmuv3-accel device. As mentioned 
> above primary-bus is the property of parent TYPE_ARM_SMMU device.
>
> Thanks,
> Shameer

Hi Eric,

> -----Original Message-----
> From: Eric Auger <eric.auger@redhat.com>
> Sent: Wednesday, March 12, 2025 4:42 PM
> To: Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
> qemu-devel@nongnu.org
> Cc: peter.maydell@linaro.org; jgg@nvidia.com; nicolinc@nvidia.com;
> ddutile@redhat.com; berrange@redhat.com; nathanc@nvidia.com;
> mochs@nvidia.com; smostafa@google.com; Linuxarm
> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
> Subject: Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a pxb-
> pcie bus
> 
> 
> 
> 
> On 3/12/25 5:34 PM, Shameerali Kolothum Thodi wrote:
> > Hi Eric,
> >
> >> -----Original Message-----
> >> From: Eric Auger <eric.auger@redhat.com>
> >> Sent: Wednesday, March 12, 2025 4:08 PM
> >> To: Shameerali Kolothum Thodi
> >> <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
> >> qemu-devel@nongnu.org
> >> Cc: peter.maydell@linaro.org; jgg@nvidia.com; nicolinc@nvidia.com;
> >> ddutile@redhat.com; berrange@redhat.com; nathanc@nvidia.com;
> >> mochs@nvidia.com; smostafa@google.com; Linuxarm
> >> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> >> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
> >> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
> >> Subject: Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a
> pxb-
> >> pcie bus
> >>
> >> Hi Shameer,
> >>
> >>
> >> On 3/11/25 3:10 PM, 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.
> >> why do we require a pxb-pcie root bus? why can't pci.0 root bus be used
> >> for simpler use cases (ie. I just want to passthough a NIC in
> >> accelerated mode). Or may pci.0 is also called a pax-pcie root bus?
> > The idea was since pcie.0 is the default RC with virt, leave that to cases
> where
> > we want to attach any emulated devices and use pxb-pcie based RCs for
> vfio-pci.
> yes but for simpler use case you may not want the extra pain to
> instantiate a pxb-pcie device. Actually libvirt does not instantiate it
> by default.
> >
> >> Besides, why do we put the constraint to plug on a root bus. I know that
> >> at this point we always plug to pci.0 but with the new -device option it
> >> would be possible to plug it anywhere in the pcie hierarchy. At SOC
> >> level can't an SMMU be plugged anywhere protecting just a few RIDs?
> > In my understanding normally(or atleast in the most common cases) it is
> attached
> > to root complexes. Also IORT mappings are at the root complex level,
> right?
> Yes I do agree the IORT describes ID mappings between RC and SMMU but
> the actual ID mappings allow you to be much more precise and state that
> a given SMMU only translates few RIDs within that RID space. If you
> force the device bus to be a root bus you can't model that anymore.
>

Do we really need to support that? What if the user then have another smmuv3-accel
in the associated upstream buses/RC as well? Not sure how to handle that.
 
Thanks,
Shameer

On 3/13/25 9:22 AM, Shameerali Kolothum Thodi wrote:
> Hi Eric,
>
>> -----Original Message-----
>> From: Eric Auger <eric.auger@redhat.com>
>> Sent: Wednesday, March 12, 2025 4:42 PM
>> To: Shameerali Kolothum Thodi
>> <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
>> qemu-devel@nongnu.org
>> Cc: peter.maydell@linaro.org; jgg@nvidia.com; nicolinc@nvidia.com;
>> ddutile@redhat.com; berrange@redhat.com; nathanc@nvidia.com;
>> mochs@nvidia.com; smostafa@google.com; Linuxarm
>> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
>> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
>> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
>> Subject: Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a pxb-
>> pcie bus
>>
>>
>>
>>
>> On 3/12/25 5:34 PM, Shameerali Kolothum Thodi wrote:
>>> Hi Eric,
>>>
>>>> -----Original Message-----
>>>> From: Eric Auger <eric.auger@redhat.com>
>>>> Sent: Wednesday, March 12, 2025 4:08 PM
>>>> To: Shameerali Kolothum Thodi
>>>> <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
>>>> qemu-devel@nongnu.org
>>>> Cc: peter.maydell@linaro.org; jgg@nvidia.com; nicolinc@nvidia.com;
>>>> ddutile@redhat.com; berrange@redhat.com; nathanc@nvidia.com;
>>>> mochs@nvidia.com; smostafa@google.com; Linuxarm
>>>> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
>>>> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
>>>> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
>>>> Subject: Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a
>> pxb-
>>>> pcie bus
>>>>
>>>> Hi Shameer,
>>>>
>>>>
>>>> On 3/11/25 3:10 PM, 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.
>>>> why do we require a pxb-pcie root bus? why can't pci.0 root bus be used
>>>> for simpler use cases (ie. I just want to passthough a NIC in
>>>> accelerated mode). Or may pci.0 is also called a pax-pcie root bus?
>>> The idea was since pcie.0 is the default RC with virt, leave that to cases
>> where
>>> we want to attach any emulated devices and use pxb-pcie based RCs for
>> vfio-pci.
>> yes but for simpler use case you may not want the extra pain to
>> instantiate a pxb-pcie device. Actually libvirt does not instantiate it
>> by default.
>>>> Besides, why do we put the constraint to plug on a root bus. I know that
>>>> at this point we always plug to pci.0 but with the new -device option it
>>>> would be possible to plug it anywhere in the pcie hierarchy. At SOC
>>>> level can't an SMMU be plugged anywhere protecting just a few RIDs?
>>> In my understanding normally(or atleast in the most common cases) it is
>> attached
>>> to root complexes. Also IORT mappings are at the root complex level,
>> right?
>> Yes I do agree the IORT describes ID mappings between RC and SMMU but
>> the actual ID mappings allow you to be much more precise and state that
>> a given SMMU only translates few RIDs within that RID space. If you
>> force the device bus to be a root bus you can't model that anymore.
>>
> Do we really need to support that? What if the user then have another smmuv3-accel
> in the associated upstream buses/RC as well? Not sure how to handle that.
Well I agree we would need to reject such kind of config. Maybe we can
relax this requirement and connect the smmu to a root bus (including
pci.0 though). Then this SMMU would translate all the input RIDs. This
is not as flexible as what the IORT allows but maybe that's enough for
our use cases.

Thanks

Eric
>  
> Thanks,
> Shameer

On Wed, Mar 12, 2025 at 04:34:18PM +0000, Shameerali Kolothum Thodi wrote:
> Hi Eric,
> 
> > -----Original Message-----
> > From: Eric Auger <eric.auger@redhat.com>
> > Sent: Wednesday, March 12, 2025 4:08 PM
> > To: Shameerali Kolothum Thodi
> > <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
> > qemu-devel@nongnu.org
> > Cc: peter.maydell@linaro.org; jgg@nvidia.com; nicolinc@nvidia.com;
> > ddutile@redhat.com; berrange@redhat.com; nathanc@nvidia.com;
> > mochs@nvidia.com; smostafa@google.com; Linuxarm
> > <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> > jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
> > <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
> > Subject: Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a pxb-
> > pcie bus
> > 
> > Hi Shameer,
> > 
> > 
> > On 3/11/25 3:10 PM, 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.
> > why do we require a pxb-pcie root bus? why can't pci.0 root bus be used
> > for simpler use cases (ie. I just want to passthough a NIC in
> > accelerated mode). Or may pci.0 is also called a pax-pcie root bus?
> 
> The idea was since pcie.0 is the default RC with virt, leave that to cases where
> we want to attach any emulated devices and use pxb-pcie based RCs for vfio-pci.

The majority of management applications will never do anything other
than a flat PCI(e) topology by default. Some might enable pxb-pcie as
an optional but plenty won't ever support it. If you want to maximise
the potential usefulness of the ssmmuv3-accel, and it is technically
viable, it would be worth permitting choice of attachment to the root
bus as an alteranative to the pxb.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

> -----Original Message-----
> From: Daniel P. Berrangé <berrange@redhat.com>
> Sent: Wednesday, March 12, 2025 4:39 PM
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: eric.auger@redhat.com; qemu-arm@nongnu.org; qemu-
> devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com;
> nicolinc@nvidia.com; ddutile@redhat.com; nathanc@nvidia.com;
> mochs@nvidia.com; smostafa@google.com; Linuxarm
> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
> Subject: Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a pxb-
> pcie bus
> 
> On Wed, Mar 12, 2025 at 04:34:18PM +0000, Shameerali Kolothum Thodi
> wrote:
> > Hi Eric,
> >
> > > -----Original Message-----
> > > From: Eric Auger <eric.auger@redhat.com>
> > > Sent: Wednesday, March 12, 2025 4:08 PM
> > > To: Shameerali Kolothum Thodi
> > > <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
> > > qemu-devel@nongnu.org
> > > Cc: peter.maydell@linaro.org; jgg@nvidia.com; nicolinc@nvidia.com;
> > > ddutile@redhat.com; berrange@redhat.com; nathanc@nvidia.com;
> > > mochs@nvidia.com; smostafa@google.com; Linuxarm
> > > <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> > > jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
> > > <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
> > > Subject: Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a
> pxb-
> > > pcie bus
> > >
> > > Hi Shameer,
> > >
> > >
> > > On 3/11/25 3:10 PM, 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.
> > > why do we require a pxb-pcie root bus? why can't pci.0 root bus be used
> > > for simpler use cases (ie. I just want to passthough a NIC in
> > > accelerated mode). Or may pci.0 is also called a pax-pcie root bus?
> >
> > The idea was since pcie.0 is the default RC with virt, leave that to cases
> where
> > we want to attach any emulated devices and use pxb-pcie based RCs for
> vfio-pci.
> 
> The majority of management applications will never do anything other
> than a flat PCI(e) topology by default. Some might enable pxb-pcie as
> an optional but plenty won't ever support it. If you want to maximise
> the potential usefulness of the ssmmuv3-accel, and it is technically
> viable, it would be worth permitting choice of attachment to the root
> bus as an alteranative to the pxb.

Ok. I will look into this. Though I am not sure when we have smmuv3-accel
to pcie.0 we can still have additional smmuv3-accel with pxb-pcie or not.
It looks like pxb-pcie will be plugged into pcie.0. And if that is the case
IORT mappings will be difficult I guess. I need to double check.

Thanks,
Shameer

Hi Shameer,

On 3/12/25 6:28 PM, Shameerali Kolothum Thodi wrote:
>
>> -----Original Message-----
>> From: Daniel P. Berrangé <berrange@redhat.com>
>> Sent: Wednesday, March 12, 2025 4:39 PM
>> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
>> Cc: eric.auger@redhat.com; qemu-arm@nongnu.org; qemu-
>> devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com;
>> nicolinc@nvidia.com; ddutile@redhat.com; nathanc@nvidia.com;
>> mochs@nvidia.com; smostafa@google.com; Linuxarm
>> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
>> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
>> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
>> Subject: Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a pxb-
>> pcie bus
>>
>> On Wed, Mar 12, 2025 at 04:34:18PM +0000, Shameerali Kolothum Thodi
>> wrote:
>>> Hi Eric,
>>>
>>>> -----Original Message-----
>>>> From: Eric Auger <eric.auger@redhat.com>
>>>> Sent: Wednesday, March 12, 2025 4:08 PM
>>>> To: Shameerali Kolothum Thodi
>>>> <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
>>>> qemu-devel@nongnu.org
>>>> Cc: peter.maydell@linaro.org; jgg@nvidia.com; nicolinc@nvidia.com;
>>>> ddutile@redhat.com; berrange@redhat.com; nathanc@nvidia.com;
>>>> mochs@nvidia.com; smostafa@google.com; Linuxarm
>>>> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
>>>> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
>>>> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
>>>> Subject: Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a
>> pxb-
>>>> pcie bus
>>>>
>>>> Hi Shameer,
>>>>
>>>>
>>>> On 3/11/25 3:10 PM, 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.
>>>> why do we require a pxb-pcie root bus? why can't pci.0 root bus be used
>>>> for simpler use cases (ie. I just want to passthough a NIC in
>>>> accelerated mode). Or may pci.0 is also called a pax-pcie root bus?
>>> The idea was since pcie.0 is the default RC with virt, leave that to cases
>> where
>>> we want to attach any emulated devices and use pxb-pcie based RCs for
>> vfio-pci.
>>
>> The majority of management applications will never do anything other
>> than a flat PCI(e) topology by default. Some might enable pxb-pcie as
>> an optional but plenty won't ever support it. If you want to maximise
>> the potential usefulness of the ssmmuv3-accel, and it is technically
>> viable, it would be worth permitting choice of attachment to the root
>> bus as an alteranative to the pxb.
> Ok. I will look into this. Though I am not sure when we have smmuv3-accel
> to pcie.0 we can still have additional smmuv3-accel with pxb-pcie or not.
> It looks like pxb-pcie will be plugged into pcie.0. And if that is the case
> IORT mappings will be difficult I guess. I need to double check.

Indeed it makes things more difficult in terms of id mapping but I think
it would bring some benefits to be able to plug the accel smmu on pci.0 too.

some logic should be there already because you can bypass the SMMU on a
given pxb while enabled on pci.0:
see

[PATCH v5 0/9] IOMMU: Add support for IOMMU Bypass Feature <https://lore.kernel.org/all/1625748919-52456-1-git-send-email-wangxingang5@huawei.com/#r>
https://lore.kernel.org/all/1625748919-52456-1-git-send-email-wangxingang5@huawei.com/

Eric

>
> Thanks,
> Shameer