Received-SPF: pass (google.com: domain of linux-kernel+bounces-12519-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <77f8a549-4d81-4ad4-8acf-bff3b2cafcb4@intel.com>
Date: Thu, 28 Dec 2023 16:42:53 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v8 10/10] iommu/vt-d: Add iotlb flush for nested domain
Content-Language: en-US
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: <20231227161354.67701-1-yi.l.liu@intel.com>
 <20231227161354.67701-11-yi.l.liu@intel.com>
 <BN9PR11MB527696416E2B1F33900007968C9EA@BN9PR11MB5276.namprd11.prod.outlook.com>
From: Yi Liu <yi.l.liu@intel.com>
In-Reply-To: <BN9PR11MB527696416E2B1F33900007968C9EA@BN9PR11MB5276.namprd11.prod.outlook.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SWtrWlBoeEdQQ0lwSEhRbC9RUE1oS2RNd0psK3VUL2lHNCs4cmgwQ1VwZytP?=
 =?utf-8?B?eDdESHllbzErS29CRkhVT1lhVlRidFlocHQ1ZCtTa3BuUWRsc1pZdW95TEdT?=
 =?utf-8?B?MEkvcEcwZ1Fnb3R1MjlieTNQV2VCUGlHQU5sOWVtUHVHeld1VjRVanZNOGVr?=
 =?utf-8?B?L1NTZkNFdFFFdkszaVZjVStoTDZ0b3RPOUZReVRGempoaDJ5OFFaeC9lRWM4?=
 =?utf-8?B?Mnl6aGI2QWtualJpakZDdFVFa05VYjJvMnNNZkVUNkZUNXJSbzI4UVZSVWQr?=
 =?utf-8?B?MVByRVR3VWxlQTlnUDBXZkZOUk9aelEyZmhUcVMvYnpXVkJ6SmY0TU8vQ0tw?=
 =?utf-8?B?blhsVmY1d0xkMFZkcWJib1VVVEpDbDBZMDBhN3AzOEtZUEdmS1l5S1VmaktL?=
 =?utf-8?B?VXFsaDRvYXBaa2k0ZHk5a3hnaDNOUjdNc2tia2FwU2QwdkRrY2xJcmd0WGI2?=
 =?utf-8?B?NU5SdUsvaE5GcVJIczVLNkRjUVhiWTh3cWlrTlp3SkFWdllDN1ZXVmJLaTM1?=
 =?utf-8?B?V1ErRnExbUVDVFArMlBWUWVOMnVMdTVJZlJuV1dBcVYvbS9uSmt0cnQybk5x?=
 =?utf-8?B?UTNQUzJ6NUxpWm1naGVvcHJBTW9FRDQ0MVpLR2I1ZElmczBmY1B0bG5BV3Ns?=
 =?utf-8?B?c2EwRDRTSGtTR09WRUlVSDBsTGwxWjNkU3JKNTNvdmlsMk5YTXRZOWE2ZW5U?=
 =?utf-8?B?a2FuZ2tORlpCU3JudHN2aVVnT0c0ZGJTV2dLZEkzUk1DRHgrU01LWno1MjZI?=
 =?utf-8?B?ejZwcVJ4SHJXelBQd2dwdlpvY2JkTFdyN2VzNnBSSFF4cWZKR2FMZTRObUJq?=
 =?utf-8?B?MEprZjhjNERmYXNKTk1pWHgyUlZ1K2d3U2xLa2hZS3Rlb2RJNWc2UE51NXg2?=
 =?utf-8?B?S3pXRjhVMnBybG5uY0xiZG01NGlyTGR1eFlNWFhqUnEzbTJvSCtQTG1qRGt6?=
 =?utf-8?B?bjZGN2gxNUo4V0FicmpQUHNrOHRZRnVKSk1RWXgyYkpkMHpLNVdGSjNtYjlQ?=
 =?utf-8?B?TGhBM0hvcUZCcFVNZmFNT3VxLzVtSUJRbEdHYUZLMktHTWptQlNBcXJ4YU5T?=
 =?utf-8?B?cE1hYVpIc0FxOHFTdnJnZDBnZk1OanVJY05NbWNQLzdibld0ZXBrRGhSMW5Q?=
 =?utf-8?B?T3d6VjJKVWtaWHRjZGgzWGhpdVVnSzZ5d0NUdm5pSWxkcEpIYlpQY2gxTmlO?=
 =?utf-8?B?Mm5iRkxzTzdRWFpIbmRnMkZ4b1VYMlhYK2MzTUszRGJiV2VRUCtDYVRXK2c4?=
 =?utf-8?B?TkQ0ZndXdWtkQjg2WHRPdENseTA2d0xBajVaUnBna2xDV1A5eC9td2NySERa?=
 =?utf-8?B?WVBwcnB4bGZWTHN2aFBocm5SSEcvUjVuTFhReEdBQllReWZDL1RtQTNnNGJH?=
 =?utf-8?B?ZlRaUnlPZURaelZPK0RyY2d1TS8vNVBsK21zNlBKdDQwOHdGeUU0UmdHMkpn?=
 =?utf-8?B?YWN3Y2RlYUVNNHdvUmp0V1gveHJGUHNySnFLMFFSVEVidms5OXd3Qm9DbU9Q?=
 =?utf-8?B?eDB4cnNzODZJaWVTQ3BEWjdYUWh0a2Z0Nm1sQlE3YlZYR3FJbTBJR3dtdFlu?=
 =?utf-8?B?YXVSWU9uZVp3cGNkdnExRjlpVEpQTG9UakVYWkdodUxraE9IU2Z4NmN2TnpL?=
 =?utf-8?B?SDVnMFhLTkpNWklqc1NKOGpnaTB3WmdSSURLU3haS0JDNmRmSHpqK2RPWmF0?=
 =?utf-8?B?aFVXTUNKcVdMMHFvdysrN2VMZGRlb1oxM01jb2tqQ3JwOVcvR3RxWm9sa0xC?=
 =?utf-8?B?YmVQL1FaTTF5RzNFK3hHWWxFTDdEUEVPSktLUlg2elBPWUJJZ2NLa2UxbVZV?=
 =?utf-8?B?ZkJ3QTRTbEJhcjhCNUhVUjdUeGZLZ2E5eHB5UDErTUlwZGZpaXNaeDdsV0FF?=
 =?utf-8?B?b1VnbGhSazJtaFV5Y0lkcDN4SG1RTWF4NmhYWEhEYTRlY0lLNmpySCtmSEc4?=
 =?utf-8?B?aDg4MlBGS0NzR2l1WkZRM0lhdGFuVUMrOEZ6aHI5ZWtoM0pNdU5TVDQ5TkhW?=
 =?utf-8?B?VTVjekxCcElFT3pvdW5JU1dYOUlvVnZ6VkVld1hVT0V1YW5xRGY5UUZhQmZS?=
 =?utf-8?B?MkhTUG1vb2taamVCaEU3bXVvMmgrb2ZkU29ySG94QjQwYTZ3bXhLREd2U21r?=
 =?utf-8?Q?ymZy4hyXf8WEbk7R7mDhdDJYP?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 52351878-64c2-424a-445e-08dc078096c3
