From patchwork Thu Dec 1 11:23:14 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Cooper X-Patchwork-Id: 9455865 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id CD48E6074E for ; Thu, 1 Dec 2016 11:25:22 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C0EDA284D3 for ; Thu, 1 Dec 2016 11:25:22 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B077A284CF; Thu, 1 Dec 2016 11:25:22 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 277AA284CF for ; Thu, 1 Dec 2016 11:25:22 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cCPST-0004cG-1p; Thu, 01 Dec 2016 11:23:21 +0000 Received: from mail6.bemta6.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cCPSR-0004c2-Vw for xen-devel@lists.xen.org; Thu, 01 Dec 2016 11:23:20 +0000 Received: from [193.109.254.147] by server-3.bemta-6.messagelabs.com id 16/5C-08948-72800485; Thu, 01 Dec 2016 11:23:19 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrIIsWRWlGSWpSXmKPExsWyU9JRQleNwyH C4PRJTYslHxezODB6HN39mymAMYo1My8pvyKBNePM1l7mgqcyFSsmb2BsYHws2sXIySEh4Cfx +sNWZhBbWMBX4mXLJHYQW0RAWaL312+WLkYuDiGB3YwSX3u/ABWxczALZEl0x4GUsAnoS+x+8 YkJxOYVsJV4u+weUCsHB4uAisTkXn0QU1QgWGLzNjeICkGJkzOfsIDYnAL2ElfuzWUFsZkFDC SOLJoDZctLbH87B+wYIQE1iWv9l9ghjkyXmPish2UCI/8sJKNmIWmfhaR9ASPzKkb14tSistQ iXUu9pKLM9IyS3MTMHF1DAzO93NTi4sT01JzEpGK95PzcTYzA4GMAgh2MdzcFHGKU5GBSEuUt K7GPEOJLyk+pzEgszogvKs1JLT7EKMPBoSTBe4XNIUJIsCg1PbUiLTMHGAcwaQkOHiUR3pMga d7igsTc4sx0iNQpRl2Oac8WP2USYsnLz0uVEuf9AFIkAFKUUZoHNwIWk5cYZaWEeRmBjhLiKU gtys0sQZV/xSjOwagkzMvNDjSFJzOvBG7TK6AjmICO6LhuD3JESSJCSqqBUTf3ypSfIqK/5nD H2+zMS3pgbfjmzbNDHLV8K+8c9PH6FbM0RfbKFdbtmnN7TpYY312zV2Lez8Ue9/u7VWd0nY1L Lk5vff3Xe81qpr9ld3oWX+3eeqr9JxuD/rbte3JEl9yTamW/c18x/fgHt9Y4v+vZGXNlXzc2G DGc+qFyJJ9FpfWARMzZj0osxRmJhlrMRcWJAGPV0qHEAgAA X-Env-Sender: prvs=136206f05=Andrew.Cooper3@citrix.com X-Msg-Ref: server-7.tower-27.messagelabs.com!1480591397!70492375!1 X-Originating-IP: [185.25.65.24] X-SpamReason: No, hits=0.0 required=7.0 tests=received_headers: No Received headers X-StarScan-Received: X-StarScan-Version: 9.0.16; banners=-,-,- X-VirusChecked: Checked Received: (qmail 26262 invoked from network); 1 Dec 2016 11:23:18 -0000 Received: from smtp.eu.citrix.com (HELO SMTP.EU.CITRIX.COM) (185.25.65.24) by server-7.tower-27.messagelabs.com with RC4-SHA encrypted SMTP; 1 Dec 2016 11:23:18 -0000 X-IronPort-AV: E=Sophos;i="5.33,724,1477958400"; d="scan'208";a="35972749" To: Jan Beulich References: <1480513841-7565-1-git-send-email-andrew.cooper3@citrix.com> <1480513841-7565-12-git-send-email-andrew.cooper3@citrix.com> <584014B20200007800124254@prv-mh.provo.novell.com> From: Andrew Cooper Message-ID: <7a1c1196-f3a6-9854-3d9f-31d5969915ca@citrix.com> Date: Thu, 1 Dec 2016 11:23:14 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.4.0 MIME-Version: 1.0 In-Reply-To: <584014B20200007800124254@prv-mh.provo.novell.com> X-ClientProxiedBy: AMSPEX02CAS02.citrite.net (10.69.22.113) To AMSPEX02CL02.citrite.net (10.69.22.126) Cc: Paul Durrant , Tim Deegan , Xen-devel Subject: Re: [Xen-devel] [PATCH v3 11/24] x86/emul: Implement singlestep as a retire flag X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP On 01/12/16 11:16, Jan Beulich wrote: >>>> On 30.11.16 at 14:50, wrote: >> The behaviour of singlestep is to raise #DB after the instruction has been >> completed, but implementing it with inject_hw_exception() causes x86_emulate() >> to return X86EMUL_EXCEPTION, despite succesfully completing execution of the >> instruction, including register writeback. > Nice, I think that'll help simplify the privop patch a bit. > >> --- a/xen/arch/x86/mm/shadow/multi.c >> +++ b/xen/arch/x86/mm/shadow/multi.c >> @@ -3422,6 +3422,16 @@ static int sh_page_fault(struct vcpu *v, >> v->arch.paging.last_write_emul_ok = 0; >> #endif >> >> + if ( emul_ctxt.ctxt.retire.singlestep ) >> + { >> + if ( is_hvm_vcpu(v) ) >> + hvm_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC); >> + else >> + pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC); >> + >> + goto emulate_done; > This results in skipping the PAE special code (which I think is intended) Correct > but also the trace_shadow_emulate(), which I don't think is wanted. Hmm. It is only the PAE case we want to skip. Perhaps changing the PAE entry condition to would be better, along with suitable comments and style fixes? > >> @@ -3433,7 +3443,7 @@ static int sh_page_fault(struct vcpu *v, >> shadow_continue_emulation(&emul_ctxt, regs); >> v->arch.paging.last_write_was_pt = 0; >> r = x86_emulate(&emul_ctxt.ctxt, emul_ops); >> - if ( r == X86EMUL_OKAY ) >> + if ( r == X86EMUL_OKAY && !emul_ctxt.ctxt.retire.raw ) > I think this wants to have a comment attached explaining why > a blanket check of all current and future retire flags here is the > right thing (or benign). Ok. > >> @@ -3449,6 +3459,15 @@ static int sh_page_fault(struct vcpu *v, >> { >> perfc_incr(shadow_em_ex_fail); >> TRACE_SHADOW_PATH_FLAG(TRCE_SFLAG_EMULATION_LAST_FAILED); >> + >> + if ( emul_ctxt.ctxt.retire.singlestep ) >> + { >> + if ( is_hvm_vcpu(v) ) >> + hvm_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC); >> + else >> + pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC); >> + } >> + >> break; /* Don't emulate again if we failed! */ > This comment is now slightly stale. "failed to find the second half of the write". In combination with a suitable comment in the hunk above, this should be fine as is. > >> @@ -5415,11 +5414,11 @@ x86_emulate( >> if ( !mode_64bit() ) >> _regs.eip = (uint32_t)_regs.eip; >> >> - *ctxt->regs = _regs; >> + /* Was singestepping active at the start of this instruction? */ >> + if ( (rc == X86EMUL_OKAY) && (ctxt->regs->eflags & EFLG_TF) ) >> + ctxt->retire.singlestep = true; > Don't we need to avoid doing this when mov_ss is set? Or does the > hardware perhaps do the necessary deferring for us? I am currently reading up about this in the manual. I need more coffee. ~Andrew Acked-by: Tim Deegan diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c index c45d260..28ff945 100644 --- a/xen/arch/x86/mm/shadow/multi.c +++ b/xen/arch/x86/mm/shadow/multi.c @@ -3480,7 +3480,7 @@ static int sh_page_fault(struct vcpu *v, } #if GUEST_PAGING_LEVELS == 3 /* PAE guest */ - if ( r == X86EMUL_OKAY ) { + if ( r == X86EMUL_OKAY && !emul_ctxt.ctxt.retire.raw ) { int i, emulation_count=0; this_cpu(trace_emulate_initial_va) = va; /* Emulate up to four extra instructions in the hope of catching