From mboxrd@z Thu Jan  1 00:00:00 1970
From: Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [v3 4/5] vfio: implement APIs to set/put kvm to/from vfio group
Date: Wed, 9 Nov 2016 14:31:01 +0100
Message-ID: <02c27131-80fd-b882-1838-fe56f3f871d3@redhat.com>
References: <1477895706-22824-1-git-send-email-jike.song@intel.com>
 <1477895706-22824-5-git-send-email-jike.song@intel.com>
 <20161107110412.5db26fd4@t450s.home>
 <c94565d2-555d-fdd8-1131-4e7a316e2e4d@redhat.com>
 <20161107112834.2aa971df@t450s.home>
 <9e83a26f-e5dc-ed91-a1b5-c6f165eed7ed@redhat.com>
 <58231B5C.3010506@intel.com>
 <3e5299bc-e08e-f072-1001-7e0432cb1ca8@linux.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Cc: Alex Williamson <alex.williamson@redhat.com>, kwankhede@nvidia.com,
        cjia@nvidia.com, kevin.tian@intel.com, kvm@vger.kernel.org
To: Xiao Guangrong <guangrong.xiao@linux.intel.com>,
        Jike Song <jike.song@intel.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:40464 "EHLO mx1.redhat.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1752395AbcKINbG (ORCPT <rfc822;kvm@vger.kernel.org>);
        Wed, 9 Nov 2016 08:31:06 -0500
In-Reply-To: <3e5299bc-e08e-f072-1001-7e0432cb1ca8@linux.intel.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>



On 09/11/2016 14:06, Xiao Guangrong wrote:
> 
> 
> On 11/09/2016 08:49 PM, Jike Song wrote:
> 
>> +void vfio_group_attach_kvm(struct vfio_group *group, struct kvm *kvm,
>> +        void (*fn)(struct kvm *))
>> +{
>> +    mutex_lock(&group->udata.lock);
> 
> This lock is needed, please see below.

*not* needed I guess.

>> +
>> +    fn(kvm);
>> +    blocking_notifier_call_chain(&group->udata.notifier,
>> +                VFIO_GROUP_NOTIFY_ATTACH_KVM, kvm);
> 
> As this is a callback before KVM releases its last refcount, i do not
> think vendor driver need to get additional KVM refcount.

The *group* driver doesn't need it indeed.  The mdev vendor driver
however does, so it will use kvm_get_kvm under its own mutex.  That is:

- attach kvm

	mutex_lock(mdev_driver->lock);
	mdev_driver->kvm = opaque;
	kvm_get_kvm(mdev_driver->kvm);
	mutex_unlock(mdev_driver->lock);

- detach kvm

	mutex_lock(mdev_driver->lock);
	kvm_put_kvm(mdev_driver->kvm);
	WARN_ON(mdev_driver->kvm != opaque);
	mdev_driver->kvm = NULL;
	mutex_unlock(mdev_driver->lock);

- use kvm

	mutex_lock(mdev_driver->lock);
	kvm = mdev_driver->kvm;
	...
	mutex_unlock(mdev_driver->lock);

or if safe:

	mutex_lock(mdev_driver->lock);
	kvm = mdev_driver->kvm;
	kvm_get_kvm(kvm);
	mutex_unlock(mdev_driver->lock);
	...
	kvm_put_kvm(kvm);

Thanks,

Paolo