Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <6d57350a-6d07-42d7-2b1d-153bf5847b0f@amd.com>
Date:   Tue, 17 Jan 2023 12:31:07 +0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: [PATCH 3/4] iommu: Introduce IOMMU call-back for processing
 struct KVM assigned to VFIO
Content-Language: en-US
To:     Jason Gunthorpe <jgg@ziepe.ca>
Cc:     linux-kernel@vger.kernel.org, iommu@lists.linux.dev,
        joro@8bytes.org, robin.murphy@arm.com, ashish.kalra@amd.com,
        thomas.lendacky@amd.com, vasant.hegde@amd.com, jon.grimm@amd.com
References: <20230110143137.54517-1-suravee.suthikulpanit@amd.com>
 <20230110143137.54517-4-suravee.suthikulpanit@amd.com>
 <Y8F6Uoi73BK6R+mc@ziepe.ca>
From:   "Suthikulpanit, Suravee" <suravee.suthikulpanit@amd.com>
In-Reply-To: <Y8F6Uoi73BK6R+mc@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?ZFNWejZ6OW9WYWJodmdBaEVocnFPU3dETDZ1bDVmd1FHRzk3di9UQytDc1Nk?=
 =?utf-8?B?UFBtZUJGbVdwc1drOG04Ym5Xb2R2YmFIK2pjcm1lTTFRV1k3YStaMlhSbEd3?=
 =?utf-8?B?L2tSa0FYcmpXLytyMGVZY3l6OHNQcjhxeXV0Vjg2bkNHQm9SeVAyMDYxRHhD?=
 =?utf-8?B?d2dqWDVwdk1SSGZRVkJ2MmJRSHhkU1M0OW5NYVV2RGpjcnRiSFN2U0xPekwv?=
 =?utf-8?B?TGZOajNSSk53d3IxMWNSMGJLVDljVm5lNFZnVHJiZjVkYTFFWGJsNEVFVEU0?=
 =?utf-8?B?ZDViMi9CQlZzcUVvU0k5ME5QYjhOWFdpYjNqOGlnaHJDQTZiejF2eEluaXJ0?=
 =?utf-8?B?SzRTSjljOHhPSEl1UTRYRGkrRDJTZlVJOC9Yd1pvbThHK1NvUjlPRnNWYjhy?=
 =?utf-8?B?YnBnRmd4N0tuZURZclNoSUROakVvVklNbzBYTFhMcXczS1NaSWE4bUVmQy9E?=
 =?utf-8?B?MkdTNGE5V3VEMFh0djV5SXRvVXBMbDdlR3VMa2ZVOUN1V0o1MkN0Y0pOK1RD?=
 =?utf-8?B?N3VuOXdQVWliQTBxZ1F4dFFIMi9abElsM0lKRVFkdkptVUpXZEFYMmJiVFEv?=
 =?utf-8?B?Ni9veVdSaE4vVXBoVENWbXJHVWdVL2VJZjZXT0JDKzdlUUNudURQR0NoZ01z?=
 =?utf-8?B?QUMrOEtOZGJQVllIRHgrb2pmbW1yODN0bkZLUDgzQ2k3SlNuNjVNaTkxU2Iz?=
 =?utf-8?B?NE1WZXQxeHJzV1FOQzVDd2ptc2g2VjJqbTRKQ0VRRU5IRVRRYThtSnBKYVZH?=
 =?utf-8?B?RWdKbk81Y0ttaFFMamxsb0Nta2Y1TE9nUGh5bnRuS0hzdEJDY3ZKaUdLQTdi?=
 =?utf-8?B?TFVnNjNuVTBwNVFPSm4vdVdoVkRaZk9QWm1wK0pUUHJHRFhsRjFGcW9aREx1?=
 =?utf-8?B?UzIzTnA1eHVmMmpyVE5uY2dxSCtWUmVYVnpxc3EyRVhVSzYrbno3WEY3RHcw?=
 =?utf-8?B?QVQvSnRYS0Y0RkxnMjJUcmFIdnNHQWYyaktpeXN3NWRHWkFOQkVyVWNrajdZ?=
 =?utf-8?B?NkMzTkhvVmhWR2k0VkVrLzVoMDBhbnUrd3U3aDhKL2oyYk5CSnFDYitNTjZa?=
 =?utf-8?B?ajFQeU9YRlZYUUNjNExOOG12aXJUMWgxc2J3YXRNc3UzUnB5NTBiS2RNUTNv?=
 =?utf-8?B?bFZRSDV3eFlvR2VRMHBKUytZTDgweGZJTFNKVFhlRDRhSW94Z3VsbjZCM25p?=
 =?utf-8?B?TUtPekxRdnVONGt4aEdaVnl4ZVRHNVhFSGlLL25uM0tzNHBDanI5OG1ESVVa?=
 =?utf-8?B?ZHI5Z3FYaVB1dUVaQitONkVLSWszdE02OWdhRHVPSEJuVEJ3UkRTRnZBbm9F?=
 =?utf-8?B?U3BIaUxuRFRpUEZTanF6MTRaYU4wdmN0WUhIdWpwRWtkUWl0amQ1Y0J6ZmZH?=
 =?utf-8?B?elRYejdhMno4dWVCeTMybVZWbUhacU9yTGhxYlVBNGtyYVoxNW03U2FkYmx1?=
 =?utf-8?B?ZjkrZjN1SXFrOU1BTzRlUGJVV21TWGtyZzlJdDEvT0p5TW1GR0FNNzYvUFd2?=
 =?utf-8?B?bEttWm9qMlcyVmFmYm9kRzZJM2drbW15WGl6cFJ2V1JtZFA5K2k3dDE4WDNE?=
 =?utf-8?B?U0xJMGZqYlpwN3RkL3FxM21CUmFoVzE5ekM2UFBURHQ1VUo5U0I4MTQwU0Ux?=
 =?utf-8?B?Rm91TzVYTGxUN0JnN0xjcTNnUHlFbDhwT2lMaTRiK1JIaFVJbGQyK3ZTYlQ0?=
 =?utf-8?B?Z3dBM3RlWHpjbDlFRGV4b3hpUDVTeGEwVHB3ZytFL0VLcE9PNmtWU2ZRSlR3?=
 =?utf-8?B?QVo5c3IxaDRkSEM4OHBiY2xUUFZFbEZEeWVld0NtaFBRWG4zaUdicEJISzli?=
 =?utf-8?B?Rkp1SXVnZEE1OGNzaXdGSjlsRFBQZ1hReE5hbml1d2tTMVJKazBxdEJmSVN5?=
 =?utf-8?B?ZENpQ2IzcFl4VEk0RGtvcDF4S3FkNmh5MUswY3N5TXRicWVYMkRnbk10U0Rr?=
 =?utf-8?B?T2tkTEgrOU5heFYxcXUvSHl6d1YvTDEwYTVvL0szT0h1S0EzZVNkSEw2Yjda?=
 =?utf-8?B?TFo0eGhvbE1sSnh3clVyK1JvTGR2TlhsTzFyb05wRjdMdFduVVlwbC9qdXAr?=
 =?utf-8?B?aVJCYVpMaStTcERubHUrTEVtYm5kZnN3UVEycVNvZ09GN1VORG5URnpCcmwr?=
 =?utf-8?Q?Cf+Xbf3tPzDKcw9bBupi4UURJ?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 381498eb-4fb4-4f54-6c40-08daf84c0ed2
