Hi Shameer,
On 3/12/25 4:46 PM, Shameerali Kolothum Thodi wrote: > Hi Eric, > >> -----Original Message----- >> From: qemu-devel- >> [email protected] <qemu- >> [email protected]> On >> Behalf Of Eric Auger >> Sent: Wednesday, March 12, 2025 3:36 PM >> To: Shameerali Kolothum Thodi >> <[email protected]>; [email protected]; >> [email protected] >> Cc: [email protected]; [email protected]; [email protected]; >> [email protected]; [email protected]; [email protected]; >> [email protected]; [email protected]; Linuxarm >> <[email protected]>; Wangzhou (B) <[email protected]>; >> jiangkunkun <[email protected]>; Jonathan Cameron >> <[email protected]>; [email protected] >> 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 >> <[email protected]> >>> --- >>> 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. Eric > > Thanks, > Shameer
