Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
Message-ID: <a160dab7-52ea-4baa-b668-ff69ab7c8e6b@intel.com>
Date:   Fri, 22 Sep 2023 10:07:10 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.15.1
Subject: Re: [PATCH v4 3/6] iommu: Introduce mm_get_pasid() helper function
Content-Language: en-US
To:     Jason Gunthorpe <jgg@ziepe.ca>
CC:     Kevin Tian <kevin.tian@intel.com>,
        Lu Baolu <baolu.lu@linux.intel.com>,
        Michael Shavit <mshavit@google.com>,
        Vasant Hegde <vasant.hegde@amd.com>, <iommu@lists.linux.dev>,
        <linux-kernel@vger.kernel.org>
References: <20230912125936.722348-1-tina.zhang@intel.com>
 <20230912125936.722348-4-tina.zhang@intel.com>
 <20230921190259.GZ13795@ziepe.ca>
From:   Tina Zhang <tina.zhang@intel.com>
In-Reply-To: <20230921190259.GZ13795@ziepe.ca>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?czdRWUFTNU9ITlZFaWwrQlE1bXE0TUZpOGYxcG1CUmRVQ3VnUm9rbXdwTVFz?=
 =?utf-8?B?ZUFkY2tCUVp1S3V6aGo1NEZ3aFd3L0hXZDZaMUZ3bTMvdmhXNEdOam1NTHZr?=
 =?utf-8?B?T0pyd2d1Y0tpU2hub3hvU3BmdVFxY0UzSVI4a2NFVmJQWHpDZSs4bkVmWndU?=
 =?utf-8?B?eVVnc0JueVNlMWFJaVEva2hDV0ZsblVYQ2dPSm51S1NOQTJjZXNmZDFGd2Mr?=
 =?utf-8?B?NkhONVU5cFY0Y3RPZnc4Zk5TQlhJNDNQYXFkQng1bkNsSHpoZzU0TENiTXB5?=
 =?utf-8?B?OGJsbE9TOGVHYS93b3B3UW1icEMycGl6cncvdzVxdkswVTBPL0htMW5JSGw3?=
 =?utf-8?B?UEJRYjN3ZWk2dDdtRHdYN0J2czVvRFNuaEk2dktRd2puUkxtU21rYlR4bkVh?=
 =?utf-8?B?cGRGajVPb0NSNFR2VThEMmxMaTh0L0VqbHdOU0g2NlhRaGkwbWtEaVRXbFln?=
 =?utf-8?B?QWdwN1l4MHRQc1F2U3ZqQlNRNXJVVmlOT1hJNVNtY21xbUZSczFCTU42b3VH?=
 =?utf-8?B?c1FVQ0grcnlzRmMxM3JQZHVEb0lKMWdKUTVtYnRoNlg1T3NKc3VqQzg1TXJr?=
 =?utf-8?B?WEp5ckUydmp2RDFydEhuZ1JTemhYSUMvb29EOVU0Wi9vU0xFa2JYUnFnS1V1?=
 =?utf-8?B?SmxMQ0NSdzVBanRta0VLek9La3BPaDIyUmRCMGhKYXhDVFVDaldlU3ZodnVH?=
 =?utf-8?B?Yi9UZ0xFd3lUWkNNbHRZTzZhdk1UNW9pQWxmMnpEMFBGbW90OXV5UndFcHRT?=
 =?utf-8?B?R0VvOHg5Uk91MW5NZ05CUVNsbnVhanBObEl0aENBZVBBRnQxdlNqMXRmWWxS?=
 =?utf-8?B?MkF5Sm5ucVdsYTJZREpoZmFST2orQlpOa0EvODBOWGFrc3cvNFVlWlNjblYx?=
 =?utf-8?B?OS9McGpoaHZlZVZwd0JUd0pMV0FETFQyUHI0cmRNemhjaGVNaTRTa2Q3enZC?=
 =?utf-8?B?VWVsdXNmSzFsOElBa3ZROVh5T1hsMnUxQUkyQ1FoODhVV0NSanowVURmQXln?=
 =?utf-8?B?MEp0aXJoelp3U1BGNmlvSXZhWFMrc3pxVnN4RUxwZ0MvRTZTQjZGUXJ1dGdM?=
 =?utf-8?B?a1ZQVHFDTHE5YXQvc3d6eEN0Y0NVTDRibkgyRXFCcS9rVkZUcXlidnlsQ0Rq?=
 =?utf-8?B?RWRHVXpzY3dPYjgzejI0NTMyOHJVYWFRT3BMeGp5MEp1TnVGRDFYZkZWbW9P?=
 =?utf-8?B?SVg2RkY4b29wWnc5Q3BJTzhsWlMvOXVudVBpRjF2VFQrZWhmQUdBT3I3YUNk?=
 =?utf-8?B?LzdaT2d5NlpLSS90cUovL2Q4SmRKRkxOU2JGbnU1M3BEaUFHMit4TmdNR29x?=
 =?utf-8?B?aG9SbTA3MktnTVgzZTJ2ZGdGa1pJSFIvOGgyUitISy9NeDVmOU5FUHdPWmJZ?=
 =?utf-8?B?d1hDS1M4b2wyZkpsdFJVMW5iUHh6Q0tGQi9NYnI3MXFscVBpN0hWZk42alJH?=
 =?utf-8?B?a1E4OHVNdmV2V3VpTVR0ckxVYVA3S2NEZ051TFdrRit5Uk4vd1lsaEpPcUFq?=
 =?utf-8?B?UjllTWJ4bHFXTEpWSGlzejlrQmNEdWdlSkJUODcwTCtnNTkwUitpbzhSYWln?=
 =?utf-8?B?NFNRNm12ckhldjlZRndLTldWNUwzbS9vVlhicm9sdm44OHdBNlBIeUV4RWc4?=
 =?utf-8?B?VFZvcG5Nanh5S2Z2RFU0L0s2bUtYVUE2Nmo3R0ZTOHNYQ0o2Q0xDbGwxWU0w?=
 =?utf-8?B?d01XVHNkQWlPeEtHa01HOFY5aXlYK29UZmo4Nm5lVlNCaUoya1ZtL05UQU5F?=
 =?utf-8?B?eFBVMUlOeS9HU0xycmVvbHIxT2RIdWtNWlVFaWk5c251VjQ4SGM0TTJPK1Qr?=
 =?utf-8?B?M0tGMGkrWWsyai9ubHU2WWtLS3hVU1UvRWRDOHpPTEd4M0NWbHljdGc0YTVD?=
 =?utf-8?B?ckpKR0U2bmxVdEZOeVRkMC9YUjU1YzNETXZvSTBCcVRsM1ZwdlRQZUVEbXpT?=
 =?utf-8?B?U1dMUmdNZmthV3dpNk85Vm4xbkwrSytCNnFoa2NHc3NrUGdldElrMlhKR2Zv?=
 =?utf-8?B?KzM2RmloeWFWeTdJc213V05BenJrMWtFYUEyNGdIb2FxVEtnNHJEMTVvUkQ2?=
 =?utf-8?B?ZjQ0SDk3S3JURVBXb2tEaXJXVDluZXdibnNyMXRFRTRYNFgrQnU1MjNXc2l2?=
 =?utf-8?Q?8REV7Dv/PKFiDVpPd2xAgzwzi?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 32d7abde-6b28-410a-e376-08dbbb10ac1d
