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....@nongnu.org <qemu-
>> devel-bounces+shameerali.kolothum.thodi=huawei....@nongnu.org> On
>> Behalf Of Eric Auger
>> Sent: Wednesday, March 12, 2025 3:36 PM
>> To: Shameerali Kolothum Thodi
>> <shameerali.kolothum.th...@huawei.com>; qemu-...@nongnu.org;
>> qemu-devel@nongnu.org
>> Cc: peter.mayd...@linaro.org; j...@nvidia.com; nicol...@nvidia.com;
>> ddut...@redhat.com; berra...@redhat.com; nath...@nvidia.com;
>> mo...@nvidia.com; smost...@google.com; Linuxarm
>> <linux...@huawei.com>; Wangzhou (B) <wangzh...@hisilicon.com>;
>> jiangkunkun <jiangkun...@huawei.com>; Jonathan Cameron
>> <jonathan.came...@huawei.com>; zhangfei....@linaro.org
>> Subject: Re: [RFC PATCH v2 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.th...@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.
Eric
>
> Thanks,
> Shameer
