Received-SPF: pass (google.com: domain of linux-kernel+bounces-12042-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <98226e62-90b8-44a6-9804-0356235ec34d@intel.com>
Date: Wed, 27 Dec 2023 22:12:39 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v7 7/9] iommu/vt-d: Allow qi_submit_sync() to return the
 QI faults
Content-Language: en-US
To: Ethan Zhao <haifeng.zhao@linux.intel.com>, "Duan, Zhenzhong"
	<zhenzhong.duan@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>
 <ccdace4d-73a2-41c4-aa15-2d7b54e1d30e@linux.intel.com>
From: Yi Liu <yi.l.liu@intel.com>
In-Reply-To: <ccdace4d-73a2-41c4-aa15-2d7b54e1d30e@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?aVI3Uk1POElYYVBEWng3MjNoM3U4VDQzU3U2UkoySExmZWlwdGhzMUdvcXdP?=
 =?utf-8?B?MU40QSttQVFTTVdUMWp3Ym5laHZHT1IzUHE1cm5qc3NyREFwTGh5Y3NjQ1Zy?=
 =?utf-8?B?RlJ1WlhiMFdmSHRBYThqZkkzOFNPcFhXL2Q0S0NzMVFzbnBlcTh2MFhXOWla?=
 =?utf-8?B?cGU1MjFFTUlqUUJ6ZDVyREdmbno3RkZVMko2eFczWThTMFFxSHdEUWFYM08w?=
 =?utf-8?B?Q0xaaU1rZnVrWVRac0pJeGRPeUJ2Rzc3ekVrY3BZRGIwZ3lKV3UvUC81VG5w?=
 =?utf-8?B?MU1uaGROTFpJcEZNeEtyazdYTXRRVTRHbFZRejJpdGhpVXNZVWtjOVNjMVdS?=
 =?utf-8?B?U1ZBWlFubktQWFMrVml4Z0VpK1JEZlNKWW1sc0JKMjVTZkt1S3lCWUlFQzA4?=
 =?utf-8?B?aVZsY2tGWEJYVDZzM0RQYmZnQ2dsQmpXMW5NWnBld05UV09jQmNwOEhQdy94?=
 =?utf-8?B?N3o4NVkxUXNKRml0TTJ6Mm9kNlk5Y3JtY0xjVytycTVYc3M2c0g4bmR5SnBH?=
 =?utf-8?B?T2pTdk1uVGFkWnVsSnlyLytoekFxWHZPMlp5SXE0TXJSQ2tBMzZjWFl2UDZK?=
 =?utf-8?B?V3gvNW9wckVIZldXM1lOUXVQdFBiSGxhT2NOWGJtd29CUW1kcjlHQjhnR2dy?=
 =?utf-8?B?YzBKRkNrOWt1UDdnanRGMHB3N2lZanVWK3hKSDB1K1RVajF5aXkwNEJnbkcx?=
 =?utf-8?B?bk90YWU5TFQzMnh4RVhkUjVRd2hIZjdWdkFtTkQwS0pVWll0djRFSUFYZ3hN?=
 =?utf-8?B?NWxVcFdWTXlwRWxZOHAzWXlhYmhlZGNSVG5NRGZ0U21acWE2SEo4WEN2c3hE?=
 =?utf-8?B?SzE3MGo4TENjcVg4ODRtNzZhUUgyVUJqT3QwZm5Gb3FiSlFRaUg0WWg4ZkhN?=
 =?utf-8?B?RHVkeG9yTXdIUStXSW9qT2xsSmNPRGdqM1M4SFVHSjl0eHR4dUR4NWd1bGcz?=
 =?utf-8?B?ZnFCbkZjcmZZeWtRNEt3UmhXTTVVSWl2U25sUER3RzBydUVueHpLKys1U0N1?=
 =?utf-8?B?RTZvb1RMbmpseEtjeTRtQlBIMVZZTmloM1VJNXhJMjlzc3kwbXdoamJJdnM4?=
 =?utf-8?B?bXkxbkJkVkFwMHN0Tncrek4xYnBiLzByR0lLMll6ZzVSWUUvamJSTHhxZ1Vk?=
 =?utf-8?B?NVVwQjZJKzgrRGEyU1J6QmVnTnNaL2MrcUJuc3o0SWVaWUpZV0dEUDJBZWlu?=
 =?utf-8?B?WE11eGNvOXhKazZNanhNa1phVExKZ3I4Z0IxcHdJK0cvbG10bnZxRGpDaUlL?=
 =?utf-8?B?TVd0Y3BzcVUyYWdGaUtEcDNILzk4a3JlVmc5OXhJcHdqenRoa2F4VnhZU0VW?=
 =?utf-8?B?QzFFeVd1dUJBZEF1TCtMNVBGTFNUcVZRbEcwTEF6bzZpaFVuYm4wSnFpelJU?=
 =?utf-8?B?OVFLRkZTdGtQMHZTZDAxVWFka0RKbHdEcFFBTk90bVV6c3JaMVN4WHE1Q1oy?=
 =?utf-8?B?SGdlRnpIUk1iT2R2c0d6SG5XTkRNRkQrNFRDMitQMzN3dUZPSm5pYWVvZ2xF?=
 =?utf-8?B?RmtmaEI3cVVRVUlnQ01kUXBwNUt3enN5d3FyL1hKQnhVcCtqWXNJNUZKOE1F?=
 =?utf-8?B?VTRJeTlNRWNmUldOU0tmNG9XRkxPYnh0T2tNRTRISkpwKzYwUGducWN1NWF5?=
 =?utf-8?B?S3R4U0svZjQzTWZIeHprcEFxQVdpM3I2Rk9ndjBzT3N0T3Q4VjZWVUU4YXZF?=
 =?utf-8?B?UUV3ZWVNVHQyNkJoVlExSkZWZlRGYnlBRTRXMUlXR25PSUtSWGlmcW5QNFdK?=
 =?utf-8?B?VWE3c0ZiMjZsVEFwdTFDc3hsRWRtekZCWTB3cXZPTlp1MDJFY2hBdWFJVTJH?=
 =?utf-8?B?bjN1bERkWDJraFlNWEswWXhEUEdJTCt1NE9Ud0d2VE5FbXR4aEdhamp0dmJ6?=
 =?utf-8?B?R2haK21oWkRmRHFMNXRQblNnblZ1SWZYcHdlanQwT0ZHbmpzdS9qVEtITE1p?=
 =?utf-8?B?SE1Wa2hrWGhORnZob1VRNUlNd044QTJFUTk3Q0ZOR1lhZXlSSStTZk4zbndH?=
 =?utf-8?B?SGVKRUFUQ1Zob1NuY2Jld0dmTjJRMjZvUzJNWG9adE0vYUFIVmYrUEhvek5z?=
 =?utf-8?B?MndtRWwxMGVreVIxUzhFMCsxUTVrZnBYTWpNRGNuRHpHZG16akZCYjdXaW5S?=
 =?utf-8?Q?Ddop99c+NHjleYY7PRHaqGVt1?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 36096ef9-56ba-427d-870f-08dc06e57f4f
X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7529.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Dec 2023 14:09:53.6197
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: qZfcOPPkDSqIZhElviqXdDWNEwkIJFQ0c4JpTMxywOPV/sEpjrIRzICMVyAg+VBlzZyUEZgWeUJlXocAp8RiCw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR11MB6960
X-OriginatorOrg: intel.com

On 2023/12/27 17:33, Ethan Zhao wrote:
> 
> 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?

no need if a restart succeeds. I would add a *fault = 0 per the restart.

> 
> Issue a device-TLB invalidation to no response device there is possibility
> 
> will be trapped there loop for ITE , never get return.

yes. This the implementation today, in future I think we may need a kind
of timeout mechanism, so that it can return and report the error to user.
In concept, in nested translation, the page table is owned by userspace, so
it makes more sense to let userspace know it and take proper action.

-- 
Regards,
Yi Liu