linux-kernel - Re: [PATCH v2 2/8] iommu/vt-d: Improve iommu_enable_pci

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] [day] [month] [year] [list]

Date:   Fri, 11 Nov 2022 19:57:12 +0800
From:   Baolu Lu <baolu.lu@...ux.intel.com>
To:     "Tian, Kevin" <kevin.tian@...el.com>,
        "iommu@...ts.linux.dev" <iommu@...ts.linux.dev>
Cc:     baolu.lu@...ux.intel.com, Joerg Roedel <joro@...tes.org>,
        Will Deacon <will@...nel.org>,
        Robin Murphy <robin.murphy@....com>,
        "Liu, Yi L" <yi.l.liu@...el.com>,
        "Pan, Jacob jun" <jacob.jun.pan@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/8] iommu/vt-d: Improve iommu_enable_pci_caps()

On 2022/11/11 16:16, Tian, Kevin wrote:
>> From: Baolu Lu <baolu.lu@...ux.intel.com>
>> Sent: Friday, November 11, 2022 2:59 PM
>>
>> On 2022/11/11 11:45, Tian, Kevin wrote:
>>>> From: Lu Baolu <baolu.lu@...ux.intel.com>
>>>> 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...

Fair enough. We should not bury this behavior change in a cleanup
series. Okay! I will keep the previous behavior.

Best regards,
baolu