Re: [PATCH] kvm: x86: disable KVM_FAST_MMIO_BUS

[Radim KrÄmÃÅ]

2017-08-16 13:22+0200, Paolo Bonzini:
> 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.

Do you have some numbers?

We could keep the ugliness in KVM and add a new skip function with
emulate_instruction(vcpu, EMULTYPE_SKIP) to decode the length of the
instruction.  (Adding a condition just for EPT violation exit reason to
the existing skip function would be a dirtier solution.)
Slower than what we have now, but faster than full emulation.

I agree that configuring EPT to throw a violation when accessing fast
MMIO has many drawbacks (although it seems to be what Intel expected).

Thanks.

> 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.  So it would be pretty ugly in the guest-side implementation,
> but if somebody wants to do it and the virtio side is acceptable to the
> virtio maintainers, I am okay with it.
> 
> Cc: Michael S. Tsirkin <mst@xxxxxxxxxx>
> Cc: Radim KrÄmÃÅ <rkrcmar@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 68c3b4d1676d870f0453c31d5a52e7e65c7448ae
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
>  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
>