Re: [RFC PATCH v3 05/15] hw/arm/smmuv3-accel: Introduce smmuv3 accel device

[Eric Auger <eric.auger@redhat.com>]

On 9/5/25 10:22 AM, Shameer Kolothum wrote:
>
>> -----Original Message-----
>> From: Eric Auger <eric.auger@redhat.com>
>> Sent: 04 September 2025 15:33
>> To: Nicolin Chen <nicolinc@nvidia.com>; Shameer Kolothum
>> <skolothumtho@nvidia.com>
>> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
>> peter.maydell@linaro.org; Jason Gunthorpe <jgg@nvidia.com>;
>> ddutile@redhat.com; berrange@redhat.com; Nathan Chen
>> <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>;
>> smostafa@google.com; linuxarm@huawei.com; wangzhou1@hisilicon.com;
>> jiangkunkun@huawei.com; jonathan.cameron@huawei.com;
>> zhangfei.gao@linaro.org; zhenzhong.duan@intel.com;
>> shameerkolothum@gmail.com
>> Subject: Re: [RFC PATCH v3 05/15] hw/arm/smmuv3-accel: Introduce smmuv3
>> accel device
>>
>> External email: Use caution opening links or attachments
>>
>>
>> On 7/14/25 7:23 PM, Nicolin Chen wrote:
>>> On Mon, Jul 14, 2025 at 04:59:31PM +0100, Shameer Kolothum wrote:
>>>> Also setup specific PCIIOMMUOps for accel SMMUv3 as accel
>>>> SMMUv3 will have different handling for those ops callbacks in
>>>> subsequent patches.
>>>>
>>>> The "accel" property is not yet added, so users cannot set it at this
>>>> point. It will be introduced in a subsequent patch once the necessary
>>>> support is in place.
>>>>
>>>> Signed-off-by: Shameer Kolothum
>>>> <shameerali.kolothum.thodi@huawei.com>
>>> Overall the patch looks good to me,
>>> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
>>>
>>> with some nits:
>>>
>>>> @@ -61,7 +61,8 @@ arm_common_ss.add(when: 'CONFIG_ARMSSE',
>> if_true:
>>>> files('armsse.c'))
>>>>  arm_common_ss.add(when: 'CONFIG_FSL_IMX7', if_true:
>>>> files('fsl-imx7.c', 'mcimx7d-sabre.c'))
>>>>  arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP', if_true:
>>>> files('fsl-imx8mp.c'))
>>>>  arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP_EVK', if_true:
>>>> files('imx8mp-evk.c'))
>>>> -arm_common_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true:
>>>> files('smmuv3.c'))
>>>> +arm_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmuv3.c'))
>>>> +arm_ss.add(when: ['CONFIG_ARM_SMMUV3', 'CONFIG_IOMMUFD'],
>> if_true:
>>>> +files('smmuv3-accel.c'))
>>> Wondering why "arm_common_ss" is changed to "arm_ss"?
>> Indeed why did you need to change that?
> Thanks for going through the series. I will go through all the comments and
> rework the series soon, but a quick one on the above question.
>
> I was getting this compile error as now we use #include CONFIG_DEVICES to check
> CONFIG_IOMMUFD 
>
> .d -o libsystem_arm.a.p/hw_arm_smmuv3.c.o -c ../hw/arm/smmuv3.c
> In file included from ../hw/arm/smmuv3.c:35:
> ../hw/arm/smmuv3-accel.h:17:10: error: #include expects "FILENAME" or <FILENAME>
>    17 | #include CONFIG_DEVICES
>       |          ^~~~~~~~~~~~~~
> ../hw/arm/smmuv3-accel.h:55:13: error: attempt to use poisoned "CONFIG_ARM_SMMUV3"
>    55 | #if defined(CONFIG_ARM_SMMUV3) && defined(CONFIG_IOMMUFD)
>       |             ^
> ../hw/arm/smmuv3-accel.h:55:43: error: attempt to use poisoned "CONFIG_IOMMUFD"
>    55 | #if defined(CONFIG_ARM_SMMUV3) && defined(CONFIG_IOMMUFD)
>       |                                           ^
> ninja: build stopped: subcommand failed.
> make[1]: *** [Makefile:168: run-ninja] Error 1
> make[1]: Leaving directory '/root/qemu-hisi/build'
>
> And with arm_common_ss.add, it looks like the compiler is missing
> '-DCONFIG_DEVICES="aarch64-softmmu-config-devices.h"'.
I guess this is because devices are arch specific so it cannot be
"common" anymore. Looks sensible to me. Adding Philippe in the loop.

Unrelated to that, do you have a way to opt-out SMMUv3 HW acceleration.
Some people might not be interested in that feature.

Eric
>
> Not sure how to fix this with arm_common_ss.add. Please let me know
> If there is a way to address this.
>
> Thanks,
> Shameer
>
>
>