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

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@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 7d102e0647ec6d07127410b8ba34985efa4631ed..e49f9e91b9fa291a5e00ec002ce406ea84f64d0b 100644 (file)
--- 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 b80d4af6cb90c46c82deef340188d3652a75ad5a..8869a5de62538a140449c47dd13d3716c0980f30 100644 (file)
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -756,11 +756,6 @@ static void cf_check svm_set_segment_register(
     }
 }
 
-static unsigned long cf_check svm_get_shadow_gs_base(struct vcpu *v)
-{
-    return v->arch.hvm.svm.vmcb->kerngsbase;
-}
-
 static int cf_check svm_set_guest_pat(struct vcpu *v, u64 gpat)
 {
     struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
@@ -2479,6 +2474,7 @@ static bool cf_check svm_get_pending_event(
 
 static uint64_t cf_check svm_get_reg(struct vcpu *v, unsigned int reg)
 {
+    struct vcpu *curr = current;
     const struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
     struct domain *d = v->domain;
 
@@ -2487,6 +2483,11 @@ static uint64_t cf_check svm_get_reg(struct vcpu *v, unsigned int reg)
     case MSR_SPEC_CTRL:
         return vmcb->spec_ctrl;
 
+    case MSR_SHADOW_GS_BASE:
+        if ( v == curr )
+            svm_sync_vmcb(v, vmcb_in_sync);
+        return vmcb->kerngsbase;
+
     default:
         printk(XENLOG_G_ERR "%s(%pv, 0x%08x) Bad register\n",
                __func__, v, reg);
@@ -2530,7 +2531,6 @@ static struct hvm_function_table __initdata_cf_clobber 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 758df3321884405e7700d539ca7a6dc5486925cd..c075370f642ad898fca9c55be9a8f94f369a7431 100644 (file)
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1176,11 +1176,6 @@ static void cf_check vmx_set_segment_register(
     vmx_vmcs_exit(v);
 }
 
-static unsigned long cf_check vmx_get_shadow_gs_base(struct vcpu *v)
-{
-    return v->arch.hvm.vmx.shadow_gs;
-}
-
 static int cf_check vmx_set_guest_pat(struct vcpu *v, u64 gpat)
 {
     if ( !paging_mode_hap(v->domain) ||
@@ -2401,6 +2396,7 @@ static int cf_check vmtrace_reset(struct vcpu *v)
 
 static uint64_t cf_check vmx_get_reg(struct vcpu *v, unsigned int reg)
 {
+    const struct vcpu *curr = current;
     struct domain *d = v->domain;
     uint64_t val = 0;
     int rc;
@@ -2417,6 +2413,12 @@ static uint64_t cf_check vmx_get_reg(struct vcpu *v, unsigned int reg)
             domain_crash(d);
         }
         return val;
+
+    case MSR_SHADOW_GS_BASE:
+        if ( v != curr )
+            return v->arch.hvm.vmx.shadow_gs;
+        rdmsrl(MSR_SHADOW_GS_BASE, val);
+        return val;
     }
 
     /* Logic which maybe requires remote VMCS acquisition. */
@@ -2489,7 +2491,6 @@ static struct hvm_function_table __initdata_cf_clobber 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 b44bbdeb212dfc1ed8772aab518b53729ee886c4..5b7ec0cf69261b23e90ba1df0ccdf35953f39ddf 100644 (file)
--- 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)
 
@@ -759,7 +753,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 fccfb7c172837d6f85b91606c1b6cb190cdb287d..9d7f1f818b9368133f7a546154155b02f8787e61 100644 (file)
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -80,7 +80,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(