On Wed, Nov 29, 2017 at 5:22 PM, David Hildenbrand [off-list ref] wrote:

On 29.11.2017 18:20, Paolo Bonzini wrote:

quoted

On 29/11/2017 18:17, David Hildenbrand wrote:

quoted

On 29.11.2017 17:41, Christoffer Dall wrote:

quoted

As we're about to call vcpu_load() from architecture-specific
implementations of the KVM vcpu ioctls, but yet we access data
structures protected by the vcpu->mutex in the generic code, factor
this logic out from vcpu_load().

Signed-off-by: Christoffer Dall <redacted>
---
 arch/x86/kvm/vmx.c       |  4 +---
 arch/x86/kvm/x86.c       | 20 +++++++-------------
 include/linux/kvm_host.h |  2 +-
 virt/kvm/kvm_main.c      | 17 ++++++-----------
 4 files changed, 15 insertions(+), 28 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 714a067..e7c46d2 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c

@@ -9559,10 +9559,8 @@ static void vmx_switch_vmcs(struct kvm_vcpu *vcpu, struct loaded_vmcs *vmcs)
 static void vmx_free_vcpu_nested(struct kvm_vcpu *vcpu)
 {
        struct vcpu_vmx *vmx = to_vmx(vcpu);
-       int r;

-       r = vcpu_load(vcpu);
-       BUG_ON(r);
+       vcpu_load(vcpu);

I am most likely missing something, why don't we have to take the lock
in these cases?

See earlier discussion, at these points there can be no concurrent
access; the file descriptor is not accessible yet, or is already gone.

Paolo

Thanks, this belongs into the patch description then.

Fair enough, I'll add that.

Thanks for having a look.

-Christoffer