| Message ID | 53B125B9.6050306@web.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Il 30/06/2014 10:54, Jan Kiszka ha scritto: > + SVM_IOIO_SIZE_SHIFT; > gpa = svm->nested.vmcb_iopm + (port / 8); > - bit = port % 8; > - val = 0; > + start_bit = port % 8; > + iopm_len = (start_bit + size > 8) ? 2 : 1; > + mask = (0xf >> (4 - size)) << start_bit; These two lines are tricky. :) The patch looks good, I'll write a test case if you don't beat me to it. Paolo > + val = 0; > -- 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
On 2014-06-30 17:08, Paolo Bonzini wrote: > Il 30/06/2014 10:54, Jan Kiszka ha scritto: >> + SVM_IOIO_SIZE_SHIFT; >> gpa = svm->nested.vmcb_iopm + (port / 8); >> - bit = port % 8; >> - val = 0; >> + start_bit = port % 8; >> + iopm_len = (start_bit + size > 8) ? 2 : 1; >> + mask = (0xf >> (4 - size)) << start_bit; > > These two lines are tricky. :) That last line was, indeed... :) > > The patch looks good, I'll write a test case if you don't beat me to it. I'm not going to beat you if you write any test case. :) Didn't check the svm part of the unit tests recently, if they are as easily extensible for such scenarios like vmx is now. Jan
On Mon, Jun 30, 2014 at 10:54:17AM +0200, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > First, kvm_read_guest returns 0 on success. And then we need to take the > access size into account when testing the bitmap: intercept if any of > bits corresponding to the access is set. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> Reviewed-by: Joerg Roedel <jroedel@suse.de> Acked-by: Joerg Roedel <jroedel@suse.de> I have the slight hope that this fixes the issues with L2 Linux guests on L1 Windows hypervisors. Have to check that at some point :) > - if (kvm_read_guest(svm->vcpu.kvm, gpa, &val, 1)) > - val &= (1 << bit); > + if (kvm_read_guest(svm->vcpu.kvm, gpa, &val, iopm_len)) > + return NESTED_EXIT_DONE; Not related to that fix, but as a further improvement we should probably do a #vmexit(invalid-vmcb) or something if we can't read the iopm. Joerg -- 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
On 2014-07-01 17:23, Joerg Roedel wrote: > On Mon, Jun 30, 2014 at 10:54:17AM +0200, Jan Kiszka wrote: >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> First, kvm_read_guest returns 0 on success. And then we need to take the >> access size into account when testing the bitmap: intercept if any of >> bits corresponding to the access is set. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > Reviewed-by: Joerg Roedel <jroedel@suse.de> > Acked-by: Joerg Roedel <jroedel@suse.de> > > I have the slight hope that this fixes the issues with L2 Linux guests > on L1 Windows hypervisors. Have to check that at some point :) > >> - if (kvm_read_guest(svm->vcpu.kvm, gpa, &val, 1)) >> - val &= (1 << bit); >> + if (kvm_read_guest(svm->vcpu.kvm, gpa, &val, iopm_len)) >> + return NESTED_EXIT_DONE; > > Not related to that fix, but as a further improvement we should probably > do a #vmexit(invalid-vmcb) or something if we can't read the iopm. Yes, thought about this as well when thinking about kvm_read_guest failing. Some for MSR bitmap. Jan
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index c79766e1..3483ac9 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -2116,22 +2116,27 @@ static void nested_svm_unmap(struct page *page) static int nested_svm_intercept_ioio(struct vcpu_svm *svm) { - unsigned port; - u8 val, bit; + unsigned port, size, iopm_len; + u16 val, mask; + u8 start_bit; u64 gpa; if (!(svm->nested.intercept & (1ULL << INTERCEPT_IOIO_PROT))) return NESTED_EXIT_HOST; port = svm->vmcb->control.exit_info_1 >> 16; + size = (svm->vmcb->control.exit_info_1 & SVM_IOIO_SIZE_MASK) >> + SVM_IOIO_SIZE_SHIFT; gpa = svm->nested.vmcb_iopm + (port / 8); - bit = port % 8; - val = 0; + start_bit = port % 8; + iopm_len = (start_bit + size > 8) ? 2 : 1; + mask = (0xf >> (4 - size)) << start_bit; + val = 0; - if (kvm_read_guest(svm->vcpu.kvm, gpa, &val, 1)) - val &= (1 << bit); + if (kvm_read_guest(svm->vcpu.kvm, gpa, &val, iopm_len)) + return NESTED_EXIT_DONE; - return val ? NESTED_EXIT_DONE : NESTED_EXIT_HOST; + return (val & mask) ? NESTED_EXIT_DONE : NESTED_EXIT_HOST; } static int nested_svm_exit_handled_msr(struct vcpu_svm *svm)