On 10/26/2016 10:45 PM, Paolo Bonzini wrote:
> On 26/10/2016 15:44, Jike Song wrote:
>> On 10/21/2016 01:06 AM, Paolo Bonzini wrote:
>>> On 20/10/2016 03:48, Xiao Guangrong wrote:
>>>> 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.
>>
>> Hi Paolo & Guangrong,
>>
>> I walked the whole thread and became a little nervous: I don't want
>> to introduce a global mutex.
>>
>> The problem is, as I understand, vfio_group_get_usrdata() returns a
>> KVM pointer but it may be stale. To make the pointer always valid,
>> it can call kvm_get_kvm() *before* return the pointer.
>
> That doesn't work, you still have to protect get against concurrent set.
> But the mutex need not be global, it is specific to the vfio device.
> You probably have such a mutex anyway...
Thanks Paolo, I agree whatsoever a mutex is necessary. I cooked a patch
sent to you and Alex, please kindly have a look :-)
--
Thanks,
Jike
>> I would apologize in advance if this idea turns out totally
>> nonsense, but hey, please kindly help fix my whim :-)
>>
>>
>> [vfio.h]
>>
>> struct vfio_usrdata {
>> void *data;
>> void (*get)(void *data);
>> void (*put)(void *data)
>> };
>>
>> vfio_group {
>> ...
>> vfio_usrdata *usrdata;
>>
>> [kvm.ko]
>>
>> struvt vfio_usrdata kvmdata = {
>> .data = kvm,
>> .get = kvm_get_kvm,
>> .put = kvm_put_kvm,
>> };
>>
>> fn = symbol_get(vfio_group_set_usrdata)
>> fn(vfio_group, &kvmdata)
>>
>>
>> [vfio.ko]
>>
>> vfio_group_set_usrdata
>> lock
>> vfio_group->d = kvmdata
>> unlock
>>
>> void *vfio_group_get_usrdata
>> lock
>> struct vfio_usrdata *d = vfio_group->usrdata;
>> d->get(d->data);
>> unlock
>> return d->data;
>>
>> void vfio_group_put_usrdata
>> lock
>> struct vfio_usrdata *d = vfio_group->usrdata;
>> d->put(d->data)
>> unlock
>>
>> [kvmgt.ko]
>>
>> call vfio_group_get_usrdata to get kvm,
>> call vfio_group_put_usrdata to release it
>> *never* call kvm_get_kvm/kvm_put_kvm
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
