linux-kernel - Re: [PATCH v3] KVM: nVMX: Fix pending events injection

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: <20170301162505.GA20547@potion>
Date:   Wed, 1 Mar 2017 17:25:06 +0100
From:   Radim Krčmář <rkrcmar@...hat.com>
To:     Wanpeng Li <kernellwp@...il.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        Paolo Bonzini <pbonzini@...hat.com>,
        Wanpeng Li <wanpeng.li@...mail.com>,
        Jan Kiszka <jan.kiszka@...mens.com>
Subject: Re: [PATCH v3] KVM: nVMX: Fix pending events injection

2017-02-27 04:24-0800, Wanpeng Li:
> From: Wanpeng Li <wanpeng.li@...mail.com>
> 
> L2 fails to boot on a non-APICv box dues to 'commit 0ad3bed6c5ec 
> ("kvm: nVMX: move nested events check to kvm_vcpu_running")'

Applied to kvm/queue for the time being, thanks.

This patch fixes also nested apic kvm-unit-test on APICv boxes.
With this patch, the test finishes without a suberror, but fails the
"multiple nmi" subtest.  The subtest passes after reverting the whole
series that included 0ad3bed6c5ec.

We'll need to do more (something else?), but I haven't looked what
exactly, yet.

> KVM internal error. Suberror: 3
> extra data[0]: 800000ef
> extra data[1]: 1
> RAX=0000000000000000 RBX=ffffffff81f36140 RCX=0000000000000000 RDX=0000000000000000
> RSI=0000000000000000 RDI=0000000000000000 RBP=ffff88007c92fe90 RSP=ffff88007c92fe90
> R8 =ffff88007fccdca0 R9 =0000000000000000 R10=00000000fffedb3d R11=0000000000000000
> R12=0000000000000003 R13=0000000000000000 R14=0000000000000000 R15=ffff88007c92c000
> RIP=ffffffff810645e6 RFL=00000246 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0
> ES =0000 0000000000000000 ffffffff 00c00000
> CS =0010 0000000000000000 ffffffff 00a09b00 DPL=0 CS64 [-RA]
> SS =0000 0000000000000000 ffffffff 00c00000
> DS =0000 0000000000000000 ffffffff 00c00000
> FS =0000 0000000000000000 ffffffff 00c00000
> GS =0000 ffff88007fcc0000 ffffffff 00c00000
> LDT=0000 0000000000000000 ffffffff 00c00000
> TR =0040 ffff88007fcd4200 00002087 00008b00 DPL=0 TSS64-busy
> GDT=     ffff88007fcc9000 0000007f
> IDT=     ffffffffff578000 00000fff
> CR0=80050033 CR2=00000000ffffffff CR3=0000000001e0a000 CR4=003406e0
> DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000 DR3=0000000000000000 
> DR6=00000000fffe0ff0 DR7=0000000000000400
> EFER=0000000000000d01
> 
> We should try to reinject previous events if any before trying to inject 
> new event if pending. If vmexit is triggered by L2 guest and L0 interested
> in, we should reinject IDT-vectoring info to L2 through vmcs02 if any,
> otherwise, we can consider new IRQs/NMIs which can be injected and call 
> nested events callback to switch from L2 to L1 if needed and inject the 
> proper vmexit events. However, 'commit 0ad3bed6c5ec ("kvm: nVMX: move 
> nested events check to kvm_vcpu_running")' results in the handle events 
> order reversely on non-APICv box. This patch fixes it by bailing out for  
> pending events and not consider new events in this scenario.
> 
> Cc: Paolo Bonzini <pbonzini@...hat.com>
> Cc: Radim Krčmář <rkrcmar@...hat.com>
> Cc: Jan Kiszka <jan.kiszka@...mens.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@...mail.com>
> ---
> v2 -> v3:
>  * remove the duplicate check for vcpu->arch.interrupt.pending
> v1 -> v2:
>  * bail out for pending events in vmx_check_nested_events() and not consider 
>     new events in this scenario
> 
>  arch/x86/kvm/vmx.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index ef4ba71..246a910 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -10642,6 +10642,11 @@ static int vmx_check_nested_events(struct kvm_vcpu *vcpu, bool external_intr)
>  {
>  	struct vcpu_vmx *vmx = to_vmx(vcpu);
>  
> +	if (vcpu->arch.exception.pending ||
> +		vcpu->arch.nmi_injected ||
> +		vcpu->arch.interrupt.pending)
> +		return -EBUSY;
> +
>  	if (nested_cpu_has_preemption_timer(get_vmcs12(vcpu)) &&
>  	    vmx->nested.preemption_timer_expired) {
>  		if (vmx->nested.nested_run_pending)
> @@ -10651,8 +10656,7 @@ static int vmx_check_nested_events(struct kvm_vcpu *vcpu, bool external_intr)
>  	}
>  
>  	if (vcpu->arch.nmi_pending && nested_exit_on_nmi(vcpu)) {
> -		if (vmx->nested.nested_run_pending ||
> -		    vcpu->arch.interrupt.pending)
> +		if (vmx->nested.nested_run_pending)
>  			return -EBUSY;
>  		nested_vmx_vmexit(vcpu, EXIT_REASON_EXCEPTION_NMI,
>  				  NMI_VECTOR | INTR_TYPE_NMI_INTR |
> -- 
> 2.7.4
>