[PATCH v4 1/3] x86/msr: Dispatch Xen and Viridian MSRs from guest_{wr, rd}msr()

[Andrew Cooper <andrew.cooper3@citrix.com>]

Despite the complicated diff in {svm,vmx}_msr_write_intercept(), it is just
the 0 case losing one level of indentation, as part of removing the call to
wrmsr_hypervisor_regs().

The case blocks in guest_{wr,rd}msr() use raw numbers, partly for consistency
with the CPUID side of things, but mainly because this is clearer code to
follow.  In particular, the Xen block may overlap with the Viridian block if
Viridian is not enabled for the domain, and trying to express this with named
literals caused more confusion that it solved.

Future changes with clean up the individual APIs, including allowing these
MSRs to be usable for vcpus other than current (no callers exist with v !=
current).

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Sergey Dyasli <sergey.dyasli@citrix.com>

v3:
 * Split out of previous series.  Retain appropriate R-by's
v4:
 * Retain switch() for interpreting the result of wrmsr_hypervisor_regs()
---
 xen/arch/x86/hvm/svm/svm.c     | 27 +++-----------------------
 xen/arch/x86/hvm/vmx/vmx.c     | 28 ++++-----------------------
 xen/arch/x86/msr.c             | 43 ++++++++++++++++++++++++++++++++++++++----
 xen/arch/x86/pv/emul-priv-op.c |  6 ------
 4 files changed, 46 insertions(+), 58 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 34d55b4..ef8f271 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2016,10 +2016,6 @@ static int svm_msr_read_intercept(unsigned int msr, uint64_t *msr_content)
         else if ( ret )
             break;
 
-        if ( rdmsr_viridian_regs(msr, msr_content) ||
-             rdmsr_hypervisor_regs(msr, msr_content) )
-            break;
-
         if ( rdmsr_safe(msr, *msr_content) == 0 )
             break;
 
@@ -2218,28 +2214,11 @@ static int svm_msr_write_intercept(unsigned int msr, uint64_t msr_content)
         else if ( ret )
             break;
 
-        if ( wrmsr_viridian_regs(msr, msr_content) )
+        /* Match up with the RDMSR side; ultimately this should go away. */
+        if ( rdmsr_safe(msr, msr_content) == 0 )
             break;
 
-        switch ( wrmsr_hypervisor_regs(msr, msr_content) )
-        {
-        case -ERESTART:
-            result = X86EMUL_RETRY;
-            break;
-        case 0:
-            /*
-             * Match up with the RDMSR side for now; ultimately this entire
-             * case block should go away.
-             */
-            if ( rdmsr_safe(msr, msr_content) == 0 )
-                break;
-            goto gpf;
-        case 1:
-            break;
-        default:
-            goto gpf;
-        }
-        break;
+        goto gpf;
     }
 
     return result;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index b2e1a28..bf90e22 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2965,10 +2965,6 @@ static int vmx_msr_read_intercept(unsigned int msr, uint64_t *msr_content)
             break;
         }
 
-        if ( rdmsr_viridian_regs(msr, msr_content) ||
-             rdmsr_hypervisor_regs(msr, msr_content) )
-            break;
-
         if ( rdmsr_safe(msr, *msr_content) == 0 )
             break;
 
@@ -3249,31 +3245,15 @@ static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content)
         if ( passive_domain_do_wrmsr(msr, msr_content) )
             return X86EMUL_OKAY;
 
-        if ( wrmsr_viridian_regs(msr, msr_content) ) 
-            break;
-
         if ( vmx_write_guest_msr(v, msr, msr_content) == 0 ||
              is_last_branch_msr(msr) )
             break;
 
