linux-kernel - Re: [PATCH 1/1] iommu/vt-d: Fix missed device TLB cache tag

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, 21 Jun 2024 09:44:27 +0800
From: Baolu Lu <baolu.lu@...ux.intel.com>
To: Jason Gunthorpe <jgg@...pe.ca>, Vasant Hegde <vasant.hegde@....com>
Cc: baolu.lu@...ux.intel.com, "Tian, Kevin" <kevin.tian@...el.com>,
 Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>,
 Robin Murphy <robin.murphy@....com>,
 Jacek Lawrynowicz <jacek.lawrynowicz@...ux.intel.com>,
 "iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] iommu/vt-d: Fix missed device TLB cache tag

On 6/20/24 10:08 PM, Jason Gunthorpe wrote:
> On Thu, Jun 20, 2024 at 04:19:46PM +0530, Vasant Hegde wrote:
>>>>>> seems that for all domain attaches above is coded in a wrong order
>>>>>> as ats is enabled after the cache tag is assigned.
>>>>> Yes, exactly. But simply changing the order isn't future-proof,
>>>>> considering ATS control will eventually be moved out of iommu drivers.
>>>> [Unrelated to this patch]
>>>>
>>>> You mean ATS setup will be moved to individual device driver? Is there any
>>>> reason for that?
>>> Not exactly to individual device drivers, but it should be out of the
>>> iommu drivers.
>>>
>>> https://lore.kernel.org/linux-iommu/BL1PR12MB51441FC4303BD0442EDB7A9CF7FFA@BL1PR12MB5144.namprd12.prod.outlook.com/
>> Got it. Thanks.
>>
>> I remember of this discussion. May be we can provide API from IOMMU driver so
>> that individual driver can enable/disable ATS (like iommu_dev_enable_feature()).
> But I have a feeling if we do that it should be done by re-attaching
> the domain.
> 
> For instance if you look at how I structued SMMUv3, the ATSness is an
> effective property of the domain type and ATS switches on and off
> dynamically already.
> 
> Having an additional input to domain attach "inhibit ats", as a flag
> would be all the support the driver would need to provide for the core
> code to manage this with some kind of global policy.
> 
> I would suggest to steer VTD in that direction too and make the ATS
> enable be done on domain attach, and put the first ATS enable in
> attach, not in probe. The logic in smmuv3 would apply just as well to
> VTD, though you'd need the hitless update logic too 🙂

Enabling/Disabling ATS on domain attach seems like a feasible approach.
The ATS requirement information (required/disallowed/neutral) could be
included as an opt-in option in the domain attach path. This likely
applies to PASID attachments and VF/PF devices. The iommu driver
maintains the per-device ATS refcount and enables it for the first
request and disables it after the last one. The attachment fails
accordingly if the hardware capability doesn't match the domain attach
requirement.

Perhaps we could further include PRI as a domain attach option,
indicating that the domain requires IOPF functionality. This would allow
us to simplify the SVA and IOMMUFD by hiding device and IOMMU details
within the IOMMU driver.

Best regards,
baolu