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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 3 Mar 2023 08:19:29 +0000
From:   "Tian, Kevin" <kevin.tian@...el.com>
To:     Jason Gunthorpe <jgg@...dia.com>,
        Robin Murphy <robin.murphy@....com>
CC:     Baolu Lu <baolu.lu@...ux.intel.com>,
        "iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
        Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 1/1] iommu/vt-d: Add opt-in for ATS support on discrete
 devices

> From: Jason Gunthorpe <jgg@...dia.com>
> 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