From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH v2] KVM: nVMX: Improve I/O exit handling Date: Thu, 14 Feb 2013 13:22:01 +0100 Message-ID: <511CD6E9.7080509@siemens.com> References: <51180635.3060003@web.de> <20130211100721.GA11107@fermat.math.technion.ac.il> <5118C4F9.707@web.de> <5118D3B5.5010406@siemens.com> <20130214093257.GK9817@redhat.com> <511CC83E.1080208@siemens.com> <20130214121129.GM9817@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , "Nadav Har'El" , kvm , Orit Wasserman To: Gleb Natapov Return-path: Received: from david.siemens.de ([192.35.17.14]:22741 "EHLO david.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934237Ab3BNMWK (ORCPT ); Thu, 14 Feb 2013 07:22:10 -0500 In-Reply-To: <20130214121129.GM9817@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 2013-02-14 13:11, Gleb Natapov wrote: > 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 >>>> --- >>>> >>>> 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? To my understanding, nested_vmx_failValid/Invalid are related to errors directly related to vm instruction execution. This one is triggered by the guest later on. > >> 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; OK. > >> 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? Will follow. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux