Re: [RFC PATCH 2/2] KVM: task switch: generate #DB trap if TSS.T is set

[Paolo Bonzini <pbonzini@redhat.com>]

On 28/03/2015 23:27, Eugene Korenevsky wrote:
> Emulate #DB generation on task switch if TSS.T is set according to Intel SDM.
> The processor generates a debug exception after a task switch if the T flag of
> the new task's TSS is set. This exception is generated after program control
> has passed to the new task, and prior to the execution of the first instruction
> of that task. DR6.BT bit should be set to indicate this condition.
> 
> Signed-off-by: Eugene Korenevsky <ekorenevsky@gmail.com>
> ---
>  arch/x86/include/asm/kvm_host.h |  1 +
>  arch/x86/kvm/emulate.c          | 13 +++++++++++++
>  arch/x86/kvm/vmx.c              |  5 -----
>  arch/x86/kvm/x86.c              |  4 ++++
>  4 files changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index a236e39..981e9ea 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -147,6 +147,7 @@ enum {
>  
>  #define DR6_BD		(1 << 13)
>  #define DR6_BS		(1 << 14)
> +#define DR6_BT		(1 << 15)
>  #define DR6_RTM		(1 << 16)
>  #define DR6_FIXED_1	0xfffe0ff0
>  #define DR6_INIT	0xffff0ff0
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 3a494f3..4ef1c27 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -2783,6 +2783,19 @@ static int load_state_from_tss32(struct x86_emulate_ctxt *ctxt,
>  	ret = __load_segment_descriptor(ctxt, tss->gs, VCPU_SREG_GS, cpl,
>  					X86_TRANSFER_TASK_SWITCH, NULL);
>  
> +	/*
> +	 * The last thing to do is injecting #DB trap if TSS.T bit is set
> +	 */
> +	if (ret == X86EMUL_CONTINUE && tss->t) {
> +		ulong dr6;
> +
> +		ctxt->ops->get_dr(ctxt, 6, &dr6);
> +		dr6 |= DR6_BT | DR6_FIXED_1 | DR6_RTM;
> +		ctxt->ops->set_dr(ctxt, 6, dr6);
> +		ctxt->have_exception = true;
> +		emulate_db(ctxt);
> +	}
> +
>  	return ret;
>  }
>  
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index f7b20b4..d922fd8 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -5696,11 +5696,6 @@ static int handle_task_switch(struct kvm_vcpu *vcpu)
>  	/* clear all local breakpoint enable flags */
>  	vmcs_writel(GUEST_DR7, vmcs_readl(GUEST_DR7) & ~0x155);
>  
> -	/*
> -	 * TODO: What about debug traps on tss switch?
> -	 *       Are we supposed to inject them and update dr6?
> -	 */
> -
>  	return 1;
>  }
>  
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index bd7a70b..66ac520 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6736,6 +6736,7 @@ int kvm_task_switch(struct kvm_vcpu *vcpu, u16 tss_selector, int idt_index,
>  	int ret;
>  
>  	init_emulate_ctxt(vcpu);
> +	ctxt->have_exception = false;

Please instead add the statement to init_emulate_ctxt, and remove it
from x86_emulate_instruction.

>  
>  	ret = emulator_task_switch(ctxt, tss_selector, idt_index, reason,
>  				   has_error_code, error_code);
> @@ -6745,6 +6746,9 @@ int kvm_task_switch(struct kvm_vcpu *vcpu, u16 tss_selector, int idt_index,
>  
>  	kvm_rip_write(vcpu, ctxt->eip);
>  	kvm_set_rflags(vcpu, ctxt->eflags);
> +	/* Generate #DB trap if T bit is set in new TSS */
> +	if (ctxt->have_exception && ctxt->exception.vector == DB_VECTOR)
> +		kvm_queue_exception(vcpu, DB_VECTOR);

I think you should just call kvm_multiple_exception directly, because
it's also possible that you'd have to inject a #TS exception here.

Paolo

>  	kvm_make_request(KVM_REQ_EVENT, vcpu);
>  	return EMULATE_DONE;
>  }
>