-        switch ( wrmsr_hypervisor_regs(msr, msr_content) )
-        {
-        case -ERESTART:
-            return X86EMUL_RETRY;
-        case 0:
-            /*
-             * Match up with the RDMSR side for now; ultimately this
-             * entire case block should go away.
-             */
-            if ( rdmsr_safe(msr, msr_content) == 0 )
-                break;
-            goto gp_fault;
-        case 1:
+        /* Match up with the RDMSR side; ultimately this should go away. */
+        if ( rdmsr_safe(msr, msr_content) == 0 )
             break;
-        default:
-            goto gp_fault;
-        }
-        break;
+
+        goto gp_fault;
     }
 
     return X86EMUL_OKAY;
diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index b675f3a..d7c38f1 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -114,9 +114,11 @@ int init_vcpu_msr_policy(struct vcpu *v)
 
 int guest_rdmsr(const struct vcpu *v, uint32_t msr, uint64_t *val)
 {
-    const struct cpuid_policy *cp = v->domain->arch.cpuid;
-    const struct msr_policy *mp = v->domain->arch.msr;
+    const struct domain *d = v->domain;
+    const struct cpuid_policy *cp = d->arch.cpuid;
+    const struct msr_policy *mp = d->arch.msr;
     const struct vcpu_msrs *msrs = v->arch.msrs;
+    int ret = X86EMUL_OKAY;
 
     switch ( msr )
     {
@@ -145,11 +147,25 @@ int guest_rdmsr(const struct vcpu *v, uint32_t msr, uint64_t *val)
         *val = msrs->misc_features_enables.raw;
         break;
 
+    case 0x40000000 ... 0x400001ff:
+        if ( is_viridian_domain(d) )
+        {
+            ret = (rdmsr_viridian_regs(msr, val)
+                   ? X86EMUL_OKAY : X86EMUL_EXCEPTION);
+            break;
+        }
+
+        /* Fallthrough. */
+    case 0x40000200 ... 0x400002ff:
+        ret = (rdmsr_hypervisor_regs(msr, val)
+               ? X86EMUL_OKAY : X86EMUL_EXCEPTION);
+        break;
+
     default:
         return X86EMUL_UNHANDLEABLE;
     }
 
-    return X86EMUL_OKAY;
+    return ret;
 
  gp_fault:
     return X86EMUL_EXCEPTION;
@@ -162,6 +178,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
     const struct cpuid_policy *cp = d->arch.cpuid;
     const struct msr_policy *mp = d->arch.msr;
     struct vcpu_msrs *msrs = v->arch.msrs;
+    int ret = X86EMUL_OKAY;
 
     switch ( msr )
     {
@@ -252,11 +269,29 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
         break;
     }
 
+    case 0x40000000 ... 0x400001ff:
+        if ( is_viridian_domain(d) )
+        {
+            ret = (wrmsr_viridian_regs(msr, val)
+                   ? X86EMUL_OKAY : X86EMUL_EXCEPTION);
+            break;
+        }
+
+        /* Fallthrough. */
+    case 0x40000200 ... 0x400002ff:
+        switch ( wrmsr_hypervisor_regs(msr, val) )
+        {
+        case -ERESTART: ret = X86EMUL_RETRY;     break;
+        case 1:         ret = X86EMUL_OKAY;      break;
+        default:        ret = X86EMUL_EXCEPTION; break;
+        }
+        break;
+
     default:
         return X86EMUL_UNHANDLEABLE;
     }
 
-    return X86EMUL_OKAY;
+    return ret;
 
  gp_fault:
     return X86EMUL_EXCEPTION;
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 45941ea..6422f91 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -950,9 +950,6 @@ static int read_msr(unsigned int reg, uint64_t *val,
         }
         /* fall through */
     default:
-        if ( rdmsr_hypervisor_regs(reg, val) )
-            return X86EMUL_OKAY;
-
         rc = vmce_rdmsr(reg, val);
         if ( rc < 0 )
             break;
@@ -1149,9 +1146,6 @@ static int write_msr(unsigned int reg, uint64_t val,
         }
         /* fall through */
     default:
-        if ( wrmsr_hypervisor_regs(reg, val) == 1 )
-            return X86EMUL_OKAY;
-
         rc = vmce_wrmsr(reg, val);
         if ( rc < 0 )
             break;
-- 
2.1.4


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