> -----Original Message-----
> From: Eric Auger <eric.au...@redhat.com>
> Sent: Monday, May 5, 2025 9:19 AM
> To: Donald Dutile <ddut...@redhat.com>; Shameerali Kolothum Thodi
> <shameerali.kolothum.th...@huawei.com>; qemu-...@nongnu.org;
> qemu-devel@nongnu.org; Markus Armbruster <arm...@redhat.com>;
> Peter Maydell <peter.mayd...@linaro.org>; Daniel P. Berrange
> <berra...@redhat.com>; Alex Bennée <alex.ben...@linaro.org>
> Cc: j...@nvidia.com; nicol...@nvidia.com; berra...@redhat.com;
> nath...@nvidia.com; mo...@nvidia.com; smost...@google.com; Linuxarm
> <linux...@huawei.com>; Wangzhou (B) <wangzh...@hisilicon.com>;
> jiangkunkun <jiangkun...@huawei.com>; Jonathan Cameron
> <jonathan.came...@huawei.com>; zhangfei....@linaro.org
> Subject: Re: [PATCH v2 1/6] hw/arm/smmuv3: Add support to associate a
> PCIe RC
>
> Hi,
> On 5/2/25 8:16 PM, Donald Dutile wrote:
> >
> >
> > On 5/2/25 6:27 AM, Shameer Kolothum wrote:
> >> Although this change does not affect functionality at present, it lays
> >> the groundwork for enabling user-created SMMUv3 devices in
> >> future patches
> >>
> >> Signed-off-by: Shameer Kolothum
> <shameerali.kolothum.th...@huawei.com>
> >> ---
> >> hw/arm/smmuv3.c | 26 ++++++++++++++++++++++++++
> >> hw/arm/virt.c | 3 ++-
> >> 2 files changed, 28 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> >> index ab67972353..605de9b721 100644
> >> --- a/hw/arm/smmuv3.c
> >> +++ b/hw/arm/smmuv3.c
> >> @@ -24,6 +24,7 @@
> >> #include "hw/qdev-properties.h"
> >> #include "hw/qdev-core.h"
> >> #include "hw/pci/pci.h"
> >> +#include "hw/pci/pci_bridge.h"
> >> #include "cpu.h"
> >> #include "exec/target_page.h"
> >> #include "trace.h"
> >> @@ -1874,6 +1875,25 @@ static void smmu_reset_exit(Object *obj,
> >> ResetType type)
> >> smmuv3_init_regs(s);
> >> }
> >> +static int smmuv3_pcie_bus(Object *obj, void *opaque)
> >> +{
> >> + DeviceState *d = opaque;
> >> + PCIBus *bus;
> >> +
> >> + if (!object_dynamic_cast(obj, TYPE_PCI_HOST_BRIDGE)) {
> >> + return 0;
> >> + }
> >> +
> >> + bus = PCI_HOST_BRIDGE(obj)->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 non-zero as we got the bus and don't need further
> >> iteration.*/
> >> + return 1;
> >> + }
> >> + return 0;
> >> +}
> >> +
> >> static void smmu_realize(DeviceState *d, Error **errp)
> >> {
> >> SMMUState *sys = ARM_SMMU(d);
> >> @@ -1882,6 +1902,10 @@ static void smmu_realize(DeviceState *d,
> Error
> >> **errp)
> >> SysBusDevice *dev = SYS_BUS_DEVICE(d);
> >> Error *local_err = NULL;
> >> + if (!object_property_get_link(OBJECT(d), "primary-bus",
> >> &error_abort)) {
> >> + object_child_foreach_recursive(object_get_root(),
> >> smmuv3_pcie_bus, d);
> >> + }
> >> +
> >> c->parent_realize(d, &local_err);
> >> if (local_err) {
> >> error_propagate(errp, local_err);
> >> @@ -1996,6 +2020,8 @@ static void smmuv3_class_init(ObjectClass
> >> *klass, const void *data)
> >> device_class_set_parent_realize(dc, smmu_realize,
> >> &c->parent_realize);
> >> device_class_set_props(dc, smmuv3_properties);
> >> + dc->hotpluggable = false;
> >> + dc->bus_type = TYPE_PCIE_BUS;
> > Does this force legacy SMMUv3 to be tied to a PCIe bus now?
> > if so, will that break some existing legacy smmuv3 configs?, i.e.,
> > virtio-scsi attached to a legacy smmuv3.
>
> Previously the SMMU was already always attached to a PCI primary-bus
> (vms->bus ie. pci0). virtio-scsi-pci is the device being protected. The
> SMMU is not able to protect platforms devices atm.
>
> My only concern is we are highjacking the "bus" prop to record the bus
> hierarchy the SMMU is protecting. While the SMMU is a platform device
> and does not inherit the PCI device base class its bus type becomes
> "TYPE_PCIE_BUS". So in terms of qom hierachy is is seen as a PCI device
> now? I don't know if it is a problem. An alternative could be to keep
> the bus pointer and type as it was before and introduce a primary-bus
> property. Adding Markus, Peter, Daniel and Alex in to.
Yes. The SMMUV3 is a platform device and we are making the bus type
here as PCIE which is a bit odd. As replied elsewhere in this thread,
in the initial RFC we had a specific "pci-bus" property associated with
SMMUv3 device,
Eg:
-device arm-smmuv3,pci-bus=pcie.0,...
But then there were feedback that, it is more intuitive and easy for libvirt
if we can just use the default "bus" property associated with a Qemu device.
Hence the change.
>
> At some point it was envisionned to support protected platform devices
> (I think this was need for CCA). My fear is that if we turn the bus type
> to PCIE it may be difficult to extend the support to non PCIe protected
> devices. The SMMU shall remain a platform device being able to protect
> either PCI devices and, in the future, platform devices.
Ok. So in the case of platform device how do we envisage to specify the "bus"?
-device arm-smmv3, primary-bus=sysbus
Or something like having a SMMUv3 dev without any bus and each platform
device has to specify the SMMUv3?
Eg:
-device arm-smmv3,id=smmuv3.3
-device xxxx,smmuv3= smmuv3.3
If later, I think we can stick to current one.
Thanks,
Shameer
