[PATCH] x86/hvm: Drop get_shadow_gs_base() hook and use hvm_get_reg()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

This is a trivial accessor for an MSR, so use hvm_get_reg() rather than a
dedicated hook.  In arch_get_info_guest(), rework the logic to read GS_SHADOW
only once.

get_hvm_registers() is called on current, meaning that diagnostics print a
stale GS_SHADOW from the previous vcpu context switch.  Adjust both
implementations to obtain the correct value.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Wei Liu <wl@xxxxxxx>
CC: Jun Nakajima <jun.nakajima@xxxxxxxxx>
CC: Kevin Tian <kevin.tian@xxxxxxxxx>

If we care to backport the bugfix aspect, a far less invasive option would be
to read MSR_SHADOW_GS_BASE directly.

The only case where that goes wrong is when vmcb->kerngsbase has been modified
and is pending a VMLOAD.  I'm fairly sure this can only occur when we need
vcpu diagnostics, after an emulated write of MSR_SHADOW_GS_BASE.
---
 xen/arch/x86/domctl.c              |  8 ++++++--
 xen/arch/x86/hvm/svm/svm.c         | 12 ++++++------
 xen/arch/x86/hvm/vmx/vmx.c         | 16 ++++++++++------
 xen/arch/x86/include/asm/hvm/hvm.h |  7 -------
 xen/arch/x86/x86_64/traps.c        |  2 +-
 5 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 7d102e0647ec..e49f9e91b9fa 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1447,6 +1447,7 @@ void arch_get_info_guest(struct vcpu *v, 
vcpu_guest_context_u c)
     if ( is_hvm_domain(d) )
     {
         struct segment_register sreg;
+        unsigned long gs_shadow;
 
         c.nat->ctrlreg[0] = v->arch.hvm.guest_cr[0];
         c.nat->ctrlreg[2] = v->arch.hvm.guest_cr[2];
@@ -1465,15 +1466,18 @@ void arch_get_info_guest(struct vcpu *v, 
vcpu_guest_context_u c)
         c.nat->fs_base = sreg.base;
         hvm_get_segment_register(v, x86_seg_gs, &sreg);
         c.nat->user_regs.gs = sreg.sel;
+
+        gs_shadow = hvm_get_reg(v, MSR_SHADOW_GS_BASE);
+
         if ( ring_0(&c.nat->user_regs) )
         {
             c.nat->gs_base_kernel = sreg.base;
-            c.nat->gs_base_user = hvm_get_shadow_gs_base(v);
+            c.nat->gs_base_user = gs_shadow;
         }
         else
         {
             c.nat->gs_base_user = sreg.base;
-            c.nat->gs_base_kernel = hvm_get_shadow_gs_base(v);
+            c.nat->gs_base_kernel = gs_shadow;
         }
     }
     else
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index bb6b8e560a9f..e3bc88e78058 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -752,11 +752,6 @@ static void svm_set_segment_register(struct vcpu *v, enum 
x86_segment seg,
     }
 }
 
