On 9/8/25 11:15 AM, Shameer Kolothum wrote:
>
>> -----Original Message-----
>> From: Eric Auger <[email protected]>
>> Sent: 05 September 2025 11:18
>> To: Shameer Kolothum <[email protected]>; Nicolin Chen
>> <[email protected]>
>> Cc: [email protected]; [email protected];
>> [email protected]; Jason Gunthorpe <[email protected]>;
>> [email protected]; [email protected]; Nathan Chen
>> <[email protected]>; Matt Ochs <[email protected]>;
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; Philippe Mathieu-Daudé <[email protected]>
>> Subject: Re: [RFC PATCH v3 05/15] hw/arm/smmuv3-accel: Introduce
>> smmuv3 accel device
>>
>> External email: Use caution opening links or attachments
>>
>>
>> On 9/5/25 10:22 AM, Shameer Kolothum wrote:
>>>> -----Original Message-----
>>>> From: Eric Auger <[email protected]>
>>>> Sent: 04 September 2025 15:33
>>>> To: Nicolin Chen <[email protected]>; Shameer Kolothum
>>>> <[email protected]>
>>>> Cc: [email protected]; [email protected];
>>>> [email protected]; Jason Gunthorpe <[email protected]>;
>>>> [email protected]; [email protected]; Nathan Chen
>>>> <[email protected]>; Matt Ochs <[email protected]>;
>>>> [email protected]; [email protected];
>> [email protected];
>>>> [email protected]; [email protected];
>>>> [email protected]; [email protected];
>>>> [email protected]
>>>> 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
>>>>>> <[email protected]>
>>>>> Overall the patch looks good to me,
>>>>> Reviewed-by: Nicolin Chen <[email protected]>
>>>>>
>>>>> 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.
> You mean a config option like ARM_SMMUV3_ACCEL to opt out during compile?
Yes that's what I meant
Eric
> Can do.
>
> Thanks,
> Shameer
>
>
