Received-SPF: pass (google.com: domain of linux-kernel+bounces-136448-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: <ac7fdf0f-ecb9-415f-9c1b-600536b92ca5@intel.com>
Date: Tue, 9 Apr 2024 16:27:58 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/2] iommu/vt-d: Avoid unnecessary device TLB flush in map
 path
Content-Language: en-US
To: Lu Baolu <baolu.lu@linux.intel.com>, <iommu@lists.linux.dev>
CC: Kevin Tian <kevin.tian@intel.com>, Joerg Roedel <joro@8bytes.org>, "Will
 Deacon" <will@kernel.org>, Robin Murphy <robin.murphy@arm.com>,
	<linux-kernel@vger.kernel.org>
References: <20240407144232.190355-1-baolu.lu@linux.intel.com>
From: Yi Liu <yi.l.liu@intel.com>
In-Reply-To: <20240407144232.190355-1-baolu.lu@linux.intel.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?bnVTRlVBNDU5VzZ2MndDeDN4WW02VUVqZmtwRlE4L2hWUjZzd3lzNkhKN0RN?=
 =?utf-8?B?Y0Z6eC9MT2hrL3lvUEY5VFAxN280M0xXUEN6STlRSENnbzNpRDBVajc3cXZW?=
 =?utf-8?B?RHVjbENiM1Zoc2NldEhPeHhDK2FYQmFKQU5HRU12emxSazZUL3dKcjI1STdk?=
 =?utf-8?B?VmRpOEJyWTl6OFFHcEdqck9sTjBVT0VnTmg0TEhhbjh4RVFJUG5KMm5Zem1w?=
 =?utf-8?B?TWVTbDBPVVVwWG1YZVVjdFUwQmhkbmlZNkRFY1NFSVdtUi9DY3dibjRNV0dm?=
 =?utf-8?B?QXNOZzZJSnNCbGJrOXB0VXF1bzFaeVlmQllGUXc1WGMxaXMrK3h6eFYwenNh?=
 =?utf-8?B?RENiSHB3bjBXVnhtTHI5K3ByUXZBZ05qNkgzQUdtWGpFTXRxNWhVK0dZb1V2?=
 =?utf-8?B?aEZNYVpxTVdQVlFBcGx1Ui9PRzRKR2pkb2NXRjB2OHVLQzBCWGlISWw5dm9z?=
 =?utf-8?B?ZExOSmRxbnNqS01Vem43dzNYTDlyZXMyN3pEK2JHcnlnQjBSRmIvUzFJTXVz?=
 =?utf-8?B?TDdjUm5sTjRtdVMvZ3kxcDN6NTdGZVY4NmJXUWZEQjhtTWxhRityTERqQ3dP?=
 =?utf-8?B?QnNmL0JQMzJwTzl4a2xUa1NkdGVjWjYxM0VSUDYvUWd5S2FKa3NaWTJsdDBK?=
 =?utf-8?B?ZUVkdXF6U3JUVlJiZTBsK1ZENmQyTVVLRXBST0taOHQ1THgyRGZXM0tWbkQ1?=
 =?utf-8?B?aUdpUi9SUldLcHNJKzd2NlZnajhHVmpWcHE0UUpwcHRQOWEvZ2EwSnhOdU4r?=
 =?utf-8?B?MUpBeEtkZEN2dmdTWkRWQ2cvckVhL2IzMjYrTDBObWVNVjZ3YVhIUnEzRkhz?=
 =?utf-8?B?OWpiakVZNXB1NWE5U2hPckw0a1FleHdtaWk3YUc2WUVPYWhiRjRNNjhLZ3oy?=
 =?utf-8?B?dGRzSE5SZWV5T0hKdFBGdzdvSk9CSHpVZmRpZkNqMWNiOHBkZHp1WXg4ckpU?=
 =?utf-8?B?dE15VVRjY0VSMzRZd2hGUnBJYWxxT1FENnp5T0NtTmpyUlNjckJwajhzY3Rm?=
 =?utf-8?B?UFFxWWRyMTRjR05rZG1LdFFpUnQ5WjVpV1lNbHRqZFIzR3pGamVTVnhFSjE5?=
 =?utf-8?B?dmFLanZGQ1BObjdoTENFR2tSNjRQMUlSSE9NNkIxV2hSL3hHTHJjTTBDUS9M?=
 =?utf-8?B?UFIvTUZQTXpVRk1zUC9BUHpxQ2lqSEIyMUdNTWd4UXQ3TkdEWXhRdEJZTWlC?=
 =?utf-8?B?VC84a3FLbnYxRlNVQzJtRHpTZmhPWVlOdFZ0NGo0LzVkdmI1RGlBNTBxY3d0?=
 =?utf-8?B?cnFKVEtiNW9qK0JHd0hMM1lWTGNkQlVuWU1ic2Q0bnRmOG5sYlkvTUZ5YTJB?=
 =?utf-8?B?LzZCTUJ0b0g3ZnVCd1Z4aVFXV3BiZW9jdExHdXI1ZDdhUC9QblJBR0pXdE5z?=
 =?utf-8?B?ZlFYVE1RUXRNM01hVHFxMDhKckx2YUlJSXY4Y0s2aDY1cTNPb2o0VkFjRlM0?=
 =?utf-8?B?cnh4OERpOWp5Ti9PRlJaMHlPaHFXN3lWSGQxc21kUzkzS2hWU2t1RDRBMDlO?=
 =?utf-8?B?a2ZyWjY3REZ0SFM5UklKdFRGLzdnYnNxK01MY1lVbXlETkxQT2JQdFRVODJk?=
 =?utf-8?B?aVUvOWZlN3RxMXYyNlY0LzFjSlRXY1FuUzNNMkdBYVR3WThLYU9PdUUzWjhR?=
 =?utf-8?B?VnVya0o4V1M3cm9UOHExcXRtTXJmSFBZaC9XdVBCYU8rWVFWUkFNVEFzMWpC?=
 =?utf-8?B?SkNVSVJyMU81MG1BRHJ4VU9zZDRsVTRLaXRwbDBpcUZNbDc2RjdnQk9Fb1k0?=
 =?utf-8?B?NzRYeTJOOEtMODhNajNjTXppTjZMQVN0dmNhYkxjbVhSenNCRktvdDlxS3po?=
 =?utf-8?B?VEZ4b0EzQWVBRENBR3RXaWcvejdqWlA5RFkzT3VmUW5TUnIvRHJuckozNGs0?=
 =?utf-8?B?dkJ2bjVrQzFlUFJ4aWwrdnd6dUgyenNRVkcwQStzcUJkWlhnSE5VRmhDT2ll?=
 =?utf-8?B?dTFndnBpeEdvSjhFcTZkRGxycDJNVWNrbWtOQlJHRGE1c1I5QXIzZ2dRYW5Z?=
 =?utf-8?B?TVkvOTFMMGg3N2JzOERRZm41akxDSjdUdzNGbVFFZGV0dXdsRkhubW1HSmNt?=
 =?utf-8?B?eFJBaytBNmpncFhERHhFWU5BNlNLbFc1WEx5c29GTHpLdzVVamNlcWhhNkhj?=
 =?utf-8?Q?JeXUfMSkn7+6biiRkbD0p4j7w?=
