[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: t
From: |
Xiao, Guangrong |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot |
Date: |
Thu, 20 Oct 2016 17:19:26 +0000 |
-----Original Message-----
From: Paolo Bonzini [mailto:address@hidden On Behalf Of Paolo Bonzini
Sent: Friday, October 21, 2016 1:07 AM
To: Xiao Guangrong <address@hidden>; Xiao, Guangrong <address@hidden>; Song,
Jike <address@hidden>; Alex Williamson <address@hidden>
Cc: Tian, Kevin <address@hidden>; Neo Jia <address@hidden>; address@hidden;
qemu-devel <address@hidden>; Chen, Xiaoguang <address@hidden>; Kirti Wankhede
<address@hidden>
Subject: Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type:
track_flush_slot
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.
Yes, mutex can fix it and is good to me. :)
- Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot, (continued)
- Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot, Alex Williamson, 2016/10/17
- Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot, Jike Song, 2016/10/18
- Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot, Alex Williamson, 2016/10/18
- Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot, Jike Song, 2016/10/18
- Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot, Xiao Guangrong, 2016/10/19
- Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot, Paolo Bonzini, 2016/10/19
- Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot, Xiao Guangrong, 2016/10/19
- Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot, Paolo Bonzini, 2016/10/19
- Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot, Xiao Guangrong, 2016/10/19
- Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot, Paolo Bonzini, 2016/10/20
- Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot,
Xiao, Guangrong <=
- Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot, Jike Song, 2016/10/20
- Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot, Jike Song, 2016/10/26
- Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot, Paolo Bonzini, 2016/10/26
- Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot, Jike Song, 2016/10/29
- Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot, Eric Blake, 2016/10/19
- Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot, Jike Song, 2016/10/24