X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7529.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Dec 2023 08:40:04.9832
 (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: 5JF6usyWtjX2rpWv6hh4COevCzjQAoDDS/3pWsEdT3rQKG+xvhBcxPnplQ5P0+VsN3I66kZNY074dQcwBBD3aQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR11MB8513
X-OriginatorOrg: intel.com

On 2023/12/28 15:11, Tian, Kevin wrote:
>> From: Liu, Yi L <yi.l.liu@intel.com>
>> Sent: Thursday, December 28, 2023 12:14 AM
>>
>>
>> +static void nested_flush_pasid_iotlb(struct intel_iommu *iommu,
>> +				     struct dmar_domain *domain, u64 addr,
>> +				     unsigned long npages, bool ih)
>> +{
>> +	u16 did = domain_id_iommu(domain, iommu);
>> +	unsigned long flags;
>> +
>> +	spin_lock_irqsave(&domain->lock, flags);
>> +	if (!list_empty(&domain->devices))
>> +		qi_flush_piotlb(iommu, did, IOMMU_NO_PASID, addr,
>> +				npages, ih, NULL);
> 
> Not sure the check of domain->devices makes sense here. The outer
> loop is xa_for_each(&domain->iommu_array, i, info) which, if found,
> then implies certain devices behind this iommu must be attached to
> this domain.

yes.

> I wonder whether it's clearer to remove this function and just call
> qi_flush_piotlb() from intel_nested_flush_cache().

yeah, with above remark, it would be more clear to call qi_flush_piotlb()
from intel_nested_flush_cache().

>> +
>> +	/*
>> +	 * Invalidation queue error (i.e. IQE) will not be reported to user
>> +	 * as it's caused only by driver internal bug.
>> +	 */
> 
> here lack of:
> 
> 	*error = 0;

yes.

>> +	if (fault & DMA_FSTS_ICE)
>> +		*error |= IOMMU_HWPT_INVALIDATE_VTD_S1_ICE;
>> +	if (fault & DMA_FSTS_ITE)
>> +		*error |= IOMMU_HWPT_INVALIDATE_VTD_S1_ITE;
>> +}
>> +

-- 
Regards,
Yi Liu