RE: [PATCH v2 2/8] iommu/vt-d: Improve iommu_enable_pci_caps()

[Tian, Kevin]

> From: Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx>
> Sent: Friday, November 11, 2022 2:59 PM
> 
> On 2022/11/11 11:45, Tian, Kevin wrote:
> >> From: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> >> Sent: Tuesday, November 8, 2022 3:34 PM
> >>
> >> The PCI subsystem triggers WARN() if a feature is repeatedly enabled.
> >> This improves iommu_enable_pci_caps() to avoid unnecessary kernel
> >> traces through checking and enabling. This also adds kernel messages
> >> if any feature enabling results in failure. It is worth noting that
> >> PRI depends on ATS. This adds a check as well.
> >
> > Cannot we have a helper to check whether this device has been attached
> > to any domain? If no in the blocking path then disable PCI caps. If no
> > in the attaching path then enable PCI caps.
> >
> > I just didn't get the point of leaving them enabled while the device can
> > not do any DMA at all.
> 
> Ideally, the kernel owns the default policy (default on or off). The
> upper layers are able to control it over IOMMUFD uAPI or kerneld kAPI.
> I can't see the benefits of associating these features with the
> existence of any domain.

we don't have such uAPI or kAPI today.

the current behavior before your change is default off and then toggled
along with attach/detach domain. as only one domain is allowed per
RID it implies the capabilities are toggled along with DMA allow/block.

now you change it to a messy model:

  - default off when the device is probed
  - turn on at the 1st domain attach and never turn off until release
  - but iommu_enable_pci_caps() is still called at every domain attach
    with band-aid to allow re-entrant

this isn't like a good cleanup...