RE: [RFC PATCH v2 04/20] hw/arm/virt: Add support for smmuv3-accel

[Shameerali Kolothum Thodi via <qemu-devel@nongnu.org>]

> -----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: Wednesday, March 12, 2025 4:13 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 04/20] hw/arm/virt: Add support for smmuv3-
> accel
> 
> Hi Shameer,
> 
> 
> On 3/12/25 4:46 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: Wednesday, March 12, 2025 3:36 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 04/20] hw/arm/virt: Add support for
> >> smmuv3- accel
> >>
> >> Hi Shameer,
> >>
> >>
> >> On 3/11/25 3:10 PM, Shameer Kolothum wrote:
> >>> Allow cold-plug smmuv3-accel to virt If the machine wide smmuv3 is
> >>> not specified.
> >>>
> >>> No FDT support is added for now.
> >>>
> >>> Signed-off-by: Shameer Kolothum
> >> <shameerali.kolothum.thodi@huawei.com>
> >>> ---
> >>>  hw/arm/virt.c         | 12 ++++++++++++
> >>>  hw/core/sysbus-fdt.c  |  1 +
> >>>  include/hw/arm/virt.h |  1 +
> >>>  3 files changed, 14 insertions(+)
> >>>
> >>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c index
> >>> 4a5a9666e9..84a323da55 100644
> >>> --- a/hw/arm/virt.c
> >>> +++ b/hw/arm/virt.c
> >>> @@ -73,6 +73,7 @@
> >>>  #include "qobject/qlist.h"
> >>>  #include "standard-headers/linux/input.h"
> >>>  #include "hw/arm/smmuv3.h"
> >>> +#include "hw/arm/smmuv3-accel.h"
> >>>  #include "hw/acpi/acpi.h"
> >>>  #include "target/arm/cpu-qom.h"
> >>>  #include "target/arm/internals.h"
> >>> @@ -2911,6 +2912,16 @@ static void
> >> virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
> >>>              platform_bus_link_device(PLATFORM_BUS_DEVICE(vms-
> >>> platform_bus_dev),
> >>>                                       SYS_BUS_DEVICE(dev));
> >>>          }
> >>> +        if (object_dynamic_cast(OBJECT(dev),
> >>> + TYPE_ARM_SMMUV3_ACCEL))
> >> {
> >>> +            if (vms->iommu == VIRT_IOMMU_SMMUV3) {
> >> maybe just check whether it is != VIRT_IOMMU_NONE?
> >>> +                error_setg(errp,
> >>> +                           "iommu=smmuv3 is already specified.
> >>> + can't create
> >> smmuv3-accel dev");
> >> I would clearly state "iommu=smmuv3 virt machine option is alreadt set"
> >> and use an error hint to say both are not compatible.
> >>> +                return;
> >>> +            }
> >>> +            if (vms->iommu != VIRT_IOMMU_SMMUV3_ACCEL) {
> >>> +                vms->iommu = VIRT_IOMMU_SMMUV3_ACCEL;
> >> I know there were quite a lot of dicussions on the 1st multi
> >> instantiation series related to the way we instanatiate that device
> >> and maybe I missed some blockers but why wouldn't we allow the
> >> instantiation of the legacy smmu device with -device too. I think
> >> this would be simpler for libvirt and we would somehow deprecate the
> >> machine option method? would that make a problem if you were to use
> >> -device smmu,accel or something alike?
> > Thanks for taking a look. I am just jumping on this one for now.  Yes,
> > there were discussions around that. But I was not sure we concluded on
> > deprecating the machine option. So if I get you correctly the idea is,
> >
> > if we have,
> > -device smmuv3 it will instantiate the current machine wide smmuv3 and
> > for -device smmuv3,accel this device?
> yes that would be my preference.

Ok. I will look into that in my next respin. A quick question. Does qemu
DEVICE model support the differentiation like above easily? Or we have
to manage it with properties?

Any example device implementation like above already there?
Please let me know.

Thanks,
Shameer