-static unsigned long svm_get_shadow_gs_base(struct vcpu *v)
-{
-    return v->arch.hvm.svm.vmcb->kerngsbase;
-}
-
 static int svm_set_guest_pat(struct vcpu *v, u64 gpat)
 {
     struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
@@ -2471,10 +2466,16 @@ static bool svm_get_pending_event(struct vcpu *v, 
struct x86_event *info)
 
 static uint64_t svm_get_reg(struct vcpu *v, unsigned int reg)
 {
+    struct vcpu *curr = current;
     struct domain *d = v->domain;
 
     switch ( reg )
     {
+    case MSR_SHADOW_GS_BASE:
+        if ( v == curr )
+            svm_sync_vmcb(v, vmcb_in_sync);
+        return v->arch.hvm.svm.vmcb->kerngsbase;
+
     default:
         printk(XENLOG_G_ERR "%s(%pv, 0x%08x) Bad register\n",
                __func__, v, reg);
@@ -2513,7 +2514,6 @@ static struct hvm_function_table __initdata 
svm_function_table = {
     .get_cpl              = svm_get_cpl,
     .get_segment_register = svm_get_segment_register,
     .set_segment_register = svm_set_segment_register,
-    .get_shadow_gs_base   = svm_get_shadow_gs_base,
     .update_guest_cr      = svm_update_guest_cr,
     .update_guest_efer    = svm_update_guest_efer,
     .cpuid_policy_changed = svm_cpuid_policy_changed,
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index c44cf8f5d425..27c36af6027d 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1183,11 +1183,6 @@ static void vmx_set_segment_register(struct vcpu *v, 
enum x86_segment seg,
     vmx_vmcs_exit(v);
 }
 
-static unsigned long vmx_get_shadow_gs_base(struct vcpu *v)
-{
-    return v->arch.hvm.vmx.shadow_gs;
-}
-
 static int vmx_set_guest_pat(struct vcpu *v, u64 gpat)
 {
     if ( !paging_mode_hap(v->domain) ||
@@ -2401,6 +2396,7 @@ static int vmtrace_reset(struct vcpu *v)
 
 static uint64_t vmx_get_reg(struct vcpu *v, unsigned int reg)
 {
+    struct vcpu *curr = current;
     struct domain *d = v->domain;
     uint64_t val = 0;
     int rc;
@@ -2417,6 +2413,15 @@ static uint64_t vmx_get_reg(struct vcpu *v, unsigned int 
reg)
             domain_crash(d);
         }
         return val;
+
+    case MSR_SHADOW_GS_BASE:
+        if ( v == curr )
+        {
+            rdmsrl(MSR_SHADOW_GS_BASE, val);
+            return val;
+        }
+        else
+            return v->arch.hvm.vmx.shadow_gs;
     }
 
     /* Logic which maybe requires remote VMCS acquisition. */
@@ -2489,7 +2494,6 @@ static struct hvm_function_table __initdata 
vmx_function_table = {
     .get_cpl              = _vmx_get_cpl,
     .get_segment_register = vmx_get_segment_register,
     .set_segment_register = vmx_set_segment_register,
-    .get_shadow_gs_base   = vmx_get_shadow_gs_base,
     .update_host_cr3      = vmx_update_host_cr3,
     .update_guest_cr      = vmx_update_guest_cr,
     .update_guest_efer    = vmx_update_guest_efer,
diff --git a/xen/arch/x86/include/asm/hvm/hvm.h 
b/xen/arch/x86/include/asm/hvm/hvm.h
index 842f98763c4b..feb9d221a6a6 100644
--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -128,7 +128,6 @@ struct hvm_function_table {
                                  struct segment_register *reg);
     void (*set_segment_register)(struct vcpu *v, enum x86_segment seg,
                                  struct segment_register *reg);
-    unsigned long (*get_shadow_gs_base)(struct vcpu *v);
 
     /* 
      * Re-set the value of CR3 that Xen runs on when handling VM exits.
@@ -469,11 +468,6 @@ hvm_get_cpl(struct vcpu *v)
     return alternative_call(hvm_funcs.get_cpl, v);
 }
 
-static inline unsigned long hvm_get_shadow_gs_base(struct vcpu *v)
-{
-    return alternative_call(hvm_funcs.get_shadow_gs_base, v);
-}
-
 #define has_hvm_params(d) \
     ((d)->arch.hvm.params != NULL)
 
@@ -753,7 +747,6 @@ void hvm_set_reg(struct vcpu *v, unsigned int reg, uint64_t 
val);
  * needed because DCE will kick in.
  */
 int hvm_guest_x86_mode(struct vcpu *v);
-unsigned long hvm_get_shadow_gs_base(struct vcpu *v);
 void hvm_cpuid_policy_changed(struct vcpu *v);
 void hvm_set_tsc_offset(struct vcpu *v, uint64_t offset, uint64_t at_tsc);
 
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index d661d7ffcaaf..d97bf07b22bc 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -79,7 +79,7 @@ static void get_hvm_registers(struct vcpu *v, struct 
cpu_user_regs *regs,
     hvm_get_segment_register(v, x86_seg_ss, &sreg);
     regs->ss = sreg.sel;
 
-    crs[7] = hvm_get_shadow_gs_base(v);
+    crs[7] = hvm_get_reg(v, MSR_SHADOW_GS_BASE);
 }
 
 static void _show_registers(
-- 
2.11.0