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: <c5a1a51d-b309-4237-b2c5-fbdaa1f4f9a8@linux.intel.com>
Date:   Wed, 31 May 2023 12:59:44 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Cc:     baolu.lu@linux.intel.com,
        Jacob Pan <jacob.jun.pan@linux.intel.com>,
        LKML <linux-kernel@vger.kernel.org>,
        "iommu@lists.linux.dev" <iommu@lists.linux.dev>,
        Joerg Roedel <joro@8bytes.org>,
        Robin Murphy <robin.murphy@arm.com>,
        Jean-Philippe Brucker <jean-philippe@linaro.com>,
        "dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>,
        "vkoul@kernel.org" <vkoul@kernel.org>,
        Will Deacon <will@kernel.org>,
        David Woodhouse <dwmw2@infradead.org>,
        "Raj, Ashok" <ashok.raj@intel.com>,
        "Liu, Yi L" <yi.l.liu@intel.com>,
        "Yu, Fenghua" <fenghua.yu@intel.com>,
        "Jiang, Dave" <dave.jiang@intel.com>,
        "Luck, Tony" <tony.luck@intel.com>,
        "Zanussi, Tom" <tom.zanussi@intel.com>,
        "Ranganathan, Narayan" <narayan.ranganathan@intel.com>
Subject: Re: [PATCH v7 3/4] iommu/vt-d: Add set_dev_pasid callback for dma
 domain
Content-Language: en-US
To:     Jason Gunthorpe <jgg@nvidia.com>,
        "Tian, Kevin" <kevin.tian@intel.com>
References: <20230523173451.2932113-1-jacob.jun.pan@linux.intel.com>
 <20230523173451.2932113-4-jacob.jun.pan@linux.intel.com>
 <BN9PR11MB52760139A4844C8DF0EE1BE98C469@BN9PR11MB5276.namprd11.prod.outlook.com>
 <ZHYtkioNR2YfC18C@nvidia.com>
From:   Baolu Lu <baolu.lu@linux.intel.com>
In-Reply-To: <ZHYtkioNR2YfC18C@nvidia.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 5/31/23 1:08 AM, Jason Gunthorpe wrote:
>>> +	/*
>>> +	 * The SVA implementation needs to stop mm notification, drain the
>>> +	 * pending page fault requests before tearing down the pasid entry.
>>> +	 * The VT-d spec (section 6.2.3.1) also recommends that software
>>> +	 * could use a reserved domain id for all first-only and pass-through
>>> +	 * translations. Hence there's no need to call
>>> domain_detach_iommu()
>>> +	 * in the sva domain case.
>>> +	 */
>>
>> It's probably clearer to say:
> 
> Is this what is going on ??
> 
>> /*
>>   * SVA domain requires special treatment before tearing down the pasid
>>   * entry:
>>   *   1) pasid is stored in mm instead of in dev_pasid;
> 
> Why? The mm pasid should not be used by any driver code, the PASID the
> SVA is hooked to does NOT have to be the mm PASID.

Yes. The individual driver should not be aware of mm->pasid. The
set/remove_dev_pasid callbacks have already take pasid as the inputs.

> 
>>   *   2) all SVA domains share a reserved domain id per recommendation
>>   *      from VT-d spec (section 6.2.3.1) so domain_detach_iommu() is
>>   *      not required;
> 
> The DID should be managed and allocated for the S2 pointer and the
> flushing logic should work genericly by tracking the S2's being used
> and flushing their DIDs when all the S2s fall out of use. The special
> identity S2 just gets a static DID that never falls out of use.

The DID logic in this case is not about invalidating the DID. Instead,
it is about managing the DID's reference count to track its allocation
and release. The reference count is increased every time the DID is
assigned to hardware, and it is decreased when the DID is removed from
hardware. The DID is released when the count hits 0.

Some DIDs are reserved for special domains, like identity domain, SVA
domain, etc. For those DIDs, there's no need for reference count, hence
no need to call the helper.

For code simplify, perhaps we can enhance the helpers to take no action
for those special DIDs.

Best regards,
baolu