Message-ID: <156f9e28-e7d2-b443-1aa5-1f5447cb9bbd@intel.com>
Date:   Tue, 7 Mar 2023 08:33:07 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.11.0
Subject: Re: [PATCH v2 08/16] iommu: define and export
 iommu_access_remote_vm()
Content-Language: en-US
To:     Jean-Philippe Brucker <jean-philippe@linaro.org>,
        Jacob Pan <jacob.jun.pan@linux.intel.com>
CC:     Vinod Koul <vkoul@kernel.org>, Dave Jiang <dave.jiang@intel.com>,
        <dmaengine@vger.kernel.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Alistair Popple <apopple@nvidia.com>,
        Joerg Roedel <joro@8bytes.org>,
        "Will Deacon" <will@kernel.org>,
        Robin Murphy <robin.murphy@arm.com>,
        "Lorenzo Stoakes" <lstoakes@gmail.com>,
        Christoph Hellwig <hch@infradead.org>, <iommu@lists.linux.dev>
References: <20230306163138.587484-1-fenghua.yu@intel.com>
 <20230306163138.587484-9-fenghua.yu@intel.com>
 <20230307084058.GA1330745@myrica>
From:   Fenghua Yu <fenghua.yu@intel.com>
In-Reply-To: <20230307084058.GA1330745@myrica>
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?VmVlL2FkR0p4Ky9DK0djak94Z3pIQkVzaXdyM2cyOGVoTHE4dEZNQ1ZINUh3?=
 =?utf-8?B?R1hnV2txQ3dEZ0NuK2tLSVNHWlJNUzV2VWk2VE94MGtYaUI0TDJlekhrT1JU?=
 =?utf-8?B?c2p3SkV2RGtYU0JoVWZPTkFKeXV4SjNwN3RXWkNxUHY1VEhpblg4RVJhcFBr?=
 =?utf-8?B?Q0hnSE8vQ2hXOVRLUnFkRjRTc0hjQlk1eWk0cmZCS3hHekFnMnh2dlZUTVV5?=
 =?utf-8?B?cUJ5MGNuT2I5OWFpM2Jud0YzcEdjdHp2MC9IWDNOQml0MDUxSzZNckpEcGZH?=
 =?utf-8?B?RkFlVWFMT3JDRUI1MjdVMG1yWUI5YmFGM0kvems0RU5rWlhBUGxVMER4Q0pD?=
 =?utf-8?B?dDRlVE5WUGg3VFZGUXhHdlRKODNCS0NsMkM4bm1BaWIydFBOU3hMY2NrY1cx?=
 =?utf-8?B?cXN5aTZsY0Z5YUtIazlWQjFtY3RmUmtqTjJCYmtPYWlEL0I4Q0tFc2tuYzJR?=
 =?utf-8?B?UWZHeHU1bkZnWDBNQ0ppSFl4NFM1eVNjdCtDa3FiWE9mWFl2VVhLMnhmdkJm?=
 =?utf-8?B?c3NzMmp1S1dKUUU0Rng5bGxDMWFQUXdVbkJFeGg3SHFvdWlsbjFHMHdmNkgw?=
 =?utf-8?B?MGEzOUtYbnQ2RXlGMDFoU2JScWtCWmYyQURsOVlkV1FzZG9adHJ4SExvM0Nh?=
 =?utf-8?B?QzJYNTFSbnVpa0pLV0djTUh2K3NaelVIQ0I4OFlxTGlkeGFjdm9IcE5lOVVi?=
 =?utf-8?B?Vkd4Z3ljZnRMS21EbEFFa1J6amxaZEhqa1BFYmtQUVY4YkZYRkh4ai90aVlz?=
 =?utf-8?B?TktaRkZDTkR0TkdpeGNNNjFVLzNkWVhWTFZIY3NBZktXeUVxTktpNlZDRElU?=
 =?utf-8?B?cmpwbm5kUHdZZy9FdFVKRVR6KzdaYUNSVXY0amxSZTJYa01ocnpsdHpQeUJR?=
 =?utf-8?B?MjUzMjlLbGhkbmVRRDAyanhEN2lkaGhRUy9VZUhONXhUZVlRWENMTzFlcWhB?=
 =?utf-8?B?T1ozSjA1a1l0eko4U1NjUXZ3dzNuRXMyMXpkQzU5a2UwUmZDcGhaYTNWSzkw?=
 =?utf-8?B?QjA2OFduMGNqS1hwZFh5SzBpaXpTTmhuRUp1LzVPSm5RQnNNQXNoNXBXZVpO?=
 =?utf-8?B?eFFtYnoyenA0QmZxcnE4QWl2UGNYUzF4VVRhNmRUVFBBdndEZ0ozMWxmYnRt?=
 =?utf-8?B?L1puM0ExYmFRa2N1S1JvYVR3L0ZuNG4xaGZoLzNHdjRONHZhQWE2L1ZKVkxt?=
 =?utf-8?B?VCsvcmxNaHpuckp0MWdGTGM3UTBiOEJpeDBRYXRqN3JVclFOR0RoM3p4Uyts?=
 =?utf-8?B?K0JTaUZTMlRUdzF4L2xtb0k5bnVVYXhtMW55QWJjbVdpVlFFNXZWVDV2QTJ3?=
 =?utf-8?B?eGZoVjJCNFVVVXFPYm1MeHJZdTNHQVFPNm9jQzNxK1lmaUxiMmpYUnl0K3Qv?=
 =?utf-8?B?NkNyN1RrVkJRZm9TMGlTTTM4eDl2b3dJUlpMcENJblVpZk9JRVMxZW1teDFl?=
 =?utf-8?B?a3U5SEhXMmhxdXZaVVFPSTUwMG1ibDcxVVFlNUVTbWUwVURiY3NlQTZWK0tL?=
 =?utf-8?B?NEMxdW5uWjNqT1JJUDU3TDFtakRlMVdxVE9DWWdQRTAwcUtTS2JmMHJWeVY3?=
 =?utf-8?B?YjZPNmwxWXV2ektyWFovL3dUUnBYelhOa05JaFNSOUV4bm9kOVFDU3V6eklt?=
 =?utf-8?B?MUlmVUd3TFZxaElZYmtUdWowMDdWRGZYTHhSSURLT1crSFJhTElZZk5VenZJ?=
 =?utf-8?B?ZEdiQlJPYjF6V0pvaDFCZzdvWVlQeHFVMFFFaXFpdDliL2FlSXEwSUg0T1c0?=
 =?utf-8?B?aWt1QjQvelhVVE5OU3Irb2JOdGhYYWJJNGV4S2V3ZlpGS3p1NS9LWS90b1Bt?=
 =?utf-8?B?WVpobGUvYkhna0ZGNWNYVFNjT0RmclhMNXg0NDRuaDhnQTV0UUEzblNnQWtV?=
 =?utf-8?B?bktXZEYySDJ1NndidmRUaklycFl5bGV0RVhmcXlpNkQrTVlRanZaT2x2TWVp?=
 =?utf-8?B?UWhFLytxZXVFMmpZeGFUYVkzQmREVS9LZGxNTEFMNlRrVUdydjVDdGVEYzRm?=
 =?utf-8?B?Y2tlNllQYUFFKzU1ZWtTQ3lWVFJYMzJSM2h1SzNHTDI0ZHV5T2ZLMGFWKzhC?=
 =?utf-8?B?VVBJRE1IcnRrbmpSQXlnQ1lxRjhPZHRyZWIwcEpVcnZkWHpDYlJYR1pvUE45?=
 =?utf-8?Q?/sIyrIJ6nnKeFk5SUz07HlAcZ?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 86c9d617-f33d-495a-452d-08db1f29a475
