| Message ID | 1431013779-28910-1-git-send-email-thuth@redhat.com |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, May 07, 2015 at 05:49:39PM +0200, Thomas Huth wrote: > Both functions are doing the same thing - looking up the struct > kvm_vcpu pointer for a given vCPU ID. No, kvm_get_vcpu(n) returns the n'th vcpu created, which is often but not necessarily the same as the vcpu with id n. In fact, due to the way we do threading in HV KVM, it's quite common for userspace to create vcpus with ids 0, 8, 16, ..., which means that the n'th vcpu created does not have id n (except of course for vcpu 0). So, NAK. Paul. -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 8 May 2015 09:07:18 +1000 Paul Mackerras <paulus@samba.org> wrote: > On Thu, May 07, 2015 at 05:49:39PM +0200, Thomas Huth wrote: > > Both functions are doing the same thing - looking up the struct > > kvm_vcpu pointer for a given vCPU ID. > > No, kvm_get_vcpu(n) returns the n'th vcpu created, which is often > but not necessarily the same as the vcpu with id n. In fact, due to > the way we do threading in HV KVM, it's quite common for userspace to > create vcpus with ids 0, 8, 16, ..., which means that the n'th vcpu > created does not have id n (except of course for vcpu 0). Ah, ok, thanks a lot for the explanation ... I thought that KVM would always put the VCPUs at the position of their ID into the array, but looking at kvm_vm_ioctl_create_vcpu(), you're right, it's using the amount of online CPUs as index instead: kvm->vcpus[atomic_read(&kvm->online_vcpus)] = vcpu; So never mind, and sorry for the nuisance! Thomas -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2015-05-08 at 10:19 +0200, Thomas Huth wrote: > On Fri, 8 May 2015 09:07:18 +1000 > Paul Mackerras <paulus@samba.org> wrote: > > > On Thu, May 07, 2015 at 05:49:39PM +0200, Thomas Huth wrote: > > > Both functions are doing the same thing - looking up the struct > > > kvm_vcpu pointer for a given vCPU ID. > > > > No, kvm_get_vcpu(n) returns the n'th vcpu created, which is often > > but not necessarily the same as the vcpu with id n. In fact, due to > > the way we do threading in HV KVM, it's quite common for userspace to > > create vcpus with ids 0, 8, 16, ..., which means that the n'th vcpu > > created does not have id n (except of course for vcpu 0). A patch adding a comment explaining that subtlety would be welcome from anyone who sent it :) cheers -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index 48d3c5d..78e62cf 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -292,22 +292,6 @@ void kvmppc_dump_regs(struct kvm_vcpu *vcpu) vcpu->arch.last_inst); } -struct kvm_vcpu *kvmppc_find_vcpu(struct kvm *kvm, int id) -{ - int r; - struct kvm_vcpu *v, *ret = NULL; - - mutex_lock(&kvm->lock); - kvm_for_each_vcpu(r, v, kvm) { - if (v->vcpu_id == id) { - ret = v; - break; - } - } - mutex_unlock(&kvm->lock); - return ret; -} - static void init_vpa(struct kvm_vcpu *vcpu, struct lppaca *vpa) { vpa->__old_status |= LPPACA_OLD_SHARED_PROC; @@ -358,7 +342,7 @@ static unsigned long do_h_register_vpa(struct kvm_vcpu *vcpu, int subfunc; struct kvmppc_vpa *vpap; - tvcpu = kvmppc_find_vcpu(kvm, vcpuid); + tvcpu = kvm_get_vcpu(kvm, vcpuid); if (!tvcpu) return H_PARAMETER; @@ -678,7 +662,7 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu) break; case H_PROD: target = kvmppc_get_gpr(vcpu, 4); - tvcpu = kvmppc_find_vcpu(vcpu->kvm, target); + tvcpu = kvm_get_vcpu(vcpu->kvm, target); if (!tvcpu) { ret = H_PARAMETER; break; @@ -696,7 +680,7 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu) target = kvmppc_get_gpr(vcpu, 4); if (target == -1) break; - tvcpu = kvmppc_find_vcpu(vcpu->kvm, target); + tvcpu = kvm_get_vcpu(vcpu->kvm, target); if (!tvcpu) { ret = H_PARAMETER; break;
Both functions are doing the same thing - looking up the struct kvm_vcpu pointer for a given vCPU ID. So there's no need for the kvmppc_find_vcpu() function, simply use the common function instead. Signed-off-by: Thomas Huth <thuth@redhat.com> --- arch/powerpc/kvm/book3s_hv.c | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-)