X-MS-Exchange-CrossTenant-AuthSource: MW5PR11MB5881.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Sep 2023 02:07:28.6978
 (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: loKeNqYomXBqDjaGrfyRjqq83o2dOWUST1izDsDeHVuxOzbRdOBD3L20VDicHxihk9q72XtjkqV1Havygsyd/A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB8084
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.3 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Thu, 21 Sep 2023 19:07:44 -0700 (PDT)


On 9/22/23 03:02, Jason Gunthorpe wrote:
> On Tue, Sep 12, 2023 at 08:59:33PM +0800, Tina Zhang wrote:
>> Use the helper function mm_get_pasid() to get a mm assigned pasid
>> value. The motivation is to replace mm->pasid with an iommu private
>> data structure that is introduced in a later patch.
>>
>> Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
>> Signed-off-by: Tina Zhang <tina.zhang@intel.com>
>> ---
>>
>> Changes in v4:
>> - Rebase to v6.6-rc1.
>>
>> Changes in v2:
>> - Update commit message.
>> - Let mm_get_enqcmd_pasid() call mm_get_pasid() to get pasid.
>>
>>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 12 ++++++------
>>   drivers/iommu/iommu-sva.c                       | 12 ++++++------
>>   include/linux/iommu.h                           | 10 +++++++++-
>>   3 files changed, 21 insertions(+), 13 deletions(-)
> 
> Michael already has a series to remove this mis-use from the ARM
> driver like you did for the Intel driver.
> 
> I think for the interm it is fine to just use mm_get_enqcmd_pasid()
> and Michael can remove it.
> 
> It helps point out this is just wrong anyhow and better discourages
> anyone else from using it.
Sure, I'll update the commit message.

Thanks,
-Tina
> 
>> diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
>> index b78671a8a914..0f956ecd0c9b 100644
>> --- a/drivers/iommu/iommu-sva.c
>> +++ b/drivers/iommu/iommu-sva.c
>> @@ -23,7 +23,7 @@ static int iommu_sva_alloc_pasid(struct mm_struct *mm, struct device *dev)
>>   	mutex_lock(&iommu_sva_lock);
>>   	/* Is a PASID already associated with this mm? */
>>   	if (mm_valid_pasid(mm)) {
>> -		if (mm->pasid >= dev->iommu->max_pasids)
>> +		if (mm_get_pasid(mm) >= dev->iommu->max_pasids)
>>   			ret = -EOVERFLOW;
>>
>>   		goto out;
>>   	}
>>
>> @@ -73,7 +73,7 @@ struct iommu_sva *iommu_sva_bind_device(struct device *dev, struct mm_struct *mm
>>   
>>   	mutex_lock(&iommu_sva_lock);
>>   	/* Search for an existing domain. */
>> -	domain = iommu_get_domain_for_dev_pasid(dev, mm->pasid,
>> +	domain = iommu_get_domain_for_dev_pasid(dev, mm_get_pasid(mm),
>>   						IOMMU_DOMAIN_SVA);
>>   	if (IS_ERR(domain)) {
>>   		ret = PTR_ERR(domain);
>> @@ -92,7 +92,7 @@ struct iommu_sva *iommu_sva_bind_device(struct device *dev, struct mm_struct *mm
>>   		goto out_unlock;
>>   	}
>>   
>> -	ret = iommu_attach_device_pasid(domain, dev, mm->pasid);
>> +	ret = iommu_attach_device_pasid(domain, dev, mm_get_pasid(mm));
>>   	if (ret)
>>   		goto out_free_domain;
>>   	domain->users = 1;
> 
> These ones are all manipulating the enqcmd_pasid really..  If it
> wasn't for enqcmd then we'd simply make each device have its own PASID
> and then we wouldn't have the issue that a valid pasid is incompatible
> with a certain device.
> 
> Jason