From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f193.google.com ([209.85.128.193]:32874 "EHLO mail-wr0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751483AbdF0IU5 (ORCPT ); Tue, 27 Jun 2017 04:20:57 -0400 Subject: Re: [PATCH] KVM: x86: fix singlestepping over syscall To: Wanpeng Li , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Cc: kvm , P J P , Steve Rutherford , Andrew Honig , Andy Lutomirski , "# v3 . 10+" References: <20170622151040.16231-1-rkrcmar@redhat.com> From: Paolo Bonzini Message-ID: Date: Tue, 27 Jun 2017 10:20:51 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: stable-owner@vger.kernel.org List-ID: On 27/06/2017 05:41, Wanpeng Li wrote: >> KVM emulates syscall so that it can trap 32-bit syscall on Intel processors. > > We have a discussion to not expose syscall/sysret to Intel 32-bit > guest two years ago. https://lkml.org/lkml/2015/11/19/225 The > syscall/sysret just makes sense against long mode instead of > compatibility/legacy mode of Intel CPU. We will get a #UD in 32-bit > guest, and syscall emulation is introduced by commit 66bb2ccd (KVM: > x86 emulator: add syscall emulation) to handle it. So why we still > expose syscall/sysret to Intel 32-bit guest? Because you didn't post v2 of that patch, I guess. :) >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index 87d3cb901935..0e846f0cb83b 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -5313,6 +5313,8 @@ static void init_emulate_ctxt(struct kvm_vcpu *vcpu) >> kvm_x86_ops->get_cs_db_l_bits(vcpu, &cs_db, &cs_l); >> >> ctxt->eflags = kvm_get_rflags(vcpu); >> + ctxt->tf = (ctxt->eflags & X86_EFLAGS_TF) != 0; >> + > > I guess this is used for "the sysret is executed the #DB is taken "as > if" the syscall insn just completed", however, there is no sysret > emulation, so how the #DB is taken after the sysret? No, it's used for instructions other than syscall and sysret: > + if (r == EMULATE_DONE && > + (ctxt->tf || (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP))) > + kvm_vcpu_do_singlestep(vcpu, &r); syscall (and sysret if it were emulated) overwrite ctxt->tf with the value of TF at the end of the instruction. Other instructions don't, so that singlestep depends on EFLAGS.TF before the instruction is executed. Paolo