On 3/12/25 5:22 PM, Shameerali Kolothum Thodi wrote:
>
>> -----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 4:13 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/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.
> 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?
Not sure if I understand you question. I meant it can be a boolean
device property (DEFINE_PROP_BOOL) smmuv3,accel=on
No?
Eric
>
> Any example device implementation like above already there?
> Please let me know.
>
> Thanks,
> Shameer
>
>
>
>