[Xen-devel] [PATCH 2/3] x86/msr: Drop {MISC_ENABLES, PLATFORM_INFO}.available

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

These MSRs are non-architectural and the available booleans were used in lieu
of an architectural signal of availability.  The MSRs are unconditionally
available to HVM guests, but currently for PV guests, are hidden when CPUID
faulting is unavailable.

However, in hindsight, the additional booleans make toolstack MSR interactions
more complicated.  As the behaviour of the MSRs is reserved when unavailable,
unconditionally letting the MSRs be accessible is compatible behaviour, even
for PV guests.

The new behaviour is:
  * PLATFORM_INFO is unconditionally readable even for PV guests and will
    indicate the presense or absense of CPUID Faulting in bit 31.
  * MISC_FEATURES_ENABLES is uncondtionally readable, and bit 0 may be set iff
    PLATFORM_INFO reports that CPUID Faulting is available.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>
---
 xen/arch/x86/cpu/common.c |  1 -
 xen/arch/x86/msr.c        | 30 +-----------------------------
 xen/include/asm-x86/msr.h |  2 --
 3 files changed, 1 insertion(+), 32 deletions(-)

diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 3548b12..b9ff6f8 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -119,7 +119,6 @@ bool __init probe_cpuid_faulting(void)
        {
                struct msr_domain_policy *dp = &raw_msr_domain_policy;
 
-               dp->plaform_info.available = true;
                if (val & MSR_PLATFORM_INFO_CPUID_FAULTING)
                        dp->plaform_info.cpuid_faulting = true;
        }
diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index 1e12ccb..6599f10 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -54,35 +54,21 @@ static void __init calculate_host_policy(void)
 static void __init calculate_hvm_max_policy(void)
 {
     struct msr_domain_policy *dp = &hvm_max_msr_domain_policy;
-    struct msr_vcpu_policy *vp = &hvm_max_msr_vcpu_policy;
 
     if ( !hvm_enabled )
         return;
 
     *dp = host_msr_domain_policy;
 
-    /* 0x000000ce  MSR_INTEL_PLATFORM_INFO */
     /* It's always possible to emulate CPUID faulting for HVM guests */
-    if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL ||
-         boot_cpu_data.x86_vendor == X86_VENDOR_AMD )
-    {
-        dp->plaform_info.available = true;
-        dp->plaform_info.cpuid_faulting = true;
-    }
-
-    /* 0x00000140  MSR_INTEL_MISC_FEATURES_ENABLES */
-    vp->misc_features_enables.available = dp->plaform_info.cpuid_faulting;
+    dp->plaform_info.cpuid_faulting = true;
 }
 
 static void __init calculate_pv_max_policy(void)
 {
     struct msr_domain_policy *dp = &pv_max_msr_domain_policy;
-    struct msr_vcpu_policy *vp = &pv_max_msr_vcpu_policy;
 
     *dp = host_msr_domain_policy;
-
-    /* 0x00000140  MSR_INTEL_MISC_FEATURES_ENABLES */
-    vp->misc_features_enables.available = dp->plaform_info.cpuid_faulting;
 }
 
 void __init init_guest_msr_policy(void)
@@ -107,10 +93,7 @@ int init_domain_msr_policy(struct domain *d)
 
     /* See comment in intel_ctxt_switch_levelling() */
     if ( is_control_domain(d) )
-    {
-        dp->plaform_info.available = false;
         dp->plaform_info.cpuid_faulting = false;
-    }
 
     d->arch.msr = dp;
 
@@ -130,10 +113,6 @@ int init_vcpu_msr_policy(struct vcpu *v)
     *vp = is_pv_domain(d) ? pv_max_msr_vcpu_policy :
                             hvm_max_msr_vcpu_policy;
 
-    /* See comment in intel_ctxt_switch_levelling() */
-    if ( is_control_domain(d) )
-        vp->misc_features_enables.available = false;
-
     v->arch.msr = vp;
 
     return 0;
@@ -160,8 +139,6 @@ int guest_rdmsr(const struct vcpu *v, uint32_t msr, 
uint64_t *val)
         break;
 
     case MSR_INTEL_PLATFORM_INFO:
-        if ( !dp->plaform_info.available )
-            goto gp_fault;
         *val = (uint64_t)dp->plaform_info.cpuid_faulting <<
                _MSR_PLATFORM_INFO_CPUID_FAULTING;
         break;
@@ -171,8 +148,6 @@ int guest_rdmsr(const struct vcpu *v, uint32_t msr, 
uint64_t *val)
         goto gp_fault;
 
     case MSR_INTEL_MISC_FEATURES_ENABLES:
-        if ( !vp->misc_features_enables.available )
-            goto gp_fault;
         *val = (uint64_t)vp->misc_features_enables.cpuid_faulting <<
                _MSR_MISC_FEATURES_CPUID_FAULTING;
         break;
@@ -258,9 +233,6 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
     {
         bool old_cpuid_faulting = vp->misc_features_enables.cpuid_faulting;
 
-        if ( !vp->misc_features_enables.available )
-            goto gp_fault;
-
         rsvd = ~0ull;
         if ( dp->plaform_info.cpuid_faulting )
             rsvd &= ~MSR_MISC_FEATURES_CPUID_FAULTING;
diff --git a/xen/include/asm-x86/msr.h b/xen/include/asm-x86/msr.h
index afbeb7f..c28371c 100644
--- a/xen/include/asm-x86/msr.h
+++ b/xen/include/asm-x86/msr.h
@@ -262,7 +262,6 @@ struct msr_domain_policy
 {
     /* 0x000000ce  MSR_INTEL_PLATFORM_INFO */
     struct {
-        bool available; /* This MSR is non-architectural */
         bool cpuid_faulting;
     } plaform_info;
 };
@@ -290,7 +289,6 @@ struct msr_vcpu_policy
 
     /* 0x00000140  MSR_INTEL_MISC_FEATURES_ENABLES */
     struct {
-        bool available; /* This MSR is non-architectural */
         bool cpuid_faulting;
     } misc_features_enables;
 };
-- 
2.1.4


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