From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34200) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bxGnz-0003ib-8F for qemu-devel@nongnu.org; Thu, 20 Oct 2016 13:07:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bxGnv-0001c7-9B for qemu-devel@nongnu.org; Thu, 20 Oct 2016 13:06:59 -0400 Received: from mail-lf0-x242.google.com ([2a00:1450:4010:c07::242]:34076) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1bxGnv-0001bY-0o for qemu-devel@nongnu.org; Thu, 20 Oct 2016 13:06:55 -0400 Received: by mail-lf0-x242.google.com with SMTP id x23so1062309lfi.1 for ; Thu, 20 Oct 2016 10:06:54 -0700 (PDT) Sender: Paolo Bonzini References: <1259cdba-c137-c3da-abe2-ecf51aec6738@linux.intel.com> <523e1446-75f1-fe3a-d818-f7d238d57751@redhat.com> <5800B579.9000705@intel.com> <20161014084158.623087aa@t450s.home> <20161014084601.2a50ba87@t450s.home> <20161014163545.GA6121@nvidia.com> <20161014105124.42b438a6@t450s.home> <20161014221901.GA8865@nvidia.com> <20161017100229.1474ae33@t450s.home> <580617BD.8000300@intel.com> <20161018085918.61ec0e93@t450s.home> <5806DB2D.6090306@intel.com> <2f04a53d-261c-7fb5-6825-117da6a1307d@intel.com> <06340187-61d8-ed7a-e40d-264ca3eb4b37@linux.intel.com> <6067bf7d-42ba-0ffd-5131-da74f60296d4@redhat.com> From: Paolo Bonzini Message-ID: Date: Thu, 20 Oct 2016 19:06:48 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Xiao Guangrong , Xiao Guangrong , Jike Song , Alex Williamson Cc: "Tian, Kevin" , Neo Jia , kvm@vger.kernel.org, qemu-devel , Xiaoguang Chen , Kirti Wankhede On 20/10/2016 03:48, Xiao Guangrong wrote: > > > On 10/19/2016 10:14 PM, Paolo Bonzini wrote: >> >> >> On 19/10/2016 15:39, Xiao Guangrong wrote: >>> >>> >>> On 10/19/2016 07:56 PM, Paolo Bonzini wrote: >>>> >>>> >>>> On 19/10/2016 07:45, Xiao Guangrong wrote: >>>>> >>>>> >>>>> On 10/19/2016 10:32 AM, Jike Song wrote: >>>>> +EXPORT_SYMBOL_GPL(vfio_group_set_usrdata); >>>>>>>> + >>>>>>>> +void *vfio_group_get_usrdata(struct vfio_group *group) >>>>>>>> +{ >>>>>>>> + return group->usrdata; >>>>>>>> +} >>>>>>>> +EXPORT_SYMBOL_GPL(vfio_group_get_usrdata); >>>>>>>> + >>>>>>>> +void *vfio_group_get_usrdata_by_device(struct device *dev) >>>>>>>> +{ >>>>>>>> + struct vfio_group *vfio_group; >>>>>>>> + >>>>>>>> + vfio_group = __vfio_group_get_from_iommu(dev->iommu_group); >>>>>>> >>>>>>> We actually need to use iommu_group_get() here. Kirti adds a >>>>>>> vfio_group_get_from_dev() in v9 03/12 that does this properly. >>>>>>> >>>>>>>> + if (!vfio_group) >>>>>>>> + return NULL; >>>>>>>> + >>>>>>>> + return vfio_group_get_usrdata(vfio_group); >>>>> >>>>> I am worrying if the kvm instance got from group->usrdata is safe >>>>> enough? What happens if you get the instance after kvm released >>>>> kvm-vfio device? >>>> >>>> It shouldn't happen if you use kvm_get_kvm and kvm_put_kvm >>>> properly. It >>>> is almost okay in the patch, just: >>> >>> How about if KVM releases kvm-vfio device between >>> vfio_group_get_usrdata() >>> and get_kvm()? >> >> That cannot happen as long as there is a struct file* for the device >> (see kvm_ioctl_create_device and kvm_device_release). Since you're >> sending a ioctl to it, it's fine. > > I understood that KVM side is safe, however, vfio side is independent with > kvm and the user of usrdata can fetch kvm struct at any time, consider > this scenario: > > CPU 0 CPU 1 > KVM: VFIO/userdata user > kvm_ioctl_create_device > get_kvm() > vfio_group_get_usrdata(vfio_group) > kvm_device_release > put_kvm() > !!! kvm refcount has gone > use KVM struct > > Then, the user of userdata have fetched kvm struct but the refcount has > already gone. vfio_group_set_usrdata (actually) kvm_vfio_group_set_kvm has called kvm_get_kvm too, however. What you need is a mutex that is taken by vfio_group_set_usrdata and by the callers of vfio_group_get_usrdata. Paolo > What i missed? > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >