On Mon, Jan 23, 2017 at 3:25 PM, David Hildenbrand <david@xxxxxxxxxx> 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@xxxxxxxxxx>
>> Cc: Joerg Roedel <joro@xxxxxxxxxx>
>> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
>> Cc: "Radim Krčmář" <rkrcmar@xxxxxxxxxx>
>> Cc: kvm@xxxxxxxxxxxxxxx
>> Cc: syzkaller@xxxxxxxxxxxxxxxx
>> ---
>> 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 = &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...