X-MS-Exchange-CrossTenant-Network-Message-Id: fe201bad-89d1-491d-b7b0-08dc586e7749
X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7529.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Apr 2024 08:24:25.4054
 (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: uTH0l6UEaBiUQhop8firFLqQpRGEfgDPYGUNivEd+C6ZJO3oItde4Mm97ZajjtdpUKOYaMTOBpIpqwKOWUTCvw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB6345
X-OriginatorOrg: intel.com

On 2024/4/7 22:42, Lu Baolu wrote:
> iommu_flush_iotlb_psi() is called in map and unmap paths. The caching
> mode check before device TLB invalidation will cause device TLB
> invalidation always issued if IOMMU is not running in the caching mode.
> This is inefficient and causes performance overhead.
> 
> Make device TLB invalidation behavior consistent between batched mode
> unmapping and strict mode unmapping. Device TLB invalidation should only
> be requested in the unmap path if the IOMMU is not in caching mode.
> 
> Fixes: bf92df30df90 ("intel-iommu: Only avoid flushing device IOTLB for domain ID 0 in caching mode")
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>   drivers/iommu/intel/iommu.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 50eb9aed47cc..493b6a600394 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -1501,11 +1501,7 @@ static void iommu_flush_iotlb_psi(struct intel_iommu *iommu,
>   	else
>   		__iommu_flush_iotlb_psi(iommu, did, pfn, pages, ih);
>   
> -	/*
> -	 * In caching mode, changes of pages from non-present to present require
> -	 * flush. However, device IOTLB doesn't need to be flushed in this case.
> -	 */
> -	if (!cap_caching_mode(iommu->cap) || !map)
> +	if (!cap_caching_mode(iommu->cap) && !map)
>   		iommu_flush_dev_iotlb(domain, addr, mask);
>   }
>   

The existing code works but kind of confusing. The iommu_flush_iotlb_psi()
helper will be called in both the map and unmap path. But device-TLB only
needed to be flushed in the unmap path since there is no chance for
device ATC to have cache for a non-present mapping. And only when caching
mode is reported, then should the helper be called in the map path. So the
fact is if caching mode is 0, the @map should always be false. If caching
mode is 1, then @map can be either false or true. To be simpler, it should
be enough to check @map before flushing device-TLB. no matter caching mode
or not.

-- 
Regards,
Yi Liu