diff options
| author | Paul Mackerras <paulus@ozlabs.org> | 2019-05-28 21:54:00 -0400 |
|---|---|---|
| committer | Paul Mackerras <paulus@ozlabs.org> | 2019-05-28 23:44:36 -0400 |
| commit | 1659e27d2bc1ef47b6d031abe01b467f18cb72d9 (patch) | |
| tree | 9624c611a13ad720ec5593df5cccf04afef21bc1 | |
| parent | 0d4ee88d92884c661fcafd5576da243aa943dc24 (diff) | |
KVM: PPC: Book3S: Use new mutex to synchronize access to rtas token list
Currently the Book 3S KVM code uses kvm->lock to synchronize access
to the kvm->arch.rtas_tokens list. Because this list is scanned
inside kvmppc_rtas_hcall(), which is called with the vcpu mutex held,
taking kvm->lock cause a lock inversion problem, which could lead to
a deadlock.
To fix this, we add a new mutex, kvm->arch.rtas_token_lock, which nests
inside the vcpu mutexes, and use that instead of kvm->lock when
accessing the rtas token list.
This removes the lockdep_assert_held() in kvmppc_rtas_tokens_free().
At this point we don't hold the new mutex, but that is OK because
kvmppc_rtas_tokens_free() is only called when the whole VM is being
destroyed, and at that point nothing can be looking up a token in
the list.
Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
| -rw-r--r-- | arch/powerpc/include/asm/kvm_host.h | 1 | ||||
| -rw-r--r-- | arch/powerpc/kvm/book3s.c | 1 | ||||
| -rw-r--r-- | arch/powerpc/kvm/book3s_rtas.c | 14 |
3 files changed, 8 insertions, 8 deletions
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index 26b3ce487ddc..d10df677c452 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h | |||
| @@ -309,6 +309,7 @@ struct kvm_arch { | |||
| 309 | #ifdef CONFIG_PPC_BOOK3S_64 | 309 | #ifdef CONFIG_PPC_BOOK3S_64 |
| 310 | struct list_head spapr_tce_tables; | 310 | struct list_head spapr_tce_tables; |
| 311 | struct list_head rtas_tokens; | 311 | struct list_head rtas_tokens; |
| 312 | struct mutex rtas_token_lock; | ||
| 312 | DECLARE_BITMAP(enabled_hcalls, MAX_HCALL_OPCODE/4 + 1); | 313 | DECLARE_BITMAP(enabled_hcalls, MAX_HCALL_OPCODE/4 + 1); |
| 313 | #endif | 314 | #endif |
| 314 | #ifdef CONFIG_KVM_MPIC | 315 | #ifdef CONFIG_KVM_MPIC |
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c index 61a212d0daf0..ac5664845aca 100644 --- a/arch/powerpc/kvm/book3s.c +++ b/arch/powerpc/kvm/book3s.c | |||
| @@ -902,6 +902,7 @@ int kvmppc_core_init_vm(struct kvm *kvm) | |||
| 902 | #ifdef CONFIG_PPC64 | 902 | #ifdef CONFIG_PPC64 |
| 903 | INIT_LIST_HEAD_RCU(&kvm->arch.spapr_tce_tables); | 903 | INIT_LIST_HEAD_RCU(&kvm->arch.spapr_tce_tables); |
| 904 | INIT_LIST_HEAD(&kvm->arch.rtas_tokens); | 904 | INIT_LIST_HEAD(&kvm->arch.rtas_tokens); |
| 905 | mutex_init(&kvm->arch.rtas_token_lock); | ||
| 905 | #endif | 906 | #endif |
| 906 | 907 | ||
| 907 | return kvm->arch.kvm_ops->init_vm(kvm); | 908 | return kvm->arch.kvm_ops->init_vm(kvm); |
diff --git a/arch/powerpc/kvm/book3s_rtas.c b/arch/powerpc/kvm/book3s_rtas.c index 4e178c4c1ea5..b7ae3dfbf00e 100644 --- a/arch/powerpc/kvm/book3s_rtas.c +++ b/arch/powerpc/kvm/book3s_rtas.c | |||
| @@ -146,7 +146,7 @@ static int rtas_token_undefine(struct kvm *kvm, char *name) | |||
| 146 | { | 146 | { |
| 147 | struct rtas_token_definition *d, *tmp; | 147 | struct rtas_token_definition *d, *tmp; |
| 148 | 148 | ||
| 149 | lockdep_assert_held(&kvm->lock); | 149 | lockdep_assert_held(&kvm->arch.rtas_token_lock); |
| 150 | 150 | ||
| 151 | list_for_each_entry_safe(d, tmp, &kvm->arch.rtas_tokens, list) { | 151 | list_for_each_entry_safe(d, tmp, &kvm->arch.rtas_tokens, list) { |
| 152 | if (rtas_name_matches(d->handler->name, name)) { | 152 | if (rtas_name_matches(d->handler->name, name)) { |
| @@ -167,7 +167,7 @@ static int rtas_token_define(struct kvm *kvm, char *name, u64 token) | |||
| 167 | bool found; | 167 | bool found; |
| 168 | int i; | 168 | int i; |
| 169 | 169 | ||
| 170 | lockdep_assert_held(&kvm->lock); | 170 | lockdep_assert_held(&kvm->arch.rtas_token_lock); |
| 171 | 171 | ||
| 172 | list_for_each_entry(d, &kvm->arch.rtas_tokens, list) { | 172 | list_for_each_entry(d, &kvm->arch.rtas_tokens, list) { |
| 173 | if (d->token == token) | 173 | if (d->token == token) |
| @@ -206,14 +206,14 @@ int kvm_vm_ioctl_rtas_define_token(struct kvm *kvm, void __user *argp) | |||
| 206 | if (copy_from_user(&args, argp, sizeof(args))) | 206 | if (copy_from_user(&args, argp, sizeof(args))) |
| 207 | return -EFAULT; | 207 | return -EFAULT; |
| 208 | 208 | ||
| 209 | mutex_lock(&kvm->lock); | 209 | mutex_lock(&kvm->arch.rtas_token_lock); |
| 210 | 210 | ||
| 211 | if (args.token) | 211 | if (args.token) |
| 212 | rc = rtas_token_define(kvm, args.name, args.token); | 212 | rc = rtas_token_define(kvm, args.name, args.token); |
| 213 | else | 213 | else |
| 214 | rc = rtas_token_undefine(kvm, args.name); | 214 | rc = rtas_token_undefine(kvm, args.name); |
| 215 | 215 | ||
| 216 | mutex_unlock(&kvm->lock); | 216 | mutex_unlock(&kvm->arch.rtas_token_lock); |
| 217 | 217 | ||
| 218 | return rc; | 218 | return rc; |
| 219 | } | 219 | } |
| @@ -245,7 +245,7 @@ int kvmppc_rtas_hcall(struct kvm_vcpu *vcpu) | |||
| 245 | orig_rets = args.rets; | 245 | orig_rets = args.rets; |
| 246 | args.rets = &args.args[be32_to_cpu(args.nargs)]; | 246 | args.rets = &args.args[be32_to_cpu(args.nargs)]; |
| 247 | 247 | ||
| 248 | mutex_lock(&vcpu->kvm->lock); | 248 | mutex_lock(&vcpu->kvm->arch.rtas_token_lock); |
| 249 | 249 | ||
| 250 | rc = -ENOENT; | 250 | rc = -ENOENT; |
| 251 | list_for_each_entry(d, &vcpu->kvm->arch.rtas_tokens, list) { | 251 | list_for_each_entry(d, &vcpu->kvm->arch.rtas_tokens, list) { |
| @@ -256,7 +256,7 @@ int kvmppc_rtas_hcall(struct kvm_vcpu *vcpu) | |||
| 256 | } | 256 | } |
| 257 | } | 257 | } |
| 258 | 258 | ||
| 259 | mutex_unlock(&vcpu->kvm->lock); | 259 | mutex_unlock(&vcpu->kvm->arch.rtas_token_lock); |
| 260 | 260 | ||
| 261 | if (rc == 0) { | 261 | if (rc == 0) { |
| 262 | args.rets = orig_rets; | 262 | args.rets = orig_rets; |
| @@ -282,8 +282,6 @@ void kvmppc_rtas_tokens_free(struct kvm *kvm) | |||
| 282 | { | 282 | { |
| 283 | struct rtas_token_definition *d, *tmp; | 283 | struct rtas_token_definition *d, *tmp; |
| 284 | 284 | ||
| 285 | lockdep_assert_held(&kvm->lock); | ||
| 286 | |||
| 287 | list_for_each_entry_safe(d, tmp, &kvm->arch.rtas_tokens, list) { | 285 | list_for_each_entry_safe(d, tmp, &kvm->arch.rtas_tokens, list) { |
| 288 | list_del(&d->list); | 286 | list_del(&d->list); |
| 289 | kfree(d); | 287 | kfree(d); |
