linux-kernel - Re: [PATCH 1/1] iommu/vt-d: Enable PASID during iommu device probe

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]

Message-ID: <af6a3ae8-d4b2-517d-8b14-15dd20392a00@linux.intel.com>
Date:   Tue, 13 Sep 2022 14:01:07 +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>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] iommu/vt-d: Enable PASID during iommu device probe

Hi Kevin,

On 2022/9/13 11:13, Tian, Kevin wrote:
>> From: Lu Baolu <baolu.lu@...ux.intel.com>
>> Sent: Monday, September 12, 2022 10:48 AM
>>
>> Previously PASID supports on both IOMMU and PCI devices are enabled in
>> the
>> iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA) path. It's
>> functionally
>> correct as the SVA is the only feature that requires PASID setup. However,
>> looking ahead, we will add more features that need to enable pasid (for
>> example, kernel DMA with PASID, SIOV, VM guest SVA, etc.). It makes more
>> sense to enable PASID during iommu probing device.
>>
>> This enables PASID during iommu probing device and deprecates the
>> intel_iommu_enable_pasid() helper. This is safe because the IOMMU
>> hardware
>> will block any PCI TLP with a PASID prefix if there is no IOMMU domain
>> attached to the PASID of the device.
>>
> 
> IMHO it's better to enable something only when it's actually required,
> e.g. does it make more sense to have a IOMMU_DEV_FEAT_PASID
> instead?

PASID is a capability (not a feature) of a device. Hence from my point
of view, the IOMMU driver could enable it by default as long as the
IOMMU can handle transactions with PASID. Currently other PCIe
capabilities like ATS and PRI are also handled in this way.

> 
> What this patch does has one problem. It's an intel-iommu driver
> internal policy. How can a device driver reliably tell that the pasid
> capability has been enabled by the iommu driver?

If *necessary*, I do not object to letting the device drivers enable or
disable PCI/PASID through an IOMMU interface. In that case, we may need
a reference counter, and explicitly tell the device driver that
disabling PASID will only take effect when the reference counter
becomes 0.

Best regards,
baolu