On 10/27/25 6:54 PM, Shameer Kolothum wrote:
>
>> -----Original Message-----
>> From: Eric Auger <[email protected]>
>> Sent: 27 October 2025 17:38
>> To: Nicolin Chen <[email protected]>
>> Cc: Shameer Kolothum <[email protected]>; qemu-
>> [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: [PATCH v4 22/27] hw/arm/smmuv3-accel: Add support for ATS
>>
>> External email: Use caution opening links or attachments
>>
>>
>> On 10/27/25 6:13 PM, Nicolin Chen wrote:
>>> Hi Eric,
>>>
>>> On Mon, Oct 27, 2025 at 05:59:13PM +0100, Eric Auger wrote:
>>>> On 9/29/25 3:36 PM, Shameer Kolothum wrote:
>>>>> QEMU SMMUv3 does not enable ATS (Address Translation Services) by
>> default.
>>>>> When accelerated mode is enabled and the host SMMUv3 supports ATS, it
>> can
>>>>> be useful to report ATS capability to the guest so it can take advantage
>>>>> of it if the device also supports ATS.
>>>>>
>>>>> Note: ATS support cannot be reliably detected from the host SMMUv3 IDR
>>>>> registers alone, as firmware ACPI IORT tables may override them. The
>>>>> user must therefore ensure the support before enabling it.
>>>> This looks incomplete to me. ATS is a big topic in itself. I would
>>>> prefer we do not advertise it until we do not have full support for it
>>>> (including emulation). Comparing to
>>>> c049bf5bb9dd ("intel_iommu: Add support for ATS") which was recently
>>>> contributed we miss at least translation request implementations
>>>> (PCIIOMMUOps ats_request_translation callback implementation).
>>>>
>>>> See:
>>>> https://lore.kernel.org/all/20250628180226.133285-11-
>> [email protected]/#r
>>> In accelerated SMMUv3 case, ATS translation and invalidation are
>>> done by the physical SMMU. Wondering why do we need this?
>> in 06/27 you still can have emulated EPs hotplugged in the loop, no?
> That can be prevented by propagating an error from the get_address_space()
> callback. It’s already on my TODO list as a follow-up series, but I can move
> it
> forward if this is a major concern here.
>
>> I remember some discussions with Peter who was also reluctant in general
>> to introduce some partial feature support. I think in general this is a
>> good policy to have features emulated and then accelerated. That's also
>> good for testing in case we can bring up some test env.
> Understood. However, ATS and PASID are the two key features required to make
> any meaningful use of the accelerated support. I’m not suggesting we skip
> emulation for ATS or PASID entirely, just that we prioritize the acceleration
> path first. 😊
I understand this is needed for vSVA. Nevertheless accel SMMU can
already work and be tested in a stdalone manner without ATS and PASID.
Those 2 features add a significant complexity on top of the core series
including from the review pov. This partial support support is not
necessarily great. Also I have not closely followed the thread related
to "[PATCH v4 26/27] vfio: Synthesize vPASID capability to VM" touching
pci.c. I don't know how much this is contreversial. just reading the
commit msg frightens me a little bit: "This is a choice in the good hope
of no conflict with any existing cap or hidden registers" ;-) Eric
>
> Please let me know if there are any strong opinions on this.
>
> Thanks,
> Shameer
