Re: [PATCH v3] KVM: nVMX: Improve I/O exit handling

[Gleb Natapov <gleb@redhat.com>]

On Mon, Feb 18, 2013 at 07:32:53AM +0100, Jan Kiszka wrote:
> On 2013-02-14 19:46, 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@siemens.com>
> > ---
> > 
> > Changes in v3:
> >  - trap unconditionally if bitmap access fails
> > 
> >  arch/x86/kvm/vmx.c |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 files changed, 53 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index 6667042..2633199 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -5908,6 +5908,58 @@ 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;
> 
> Nonsense, rep ins/outs always works against the same port. We can simply
> drop this check and be done with the feature. I'll come up with v4.
> 
Actually this reminds me that we should check range of ports depending
on operand size, not one port. But here is a catch, older cpus do not
provide operand size as part of exit information.

> Jan
> 
> > +
> > +	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)
> > +			if (kvm_read_guest(vcpu->kvm, bitmap, &b, 1))
> > +				return 1;
> > +		if (b & (1 << (port & 7)))
> > +			return 1;
> > +
> > +		port++;
> > +		size--;
> > +		last_bitmap = bitmap;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  /*
> >   * Return 1 if we should exit from L2 to L1 to handle an MSR access access,
> >   * rather than handle it ourselves in L0. I.e., check whether L1 expressed
> > @@ -6097,8 +6149,7 @@ static bool nested_vmx_exit_handled(struct kvm_vcpu *vcpu)
> >  	case EXIT_REASON_DR_ACCESS:
> >  		return nested_cpu_has(vmcs12, CPU_BASED_MOV_DR_EXITING);
> >  	case EXIT_REASON_IO_INSTRUCTION:
> > -		/* TODO: support IO bitmaps */
> > -		return 1;
> > +		return nested_vmx_exit_handled_io(vcpu, vmcs12);
> >  	case EXIT_REASON_MSR_READ:
> >  	case EXIT_REASON_MSR_WRITE:
> >  		return nested_vmx_exit_handled_msr(vcpu, vmcs12, exit_reason);
> > 
> 
> 



--
			Gleb.