X-MS-Exchange-CrossTenant-AuthSource: DM8PR12MB5445.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Jan 2023 05:31:17.9970
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: haACUWwN6EdV0GHP0CZWSQZ30Dt0LiTTAGl5tACSoUDbnZkrtLYWYiyNwhccK14/SedqyN8PN8KHOrCQHO1gtA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR12MB7489
X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Jason,

On 1/13/2023 10:35 PM, Jason Gunthorpe wrote:
> On Tue, Jan 10, 2023 at 08:31:36AM -0600, Suravee Suthikulpanit wrote:
>> Currently, VFIO provide an kvm_vfio_file_set_kvm() interface for assigning
>> a KVM structure to a VFIO group. The information in struct KVM is also
>> useful for IOMMU drivers when setting up VFIO domain.
>>
>> Introduce struct iommu_domain_ops.set_kvm call-back function to allow
>> IOMMU drivers to provide call-back to process the struct KVM
>> assigned.
> 
> Also NAK
> 
> Connecting the iommu driver to KVM has to be properly architected
> though iommufd.
> 

My understanding is the kvm_vfio_file_set_kvm() from the following 
call-path:

* kvm_vfio_group_add()
* kvm_vfio_group_del()
* kvm_vfio_destroy()

to attach/detach KVM to/from a particular VFIO domain. This is an 
existing interface from kvm_vfio_set_group()

Here is the call-path:

kvm_vfio_file_set_kvm()
	vfio_file_set_kvm()
		iommu_set_kvm() <-- New interface
			amd_iommu_set_kvm()

Could you please elaborate what you have in mind for a properly 
architected interface via iommufd?

>> @@ -1652,6 +1652,7 @@ void vfio_file_set_kvm(struct file *file, struct kvm *kvm)
>>   
>>   	mutex_lock(&group->group_lock);
>>   	group->kvm = kvm;
>> +	iommu_set_kvm(group->iommu_group, kvm);
>>   	mutex_unlock(&group->group_lock);
>>   }
> 
> This also has obvious lifetime bugs

Could you please also elaborate on this part? For detaching case, KVM is 
NULL, and the same information is passed to the IOMMU driver to handle 
the detaching case. Am I missing anything?

Thanks,
Suravee

> 
> Jason