Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <af6a3ae8-d4b2-517d-8b14-15dd20392a00@linux.intel.com>
Date:   Tue, 13 Sep 2022 14:01:07 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Cc:     baolu.lu@linux.intel.com, Joerg Roedel <joro@8bytes.org>,
        Will Deacon <will@kernel.org>,
        Robin Murphy <robin.murphy@arm.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
To:     "Tian, Kevin" <kevin.tian@intel.com>,
        "iommu@lists.linux.dev" <iommu@lists.linux.dev>
References: <20220912024826.1684913-1-baolu.lu@linux.intel.com>
 <BN9PR11MB5276B65B2EAF97D53719D2FE8C479@BN9PR11MB5276.namprd11.prod.outlook.com>
Content-Language: en-US
From:   Baolu Lu <baolu.lu@linux.intel.com>
Subject: Re: [PATCH 1/1] iommu/vt-d: Enable PASID during iommu device probe
In-Reply-To: <BN9PR11MB5276B65B2EAF97D53719D2FE8C479@BN9PR11MB5276.namprd11.prod.outlook.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Kevin,

On 2022/9/13 11:13, Tian, Kevin wrote:
>> From: Lu Baolu <baolu.lu@linux.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