| Message ID | 1382021447-20216-3-git-send-email-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Oct 17, 2013 at 04:50:47PM +0200, Paolo Bonzini wrote: > The loop was always using 0 as the index. This means that > any rubbish after the first element of the array went undetected. > It seems reasonable to assume that no KVM userspace did that. It is not a typo, look at __kvm_set_xcr when setting guest_xcrs->xcrs[i].value, with i != 0. The code is not prepared to deal with XCR != 0 (because its not implemented in hw). > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > arch/x86/kvm/x86.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index f4e1391..f91dff2 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3062,9 +3062,9 @@ static int kvm_vcpu_ioctl_x86_set_xcrs(struct kvm_vcpu *vcpu, > > for (i = 0; i < guest_xcrs->nr_xcrs; i++) > /* Only support XCR0 currently */ > - if (guest_xcrs->xcrs[0].xcr == XCR_XFEATURE_ENABLED_MASK) { > + if (guest_xcrs->xcrs[i].xcr == XCR_XFEATURE_ENABLED_MASK) { > r = __kvm_set_xcr(vcpu, XCR_XFEATURE_ENABLED_MASK, > - guest_xcrs->xcrs[0].value); > + guest_xcrs->xcrs[i].value); > break; > } > if (r) > -- > 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Il 18/10/2013 02:04, Marcelo Tosatti ha scritto: > On Thu, Oct 17, 2013 at 04:50:47PM +0200, Paolo Bonzini wrote: >> The loop was always using 0 as the index. This means that >> any rubbish after the first element of the array went undetected. >> It seems reasonable to assume that no KVM userspace did that. > > It is not a typo, look at __kvm_set_xcr when setting > guest_xcrs->xcrs[i].value, with i != 0. i is not the index of the XCR register, it's the index in the array. The index is currently hardcoded to 0 when it is passed to __kvm_set_xcr; but very reasonably __kvm_set_xcr returns 1 when index != 0. IMO even the "if" in kvm_vcpu_ioctl_x86_set_xcrs is wrong: setting XCR above XCR0 should fail KVM_SET_XCRS, while currently is ignored. The body of the loop should be simply: r = __kvm_set_xcr(vcpu, guest_xcrs->xcrs[i].xcr, guest_xcrs->xcrs[i].value); if (r) break; Paolo > The code is not prepared to deal with XCR != 0 (because its not > implemented in hw). > >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> --- >> arch/x86/kvm/x86.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index f4e1391..f91dff2 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -3062,9 +3062,9 @@ static int kvm_vcpu_ioctl_x86_set_xcrs(struct kvm_vcpu *vcpu, >> >> for (i = 0; i < guest_xcrs->nr_xcrs; i++) >> /* Only support XCR0 currently */ >> - if (guest_xcrs->xcrs[0].xcr == XCR_XFEATURE_ENABLED_MASK) { >> + if (guest_xcrs->xcrs[i].xcr == XCR_XFEATURE_ENABLED_MASK) { >> r = __kvm_set_xcr(vcpu, XCR_XFEATURE_ENABLED_MASK, >> - guest_xcrs->xcrs[0].value); >> + guest_xcrs->xcrs[i].value); >> break; >> } >> if (r) >> -- >> 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe kvm" 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/x86/kvm/x86.c b/arch/x86/kvm/x86.c index f4e1391..f91dff2 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3062,9 +3062,9 @@ static int kvm_vcpu_ioctl_x86_set_xcrs(struct kvm_vcpu *vcpu, for (i = 0; i < guest_xcrs->nr_xcrs; i++) /* Only support XCR0 currently */ - if (guest_xcrs->xcrs[0].xcr == XCR_XFEATURE_ENABLED_MASK) { + if (guest_xcrs->xcrs[i].xcr == XCR_XFEATURE_ENABLED_MASK) { r = __kvm_set_xcr(vcpu, XCR_XFEATURE_ENABLED_MASK, - guest_xcrs->xcrs[0].value); + guest_xcrs->xcrs[i].value); break; } if (r)
The loop was always using 0 as the index. This means that any rubbish after the first element of the array went undetected. It seems reasonable to assume that no KVM userspace did that. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- arch/x86/kvm/x86.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)