Received-SPF: pass (google.com: domain of linux-kernel+bounces-11419-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <dec6b647-5ff2-4c57-8b10-b2d5c544566a@intel.com>
Date: Tue, 26 Dec 2023 16:44:03 +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
From: Yi Liu <yi.l.liu@intel.com>
To: "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>, "Duan,
 Zhenzhong" <zhenzhong.duan@intel.com>, "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>
In-Reply-To: <72ce37d6-69bc-4580-a795-7a6638eae0ee@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?RmJMSHJONEp2NkV1TXdZZXdVTjNnUFI4WlpYbWRCK3VqWU1LZHBuYnF3R1ZG?=
 =?utf-8?B?aUlEYmRSMU9icU5FblhIUmpBREt5TWNMWlJFTGQ1ODFET1NvVW40bEVHdWh5?=
 =?utf-8?B?Vnc0TVJTODBOSkJtdjAxVGpCWDNaQ1pDdjNjQUdhNFVnUVZCNnFoaERzZTFj?=
 =?utf-8?B?NytXRXNDaUtjTmpNZW90dFJJNzQ3MDlFRVhGay9MRmNrZTdKbGpmSzU5ZEVj?=
 =?utf-8?B?VzAxeW5UZC8za0JHVERML2JFL0wyaWh6Sk0xcTNxK1RoY09XdFlOOVFMWUJK?=
 =?utf-8?B?bVNrMnJVWms3RUNVUnlVaEtDWVpDaVZIcWlPcFp0OVVtUnh0RDVkcy9oU0hu?=
 =?utf-8?B?ZzNnSVVabHZkZGNHaFFDRXA2N2tYOG9pa2FoQitPOU5tMVVSS1RGZXJxL0pT?=
 =?utf-8?B?cTI1dVFVUjFaU2YyRVlVZEdlcnJwQzg2TDVUcGhSNDFGWHFNRkNyWjU0L2Qz?=
 =?utf-8?B?TVNzL2x3RDdpeG90ekdnWkxVNUc3bDZOa1hYZnR0YkF3Sit5emRxRnFFUlB6?=
 =?utf-8?B?TmJIbzlYN2pCVVpaRUpxZFM4YkQ1azhicTNiVG5vR3lYcnJMcVFTMVlYTVFD?=
 =?utf-8?B?UjJYQndXckNoVmRycGt2U1Rla3RaVXEzV3JjVHNRMDZxajM0WFVVVWVXRkFH?=
 =?utf-8?B?OFpQN3RRZTlMMFo3RzhleHByNWFPSTR2bHphQzl1NUtXaUJEK1BNZXM5dm5W?=
 =?utf-8?B?MWNJejdBL0JqOXo1Sld4VE04QlREUEQweERaZC9NMkpqenhla3hUc2NQdHZx?=
 =?utf-8?B?NTExVCtJZTl3UkdYdjBIVnJoeU5nYVZla05ac2hOckxjd3hiSzAycXBra3ZE?=
 =?utf-8?B?aWp2RTE4T2hHSWlMaXpDd0tNUkI5S1E0bGl6UlZFR1h2bDhQcGFDWnAxQW93?=
 =?utf-8?B?bEFJaUdxWkxjeTFjRmpSSTRoOFZqK0lYeXArcXgrL25EbmNuQTdKV2pMekNW?=
 =?utf-8?B?dVlwZmlEdXdjOXRab21XcXgxQWd5VkNGdzBheFRXK0RnamZhL0hPNy8wdVhB?=
 =?utf-8?B?QUtRVEYvTE1QNyt3TmRPV3lSN21pV2FUNDNwTUhDbHY3UVI5NU5JV0lWNWpI?=
 =?utf-8?B?VHo3QmdZVXNvcjVJYkpRT1pIMDJocXB2QkljNlpoZUtCZlJiTzNNZmpWQ0s2?=
 =?utf-8?B?SG9UUDJyN2FpMk9KR3FSMEVYenJoNHpxMklEUkl5NnJubkdJTi9sZmJ1MWRo?=
 =?utf-8?B?aklBdGg4dE5lZ09zeEw2dnp4QVhvRndmUVk0SWRkZ3lkM2ZjT1VjUnYrL09t?=
 =?utf-8?B?QUNWVDVINFRBeEtxNFVEQVhod2c0ZlQwYUtjRGJFOE4xVmRGdnI4d09qV0Vp?=
 =?utf-8?B?d2poM01lNTlQcmt5VUs1TlFwQXdRT2tUZnJDUk1KQTk1eUZjMWdPNFBaRGNO?=
 =?utf-8?B?eDlZYnBYYmdTRDd3VHRDd3grdy9oV1MyTzFhVjVHdjl1SjVWNnJGVEZqWkhz?=
 =?utf-8?B?RU5VUjBycnRzVkJqK29QZ3VCM1pnT3JsYTdhNWNYcTRSM3Y5RUtUV3Y0NnFN?=
 =?utf-8?B?L2hielM4T2hnbTRPbnV5WWorVitVS1pZN2JTa0VBMFJEWCthSHFkM093NW9G?=
 =?utf-8?B?S3RZOHY1bFVYOVBQSk5LWHV2eHZ5SHBPQ2lNdHA5a0ZzK2hBTVV6Q1g4Q2Q3?=
 =?utf-8?B?bjBlZkE5TGkrQytPakZpZSt3RGIwSlhlNzdZOFZFcUd3Qnp1akJlNVk2RWc3?=
 =?utf-8?B?R2lBRjVxdGhJa0RtMHlzeXo2VzJMdVJxbGNKR0dDeHMzRjVnQnZHcW5aZTFu?=
 =?utf-8?B?Z3dsc05GbEZHdGx4d0I3SEdqd2Y1MndROTkrc1NBNzVESzIzYi9oUXROaVZC?=
 =?utf-8?B?alpaRUJOMXdpODFTd3BDVkRhYk9GZjBacXJXV1UxYTB6c3Y3VUwwWk1GSi9m?=
 =?utf-8?B?UGp6UVhwek9CZDBpTTJLTk01UDVvSkErQ2Z1N1MzdGlSTkNqSVZCSGE2NU96?=
 =?utf-8?B?aGZpSnhkTFNuaFZwclkzdmZwWXhEV1UzbXN3OWNBYldOdVhjY1BMc3NWdHEx?=
 =?utf-8?B?MnlJWnJzdjdpNmlDNlEwbVJsbE5IdXlHMzhjNWhLc2hrREpaZ0FaanJiQnZQ?=
 =?utf-8?B?R0taQ2VpNmRHV1E3b3p4Q0UyMW40TEtKUzg4bHZJN2R1OWNwejVWS3Exc014?=
 =?utf-8?Q?d4tbtYPL0QXsGb2goLNCJI4mD?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 0a92c577-e040-49d1-2f2a-08dc05ee6c2c
X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7529.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Dec 2023 08:41:15.8577
 (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: 0T+p9LTzMEpgPC45ogzePfk2LCtdoPNLhv/HsOPSy+sp7E97xfNzXXbHAb8mxaY2Otglz8ynDwj9iabmpSXJ2A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB8089
X-OriginatorOrg: intel.com

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.

-- 
Regards,
Yi Liu