From mboxrd@z Thu Jan  1 00:00:00 1970
From: Dmitry Vyukov <dvyukov@google.com>
Subject: Re: [PATCH] kvm: initialize SVM spinlock
Date: Mon, 23 Jan 2017 15:52:27 +0100
Message-ID: <CACT4Y+bdRx13Dg_D3so=5t=VQyJjiASL=EzCt7oaqzNC-WsLow@mail.gmail.com>
References: <20170122090453.88101-1-dvyukov@google.com> <f9533654-4dd8-faa6-1472-d0070fbb9ec1@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Cc: joro@8bytes.org, Paolo Bonzini <pbonzini@redhat.com>,
        =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= <rkrcmar@redhat.com>,
        KVM list <kvm@vger.kernel.org>,
        syzkaller <syzkaller@googlegroups.com>
To: David Hildenbrand <david@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-ua0-f177.google.com ([209.85.217.177]:32849 "EHLO
        mail-ua0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1751081AbdAWOwt (ORCPT <rfc822;kvm@vger.kernel.org>);
        Mon, 23 Jan 2017 09:52:49 -0500
Received: by mail-ua0-f177.google.com with SMTP id i68so111255436uad.0
        for <kvm@vger.kernel.org>; Mon, 23 Jan 2017 06:52:48 -0800 (PST)
In-Reply-To: <f9533654-4dd8-faa6-1472-d0070fbb9ec1@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Mon, Jan 23, 2017 at 3:25 PM, David Hildenbrand <david@redhat.com> wrote=
:
> Am 22.01.2017 um 10:04 schrieb Dmitry Vyukov:
>> Currently svm_vm_data_hash_lock is left uninitialized.
>> This causes lockdep warnings. Properly initialize it.
>>
>> Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
>> Cc: Joerg Roedel <joro@8bytes.org>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: "Radim Kr=C4=8Dm=C3=A1=C5=99" <rkrcmar@redhat.com>
>> Cc: kvm@vger.kernel.org
>> Cc: syzkaller@googlegroups.com
>> ---
>>  arch/x86/kvm/svm.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>> index 08a4d3ab3455..b928a9c34987 100644
>> --- a/arch/x86/kvm/svm.c
>> +++ b/arch/x86/kvm/svm.c
>> @@ -972,7 +972,7 @@ static void svm_disable_lbrv(struct vcpu_svm *svm)
>>   */
>>  #define SVM_VM_DATA_HASH_BITS        8
>>  DECLARE_HASHTABLE(svm_vm_data_hash, SVM_VM_DATA_HASH_BITS);
>> -static spinlock_t svm_vm_data_hash_lock;
>> +static DEFINE_SPINLOCK(svm_vm_data_hash_lock);
>>
>>  /* Note:
>>   * This function is called from IOMMU driver to notify
>>
>
> We have
>
> spin_lock_init(&svm_vm_data_hash_lock);
>
> in svm_hardware_setup().
>
> If this isn't called, wouldn't the right fix be to find out why?


spin_lock_init is called conditionally if avic.
Then avic_vm_init returns 0, if !avic.
But then avic_vm_destroy does not check avic and unconditionally uses
the spinlock.
Perhaps the right fix then will be:

@@ -1382,6 +1383,9 @@ static void avic_vm_destroy(struct kvm *kvm)
        unsigned long flags;
        struct kvm_arch *vm_data =3D &kvm->arch;

+       if (!avic)
+               return 0;
+
        avic_free_vm_id(vm_data->avic_vm_id);

        if (vm_data->avic_logical_id_table_page)


Unfortunately I don't remember how I managed to trigger this warning
because I don't have any SVM-capable hardware...
But I remember that I did not do anything special besides just
enabling spinlock checks and then doing something trivial.
Could somebody try to use SVM with the spinlock checks enabled? I
don't feel comfortable sending a non-trivial patch without testing
it...