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.kiszka@xxxxxxxxxxx>
---
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
