From: Paul Durrant <Paul.Durrant@citrix.com>
To: Xen-devel <xen-devel@lists.xen.org>
Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>,
"Tim (Xen.org)" <tim@xen.org>, Jan Beulich <JBeulich@suse.com>
Subject: Re: [PATCH v3 11/24] x86/emul: Implement singlestep as a retire flag
Date: Wed, 30 Nov 2016 14:28:09 +0000 [thread overview]
Message-ID: <dca07173b514429aa018693d7b28f7cd@AMSPEX02CL03.citrite.net> (raw)
In-Reply-To: <1480513841-7565-12-git-send-email-andrew.cooper3@citrix.com>
> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: 30 November 2016 13:50
> To: Xen-devel <xen-devel@lists.xen.org>
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Jan Beulich
> <JBeulich@suse.com>; Tim (Xen.org) <tim@xen.org>; Paul Durrant
> <Paul.Durrant@citrix.com>
> Subject: [PATCH v3 11/24] x86/emul: Implement singlestep as a retire flag
>
> 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.
>
> Instead, use a retire flag to indicate singlestep, which causes x86_emulate()
> to return X86EMUL_OKAY.
>
> Update all callers of x86_emulate() to use the new retire flag. This fixes
> the behaviour of singlestep for shadow pagetable updates and
> mmcfg/mmio_ro
> intercepts, which previously discarded the exception.
>
> With this change, all uses of X86EMUL_EXCEPTION from x86_emulate() are
> believed to have strictly fault semantics.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Tim Deegan <tim@xen.org>
> CC: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
>
> v3:
> * New
> ---
> xen/arch/x86/hvm/emulate.c | 3 +++
> xen/arch/x86/mm.c | 11 ++++++++++-
> xen/arch/x86/mm/shadow/multi.c | 21 ++++++++++++++++++++-
> xen/arch/x86/x86_emulate/x86_emulate.c | 9 ++++-----
> xen/arch/x86/x86_emulate/x86_emulate.h | 6 ++++++
> 5 files changed, 43 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index fe62500..91c79fa 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -1788,6 +1788,9 @@ static int _hvm_emulate_one(struct
> hvm_emulate_ctxt *hvmemul_ctxt,
> if ( rc != X86EMUL_OKAY )
> return rc;
>
> + if ( hvmemul_ctxt->ctxt.retire.singlestep )
> + hvm_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
> +
> new_intr_shadow = hvmemul_ctxt->intr_shadow;
>
> /* MOV-SS instruction toggles MOV-SS shadow, else we just clear it. */
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index b7c7122..231c7bf 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -5382,6 +5382,9 @@ int ptwr_do_page_fault(struct vcpu *v, unsigned
> long addr,
> if ( rc == X86EMUL_UNHANDLEABLE )
> goto bail;
>
> + if ( ptwr_ctxt.ctxt.retire.singlestep )
> + pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
> +
> perfc_incr(ptwr_emulations);
> return EXCRET_fault_fixed;
>
> @@ -5503,7 +5506,13 @@ int mmio_ro_do_page_fault(struct vcpu *v,
> unsigned long addr,
> else
> rc = x86_emulate(&ctxt, &mmio_ro_emulate_ops);
>
> - return rc != X86EMUL_UNHANDLEABLE ? EXCRET_fault_fixed : 0;
> + if ( rc == X86EMUL_UNHANDLEABLE )
> + return 0;
> +
> + if ( ctxt.retire.singlestep )
> + pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
> +
> + return EXCRET_fault_fixed;
> }
>
> void *alloc_xen_pagetable(void)
> diff --git a/xen/arch/x86/mm/shadow/multi.c
> b/xen/arch/x86/mm/shadow/multi.c
> index 9ee48a8..ddfb815 100644
> --- 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;
> + }
> +
> #if GUEST_PAGING_LEVELS == 3 /* PAE guest */
> if ( r == X86EMUL_OKAY ) {
> int i, emulation_count=0;
> @@ -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 )
> {
> emulation_count++;
> if ( v->arch.paging.last_write_was_pt )
> @@ -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! */
> }
> }
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
> b/xen/arch/x86/x86_emulate/x86_emulate.c
> index 8a1f1f5..0af532e 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -2417,7 +2417,6 @@ x86_emulate(
> struct x86_emulate_state state;
> int rc;
> uint8_t b, d;
> - bool tf = ctxt->regs->eflags & EFLG_TF;
> struct operand src = { .reg = PTR_POISON };
> struct operand dst = { .reg = PTR_POISON };
> enum x86_swint_type swint_type;
> @@ -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;
>
> - /* Inject #DB if single-step tracing was enabled at instruction start. */
> - if ( tf && (rc == X86EMUL_OKAY) && ops->inject_hw_exception )
> - rc = ops->inject_hw_exception(EXC_DB, -1, ctxt) ? :
> X86EMUL_EXCEPTION;
> + *ctxt->regs = _regs;
>
> done:
> _put_fpu();
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h
> b/xen/arch/x86/x86_emulate/x86_emulate.h
> index fc28976..da8924b 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.h
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.h
> @@ -483,6 +483,7 @@ struct x86_emulate_ctxt
> bool hlt:1; /* Instruction HLTed. */
> bool mov_ss:1; /* Instruction sets MOV-SS irq shadow. */
> bool sti:1; /* Instruction sets STI irq shadow. */
> + bool singlestep:1; /* Singlestepping was active. */
> };
> } retire;
> };
> @@ -572,12 +573,17 @@ static inline int x86_emulate_wrapper(
> struct x86_emulate_ctxt *ctxt,
> const struct x86_emulate_ops *ops)
> {
> + unsigned long orig_eip = ctxt->regs->eip;
> int rc = x86_emulate(ctxt, ops);
>
> /* Retire flags should only be set for successful instruction emulation. */
> if ( rc != X86EMUL_OKAY )
> ASSERT(ctxt->retire.raw == 0);
>
> + /* All cases returning X86EMUL_EXCEPTION should have fault semantics.
> */
> + if ( rc == X86EMUL_EXCEPTION )
> + ASSERT(ctxt->regs->eip == orig_eip);
> +
> return rc;
> }
>
> --
> 2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
next prev parent reply other threads:[~2016-11-30 14:28 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-30 13:50 [PATCH for-4.9 v3 00/24] XSA-191 followup Andrew Cooper
2016-11-30 13:50 ` [PATCH v3 01/24] x86/shadow: Fix #PFs from emulated writes crossing a page boundary Andrew Cooper
2016-11-30 13:50 ` [PATCH v3 02/24] x86/emul: Drop X86EMUL_CMPXCHG_FAILED Andrew Cooper
2016-11-30 13:50 ` [PATCH v3 03/24] x86/emul: Simplfy emulation state setup Andrew Cooper
2016-12-08 6:34 ` George Dunlap
2016-11-30 13:50 ` [PATCH v3 04/24] x86/emul: Rename hvm_trap to x86_event and move it into the emulation infrastructure Andrew Cooper
2016-11-30 13:50 ` [PATCH v3 05/24] x86/emul: Rename HVM_DELIVER_NO_ERROR_CODE to X86_EVENT_NO_EC Andrew Cooper
2016-11-30 13:50 ` [PATCH v3 06/24] x86/pv: Implement pv_inject_{event, page_fault, hw_exception}() Andrew Cooper
2016-12-01 10:06 ` Jan Beulich
2016-11-30 13:50 ` [PATCH v3 07/24] x86/emul: Clean up the naming of the retire union Andrew Cooper
2016-11-30 13:58 ` Paul Durrant
2016-11-30 14:02 ` Andrew Cooper
2016-11-30 14:05 ` Paul Durrant
2016-11-30 16:43 ` Jan Beulich
2016-12-01 10:08 ` Jan Beulich
2016-11-30 13:50 ` [PATCH v3 08/24] x86/emul: Correct the behaviour of pop %ss and interrupt shadowing Andrew Cooper
2016-12-01 10:18 ` Jan Beulich
2016-12-01 10:51 ` Andrew Cooper
2016-12-01 11:19 ` Jan Beulich
2016-11-30 13:50 ` [PATCH v3 09/24] x86/emul: Provide a wrapper to x86_emulate() to ASSERT() certain behaviour Andrew Cooper
2016-12-01 10:40 ` Jan Beulich
2016-12-01 10:58 ` Andrew Cooper
2016-12-01 11:21 ` Jan Beulich
2016-11-30 13:50 ` [PATCH v3 10/24] x86/emul: Always use fault semantics for software events Andrew Cooper
2016-11-30 17:55 ` Boris Ostrovsky
2016-12-01 10:53 ` Jan Beulich
2016-12-01 11:15 ` Andrew Cooper
2016-12-01 11:23 ` Jan Beulich
2016-11-30 13:50 ` [PATCH v3 11/24] x86/emul: Implement singlestep as a retire flag Andrew Cooper
2016-11-30 14:28 ` Paul Durrant [this message]
2016-12-01 11:16 ` Jan Beulich
2016-12-01 11:23 ` Andrew Cooper
2016-12-01 11:33 ` Tim Deegan
2016-12-01 12:05 ` Jan Beulich
2016-11-30 13:50 ` [PATCH v3 12/24] x86/emul: Remove opencoded exception generation Andrew Cooper
2016-11-30 13:50 ` [PATCH v3 13/24] x86/emul: Rework emulator event injection Andrew Cooper
2016-11-30 14:26 ` Paul Durrant
2016-12-01 11:35 ` Tim Deegan
2016-12-01 12:31 ` Jan Beulich
2016-11-30 13:50 ` [PATCH v3 14/24] x86/vmx: Use hvm_{get, set}_segment_register() rather than vmx_{get, set}_segment_register() Andrew Cooper
2016-11-30 13:50 ` [PATCH v3 15/24] x86/hvm: Reposition the modification of raw segment data from the VMCB/VMCS Andrew Cooper
2016-11-30 13:50 ` [PATCH v3 16/24] x86/emul: Avoid raising faults behind the emulators back Andrew Cooper
2016-11-30 13:50 ` [PATCH v3 17/24] x86/pv: " Andrew Cooper
2016-12-01 11:50 ` Tim Deegan
2016-12-01 12:57 ` Jan Beulich
2016-12-01 13:12 ` Andrew Cooper
2016-12-01 13:27 ` Jan Beulich
2016-11-30 13:50 ` [PATCH v3 18/24] x86/shadow: " Andrew Cooper
2016-12-01 11:39 ` Tim Deegan
2016-12-01 11:40 ` Andrew Cooper
2016-12-01 13:00 ` Jan Beulich
2016-12-01 13:15 ` Andrew Cooper
2016-11-30 13:50 ` [PATCH v3 19/24] x86/hvm: Extend the hvm_copy_*() API with a pagefault_info pointer Andrew Cooper
2016-11-30 13:50 ` [PATCH v3 20/24] x86/hvm: Reimplement hvm_copy_*_nofault() in terms of no pagefault_info Andrew Cooper
2016-11-30 13:50 ` [PATCH v3 21/24] x86/hvm: Rename hvm_copy_*_guest_virt() to hvm_copy_*_guest_linear() Andrew Cooper
2016-11-30 13:50 ` [PATCH v3 22/24] x86/hvm: Avoid __hvm_copy() raising #PF behind the emulators back Andrew Cooper
2016-11-30 14:29 ` Paul Durrant
2016-11-30 13:50 ` [PATCH v3 23/24] x86/emul: Prepare to allow use of system segments for memory references Andrew Cooper
2016-11-30 13:50 ` [PATCH v3 24/24] x86/emul: Use system-segment relative memory accesses Andrew Cooper
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=dca07173b514429aa018693d7b28f7cd@AMSPEX02CL03.citrite.net \
--to=paul.durrant@citrix.com \
--cc=Andrew.Cooper3@citrix.com \
--cc=JBeulich@suse.com \
--cc=tim@xen.org \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).