RE: [PATCH 1/1] iommu/vt-d: Add opt-in for ATS support on discrete devices

From: Tian, Kevin
Date: Fri Mar 03 2023 - 03:19:43 EST


> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Thursday, March 2, 2023 1:43 AM
>
> If Intel BIOS's have populated the "satcu" to say that ATS is not
> supported by the HW when the HW supports ATS perfectly fine, then get
> the BIOS fixed or patch the ACPI until it is fixed. The BIOS should
> not be saying that the HW does not support ATS when it does, it is a
> simple BIOS bug.
>

That is not the purpose of SATC.

The ATS support in VT-d side is reported in two interfaces:

1) "Device-TLB support" in Extended Capability Register;
2) Root port ATS capability in ACPI ATSR structure;

A device gets ATS enabled if 1/2 are true and !pdev->untrusted. Same
as SMMU does.

The main purpose of SATC is to describe which ATS-capable integrated
device meets the requirements of securely using ATS as stated in VT-d
spec 4.4. Kind of a way of building trust on the device which is fully
validated to not allow misusing translated transaction to attack memory
outside of its attached domain.

System software which cares about it could use the information in
a more restrictive ATS model (beyond !pdev->untrusted).

But this is moot in current intel-iommu driver which still makes decision
based on !pdev->untrusted as other IOMMU drivers do. SATC is used
kind of as a quick path to bypass ATSR check given no root port for
integrated devices (plus a small trick on HW managed ATS).

So I agree this patch is not required. We should not have a relax option
to bypass ATSR check anyway.

But Baolu, seems there is a small bug on handling satcu->atc_required.
This indicates that ATS must be enabled as a functional requirement.
Then we should handle the failure of pci_enable_ats() on such device.

Thanks
Kevin