On Wed, Jan 11, 2023 at 03:17:03PM +0100, Christian König wrote: > Am 11.01.23 um 15:14 schrieb Jason Gunthorpe: > > On Wed, Jan 11, 2023 at 09:54:03PM +0800, Baolu Lu wrote: > > > On 2023/1/11 21:44, Jason Gunthorpe wrote: > > > > > iommu_enable_pci_caps() in the Intel IOMMU driver. This also does some > > > > > handling for ATS, so here we could check the info->ats_supported flag if ACS > > > > > needs to be checked or not. > > > > *groan* this is seems wrong 🙁 Lu why are we doing this inside iommu > > > > drivers instead of in the device drivers to declare they want to use > > > > PASID? > > > Currently it's common to enable pasid in the IOMMU drivers, but device > > > driver has more knowledge of the device, hence it makes more sense to > > > move pci_enable_pasid() to the device driver. > > So, lets fix it that way. > > > > Add the flag to the pci_enable_pasid(), set the flag in the AMD > > IOMMU's special AMD GPU only path assuming the device will always use > > ATS > > That will fix at least this the AMD use case. > > > Do not set the flag in the other iommu drivers > > Don't we have other hardware which supports ATS as well and might run into > the same problem? As I said, I think we have only 1 user of the common PASID API and it was happy with things as-is, so I think for v6.2 we are fine. Honestly, not declaring ACS in a 'enterprise' multi-function device is already kind of sketchy/rare - even if ATS saves things for the PASID case. Jason
