Received-SPF: pass (google.com: domain of linux-kernel+bounces-11938-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <ccdace4d-73a2-41c4-aa15-2d7b54e1d30e@linux.intel.com>
Date: Wed, 27 Dec 2023 17:33:11 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v7 7/9] iommu/vt-d: Allow qi_submit_sync() to return the
 QI faults
To: "Duan, Zhenzhong" <zhenzhong.duan@intel.com>,
 "Liu, Yi L" <yi.l.liu@intel.com>, "Tian, Kevin" <kevin.tian@intel.com>,
 "joro@8bytes.org" <joro@8bytes.org>,
 "alex.williamson@redhat.com" <alex.williamson@redhat.com>,
 "jgg@nvidia.com" <jgg@nvidia.com>,
 "robin.murphy@arm.com" <robin.murphy@arm.com>,
 "baolu.lu@linux.intel.com" <baolu.lu@linux.intel.com>
Cc: "cohuck@redhat.com" <cohuck@redhat.com>,
 "eric.auger@redhat.com" <eric.auger@redhat.com>,
 "nicolinc@nvidia.com" <nicolinc@nvidia.com>,
 "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
 "mjrosato@linux.ibm.com" <mjrosato@linux.ibm.com>,
 "chao.p.peng@linux.intel.com" <chao.p.peng@linux.intel.com>,
 "yi.y.sun@linux.intel.com" <yi.y.sun@linux.intel.com>,
 "peterx@redhat.com" <peterx@redhat.com>,
 "jasowang@redhat.com" <jasowang@redhat.com>,
 "shameerali.kolothum.thodi@huawei.com"
 <shameerali.kolothum.thodi@huawei.com>, "lulu@redhat.com" <lulu@redhat.com>,
 "suravee.suthikulpanit@amd.com" <suravee.suthikulpanit@amd.com>,
 "iommu@lists.linux.dev" <iommu@lists.linux.dev>,
 "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
 "linux-kselftest@vger.kernel.org" <linux-kselftest@vger.kernel.org>,
 "joao.m.martins@oracle.com" <joao.m.martins@oracle.com>,
 "Zeng, Xin" <xin.zeng@intel.com>, "Zhao, Yan Y" <yan.y.zhao@intel.com>,
 "j.granados@samsung.com" <j.granados@samsung.com>
References: <20231221153948.119007-1-yi.l.liu@intel.com>
 <20231221153948.119007-8-yi.l.liu@intel.com>
 <BN9PR11MB5276468811C5113427D4CA6F8C94A@BN9PR11MB5276.namprd11.prod.outlook.com>
 <e1cf94ba-aaf7-448b-a87b-95d8a2442b20@intel.com>
 <BN9PR11MB527663D97345FC8CD9683AB28C98A@BN9PR11MB5276.namprd11.prod.outlook.com>
 <72ce37d6-69bc-4580-a795-7a6638eae0ee@intel.com>
 <dec6b647-5ff2-4c57-8b10-b2d5c544566a@intel.com>
 <SJ0PR11MB67444BD4606F7A4014801B16929FA@SJ0PR11MB6744.namprd11.prod.outlook.com>
From: Ethan Zhao <haifeng.zhao@linux.intel.com>
In-Reply-To: <SJ0PR11MB67444BD4606F7A4014801B16929FA@SJ0PR11MB6744.namprd11.prod.outlook.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit


On 12/27/2023 5:06 PM, Duan, Zhenzhong wrote:
>
>> -----Original Message-----
>> From: Liu, Yi L <yi.l.liu@intel.com>
>> Sent: Tuesday, December 26, 2023 4:44 PM
>> Subject: Re: [PATCH v7 7/9] iommu/vt-d: Allow qi_submit_sync() to return
>> the QI faults
>>
>> On 2023/12/26 14:15, Yi Liu wrote:
>>>
>>> On 2023/12/26 12:13, Tian, Kevin wrote:
>>>>> From: Liu, Yi L <yi.l.liu@intel.com>
>>>>> Sent: Tuesday, December 26, 2023 12:03 PM
>>>>>
>>>>> On 2023/12/22 12:23, Tian, Kevin wrote:
>>>>>>> From: Liu, Yi L <yi.l.liu@intel.com>
>>>>>>> Sent: Thursday, December 21, 2023 11:40 PM
>>>>>>>
>>>>>>> +    fault &= DMA_FSTS_IQE | DMA_FSTS_ITE | DMA_FSTS_ICE;
>>>>>>> +    if (fault) {
>>>>>>> +        if (fsts)
>>>>>>> +            *fsts |= fault;
>>>>>> do we expect the fault to be accumulated? otherwise it's clearer to
>>>>>> just do direct assignment instead of asking for the caller to clear
>>>>>> the variable before invocation.
>>>>> not quite get. do you mean the fault should not be cleared in the caller
>>>>> side?
>>>>>
>>>> I meant:
>>>>
>>>>      if (fsts)
>>>>          *fsts = fault;
>>>>
>>>> unless there is a reason to *OR* the original value.
>>> I guess no such a reason. :) let me modify it.
>> hmmm, replied too soon. The qi_check_fault() would be called multiple
>> times in one invalidation circle as qi_submit_sync() needs to see if any
>> fault happened before the hw writes back QI_DONE to the wait descriptor.
>> There can be ICE which may eventually result in ITE. So caller of
>> qi_check_fault()
>> would continue to wait for QI_DONE. So qi_check_fault() returns 0 to let
>> qi_submit_sync() go on though ICE detected. If we use '*fsts = fault;',
>> then ICE would be missed since the input fsts pointer is the same in
>> one qi_submit_sync() call.
> Is it necessary to return fault to user if qi_check_fault() return -EAGAIN and
> a restart run succeeds?

Issue a device-TLB invalidation to no response device there is possibility

will be trapped there loop for ITE , never get return.

Thanks,

Ethan

> Thanks
> Zhenzhong