linux-kernel - Re: [RFC PATCH 3/3] kvm: x86: skip DRn reload if previous VM exit is DR access VM exit

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: <20200423193108.GP17824@linux.intel.com>
Date:   Thu, 23 Apr 2020 12:31:08 -0700
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     Xiaoyao Li <xiaoyao.li@...el.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org,
        Nadav Amit <namit@...technion.ac.il>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 3/3] kvm: x86: skip DRn reload if previous VM exit is
 DR access VM exit

On Thu, Apr 16, 2020 at 06:15:09PM +0800, Xiaoyao Li wrote:
> When DR access vm exit, there is no DRn change throughout VM exit to
> next VM enter. Skip the DRn reload in this case and fix the comments.

Same thing as the previous patch, the hardware values aren't stable.  In
this case, MOV DR won't exit so KVM needs to ensure hardware has the guest
values, irrespective of whether breakpoints are enabled.

> Signed-off-by: Xiaoyao Li <xiaoyao.li@...el.com>
> ---
>  arch/x86/kvm/svm/svm.c | 8 +++++---
>  arch/x86/kvm/vmx/vmx.c | 8 +++++---
>  arch/x86/kvm/x86.c     | 2 +-
>  3 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 66123848448d..c6883a0bf8c3 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2287,9 +2287,11 @@ static int dr_interception(struct vcpu_svm *svm)
>  
>  	if (svm->vcpu.guest_debug == 0) {
>  		/*
> -		 * No more DR vmexits; force a reload of the debug registers
> -		 * and reenter on this instruction.  The next vmexit will
> -		 * retrieve the full state of the debug registers.
> +		 * No more DR vmexits and reenter on this instruction.
> +		 * The next vmexit will retrieve the full state of the debug
> +		 * registers and re-enable DR vmexits.
> +		 * No need to set KVM_DEBUGREG_NEED_RELOAD because no DRn change
> +		 * since this DR vmexit.
>  		 */
>  		clr_dr_intercepts(svm);
>  		svm->vcpu.arch.switch_db_regs |= KVM_DEBUGREG_WONT_EXIT;
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index aa1b8cf7c915..22eff8503048 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -4967,9 +4967,11 @@ static int handle_dr(struct kvm_vcpu *vcpu)
>  		exec_controls_clearbit(to_vmx(vcpu), CPU_BASED_MOV_DR_EXITING);
>  
>  		/*
> -		 * No more DR vmexits; force a reload of the debug registers
> -		 * and reenter on this instruction.  The next vmexit will
> -		 * retrieve the full state of the debug registers.
> +		 * No more DR vmexits and reenter on this instruction.
> +		 * The next vmexit will retrieve the full state of the debug
> +		 * registers and re-enable DR vmexits.
> +		 * No need to set KVM_DEBUGREG_NEED_RELOAD because no DRn change
> +		 * since this DR vmexit.
>  		 */
>  		vcpu->arch.switch_db_regs |= KVM_DEBUGREG_WONT_EXIT;
>  		return 1;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 71264df64001..8983848cbf45 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -8400,7 +8400,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>  	if (test_thread_flag(TIF_NEED_FPU_LOAD))
>  		switch_fpu_return();
>  
> -	if (unlikely(vcpu->arch.switch_db_regs)) {
> +	if (unlikely(vcpu->arch.switch_db_regs & KVM_DEBUGREG_NEED_RELOAD)) {
>  		set_debugreg(0, 7);
>  		set_debugreg(vcpu->arch.eff_db[0], 0);
>  		set_debugreg(vcpu->arch.eff_db[1], 1);
> -- 
> 2.20.1
>