From: John Sperbeck <jsperbeck@google.com>
To: Peter Gonda <pgonda@google.com>
Cc: kvm@vger.kernel.org, David Rientjes <rientjes@google.com>,
Sean Christopherson <seanjc@google.com>,
Paolo Bonzini <pbonzini@redhat.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] KVM: SEV: Mark nested locking of vcpu->lock
Date: Thu, 7 Apr 2022 14:17:32 -0700 [thread overview]
Message-ID: <CAFNjLiXC0AdOw5f8Ovu47D==ex7F0=WN_Ocirymz4xL=mWvC5A@mail.gmail.com> (raw)
In-Reply-To: <20220407195908.633003-1-pgonda@google.com>
On Thu, Apr 7, 2022 at 12:59 PM Peter Gonda <pgonda@google.com> 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@google.com>
> Suggested-by: David Rientjes <rientjes@google.com>
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: kvm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Peter Gonda <pgonda@google.com>
>
> ---
>
> 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?
next prev parent reply other threads:[~2022-04-07 21:17 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-07 19:59 [PATCH v3] KVM: SEV: Mark nested locking of vcpu->lock Peter Gonda
2022-04-07 21:17 ` John Sperbeck [this message]
2022-04-08 15:08 ` Peter Gonda
2022-04-20 20:14 ` Peter Gonda
2022-04-21 15:56 ` Paolo Bonzini
2022-04-26 19:06 ` Peter Gonda
2022-04-27 16:04 ` Paolo Bonzini
2022-04-27 20:18 ` Peter Gonda
2022-04-28 21:28 ` Peter Gonda
2022-04-28 23:59 ` Paolo Bonzini
2022-04-29 15:35 ` Peter Gonda
2022-04-29 15:38 ` Paolo Bonzini
2022-04-29 15:51 ` Peter Gonda
2022-04-29 15:58 ` Paolo Bonzini
2022-04-29 17:12 ` Peter Gonda
2022-04-29 17:21 ` Paolo Bonzini
2022-04-29 17:27 ` Peter Gonda
2022-04-29 17:32 ` Paolo Bonzini
2022-04-29 17:33 ` Peter Gonda
[not found] ` <20220429010312.4013-1-hdanton@sina.com>
2022-04-29 8:48 ` Paolo Bonzini
[not found] ` <20220429114012.4127-1-hdanton@sina.com>
2022-04-29 13:44 ` Paolo Bonzini
[not found] ` <20220430015008.4257-1-hdanton@sina.com>
2022-04-30 8:11 ` Paolo Bonzini
2022-04-30 8:11 ` Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAFNjLiXC0AdOw5f8Ovu47D==ex7F0=WN_Ocirymz4xL=mWvC5A@mail.gmail.com' \
--to=jsperbeck@google.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=pgonda@google.com \
--cc=rientjes@google.com \
--cc=seanjc@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).