All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xenproject.org>
Cc: "Juergen Gross" <jgross@suse.com>,
	"Kevin Tian" <kevin.tian@intel.com>,
	"Jan Beulich" <JBeulich@suse.com>, "Wei Liu" <wl@xen.org>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Jun Nakajima" <jun.nakajima@intel.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: [Xen-devel] [PATCH] x86/svm: Fix handling of EFLAGS.RF on task switch
Date: Tue, 3 Dec 2019 22:30:46 +0000	[thread overview]
Message-ID: <20191203223046.15011-1-andrew.cooper3@citrix.com> (raw)

VT-x updates RF before vmexit, so eflags written into the outgoing TSS happens
to be correct.  SVM does not update RF before vmexit, and instead provides it
via a bit in exitinfo2.

In practice, needing RF set in the outgoing state occurs when a task gate is
used to handle faults.

Extend hvm_task_switch() with an extra_eflags parameter which gets fed into
the outgoing TSS, and fill it in suitably from the SVM vmexit information.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Jun Nakajima <jun.nakajima@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
CC: Juergen Gross <jgross@suse.com>

Kevin: There is no help in the SDM about this.  RF is not mentioned in the
list of state either modified or unmodified by hardware on a task switch
vmexit.  This conclusion has been drawn from looking at the actual VMExit
state given an XTF test poking every corner of TASK_SWITCH VMExits.

Juergen: I know its getting stupidly late in the day, but this, like the
previous fixes, want backporting.  OTOH, the likelihood of not fixing it
causing harm to VMs is minimal, unlike the earlier task switch fixes.
---
 xen/arch/x86/hvm/hvm.c        | 4 ++--
 xen/arch/x86/hvm/svm/svm.c    | 3 ++-
 xen/arch/x86/hvm/vmx/vmx.c    | 3 ++-
 xen/include/asm-x86/hvm/hvm.h | 2 +-
 4 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 7f556171bd..47573f71b8 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2913,7 +2913,7 @@ void hvm_prepare_vm86_tss(struct vcpu *v, uint32_t base, uint32_t limit)
 
 void hvm_task_switch(
     uint16_t tss_sel, enum hvm_task_switch_reason taskswitch_reason,
-    int32_t errcode, unsigned int insn_len)
+    int32_t errcode, unsigned int insn_len, unsigned int extra_eflags)
 {
     struct vcpu *v = current;
     struct cpu_user_regs *regs = guest_cpu_user_regs();
@@ -2988,7 +2988,7 @@ void hvm_task_switch(
         eflags &= ~X86_EFLAGS_NT;
 
     tss.eip    = regs->eip + insn_len;
-    tss.eflags = eflags;
+    tss.eflags = eflags | extra_eflags;
     tss.eax    = regs->eax;
     tss.ecx    = regs->ecx;
     tss.edx    = regs->edx;
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 0fb1908c18..6ae43999ff 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2812,7 +2812,8 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
         if ( (vmcb->exitinfo2 >> 44) & 1 )
             errcode = (uint32_t)vmcb->exitinfo2;
 
-        hvm_task_switch(vmcb->exitinfo1, reason, errcode, insn_len);
+        hvm_task_switch(vmcb->exitinfo1, reason, errcode, insn_len,
+                        (vmcb->exitinfo2 & (1ul << 48)) ? X86_EFLAGS_RF : 0);
         break;
     }
 
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 7450cbe40d..bafc3b30c5 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3963,7 +3963,8 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
         else
              ecode = -1;
 
-        hvm_task_switch(exit_qualification, reasons[source], ecode, inst_len);
+        hvm_task_switch(exit_qualification, reasons[source], ecode, inst_len,
+                        0 /* EFLAGS.RF already updated. */);
         break;
     }
     case EXIT_REASON_CPUID:
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 17fb7efa6e..1d7b66f927 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -296,7 +296,7 @@ void hvm_set_rdtsc_exiting(struct domain *d, bool_t enable);
 enum hvm_task_switch_reason { TSW_jmp, TSW_iret, TSW_call_or_int };
 void hvm_task_switch(
     uint16_t tss_sel, enum hvm_task_switch_reason taskswitch_reason,
-    int32_t errcode, unsigned int insn_len);
+    int32_t errcode, unsigned int insn_len, unsigned int extra_eflags);
 
 enum hvm_access_type {
     hvm_access_insn_fetch,
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

             reply	other threads:[~2019-12-03 22:31 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-03 22:30 Andrew Cooper [this message]
2019-12-04  9:10 ` [Xen-devel] [PATCH] x86/svm: Fix handling of EFLAGS.RF on task switch Jan Beulich
2019-12-10  7:05 ` Tian, Kevin
2019-12-10  7:09 ` Jürgen Groß

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=20191203223046.15011-1-andrew.cooper3@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=jgross@suse.com \
    --cc=jun.nakajima@intel.com \
    --cc=kevin.tian@intel.com \
    --cc=roger.pau@citrix.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.