X-MS-Exchange-CrossTenant-AuthSource: IA1PR11MB6097.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Mar 2023 16:33:11.7575
 (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: TiUH7zi2/D8iJO8/jb8R+3+yy3p1lw55kWRVyRAssyrjlKMSRIFEtCsmKZFN0SfrC8QMcRUJGdDLFSDdnCRVkw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ1PR11MB6202
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi, Jean,

On 3/7/23 00:40, Jean-Philippe Brucker wrote:
> Hi Fenghua,
> 
> On Mon, Mar 06, 2023 at 08:31:30AM -0800, Fenghua Yu wrote:
>> Define and export iommu_access_remote_vm() to allow IOMMU related
>> drivers to access user address space by PASID.
>>
>> The IDXD driver would like to use it to write the user's completion
>> record that the hardware device is not able to write to due to user
>> page fault.
>>
>> Without the API, it's complex for IDXD driver to copy completion record
>> to a process' fault address for two reasons:
>> 1. access_remote_vm() is not exported and shouldn't be exported for
>>     drivers because drivers may easily cause mm reference issue.
>> 2. user frees fault address pages to trigger fault by IDXD device.
>>
>> The driver has to call iommu_sva_find(), kthread_use_mm(), re-implement
>> majority of access_remote_vm() etc to access remote vm.
>>
>> This IOMMU specific API hides these details and provides a clean interface
>> for idxd driver and potentially other IOMMU related drivers.
>>
>> Suggested-by: Alistair Popple <apopple@nvidia.com>
>> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
>> Cc: Joerg Roedel <joro@8bytes.org>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Robin Murphy <robin.murphy@arm.com>
>> Cc: Alistair Popple <apopple@nvidia.com>
>> Cc: Lorenzo Stoakes <lstoakes@gmail.com>
>> Cc: Christoph Hellwig <hch@infradead.org>
>> Cc: iommu@lists.linux.dev
>> ---
>> v2:
>> - Define and export iommu_access_remote_vm() for IDXD driver to write
>>    completion record to user address space. This change removes
>>    patch 8 and 9 in v1 (Alistair Popple)
>>
>>   drivers/iommu/iommu-sva.c | 35 +++++++++++++++++++++++++++++++++++
>>   include/linux/iommu.h     |  9 +++++++++
>>   2 files changed, 44 insertions(+)
>>
>> diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
>> index 24bf9b2b58aa..1d7a0aee58f7 100644
>> --- a/drivers/iommu/iommu-sva.c
>> +++ b/drivers/iommu/iommu-sva.c
>> @@ -71,6 +71,41 @@ struct mm_struct *iommu_sva_find(ioasid_t pasid)
>>   }
>>   EXPORT_SYMBOL_GPL(iommu_sva_find);
>>   
>> +/**
>> + * iommu_access_remote_vm - access another process' address space by PASID
>> + * @pasid:	Process Address Space ID assigned to the mm
>> + * @addr:	start address to access
>> + * @buf:	source or destination buffer
>> + * @len:	number of bytes to transfer
>> + * @gup_flags:	flags modifying lookup behaviour
>> + *
>> + * Another process' address space is found by PASID. A reference on @mm
>> + * is taken and released inside the function.
>> + *
>> + * Return: number of bytes copied from source to destination.
>> + */
>> +int iommu_access_remote_vm(ioasid_t pasid, unsigned long addr, void *buf,
>> +			   int len, unsigned int gup_flags)
>> +{
>> +	struct mm_struct *mm;
>> +	int copied;
>> +
>> +	mm = iommu_sva_find(pasid);
> 
> The ability to find a mm by PASID is being removed, see
> https://lore.kernel.org/linux-iommu/20230301235646.2692846-4-jacob.jun.pan@linux.intel.com/
> 

Thank you very much for pointing out this.

I talked to Jacob just now. He will keep iommu_sva_find() function
in his next version because this patch is still using the function. He 
agrees that I can still call iommu_sva_find() in this patch.

-Fenghua