[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC v1 3/7] x86/emul: Separate out instruction completion


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
  • Date: Thu, 21 May 2026 10:59:32 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=eDEJxPnWZRZ1I1hMe9c7ZBpGFl48edlwTI0Es+xq44E=; b=mqll3hVgk+CZO4oUTPcNLMv7mD9tKXFzPaYwHf6GhqlDbtn6dkEef8GcO4AWjfahtctDnPbj+N9OnanF4CJL8CKLp08kfK5UjcIB7C4PHLDXwNyQ0JjRvvnyza62Fh/n0/diK0qMuyom6rzjZfY5Et/ci5By2jfiBzhcD3uAGwRQ2ONOV5KLIrwsw8ZZ6cR2Idvuqhag2csmaWjxSrp4NhgqlKrSZF+9oodY4wR2epylzZ53KvwUrpbISr07hyWtpgK0dpmrXduS5SMGxB6DDWGD/y+EdZUXxrsHg87tNzlrD7QeMkoiM3xpISk3Yx6l5ypFc3YwzFB0e6rZStwOLw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=Wv/yM2lobqSxCfkP/YeuFSEHurgv/LOnWwKViSgzfJxIBTDLv+pYGdsB1nRYN3QX8cB2utpGNzAnBQZjv0XZqv/x91hXHb5oTvjtREs6U3RyWKjx67mqouij6WkuPBJ2w3KdVCvMYQOyQxKtHWDb7SPudKZlXn98IopspySTxtlm4P1va70ZjHGU0lB7JXhZdoMjXswhCH3uBBE91qLe8LSZst1ix/bZGgJqJu7ge/NrC/uG9awsMY5eetYDsyomAK0GeNUw91pu3244bSgFEDNLCmCp+nuzuKlB+G9eo9J0dRjb4uZrnMP4DwuApZcpVFNOKY0DVx4lrDqvNgP9Vw==
  • Authentication-results: eu.smtp.expurgate.cloud; dkim=pass header.s=selector1 header.d=citrix.com header.i="@citrix.com" header.h="From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck"
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Teddy Astie <teddy.astie@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Thu, 21 May 2026 09:59:49 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 5/19/26 11:08 AM, Jan Beulich wrote:
On 18.05.2026 15:14, Ross Lagerwall wrote:
To support merging the emulated instruction and VMEXIT paths, split out
instruction completion from the core x86 emulation so it can be called
separately.

You don't mean to use a full-fledged struct x86_emulate_ctxt on the VMEXIT
paths, do you?

For VMEXITs it uses a struct hvm_emulate_ctxt which contains struct
x86_emulate_ctxt but without going through the full emulator. See patch 4 for
details (probably makes sense to discuss this in the context of that patch).


@@ -1265,8 +1281,6 @@ x86_emulate(
      uint8_t b, d, *opc = NULL;
      unsigned int first_byte = 0, elem_bytes, insn_bytes = 0;
      uint64_t op_mask = ~0ULL;
-    bool singlestep = (_regs.eflags & X86_EFLAGS_TF) &&
-           !is_branch_step(ctxt, ops);

Nit: Yes, indentation was screwed here. Please ...

@@ -1280,6 +1294,9 @@ x86_emulate(
init_context(ctxt); + ctxt->singlestep = (_regs.eflags & X86_EFLAGS_TF) &&
+           !is_branch_step(ctxt, ops);

... correct such when moving code around.

OK.


@@ -8347,17 +8364,6 @@ x86_emulate(
      put_fpu(fpu_type, false, state, ctxt, ops);
      fpu_type = X86EMUL_FPU_none;
- /* Zero the upper 32 bits of %rip if not in 64-bit mode. */
-    if ( !mode_64bit() )
-        _regs.r(ip) = (uint32_t)_regs.r(ip);

While, because you have the new helper update ctxt->regs, the removal of
this update of _regs looks technically okay, ...

-    /* Should a singlestep #DB be raised? */
-    if ( rc == X86EMUL_OKAY && singlestep && !ctxt->retire.mov_ss )
-    {
-        ctxt->retire.singlestep = true;
-        ctxt->retire.sti = false;
-    }
-
      if ( rc != X86EMUL_DONE )
          *ctxt->regs = _regs;

... further uses of _regs here (not this one, but in general) would be at
risk of no longer working as expected (yet only in a corner case, so
possibly not covered by testing).

Any uses of _regs after assignment back to the context is surely broken anyway
because nothing outside of this function would see the result? Not that I would
expect that to happen anyway since this is essentially the end of the function.

Ross



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.