On Thu, Apr 7, 2022 at 12:59 PM Peter Gonda <pgonda@xxxxxxxxxx> wrote:
>
> svm_vm_migrate_from() uses sev_lock_vcpus_for_migration() to lock all
> source and target vcpu->locks. Mark the nested subclasses to avoid false
> positives from lockdep.
>
> Warning example:
> ============================================
> WARNING: possible recursive locking detected
> 5.17.0-dbg-DEV #15 Tainted: G O
> --------------------------------------------
> sev_migrate_tes/18859 is trying to acquire lock:
> ffff8d672d484238 (&vcpu->mutex){+.+.}-{3:3}, at: sev_lock_vcpus_for_migration+0x7e/0x150
> but task is already holding lock:
> ffff8d67703f81f8 (&vcpu->mutex){+.+.}-{3:3}, at: sev_lock_vcpus_for_migration+0x7e/0x150
> other info that might help us debug this:
> Possible unsafe locking scenario:
> CPU0
> ----
> lock(&vcpu->mutex);
> lock(&vcpu->mutex);
> *** DEADLOCK ***
> May be due to missing lock nesting notation
> 3 locks held by sev_migrate_tes/18859:
> #0: ffff9302f91323b8 (&kvm->lock){+.+.}-{3:3}, at: sev_vm_move_enc_context_from+0x96/0x740
> #1: ffff9302f906a3b8 (&kvm->lock/1){+.+.}-{3:3}, at: sev_vm_move_enc_context_from+0xae/0x740
> #2: ffff8d67703f81f8 (&vcpu->mutex){+.+.}-{3:3}, at: sev_lock_vcpus_for_migration+0x7e/0x150
>
> Fixes: b56639318bb2b ("KVM: SEV: Add support for SEV intra host migration")
> Reported-by: John Sperbeck<jsperbeck@xxxxxxxxxx>
> Suggested-by: David Rientjes <rientjes@xxxxxxxxxx>
> Suggested-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Cc: kvm@xxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Signed-off-by: Peter Gonda <pgonda@xxxxxxxxxx>
>
> ---
>
> V3
> * Updated signature to enum to self-document argument.
> * Updated comment as Seanjc@ suggested.
>
> Tested by running sev_migrate_tests with lockdep enabled. Before we see
> a warning from sev_lock_vcpus_for_migration(). After we get no warnings.
>
> ---
> arch/x86/kvm/svm/sev.c | 22 +++++++++++++++++-----
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 75fa6dd268f0..f66550ec8eaf 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -1591,14 +1591,26 @@ static void sev_unlock_two_vms(struct kvm *dst_kvm, struct kvm *src_kvm)
> atomic_set_release(&src_sev->migration_in_progress, 0);
> }
>
> +/*
> + * To suppress lockdep false positives, subclass all vCPU mutex locks by
> + * assigning even numbers to the source vCPUs and odd numbers to destination
> + * vCPUs based on the vCPU's index.
> + */
> +enum sev_migration_role {
> + SEV_MIGRATION_SOURCE = 0,
> + SEV_MIGRATION_TARGET,
> + SEV_NR_MIGRATION_ROLES,
> +};
>
> -static int sev_lock_vcpus_for_migration(struct kvm *kvm)
> +static int sev_lock_vcpus_for_migration(struct kvm *kvm,
> + enum sev_migration_role role)
> {
> struct kvm_vcpu *vcpu;
> unsigned long i, j;
>
> - kvm_for_each_vcpu(i, vcpu, kvm) {
> - if (mutex_lock_killable(&vcpu->mutex))
> + kvm_for_each_vcpu(i, vcpu, kvm) {
> + if (mutex_lock_killable_nested(
> + &vcpu->mutex, i * SEV_NR_MIGRATION_ROLES + role))
> goto out_unlock;
> }
>
> @@ -1745,10 +1757,10 @@ int sev_vm_move_enc_context_from(struct kvm *kvm, unsigned int source_fd)
> charged = true;
> }
>
> - ret = sev_lock_vcpus_for_migration(kvm);
> + ret = sev_lock_vcpus_for_migration(kvm, SEV_MIGRATION_SOURCE);
> if (ret)
> goto out_dst_cgroup;
> - ret = sev_lock_vcpus_for_migration(source_kvm);
> + ret = sev_lock_vcpus_for_migration(source_kvm, SEV_MIGRATION_TARGET);
> if (ret)
> goto out_dst_vcpu;
>
> --
> 2.35.1.1178.g4f1659d476-goog
>
Does sev_migrate_tests survive lockdep checking if
NR_MIGRATE_TEST_VCPUS is changed to 16?
