Re: [PATCH v4 22/27] hw/arm/smmuv3-accel: Add support for ATS

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

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-clement.mathieu--drif@eviden.com/#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?

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.

Thanks

Eric
>
> IIRC, intel is mixing their emulated translation and accelerated
> one, and their pasid table is not integrated like SMMU's CD table
> that we already passed entirely via the STE.
>
>> Also in SMMU spec I see other stuff like STE.EATS, ATS skipping stage 1,
>> ... Here I only see  SMMU_CMD_ATC_INV and this is only implemented for
>> accel. I think I would rather stick to the minimum in this series, ie.
>> reject in case of ATS mismatch (for testing purpose you will just need a
>> small hack until we get ATS support), work on ATS enablement in parallel
> That SMMU_CMD_ATC_INV seems to be the only thing we need for ATS..
>
> Thanks
> Nicolin
>