Avi Kivity wrote:
> On 10/03/2009 12:31 AM, Jan Kiszka wrote:
>> Give user space more flexibility /wrt its IOCTL order. So far updating
>> the rflags via KVM_SET_REGS ignored potentially set single-step flags.
>> Now they will be kept.
>>    
> 
>>
>>       kvm_rip_write(vcpu, regs->rip);
>> -    kvm_x86_ops->set_rflags(vcpu, regs->rflags);
>>
>> +    rflags = regs->rflags;
>> +    if (vcpu->guest_debug&  KVM_GUESTDBG_SINGLESTEP)
>> +        rflags |= X86_EFLAGS_TF | X86_EFLAGS_RF;
>> +    kvm_x86_ops->set_rflags(vcpu, rflags);
>>
>>    
> 
> I think we need same on popf instruction emulation.
> 

Hmmmm, good point. Mind reverting 2/2 and applying this one instead?

Jan

--------->

KVM: x86: Rework guest single-step flag injection and filtering

Push TF and RF injection and filtering on guest single-stepping into the
vender get/set_rflags callbacks. This makes the whole mechanism more
robust /wrt user space IOTCTL order and instruction emulations.

Signed-off-by: Jan Kiszka <jan.kis...@siemens.com>
---

 arch/x86/kvm/svm.c |    8 +++++++-
 arch/x86/kvm/vmx.c |    4 ++++
 arch/x86/kvm/x86.c |   24 +++++++++---------------
 3 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 279a2ae..407e1a7 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -797,11 +797,17 @@ static void svm_vcpu_put(struct kvm_vcpu *vcpu)
 
 static unsigned long svm_get_rflags(struct kvm_vcpu *vcpu)
 {
-       return to_svm(vcpu)->vmcb->save.rflags;
+       unsigned long rflags = to_svm(vcpu)->vmcb->save.rflags;
+
+       if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)
+               rflags &= ~(unsigned long)(X86_EFLAGS_TF | X86_EFLAGS_RF);
+       return rflags;
 }
 
 static void svm_set_rflags(struct kvm_vcpu *vcpu, unsigned long rflags)
 {
+       if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)
+               rflags |= X86_EFLAGS_TF | X86_EFLAGS_RF;
        to_svm(vcpu)->vmcb->save.rflags = rflags;
 }
 
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 70020e5..8e678ef 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -787,6 +787,8 @@ static unsigned long vmx_get_rflags(struct kvm_vcpu *vcpu)
        rflags = vmcs_readl(GUEST_RFLAGS);
        if (to_vmx(vcpu)->rmode.vm86_active)
                rflags &= ~(unsigned long)(X86_EFLAGS_IOPL | X86_EFLAGS_VM);
+       if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)
+               rflags &= ~(unsigned long)(X86_EFLAGS_TF | X86_EFLAGS_RF);
        return rflags;
 }
 
@@ -794,6 +796,8 @@ static void vmx_set_rflags(struct kvm_vcpu *vcpu, unsigned 
long rflags)
 {
        if (to_vmx(vcpu)->rmode.vm86_active)
                rflags |= X86_EFLAGS_IOPL | X86_EFLAGS_VM;
+       if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)
+               rflags |= X86_EFLAGS_TF | X86_EFLAGS_RF;
        vmcs_writel(GUEST_RFLAGS, rflags);
 }
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index aa5d574..5b562dd 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3840,12 +3840,6 @@ int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, 
struct kvm_regs *regs)
        regs->rip = kvm_rip_read(vcpu);
        regs->rflags = kvm_x86_ops->get_rflags(vcpu);
 
-       /*
-        * Don't leak debug flags in case they were set for guest debugging
-        */
-       if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)
-               regs->rflags &= ~(X86_EFLAGS_TF | X86_EFLAGS_RF);
-
        vcpu_put(vcpu);
 
        return 0;
@@ -3872,13 +3866,11 @@ int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, 
struct kvm_regs *regs)
        kvm_register_write(vcpu, VCPU_REGS_R13, regs->r13);
        kvm_register_write(vcpu, VCPU_REGS_R14, regs->r14);
        kvm_register_write(vcpu, VCPU_REGS_R15, regs->r15);
-
 #endif
 
        kvm_rip_write(vcpu, regs->rip);
        kvm_x86_ops->set_rflags(vcpu, regs->rflags);
 
-
        vcpu->arch.exception.pending = false;
 
        vcpu_put(vcpu);
@@ -4471,12 +4463,15 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu 
*vcpu,
                                        struct kvm_guest_debug *dbg)
 {
        unsigned long rflags;
-       int old_debug;
        int i;
 
        vcpu_load(vcpu);
 
-       old_debug = vcpu->guest_debug;
+       /*
+        * Read rflags as long as potentially injected trace flags are still
+        * filtered out.
+        */
+       rflags = kvm_x86_ops->get_rflags(vcpu);
 
        vcpu->guest_debug = dbg->control;
        if (!(vcpu->guest_debug & KVM_GUESTDBG_ENABLE))
@@ -4493,11 +4488,10 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu 
*vcpu,
                vcpu->arch.switch_db_regs = (vcpu->arch.dr7 & DR7_BP_EN_MASK);
        }
 
-       rflags = kvm_x86_ops->get_rflags(vcpu);
-       if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)
-               rflags |= X86_EFLAGS_TF | X86_EFLAGS_RF;
-       else if (old_debug & KVM_GUESTDBG_SINGLESTEP)
-               rflags &= ~(X86_EFLAGS_TF | X86_EFLAGS_RF);
+       /*
+        * Trigger an rflags update that will inject or remove the trace
+        * flags.
+        */
        kvm_x86_ops->set_rflags(vcpu, rflags);
 
        kvm_x86_ops->set_guest_debug(vcpu, dbg);

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to