linux-kernel - Re: [PATCH 3/4] iommu: Introduce IOMMU call-back for processing struct KVM assigned to VFIO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6d57350a-6d07-42d7-2b1d-153bf5847b0f@amd.com>
Date:   Tue, 17 Jan 2023 12:31:07 +0700
From:   "Suthikulpanit, Suravee" <suravee.suthikulpanit@....com>
To:     Jason Gunthorpe <jgg@...pe.ca>
Cc:     linux-kernel@...r.kernel.org, iommu@...ts.linux.dev,
        joro@...tes.org, robin.murphy@....com, ashish.kalra@....com,
        thomas.lendacky@....com, vasant.hegde@....com, jon.grimm@....com
Subject: Re: [PATCH 3/4] iommu: Introduce IOMMU call-back for processing
 struct KVM assigned to VFIO

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