linux-kernel - Re: [PATCH] kvm: x86: disable KVM_FAST_MMIO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170816161301-mutt-send-email-mst@kernel.org>
Date:   Wed, 16 Aug 2017 16:16:52 +0300
From:   "Michael S. Tsirkin" <mst@...hat.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        Radim Krčmář <rkrcmar@...hat.com>,
        stable@...r.kernel.org
Subject: Re: [PATCH] kvm: x86: disable KVM_FAST_MMIO_BUS

On Wed, Aug 16, 2017 at 03:05:51PM +0200, Paolo Bonzini wrote:
> On 16/08/2017 14:58, Michael S. Tsirkin wrote:
> > On Wed, Aug 16, 2017 at 01:22:49PM +0200, Paolo Bonzini wrote:
> >> Microsoft pointed out privately to me that KVM's handling of
> >> KVM_FAST_MMIO_BUS is invalid.  Using skip_emulation_instruction is invalid
> >> in EPT misconfiguration vmexit handlers, because neither EPT violations
> >> nor misconfigurations are listed in the manual among the VM exits that
> >> set the VM-exit instruction length field.
> >>
> >> While physical processors seem to set the field, this is not architectural
> >> and is just a side effect of the implementation.  I couldn't convince
> >> myself of any condition on the exit qualification where VM-exit
> >> instruction length "has" to be defined; there are no trap-like VM-exits
> >> that can be repurposed; and fault-like VM-exits such as descriptor-table
> >> exits provide no decoding information.  So I don't really see any elegant
> >> way to fix it except by disabling KVM_FAST_MMIO_BUS, which means virtio
> >> 1 will go slower.
> > 
> > How about I will try asking Intel about it? If they can commit to length
> > being there in the future, we are all set.
> 
> Nope, "I couldn't convince myself of any condition on the exit
> qualification where VM-exit instruction length "has" to be defined".  So
> assuming Intel can do it, it would only apply to future processors (2
> years+ for server SKUs).

Well maybe there's a reason it's actually working.  Let's see what can
be done.

> 
> Plus of course it wouldn't be guaranteed to work on nested.

Not sure I got this one.

> >> Adding a hypercall or MSR write that does a fast MMIO write to a physical
> >> address would do it, but it adds hypervisor knowledge in virtio, including
> >> CPUID handling.
> > 
> > Another issue is that it will break DPDK on virtio.
> 
> Not break, just make it slower.

I thought hypercalls can only be triggered from ring 0, userspace can't call them.
Dod I get it wrong?

> Paolo
> 
> > Hmm that's quite unfortunate as we have just completed rolling out MMIO
> > signalling across the board. We did measure a significant slowdown
> > before enabling fast mmio. 
> > 
> >     Guest TX:(TCP)
> >     size/session/+throughput%/+cpu%/-+per cpu%/
> >     64/1/[+18.9183%]/-0.2823%/[+19.2550%]/
> >     64/2/[+13.5714%]/[+2.2675%]/[+11.0533%]/
> >     64/4/[+13.1070%]/[+2.1817%]/[+10.6920%]/
> >     64/8/[+13.0426%]/[+2.0887%]/[+10.7299%]/
> >     256/1/[+36.2761%]/+6.3434%/[+28.1471%]/
> >     ...
> >     1024/1/[+44.8873%]/+2.0811%/[+41.9335%]/
> >     ...
> >     1024/4/+0.0228%/[-2.2044%]/[+2.2774%]/
> >     ...
> >     16384/2/+0.0127%/[-5.0346%]/[+5.3148%]/
> >     ...
> >     65535/1/[+0.0062%]/[-4.1183%]/[+4.3017%]/
> >     65535/2/+0.0004%/[-4.2311%]/[+4.4185%]/
> >     65535/4/+0.0107%/[-4.6106%]/[+4.8446%]/
> >     65535/8/-0.0090%/[-5.5178%]/[+5.8306%]/
> >     
> > 
> > See commit bc85ccfdf5cc045588f665c84b5707d7364c8a6c for more numbers.
> > 
> > 
> >> ---
> >>  arch/x86/kvm/vmx.c | 5 -----
> >>  1 file changed, 5 deletions(-)
> >>
> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> >> index 375dca24cf42..b3eaeb20670d 100644
> >> --- a/arch/x86/kvm/vmx.c
> >> +++ b/arch/x86/kvm/vmx.c
> >> @@ -6320,11 +6320,6 @@ static int handle_ept_misconfig(struct kvm_vcpu *vcpu)
> >>  	gpa_t gpa;
> >>  
> >>  	gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS);
> >> -	if (!kvm_io_bus_write(vcpu, KVM_FAST_MMIO_BUS, gpa, 0, NULL)) {
> >> -		trace_kvm_fast_mmio(gpa);
> >> -		return kvm_skip_emulated_instruction(vcpu);
> >> -	}
> >> -
> >>  	ret = handle_mmio_page_fault(vcpu, gpa, true);
> >>  	vcpu->arch.gpa_available = true;
> >>  	if (likely(ret == RET_MMIO_PF_EMULATE))
> >> -- 
> >> 2.13.5