| Message ID | 20170621133741.25627-1-lprosek@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2017-06-21 15:37+0200, Ladi Prosek: > kvm_skip_emulated_instruction handles the singlestep debug exception > which is something we almost always want. This commit (specifically > the change in rdmsr_interception) makes the debug.flat KVM unit test > pass on AMD. kvm_skip_emulated_instruction() also has a return value, which says whether the debug exception was requested by the userspace or by the guest (userspace has priority). This patch fixes the guest debugging, but userspace still won't receive its events. I think it would be better to fix both at once, > Signed-off-by: Ladi Prosek <lprosek@redhat.com> > --- > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > @@ -2278,7 +2278,7 @@ static int io_interception(struct vcpu_svm *svm) > port = io_info >> 16; > size = (io_info & SVM_IOIO_SIZE_MASK) >> SVM_IOIO_SIZE_SHIFT; > svm->next_rip = svm->vmcb->control.exit_info_2; > - skip_emulated_instruction(&svm->vcpu); > + kvm_skip_emulated_instruction(&svm->vcpu); > > return in ? kvm_fast_pio_in(vcpu, size, port) > : kvm_fast_pio_out(vcpu, size, port); i.e. ret = kvm_skip_emulated_instruction(&svm->vcpu); return ret && (...); > @@ -3063,7 +3063,7 @@ static int vmload_interception(struct vcpu_svm *svm) > return 1; > > svm->next_rip = kvm_rip_read(&svm->vcpu) + 3; > - skip_emulated_instruction(&svm->vcpu); > + kvm_skip_emulated_instruction(&svm->vcpu); ret = kvm_skip_emulated_instruction(&svm->vcpu); > > nested_svm_vmloadsave(nested_vmcb, svm->vmcb); > nested_svm_unmap(page); return ret; and so on ... thanks.
2017-06-21 18:55+0200, Radim Krčmář: > 2017-06-21 15:37+0200, Ladi Prosek: > > kvm_skip_emulated_instruction handles the singlestep debug exception > > which is something we almost always want. This commit (specifically > > the change in rdmsr_interception) makes the debug.flat KVM unit test > > pass on AMD. > > kvm_skip_emulated_instruction() also has a return value, which says > whether the debug exception was requested by the userspace or by the > guest (userspace has priority). > > This patch fixes the guest debugging, but userspace still won't receive > its events. I think it would be better to fix both at once, > > > Signed-off-by: Ladi Prosek <lprosek@redhat.com> > > --- > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > > @@ -2278,7 +2278,7 @@ static int io_interception(struct vcpu_svm *svm) > > port = io_info >> 16; > > size = (io_info & SVM_IOIO_SIZE_MASK) >> SVM_IOIO_SIZE_SHIFT; > > svm->next_rip = svm->vmcb->control.exit_info_2; > > - skip_emulated_instruction(&svm->vcpu); > > + kvm_skip_emulated_instruction(&svm->vcpu); > > > > return in ? kvm_fast_pio_in(vcpu, size, port) > > : kvm_fast_pio_out(vcpu, size, port); > > i.e. > ret = kvm_skip_emulated_instruction(&svm->vcpu); > > return ret && (...); Nope, the ret has to be checked afterwards ... better look at handle_io() in vmx.c. :)
On Wed, Jun 21, 2017 at 6:55 PM, Radim Krčmář <rkrcmar@redhat.com> wrote: > 2017-06-21 15:37+0200, Ladi Prosek: >> kvm_skip_emulated_instruction handles the singlestep debug exception >> which is something we almost always want. This commit (specifically >> the change in rdmsr_interception) makes the debug.flat KVM unit test >> pass on AMD. > > kvm_skip_emulated_instruction() also has a return value, which says > whether the debug exception was requested by the userspace or by the > guest (userspace has priority). > > This patch fixes the guest debugging, but userspace still won't receive > its events. I think it would be better to fix both at once, Will do, thanks!
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 4250f9a..472a1c1 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -2278,7 +2278,7 @@ static int io_interception(struct vcpu_svm *svm) port = io_info >> 16; size = (io_info & SVM_IOIO_SIZE_MASK) >> SVM_IOIO_SIZE_SHIFT; svm->next_rip = svm->vmcb->control.exit_info_2; - skip_emulated_instruction(&svm->vcpu); + kvm_skip_emulated_instruction(&svm->vcpu); return in ? kvm_fast_pio_in(vcpu, size, port) : kvm_fast_pio_out(vcpu, size, port); @@ -3063,7 +3063,7 @@ static int vmload_interception(struct vcpu_svm *svm) return 1; svm->next_rip = kvm_rip_read(&svm->vcpu) + 3; - skip_emulated_instruction(&svm->vcpu); + kvm_skip_emulated_instruction(&svm->vcpu); nested_svm_vmloadsave(nested_vmcb, svm->vmcb); nested_svm_unmap(page); @@ -3084,7 +3084,7 @@ static int vmsave_interception(struct vcpu_svm *svm) return 1; svm->next_rip = kvm_rip_read(&svm->vcpu) + 3; - skip_emulated_instruction(&svm->vcpu); + kvm_skip_emulated_instruction(&svm->vcpu); nested_svm_vmloadsave(svm->vmcb, nested_vmcb); nested_svm_unmap(page); @@ -3126,7 +3126,7 @@ static int stgi_interception(struct vcpu_svm *svm) return 1; svm->next_rip = kvm_rip_read(&svm->vcpu) + 3; - skip_emulated_instruction(&svm->vcpu); + kvm_skip_emulated_instruction(&svm->vcpu); kvm_make_request(KVM_REQ_EVENT, &svm->vcpu); enable_gif(svm); @@ -3140,7 +3140,7 @@ static int clgi_interception(struct vcpu_svm *svm) return 1; svm->next_rip = kvm_rip_read(&svm->vcpu) + 3; - skip_emulated_instruction(&svm->vcpu); + kvm_skip_emulated_instruction(&svm->vcpu); disable_gif(svm); @@ -3165,7 +3165,7 @@ static int invlpga_interception(struct vcpu_svm *svm) kvm_mmu_invlpg(vcpu, kvm_register_read(&svm->vcpu, VCPU_REGS_RAX)); svm->next_rip = kvm_rip_read(&svm->vcpu) + 3; - skip_emulated_instruction(&svm->vcpu); + kvm_skip_emulated_instruction(&svm->vcpu); return 1; } @@ -3189,7 +3189,7 @@ static int xsetbv_interception(struct vcpu_svm *svm) if (kvm_set_xcr(&svm->vcpu, index, new_bv) == 0) { svm->next_rip = kvm_rip_read(&svm->vcpu) + 3; - skip_emulated_instruction(&svm->vcpu); + kvm_skip_emulated_instruction(&svm->vcpu); } return 1; @@ -3285,7 +3285,7 @@ static int invlpg_interception(struct vcpu_svm *svm) return emulate_instruction(&svm->vcpu, 0) == EMULATE_DONE; kvm_mmu_invlpg(&svm->vcpu, svm->vmcb->control.exit_info_1); - skip_emulated_instruction(&svm->vcpu); + kvm_skip_emulated_instruction(&svm->vcpu); return 1; } @@ -3436,7 +3436,7 @@ static int dr_interception(struct vcpu_svm *svm) kvm_register_write(&svm->vcpu, reg, val); } - skip_emulated_instruction(&svm->vcpu); + kvm_skip_emulated_instruction(&svm->vcpu); return 1; } @@ -3569,7 +3569,7 @@ static int rdmsr_interception(struct vcpu_svm *svm) kvm_register_write(&svm->vcpu, VCPU_REGS_RDX, msr_info.data >> 32); svm->next_rip = kvm_rip_read(&svm->vcpu) + 2; - skip_emulated_instruction(&svm->vcpu); + kvm_skip_emulated_instruction(&svm->vcpu); } return 1; } @@ -3699,7 +3699,7 @@ static int wrmsr_interception(struct vcpu_svm *svm) kvm_inject_gp(&svm->vcpu, 0); } else { trace_kvm_msr_write(ecx, data); - skip_emulated_instruction(&svm->vcpu); + kvm_skip_emulated_instruction(&svm->vcpu); } return 1; } @@ -3730,7 +3730,7 @@ static int pause_interception(struct vcpu_svm *svm) static int nop_interception(struct vcpu_svm *svm) { - skip_emulated_instruction(&(svm->vcpu)); + kvm_skip_emulated_instruction(&(svm->vcpu)); return 1; }
kvm_skip_emulated_instruction handles the singlestep debug exception which is something we almost always want. This commit (specifically the change in rdmsr_interception) makes the debug.flat KVM unit test pass on AMD. Two call sites still call skip_emulated_instruction directly: * In svm_queue_exception where it's used only for moving the rip forward * In task_switch_interception which is analogous to handle_task_switch in VMX Signed-off-by: Ladi Prosek <lprosek@redhat.com> --- arch/x86/kvm/svm.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)