On Thu, Feb 14, 2013 at 12:19:26PM +0100, Jan Kiszka wrote:
> On 2013-02-14 10:32, Gleb Natapov wrote:
> > On Mon, Feb 11, 2013 at 12:19:17PM +0100, Jan Kiszka wrote:
> >> This prevents trapping L2 I/O exits if L1 has neither unconditional nor
> >> bitmap-based exiting enabled. Furthermore, it implements basic I/O
> >> bitmap handling. Repeated string accesses are still reported to L1
> >> unconditionally for now.
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
> >> ---
> >>
> >> Changes in v2:
> >> - fix two brown-paper-bag bugs (offset for bitmap_b, wrong direction
> >> of mask shift)
> >> - use vmcs12 argument instead of get_vmcs12
> >>
> >> arch/x86/kvm/vmx.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> >> 1 files changed, 52 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> >> index fe9a9cf..64e1233 100644
> >> --- a/arch/x86/kvm/vmx.c
> >> +++ b/arch/x86/kvm/vmx.c
> >> @@ -5913,6 +5913,57 @@ static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
> >> static const int kvm_vmx_max_exit_handlers =
> >> ARRAY_SIZE(kvm_vmx_exit_handlers);
> >>
> >> +static bool nested_vmx_exit_handled_io(struct kvm_vcpu *vcpu,
> >> + struct vmcs12 *vmcs12)
> >> +{
> >> + unsigned long exit_qualification;
> >> + gpa_t bitmap, last_bitmap;
> >> + bool string, rep;
> >> + u16 port;
> >> + int size;
> >> + u8 b;
> >> +
> >> + if (nested_cpu_has(vmcs12, CPU_BASED_UNCOND_IO_EXITING))
> >> + return 1;
> >> +
> >> + if (!nested_cpu_has(vmcs12, CPU_BASED_USE_IO_BITMAPS))
> >> + return 0;
> >> +
> >> + exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
> >> +
> >> + string = exit_qualification & 16;
> >> + rep = exit_qualification & 32;
> >> +
> >> + /* TODO: interpret instruction and check range against bitmap */
> >> + if (string && rep)
> >> + return 1;
> >> +
> >> + port = exit_qualification >> 16;
> >> + size = (exit_qualification & 7) + 1;
> >> +
> >> + last_bitmap = (gpa_t)-1;
> >> + b = -1;
> >> +
> >> + while (size > 0) {
> >> + if (port < 0x8000)
> >> + bitmap = vmcs12->io_bitmap_a;
> >> + else
> >> + bitmap = vmcs12->io_bitmap_b;
> >> + bitmap += (port & 0x7fff) / 8;
> >> +
> >> + if (last_bitmap != bitmap)
> >> + kvm_read_guest(vcpu->kvm, bitmap, &b, 1);
> > Return value is ignored.
>
> Not sure how to map a failure on real HW behaviour. I guess it's best to
Exit to L1 with nested_vmx_failValid() may be?
> simply initialize b to -1 before each call, enforcing an exit on
> unaccessible bitmaps.
>
I'd just make it explicit:
if (kvm_read_guest())
return 1;
> BTW, nested_vmx_exit_handled_msr needs some improvement in this regard, too.
>
Yes, nested_vmx_exit_handled_msr() use uninitialized 'b' from the stack.
We are leaking host kernel data to a guest here. Patch?
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
