All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/5] VMX MSRs policy for Nested Virt: part 1
@ 2018-03-22 11:32 Sergey Dyasli
  2018-03-22 11:32 ` [PATCH v6 1/5] x86/msr: add VMX MSRs definitions and populate Raw domain policy Sergey Dyasli
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Sergey Dyasli @ 2018-03-22 11:32 UTC (permalink / raw)
  To: xen-devel
  Cc: Andrew Cooper, Kevin Tian, Jan Beulich, Jun Nakajima, Sergey Dyasli

The end goal of having VMX MSRs policy is to be able to manage
L1 VMX features. This patch series is the first part of this work.
There is no functional change to what L1 sees in VMX MSRs at this
point. But each domain will have a policy object which allows to
sensibly query what VMX features the domain has. This will unblock
some other nested virtualization work items.

Currently, when nested virt is enabled, the set of L1 VMX features
is fixed and calculated by nvmx_msr_read_intercept() as an intersection
between the full set of Xen's supported L1 VMX features, the set of
actual H/W features and, for MSR_IA32_VMX_EPT_VPID_CAP, the set of
features that Xen uses.

The above makes L1 VMX feature set inconsistent between different H/W
and there is no ability to control what features are available to L1.
The overall set of issues has much in common with CPUID policy.

Part 1 adds VMX MSRs into struct msr_domain_policy and initializes them
during domain creation based on CPUID policy. In the future it should be
possible to independently configure values of VMX MSRs for each domain.

v5 --> v6:
- Various shortenings of control bit names
- Added Reviewed-by: Andrew Cooper to pathes 3,4 and 5
- Other changes are provided on per-patch basis

Sergey Dyasli (5):
  x86/msr: add VMX MSRs definitions and populate Raw domain policy
  x86/msr: add VMX MSRs into HVM_max domain policy
  x86/cpuid: update signature of hvm_cr4_guest_valid_bits()
  x86/msr: update domain policy on CPUID policy changes
  x86/msr: handle VMX MSRs with guest_rd/wrmsr()

 xen/arch/x86/domctl.c              |   1 +
 xen/arch/x86/hvm/domain.c          |   3 +-
 xen/arch/x86/hvm/hvm.c             |   7 +-
 xen/arch/x86/hvm/svm/svmdebug.c    |   4 +-
 xen/arch/x86/hvm/vmx/vmx.c         |  10 +-
 xen/arch/x86/hvm/vmx/vvmx.c        | 178 --------------------
 xen/arch/x86/msr.c                 | 320 +++++++++++++++++++++++++++++++++++
 xen/include/asm-x86/hvm/hvm.h      |   2 +-
 xen/include/asm-x86/hvm/vmx/vvmx.h |   2 -
 xen/include/asm-x86/msr.h          | 333 +++++++++++++++++++++++++++++++++++++
 xen/include/asm-x86/x86-defns.h    |  54 ++++++
 11 files changed, 718 insertions(+), 196 deletions(-)

-- 
2.14.1


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v6 1/5] x86/msr: add VMX MSRs definitions and populate Raw domain policy
  2018-03-22 11:32 [PATCH v6 0/5] VMX MSRs policy for Nested Virt: part 1 Sergey Dyasli
@ 2018-03-22 11:32 ` Sergey Dyasli
  2018-04-02  2:11   ` Tian, Kevin
  2018-03-22 11:32 ` [PATCH v6 2/5] x86/msr: add VMX MSRs into HVM_max " Sergey Dyasli
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Sergey Dyasli @ 2018-03-22 11:32 UTC (permalink / raw)
  To: xen-devel
  Cc: Andrew Cooper, Kevin Tian, Jan Beulich, Jun Nakajima, Sergey Dyasli

New definitions provide a convenient way of accessing contents of
VMX MSRs. They are separated into 5 logical blocks based on the
availability conditions of MSRs in the each block:

    1. vmx: [VMX_BASIC, VMX_VMCS_ENUM]
    2. VMX_PROCBASED_CTLS2
    3. VMX_EPT_VPID_CAP
    4. vmx_true_ctls: [VMX_TRUE_PINBASED_CTLS, VMX_TRUE_ENTRY_CTLS]
    5. VMX_VMFUNC

Every bit value is accessible by its name and bit names match existing
Xen's definitions as close as possible. There is a "raw" 64-bit field
for each MSR as well as "raw" arrays for vmx and vmx_true_ctls blocks.

Add calculate_raw_vmx_policy() which fills Raw policy with H/W values
of VMX MSRs. Host policy will contain a copy of these values (for now).

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
---
v5 --> v6:
- Removed "_bits" and "_based" from union names
- Removed "_exiting" suffixes from control bit names
- Various shortenings of control bit names
---
 xen/arch/x86/msr.c              | 118 ++++++++++++++
 xen/include/asm-x86/msr.h       | 330 ++++++++++++++++++++++++++++++++++++++++
 xen/include/asm-x86/x86-defns.h |  54 +++++++
 3 files changed, 502 insertions(+)

diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index 369b4754ce..87239e151e 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -34,10 +34,65 @@ struct msr_domain_policy __read_mostly     raw_msr_domain_policy,
 struct msr_vcpu_policy __read_mostly hvm_max_msr_vcpu_policy,
                        __read_mostly  pv_max_msr_vcpu_policy;
 
+static bool vmx_procbased_ctls2_available(const struct msr_domain_policy *dp)
+{
+    return dp->vmx.procbased_ctls.allowed_1.secondary;
+}
+
+static bool vmx_ept_vpid_cap_available(const struct msr_domain_policy *dp)
+{
+    return dp->vmx_procbased_ctls2.allowed_1.ept ||
+           dp->vmx_procbased_ctls2.allowed_1.vpid;
+}
+
+static bool vmx_true_ctls_available(const struct msr_domain_policy *dp)
+{
+    return dp->vmx.basic.default1_zero;
+}
+
+static bool vmx_vmfunc_available(const struct msr_domain_policy *dp)
+{
+    return dp->vmx_procbased_ctls2.allowed_1.vmfunc;
+}
+
+static void __init calculate_raw_vmx_policy(struct msr_domain_policy *dp)
+{
+    unsigned int i, start_msr, end_msr;
+
+    if ( !cpu_has_vmx )
+        return;
+
+    start_msr = MSR_IA32_VMX_BASIC;
+    end_msr = MSR_IA32_VMX_VMCS_ENUM;
+    for ( i = start_msr; i <= end_msr; i++ )
+        rdmsrl(i, dp->vmx.raw[i - start_msr]);
+
+    if ( vmx_procbased_ctls2_available(dp) )
+        rdmsrl(MSR_IA32_VMX_PROCBASED_CTLS2, dp->vmx_procbased_ctls2.raw);
+
+    if ( vmx_ept_vpid_cap_available(dp) )
+        rdmsrl(MSR_IA32_VMX_EPT_VPID_CAP, dp->vmx_ept_vpid_cap.raw);
+
+    if ( vmx_true_ctls_available(dp) )
+    {
+        start_msr = MSR_IA32_VMX_TRUE_PINBASED_CTLS;
+        end_msr = MSR_IA32_VMX_TRUE_ENTRY_CTLS;
+        for ( i = start_msr; i <= end_msr; i++ )
+            rdmsrl(i, dp->vmx_true_ctls.raw[i - start_msr]);
+    }
+
+    if ( vmx_vmfunc_available(dp) )
+        rdmsrl(MSR_IA32_VMX_VMFUNC, dp->vmx_vmfunc.raw);
+}
+
 static void __init calculate_raw_policy(void)
 {
+    struct msr_domain_policy *dp = &raw_msr_domain_policy;
+
     /* 0x000000ce  MSR_INTEL_PLATFORM_INFO */
     /* Was already added by probe_cpuid_faulting() */
+
+    calculate_raw_vmx_policy(dp);
 }
 
 static void __init calculate_host_policy(void)
@@ -284,6 +339,69 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
     return X86EMUL_EXCEPTION;
 }
 
+static void __init __maybe_unused build_assertions(void)
+{
+    struct msr_domain_policy dp;
+
+    BUILD_BUG_ON(sizeof(dp.vmx.basic) !=
+                 sizeof(dp.vmx.basic.raw));
+    BUILD_BUG_ON(sizeof(dp.vmx.pinbased_ctls) !=
+                 sizeof(dp.vmx.pinbased_ctls.raw));
+    BUILD_BUG_ON(sizeof(dp.vmx.procbased_ctls) !=
+                 sizeof(dp.vmx.procbased_ctls.raw));
+    BUILD_BUG_ON(sizeof(dp.vmx.exit_ctls) !=
+                 sizeof(dp.vmx.exit_ctls.raw));
+    BUILD_BUG_ON(sizeof(dp.vmx.entry_ctls) !=
+                 sizeof(dp.vmx.entry_ctls.raw));
+    BUILD_BUG_ON(sizeof(dp.vmx.misc) !=
+                 sizeof(dp.vmx.misc.raw));
+    BUILD_BUG_ON(sizeof(dp.vmx.cr0_fixed0) !=
+                 sizeof(dp.vmx.cr0_fixed0.raw));
+    BUILD_BUG_ON(sizeof(dp.vmx.cr0_fixed1) !=
+                 sizeof(dp.vmx.cr0_fixed1.raw));
+    BUILD_BUG_ON(sizeof(dp.vmx.cr4_fixed0) !=
+                 sizeof(dp.vmx.cr4_fixed0.raw));
+    BUILD_BUG_ON(sizeof(dp.vmx.cr4_fixed1) !=
+                 sizeof(dp.vmx.cr4_fixed1.raw));
+    BUILD_BUG_ON(sizeof(dp.vmx.vmcs_enum) !=
+                 sizeof(dp.vmx.vmcs_enum.raw));
+    BUILD_BUG_ON(sizeof(dp.vmx.raw) !=
+                     sizeof(dp.vmx.basic) +
+                     sizeof(dp.vmx.pinbased_ctls) +
+                     sizeof(dp.vmx.procbased_ctls) +
+                     sizeof(dp.vmx.exit_ctls) +
+                     sizeof(dp.vmx.entry_ctls) +
+                     sizeof(dp.vmx.misc) +
+                     sizeof(dp.vmx.cr0_fixed0) +
+                     sizeof(dp.vmx.cr0_fixed1) +
+                     sizeof(dp.vmx.cr4_fixed0) +
+                     sizeof(dp.vmx.cr4_fixed1) +
+                     sizeof(dp.vmx.vmcs_enum));
+
+    BUILD_BUG_ON(sizeof(dp.vmx_procbased_ctls2) !=
+                 sizeof(dp.vmx_procbased_ctls2.raw));
+
+    BUILD_BUG_ON(sizeof(dp.vmx_ept_vpid_cap) !=
+                 sizeof(dp.vmx_ept_vpid_cap.raw));
+
+    BUILD_BUG_ON(sizeof(dp.vmx_true_ctls.pinbased) !=
+                 sizeof(dp.vmx_true_ctls.pinbased.raw));
+    BUILD_BUG_ON(sizeof(dp.vmx_true_ctls.procbased) !=
+                 sizeof(dp.vmx_true_ctls.procbased.raw));
+    BUILD_BUG_ON(sizeof(dp.vmx_true_ctls.exit) !=
+                 sizeof(dp.vmx_true_ctls.exit.raw));
+    BUILD_BUG_ON(sizeof(dp.vmx_true_ctls.entry) !=
+                 sizeof(dp.vmx_true_ctls.entry.raw));
+    BUILD_BUG_ON(sizeof(dp.vmx_true_ctls.raw) !=
+                     sizeof(dp.vmx_true_ctls.pinbased) +
+                     sizeof(dp.vmx_true_ctls.procbased) +
+                     sizeof(dp.vmx_true_ctls.exit) +
+                     sizeof(dp.vmx_true_ctls.entry));
+
+    BUILD_BUG_ON(sizeof(dp.vmx_vmfunc) !=
+                 sizeof(dp.vmx_vmfunc.raw));
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/asm-x86/msr.h b/xen/include/asm-x86/msr.h
index f14f265aa5..5fdf82860e 100644
--- a/xen/include/asm-x86/msr.h
+++ b/xen/include/asm-x86/msr.h
@@ -10,6 +10,7 @@
 #include <xen/errno.h>
 #include <asm/asm_defns.h>
 #include <asm/cpufeature.h>
+#include <asm/x86-defns.h>
 
 #define rdmsr(msr,val1,val2) \
      __asm__ __volatile__("rdmsr" \
@@ -257,6 +258,127 @@ static inline void wrmsr_tsc_aux(uint32_t val)
     }
 }
 
+union vmx_pin_exec_control {
+    uint32_t raw;
+    struct {
+        bool         ext_intr:1;
+        uint32_t             :2;  /* 1:2 reserved */
+        bool              nmi:1;
+        uint32_t             :1;  /* 4 reserved */
+        bool            vnmis:1;
+        bool    preempt_timer:1;
+        bool posted_interrupt:1;
+        uint32_t             :24; /* 8:31 reserved */
+    };
+};
+
+union vmx_cpu_exec_control {
+    uint32_t raw;
+    struct {
+        uint32_t                 :2;  /* 0:1 reserved */
+        bool virtual_intr_pending:1;
+        bool           tsc_offset:1;
+        uint32_t                 :3;  /* 4:6 reserved */
+        bool                  hlt:1;
+        uint32_t                 :1;  /* 8 reserved */
+        bool               invlpg:1;
+        bool                mwait:1;
+        bool                rdpmc:1;
+        bool                rdtsc:1;
+        uint32_t                 :2;  /* 13:14 reserved */
+        bool             cr3_load:1;
+        bool            cr3_store:1;
+        uint32_t                 :2;  /* 17:18 reserved */
+        bool             cr8_load:1;
+        bool            cr8_store:1;
+        bool           tpr_shadow:1;
+        bool         vnmi_pending:1;
+        bool               mov_dr:1;
+        bool            uncond_io:1;
+        bool            io_bitmap:1;
+        uint32_t                 :1;  /* 26 reserved */
+        bool                  mtf:1;
+        bool           msr_bitmap:1;
+        bool              monitor:1;
+        bool                pause:1;
+        bool            secondary:1;
+    };
+};
+
+union vmx_vmexit_control {
+    uint32_t raw;
+    struct {
+        uint32_t               :2;  /* 0:1 reserved */
+        bool   save_guest_debug:1;
+        uint32_t               :6;  /* 3:8 reserved */
+        bool         ia32e_mode:1;
+        uint32_t               :2;  /* 10:11 reserved */
+        bool     load_perf_ctrl:1;
+        uint32_t               :2;  /* 13:14 reserved */
+        bool           ack_intr:1;
+        uint32_t               :2;  /* 16:17 reserved */
+        bool     save_guest_pat:1;
+        bool      load_host_pat:1;
+        bool    save_guest_efer:1;
+        bool     load_host_efer:1;
+        bool save_preempt_timer:1;
+        bool      clear_bndcfgs:1;
+        bool    conceal_from_pt:1;
+        uint32_t               :7;  /* 25:31 reserved */
+    };
+};
+
+union vmx_vmentry_control {
+    uint32_t raw;
+    struct {
+        uint32_t                    :2;  /* 0:1 reserved */
+        bool         load_debug_ctls:1;
+        uint32_t                    :6;  /* 3:8 reserved */
+        bool              ia32e_mode:1;
+        bool                     smm:1;
+        bool deactivate_dual_monitor:1;
+        uint32_t                    :1;  /* 12 reserved */
+        bool   load_perf_global_ctrl:1;
+        bool          load_guest_pat:1;
+        bool         load_guest_efer:1;
+        bool            load_bndcfgs:1;
+        bool conceal_entries_from_pt:1;
+        uint32_t                    :14; /* 18:31 reserved */
+    };
+};
+
+union vmx_secondary_exec_control {
+    uint32_t raw;
+    struct {
+        bool           apic_accesses:1;
+        bool                     ept:1;
+        bool              desc_table:1;
+        bool                  rdtscp:1;
+        bool  virtualize_x2apic_mode:1;
+        bool                    vpid:1;
+        bool                  wbinvd:1;
+        bool            unrestricted:1;
+        bool      apic_register_virt:1;
+        bool   virtual_intr_delivery:1;
+        bool                     ple:1;
+        bool                  rdrand:1;
+        bool                 invpcid:1;
+        bool                  vmfunc:1;
+        bool          vmcs_shadowing:1;
+        bool                   encls:1;
+        bool                  rdseed:1;
+        bool                     pml:1;
+        bool                      ve:1;
+        bool conceal_nonroot_from_pt:1;
+        bool                  xsaves:1;
+        uint32_t                    :1;  /* 21 reserved */
+        bool ept_mode_based_exec_ctl:1;
+        uint32_t                    :2;  /* 23:24 reserved */
+        bool             tsc_scaling:1;
+        uint32_t                    :6;  /* 26:31 reserved */
+    };
+};
+
 /* MSR policy object for shared per-domain MSRs */
 struct msr_domain_policy
 {
@@ -265,6 +387,214 @@ struct msr_domain_policy
         bool available; /* This MSR is non-architectural */
         bool cpuid_faulting;
     } plaform_info;
+
+    union {
+        uint64_t raw[MSR_IA32_VMX_VMCS_ENUM - MSR_IA32_VMX_BASIC + 1];
+
+        struct {
+            /* 0x00000480  MSR_IA32_VMX_BASIC */
+            union {
+                uint64_t raw;
+                struct {
+                    uint32_t      vmcs_rev_id:31;
+                    bool                  mbz:1;  /* 31 always zero */
+                    uint32_t vmcs_region_size:13;
+                    uint32_t                 :3;  /* 45:47 reserved */
+                    bool      addresses_32bit:1;
+                    bool         dual_monitor:1;
+                    uint32_t      memory_type:4;
+                    bool         ins_out_info:1;
+                    bool        default1_zero:1;
+                    uint32_t                 :8;  /* 56:63 reserved */
+                };
+            } basic;
+
+            /* 0x00000481  MSR_IA32_VMX_PINBASED_CTLS */
+            union {
+                uint64_t raw;
+                struct {
+                    union vmx_pin_exec_control allowed_0;
+                    union vmx_pin_exec_control allowed_1;
+                };
+            } pinbased_ctls;
+
+            /* 0x00000482  MSR_IA32_VMX_PROCBASED_CTLS */
+            union {
+                uint64_t raw;
+                struct {
+                    union vmx_cpu_exec_control allowed_0;
+                    union vmx_cpu_exec_control allowed_1;
+                };
+            } procbased_ctls;
+
+            /* 0x00000483  MSR_IA32_VMX_EXIT_CTLS */
+            union {
+                uint64_t raw;
+                struct {
+                    union vmx_vmexit_control allowed_0;
+                    union vmx_vmexit_control allowed_1;
+                };
+            } exit_ctls;
+
+            /* 0x00000484  MSR_IA32_VMX_ENTRY_CTLS */
+            union {
+                uint64_t raw;
+                struct {
+                    union vmx_vmentry_control allowed_0;
+                    union vmx_vmentry_control allowed_1;
+                };
+            } entry_ctls;
+
+            /* 0x00000485  MSR_IA32_VMX_MISC */
+            union {
+                uint64_t raw;
+                struct {
+                    uint32_t   preempt_timer_scale:5;
+                    bool         vmexit_stores_lma:1;
+                    bool                 hlt_state:1;
+                    bool            shutdown_state:1;
+                    bool       wait_for_sipi_state:1;
+                    uint32_t                      :5;  /* 9:13 reserved */
+                    bool                 pt_in_vmx:1;
+                    bool               ia32_smbase:1;
+                    uint32_t            cr3_target:9;
+                    uint32_t    max_msr_load_count:3;
+                    bool ia32_smm_monitor_ctl_bit2:1;
+                    bool               vmwrite_all:1;
+                    bool        inject_ilen0_event:1;
+                    uint32_t                      :1;  /* 31 reserved */
+                    uint32_t      mseg_revision_id;
+                };
+            } misc;
+
+            /* 0x00000486  MSR_IA32_VMX_CR0_FIXED0 */
+            union {
+                uint64_t raw;
+                union cr0_bits allowed_0;
+            } cr0_fixed0;
+
+            /* 0x00000487  MSR_IA32_VMX_CR0_FIXED1 */
+            union {
+                uint64_t raw;
+                union cr0_bits allowed_1;
+            } cr0_fixed1;
+
+            /* 0x00000488  MSR_IA32_VMX_CR4_FIXED0 */
+            union {
+                uint64_t raw;
+                union cr4_bits allowed_0;
+            } cr4_fixed0;
+
+            /* 0x00000489  MSR_IA32_VMX_CR4_FIXED1 */
+            union {
+                uint64_t raw;
+                union cr4_bits allowed_1;
+            } cr4_fixed1;
+
+            /* 0x0000048A  MSR_IA32_VMX_VMCS_ENUM */
+            union {
+                uint64_t raw;
+                struct {
+                    uint32_t             :1;  /* 0 reserved */
+                    uint32_t max_encoding:9;
+                    uint64_t             :54; /* 10:63 reserved */
+                };
+            } vmcs_enum;
+        };
+    } vmx;
+
+    /* 0x0000048B  MSR_IA32_VMX_PROCBASED_CTLS2 */
+    union {
+        uint64_t raw;
+        struct {
+            union vmx_secondary_exec_control allowed_0;
+            union vmx_secondary_exec_control allowed_1;
+        };
+    } vmx_procbased_ctls2;
+
+    /* 0x0000048C  MSR_IA32_VMX_EPT_VPID_CAP */
+    union {
+        uint64_t raw;
+        struct {
+            bool                   x_only:1;
+            uint32_t                     :5;  /* 1:5 reserved */
+            bool                      wl4:1;
+            uint32_t                     :1;  /* 7 reserved */
+            bool                       uc:1;
+            uint32_t                     :5;  /* 9:13 reserved */
+            bool                       wb:1;
+            uint32_t                     :1;  /* 15 reserved */
+            bool                 page_2mb:1;
+            bool                 page_1gb:1;
+            uint32_t                     :2;  /* 18:19 reserved */
+            bool                   invept:1;
+            bool                   ad_bit:1;
+            bool  advanced_ept_violations:1;
+            uint32_t                     :2;  /* 23:24 reserved */
+            bool            invept_single:1;
+            bool               invept_all:1;
+            uint32_t                     :5;  /* 27:31 reserved */
+            bool                  invvpid:1;
+            uint32_t                     :7;  /* 33:39 reserved */
+            bool  invvpid_individual_addr:1;
+            bool   invvpid_single_context:1;
+            bool      invvpid_all_context:1;
+            bool invvpid_single_nonglobal:1;
+            uint32_t                    :20; /* 44:63 reserved */
+        };
+    } vmx_ept_vpid_cap;
+
+    union {
+        uint64_t raw[MSR_IA32_VMX_TRUE_ENTRY_CTLS -
+                     MSR_IA32_VMX_TRUE_PINBASED_CTLS + 1];
+
+        struct {
+            /* 0x0000048D  MSR_IA32_VMX_TRUE_PINBASED_CTLS */
+            union {
+                uint64_t raw;
+                struct {
+                    union vmx_pin_exec_control allowed_0;
+                    union vmx_pin_exec_control allowed_1;
+                };
+            } pinbased;
+
+            /* 0x0000048E  MSR_IA32_VMX_TRUE_PROCBASED_CTLS */
+            union {
+                uint64_t raw;
+                struct {
+                    union vmx_cpu_exec_control allowed_0;
+                    union vmx_cpu_exec_control allowed_1;
+                };
+            } procbased;
+
+            /* 0x0000048F  MSR_IA32_VMX_TRUE_EXIT_CTLS */
+            union {
+                uint64_t raw;
+                struct {
+                    union vmx_vmexit_control allowed_0;
+                    union vmx_vmexit_control allowed_1;
+                };
+            } exit;
+
+            /* 0x00000490  MSR_IA32_VMX_TRUE_ENTRY_CTLS */
+            union {
+                uint64_t raw;
+                struct {
+                    union vmx_vmentry_control allowed_0;
+                    union vmx_vmentry_control allowed_1;
+                };
+            } entry;
+        };
+    } vmx_true_ctls;
+
+    /* 0x00000491  MSR_IA32_VMX_VMFUNC */
+    union {
+        uint64_t raw;
+        struct {
+            bool eptp_switching:1;
+            uint64_t           :63; /* 1:63 reserved */
+        };
+    } vmx_vmfunc;
 };
 
 /* RAW msr domain policy: contains the actual values from H/W MSRs */
diff --git a/xen/include/asm-x86/x86-defns.h b/xen/include/asm-x86/x86-defns.h
index ff8d66be3c..1d26328520 100644
--- a/xen/include/asm-x86/x86-defns.h
+++ b/xen/include/asm-x86/x86-defns.h
@@ -42,6 +42,28 @@
 #define X86_CR0_CD              0x40000000 /* Cache Disable            (RW) */
 #define X86_CR0_PG              0x80000000 /* Paging                   (RW) */
 
+#ifndef __ASSEMBLY__
+union cr0_bits {
+    uint64_t raw;
+    struct {
+        bool     pe:1;
+        bool     mp:1;
+        bool     em:1;
+        bool     ts:1;
+        bool     et:1;
+        bool     ne:1;
+        uint32_t   :10; /* 6:15 reserved */
+        bool     wp:1;
+        uint32_t   :1;  /* 17 reserved */
+        bool     am:1;
+        uint32_t   :10; /* 19:28 reserved */
+        bool     nw:1;
+        bool     cd:1;
+        bool     pg:1;
+    };
+};
+#endif /* #ifndef __ASSEMBLY__ */
+
 /*
  * Intel CPU flags in CR3
  */
@@ -71,6 +93,38 @@
 #define X86_CR4_SMAP       0x00200000 /* enable SMAP */
 #define X86_CR4_PKE        0x00400000 /* enable PKE */
 
+#ifndef __ASSEMBLY__
+union cr4_bits {
+    uint64_t raw;
+    struct {
+        bool        vme:1;
+        bool        pvi:1;
+        bool        tsd:1;
+        bool         de:1;
+        bool        pse:1;
+        bool        pae:1;
+        bool        mce:1;
+        bool        pge:1;
+        bool        pce:1;
+        bool     osfxsr:1;
+        bool osxmmexcpt:1;
+        bool       umip:1;
+        uint32_t       :1;  /* 12 reserved */
+        bool       vmxe:1;
+        bool       smxe:1;
+        uint32_t       :1;  /* 15 reserved */
+        bool   fsgsbase:1;
+        bool      pcide:1;
+        bool    osxsave:1;
+        uint32_t       :1;  /* 19 reserved */
+        bool       smep:1;
+        bool       smap:1;
+        bool        pke:1;
+        uint32_t       :9;  /* 23:31 reserved */
+    };
+};
+#endif /* #ifndef __ASSEMBLY__ */
+
 /*
  * XSTATE component flags in XCR0
  */
-- 
2.14.1


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v6 2/5] x86/msr: add VMX MSRs into HVM_max domain policy
  2018-03-22 11:32 [PATCH v6 0/5] VMX MSRs policy for Nested Virt: part 1 Sergey Dyasli
  2018-03-22 11:32 ` [PATCH v6 1/5] x86/msr: add VMX MSRs definitions and populate Raw domain policy Sergey Dyasli
@ 2018-03-22 11:32 ` Sergey Dyasli
  2018-04-02  2:14   ` Tian, Kevin
  2018-03-22 11:32 ` [PATCH v6 3/5] x86/cpuid: update signature of hvm_cr4_guest_valid_bits() Sergey Dyasli
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Sergey Dyasli @ 2018-03-22 11:32 UTC (permalink / raw)
  To: xen-devel
  Cc: Andrew Cooper, Kevin Tian, Jan Beulich, Jun Nakajima, Sergey Dyasli

Currently, when nested virt is enabled, the set of L1 VMX features
is fixed and calculated by nvmx_msr_read_intercept() as an intersection
between the full set of Xen's supported L1 VMX features, the set of
actual H/W features and, for MSR_IA32_VMX_EPT_VPID_CAP, the set of
features that Xen uses.

Add calculate_hvm_max_vmx_policy() which will save the end result of
nvmx_msr_read_intercept() on current H/W into HVM_max domain policy.
There will be no functional change to what L1 sees in VMX MSRs. But the
actual use of HVM_max domain policy will happen later, when VMX MSRs
are handled by guest_rd/wrmsr().

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
---
v5 --> v6:
- Replaced !cpu_has_vmx check with !hvm_max_cpuid_policy.basic.vmx
- Added a TODO reminder
- Added brackets around bit or expressions
---
 xen/arch/x86/msr.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 135 insertions(+)

diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index 87239e151e..01a5b52f95 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -106,6 +106,139 @@ static void __init calculate_host_policy(void)
     dp->plaform_info.cpuid_faulting = cpu_has_cpuid_faulting;
 }
 
+static void vmx_clear_policy(struct msr_domain_policy *dp)
+{
+    memset(dp->vmx.raw, 0, sizeof(dp->vmx.raw));
+    dp->vmx_procbased_ctls2.raw = 0;
+    dp->vmx_ept_vpid_cap.raw = 0;
+    memset(dp->vmx_true_ctls.raw, 0, sizeof(dp->vmx_true_ctls.raw));
+    dp->vmx_vmfunc.raw = 0;
+}
+
+static void __init calculate_hvm_max_vmx_policy(struct msr_domain_policy *dp)
+{
+    const struct msr_domain_policy *hp = &host_msr_domain_policy;
+    uint32_t supported;
+
+    if ( !hvm_max_cpuid_policy.basic.vmx )
+        return;
+
+    vmx_clear_policy(dp);
+
+     /* TODO: actually make vmx features selection sane */
+    dp->vmx.basic.raw = hp->vmx.basic.raw;
+
+    dp->vmx.pinbased_ctls.allowed_0.raw = VMX_PINBASED_CTLS_DEFAULT1;
+    dp->vmx.pinbased_ctls.allowed_1.raw = VMX_PINBASED_CTLS_DEFAULT1;
+    supported = (PIN_BASED_EXT_INTR_MASK |
+                 PIN_BASED_NMI_EXITING   |
+                 PIN_BASED_PREEMPT_TIMER);
+    dp->vmx.pinbased_ctls.allowed_1.raw |= supported;
+    dp->vmx.pinbased_ctls.allowed_1.raw &= hp->vmx.pinbased_ctls.allowed_1.raw;
+
+    dp->vmx.procbased_ctls.allowed_0.raw = VMX_PROCBASED_CTLS_DEFAULT1;
+    dp->vmx.procbased_ctls.allowed_1.raw = VMX_PROCBASED_CTLS_DEFAULT1;
+    supported = (CPU_BASED_HLT_EXITING          |
+                 CPU_BASED_VIRTUAL_INTR_PENDING |
+                 CPU_BASED_CR8_LOAD_EXITING     |
+                 CPU_BASED_CR8_STORE_EXITING    |
+                 CPU_BASED_INVLPG_EXITING       |
+                 CPU_BASED_MONITOR_EXITING      |
+                 CPU_BASED_MWAIT_EXITING        |
+                 CPU_BASED_MOV_DR_EXITING       |
+                 CPU_BASED_ACTIVATE_IO_BITMAP   |
+                 CPU_BASED_USE_TSC_OFFSETING    |
+                 CPU_BASED_UNCOND_IO_EXITING    |
+                 CPU_BASED_RDTSC_EXITING        |
+                 CPU_BASED_MONITOR_TRAP_FLAG    |
+                 CPU_BASED_VIRTUAL_NMI_PENDING  |
+                 CPU_BASED_ACTIVATE_MSR_BITMAP  |
+                 CPU_BASED_PAUSE_EXITING        |
+                 CPU_BASED_RDPMC_EXITING        |
+                 CPU_BASED_TPR_SHADOW           |
+                 CPU_BASED_ACTIVATE_SECONDARY_CONTROLS);
+    dp->vmx.procbased_ctls.allowed_1.raw |= supported;
+    dp->vmx.procbased_ctls.allowed_1.raw &=
+        hp->vmx.procbased_ctls.allowed_1.raw;
+
+    dp->vmx.exit_ctls.allowed_0.raw = VMX_EXIT_CTLS_DEFAULT1;
+    dp->vmx.exit_ctls.allowed_1.raw = VMX_EXIT_CTLS_DEFAULT1;
+    supported = (VM_EXIT_ACK_INTR_ON_EXIT   |
+                 VM_EXIT_IA32E_MODE         |
+                 VM_EXIT_SAVE_PREEMPT_TIMER |
+                 VM_EXIT_SAVE_GUEST_PAT     |
+                 VM_EXIT_LOAD_HOST_PAT      |
+                 VM_EXIT_SAVE_GUEST_EFER    |
+                 VM_EXIT_LOAD_HOST_EFER     |
+                 VM_EXIT_LOAD_PERF_GLOBAL_CTRL);
+    dp->vmx.exit_ctls.allowed_1.raw |= supported;
+    dp->vmx.exit_ctls.allowed_1.raw &= hp->vmx.exit_ctls.allowed_1.raw;
+
+    dp->vmx.entry_ctls.allowed_0.raw = VMX_ENTRY_CTLS_DEFAULT1;
+    dp->vmx.entry_ctls.allowed_1.raw = VMX_ENTRY_CTLS_DEFAULT1;
+    supported = (VM_ENTRY_LOAD_GUEST_PAT        |
+                 VM_ENTRY_LOAD_GUEST_EFER       |
+                 VM_ENTRY_LOAD_PERF_GLOBAL_CTRL |
+                 VM_ENTRY_IA32E_MODE);
+    dp->vmx.entry_ctls.allowed_1.raw |= supported;
+    dp->vmx.entry_ctls.allowed_1.raw &= hp->vmx.entry_ctls.allowed_1.raw;
+
+    dp->vmx.misc.raw = hp->vmx.misc.raw;
+    /* Do not support CR3-target feature now */
+    dp->vmx.misc.cr3_target = false;
+
+    /* PG, PE bits must be 1 in VMX operation */
+    dp->vmx.cr0_fixed0.allowed_0.pe = true;
+    dp->vmx.cr0_fixed0.allowed_0.pg = true;
+
+    /* allow 0-settings for all bits */
+    dp->vmx.cr0_fixed1.allowed_1.raw = 0xffffffff;
+
+    /* VMXE bit must be 1 in VMX operation */
+    dp->vmx.cr4_fixed0.allowed_0.vmxe = true;
+
+    /*
+     * Allowed CR4 bits will be updated during domain creation by
+     * hvm_cr4_guest_valid_bits()
+     */
+    dp->vmx.cr4_fixed1.allowed_1.raw = hp->vmx.cr4_fixed1.allowed_1.raw;
+
+    /* The max index of VVMCS encoding is 0x1f. */
+    dp->vmx.vmcs_enum.max_encoding = 0x1f;
+
+    if ( vmx_procbased_ctls2_available(dp) )
+    {
+        supported = (SECONDARY_EXEC_DESCRIPTOR_TABLE_EXITING |
+                     SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
+                     SECONDARY_EXEC_ENABLE_VPID              |
+                     SECONDARY_EXEC_UNRESTRICTED_GUEST       |
+                     SECONDARY_EXEC_ENABLE_EPT);
+        dp->vmx_procbased_ctls2.allowed_1.raw |= supported;
+        dp->vmx_procbased_ctls2.allowed_1.raw &=
+            hp->vmx_procbased_ctls2.allowed_1.raw;
+    }
+
+    if ( vmx_ept_vpid_cap_available(dp) )
+        dp->vmx_ept_vpid_cap.raw = nept_get_ept_vpid_cap();
+
+    if ( vmx_true_ctls_available(dp) )
+    {
+        dp->vmx_true_ctls.pinbased.raw = dp->vmx.pinbased_ctls.raw;
+
+        dp->vmx_true_ctls.procbased.raw = dp->vmx.procbased_ctls.raw;
+        supported = (CPU_BASED_CR3_LOAD_EXITING |
+                     CPU_BASED_CR3_STORE_EXITING);
+        dp->vmx_true_ctls.procbased.raw |= supported;
+        dp->vmx_true_ctls.procbased.raw &= hp->vmx_true_ctls.procbased.raw;
+
+        dp->vmx_true_ctls.exit.raw = dp->vmx.exit_ctls.raw;
+
+        dp->vmx_true_ctls.entry.raw = dp->vmx.entry_ctls.raw;
+    }
+
+    /* MSR_IA32_VMX_VMFUNC is N/A */
+}
+
 static void __init calculate_hvm_max_policy(void)
 {
     struct msr_domain_policy *dp = &hvm_max_msr_domain_policy;
@@ -127,6 +260,8 @@ static void __init calculate_hvm_max_policy(void)
 
     /* 0x00000140  MSR_INTEL_MISC_FEATURES_ENABLES */
     vp->misc_features_enables.available = dp->plaform_info.cpuid_faulting;
+
+    calculate_hvm_max_vmx_policy(dp);
 }
 
 static void __init calculate_pv_max_policy(void)
-- 
2.14.1


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v6 3/5] x86/cpuid: update signature of hvm_cr4_guest_valid_bits()
  2018-03-22 11:32 [PATCH v6 0/5] VMX MSRs policy for Nested Virt: part 1 Sergey Dyasli
  2018-03-22 11:32 ` [PATCH v6 1/5] x86/msr: add VMX MSRs definitions and populate Raw domain policy Sergey Dyasli
  2018-03-22 11:32 ` [PATCH v6 2/5] x86/msr: add VMX MSRs into HVM_max " Sergey Dyasli
@ 2018-03-22 11:32 ` Sergey Dyasli
  2018-04-02  2:15   ` Tian, Kevin
  2018-03-22 11:32 ` [PATCH v6 4/5] x86/msr: update domain policy on CPUID policy changes Sergey Dyasli
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Sergey Dyasli @ 2018-03-22 11:32 UTC (permalink / raw)
  To: xen-devel
  Cc: Andrew Cooper, Kevin Tian, Jan Beulich, Jun Nakajima, Sergey Dyasli

With the new cpuid infrastructure there is a domain-wide struct cpuid
policy and there is no need to pass a separate struct vcpu * into
hvm_cr4_guest_valid_bits() anymore. Make the function accept struct
domain * instead and update callers.

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
v5 --> v6:
- Added brackets to expression in vmx.c and replaced 0 with false
- Added Reviewed-by
---
 xen/arch/x86/hvm/domain.c       | 3 ++-
 xen/arch/x86/hvm/hvm.c          | 7 +++----
 xen/arch/x86/hvm/svm/svmdebug.c | 4 ++--
 xen/arch/x86/hvm/vmx/vmx.c      | 4 ++--
 xen/arch/x86/hvm/vmx/vvmx.c     | 2 +-
 xen/include/asm-x86/hvm/hvm.h   | 2 +-
 6 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/hvm/domain.c b/xen/arch/x86/hvm/domain.c
index 60474649de..ce15ce0470 100644
--- a/xen/arch/x86/hvm/domain.c
+++ b/xen/arch/x86/hvm/domain.c
@@ -111,6 +111,7 @@ static int check_segment(struct segment_register *reg, enum x86_segment seg)
 /* Called by VCPUOP_initialise for HVM guests. */
 int arch_set_info_hvm_guest(struct vcpu *v, const vcpu_hvm_context_t *ctx)
 {
+    const struct domain *d = v->domain;
     struct cpu_user_regs *uregs = &v->arch.user_regs;
     struct segment_register cs, ds, ss, es, tr;
     const char *errstr;
@@ -272,7 +273,7 @@ int arch_set_info_hvm_guest(struct vcpu *v, const vcpu_hvm_context_t *ctx)
     if ( v->arch.hvm_vcpu.guest_efer & EFER_LME )
         v->arch.hvm_vcpu.guest_efer |= EFER_LMA;
 
-    if ( v->arch.hvm_vcpu.guest_cr[4] & ~hvm_cr4_guest_valid_bits(v, 0) )
+    if ( v->arch.hvm_vcpu.guest_cr[4] & ~hvm_cr4_guest_valid_bits(d, false) )
     {
         gprintk(XENLOG_ERR, "Bad CR4 value: %#016lx\n",
                 v->arch.hvm_vcpu.guest_cr[4]);
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 5759c73dd4..fe253034f2 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -931,9 +931,8 @@ const char *hvm_efer_valid(const struct vcpu *v, uint64_t value,
         X86_CR0_CD | X86_CR0_PG)))
 
 /* These bits in CR4 can be set by the guest. */
-unsigned long hvm_cr4_guest_valid_bits(const struct vcpu *v, bool restore)
+unsigned long hvm_cr4_guest_valid_bits(const struct domain *d, bool restore)
 {
-    const struct domain *d = v->domain;
     const struct cpuid_policy *p;
     bool mce, vmxe;
 
@@ -1000,7 +999,7 @@ static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
         return -EINVAL;
     }
 
-    if ( ctxt.cr4 & ~hvm_cr4_guest_valid_bits(v, 1) )
+    if ( ctxt.cr4 & ~hvm_cr4_guest_valid_bits(d, true) )
     {
         printk(XENLOG_G_ERR "HVM%d restore: bad CR4 %#" PRIx64 "\n",
                d->domain_id, ctxt.cr4);
@@ -2350,7 +2349,7 @@ int hvm_set_cr4(unsigned long value, bool_t may_defer)
     struct vcpu *v = current;
     unsigned long old_cr;
 
-    if ( value & ~hvm_cr4_guest_valid_bits(v, 0) )
+    if ( value & ~hvm_cr4_guest_valid_bits(v->domain, false) )
     {
         HVM_DBG_LOG(DBG_LEVEL_1,
                     "Guest attempts to set reserved bit in CR4: %lx",
diff --git a/xen/arch/x86/hvm/svm/svmdebug.c b/xen/arch/x86/hvm/svm/svmdebug.c
index 091c58fa1b..6c215d19fe 100644
--- a/xen/arch/x86/hvm/svm/svmdebug.c
+++ b/xen/arch/x86/hvm/svm/svmdebug.c
@@ -121,9 +121,9 @@ bool svm_vmcb_isvalid(const char *from, const struct vmcb_struct *vmcb,
            (cr3 >> v->domain->arch.cpuid->extd.maxphysaddr))) )
         PRINTF("CR3: MBZ bits are set (%#"PRIx64")\n", cr3);
 
-    if ( cr4 & ~hvm_cr4_guest_valid_bits(v, false) )
+    if ( cr4 & ~hvm_cr4_guest_valid_bits(v->domain, false) )
         PRINTF("CR4: invalid bits are set (%#"PRIx64", valid: %#"PRIx64")\n",
-               cr4, hvm_cr4_guest_valid_bits(v, false));
+               cr4, hvm_cr4_guest_valid_bits(v->domain, false));
 
     if ( vmcb_get_dr6(vmcb) >> 32 )
         PRINTF("DR6: bits [63:32] are not zero (%#"PRIx64")\n",
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index c5cc96339e..847c314a08 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1598,8 +1598,8 @@ static void vmx_update_guest_cr(struct vcpu *v, unsigned int cr,
              * Update CR4 host mask to only trap when the guest tries to set
              * bits that are controlled by the hypervisor.
              */
-            v->arch.hvm_vmx.cr4_host_mask = HVM_CR4_HOST_MASK | X86_CR4_PKE |
-                                            ~hvm_cr4_guest_valid_bits(v, 0);
+            v->arch.hvm_vmx.cr4_host_mask = (HVM_CR4_HOST_MASK | X86_CR4_PKE |
+                                   ~hvm_cr4_guest_valid_bits(v->domain, false));
             v->arch.hvm_vmx.cr4_host_mask |= v->arch.hvm_vmx.vmx_realmode ?
                                              X86_CR4_VME : 0;
             v->arch.hvm_vmx.cr4_host_mask |= !hvm_paging_enabled(v) ?
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index dcd3b28f86..43f7297c04 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -2140,7 +2140,7 @@ int nvmx_msr_read_intercept(unsigned int msr, u64 *msr_content)
         data = X86_CR4_VMXE;
         break;
     case MSR_IA32_VMX_CR4_FIXED1:
-        data = hvm_cr4_guest_valid_bits(v, 0);
+        data = hvm_cr4_guest_valid_bits(d, false);
         break;
     case MSR_IA32_VMX_MISC:
         /* Do not support CR3-target feature now */
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 2376ed6912..bfd42b065d 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -621,7 +621,7 @@ static inline bool altp2m_vcpu_emulate_ve(struct vcpu *v)
 /* Check CR4/EFER values */
 const char *hvm_efer_valid(const struct vcpu *v, uint64_t value,
                            signed int cr0_pg);
-unsigned long hvm_cr4_guest_valid_bits(const struct vcpu *v, bool restore);
+unsigned long hvm_cr4_guest_valid_bits(const struct domain *d, bool restore);
 
 /*
  * This must be defined as a macro instead of an inline function,
-- 
2.14.1


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v6 4/5] x86/msr: update domain policy on CPUID policy changes
  2018-03-22 11:32 [PATCH v6 0/5] VMX MSRs policy for Nested Virt: part 1 Sergey Dyasli
                   ` (2 preceding siblings ...)
  2018-03-22 11:32 ` [PATCH v6 3/5] x86/cpuid: update signature of hvm_cr4_guest_valid_bits() Sergey Dyasli
@ 2018-03-22 11:32 ` Sergey Dyasli
  2018-03-22 11:32 ` [PATCH v6 5/5] x86/msr: handle VMX MSRs with guest_rd/wrmsr() Sergey Dyasli
  2018-04-30 15:46 ` [PATCH v6 0/5] VMX MSRs policy for Nested Virt: part 1 Jan Beulich
  5 siblings, 0 replies; 10+ messages in thread
From: Sergey Dyasli @ 2018-03-22 11:32 UTC (permalink / raw)
  To: xen-devel
  Cc: Andrew Cooper, Kevin Tian, Jan Beulich, Jun Nakajima, Sergey Dyasli

Availability of some MSRs depends on certain CPUID bits. Add function
recalculate_domain_msr_policy() which updates availability of MSRs
based on current domain's CPUID policy. This function is called when
CPUID policy is changed from a toolstack.

Add recalculate_domain_vmx_msr_policy() which changes availability of
VMX MSRs based on domain's nested virt settings. If it's enabled, then
the domain receives a copy of HVM_max vmx policy with allowed CR4 bits
adjusted by CPUID policy.

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
v5 --> v6:
- Updated recalculate_msr_policy() comment and commit message
- Added Reviewed-by
---
 xen/arch/x86/domctl.c     |  1 +
 xen/arch/x86/msr.c        | 35 +++++++++++++++++++++++++++++++++++
 xen/include/asm-x86/msr.h |  3 +++
 3 files changed, 39 insertions(+)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 8fbbf3aeb3..5bde1a22b7 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -125,6 +125,7 @@ static int update_domain_cpuid_info(struct domain *d,
     }
 
     recalculate_cpuid_policy(d);
+    recalculate_msr_policy(d);
 
     switch ( ctl->input[0] )
     {
diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index 01a5b52f95..26d987098b 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -23,6 +23,7 @@
 #include <xen/lib.h>
 #include <xen/sched.h>
 #include <asm/msr.h>
+#include <asm/hvm/nestedhvm.h>
 
 DEFINE_PER_CPU(uint32_t, tsc_aux);
 
@@ -283,6 +284,39 @@ void __init init_guest_msr_policy(void)
     calculate_pv_max_policy();
 }
 
+static void vmx_copy_policy(const struct msr_domain_policy *src,
+                                  struct msr_domain_policy *dst)
+{
+    memcpy(dst->vmx.raw, src->vmx.raw, sizeof(dst->vmx.raw));
+    dst->vmx_procbased_ctls2.raw = src->vmx_procbased_ctls2.raw;
+    dst->vmx_ept_vpid_cap.raw = src->vmx_ept_vpid_cap.raw;
+    memcpy(dst->vmx_true_ctls.raw, src->vmx_true_ctls.raw,
+           sizeof(dst->vmx_true_ctls.raw));
+    dst->vmx_vmfunc.raw = src->vmx_vmfunc.raw;
+}
+
+static void recalculate_vmx_msr_policy(struct domain *d)
+{
+    struct msr_domain_policy *dp = d->arch.msr;
+
+    if ( !nestedhvm_enabled(d) || !d->arch.cpuid->basic.vmx )
+    {
+        vmx_clear_policy(dp);
+
+        return;
+    }
+
+    vmx_copy_policy(&hvm_max_msr_domain_policy, dp);
+
+    /* Get allowed CR4 bits from CPUID policy */
+    dp->vmx.cr4_fixed1.allowed_1.raw = hvm_cr4_guest_valid_bits(d, false);
+}
+
+void recalculate_msr_policy(struct domain *d)
+{
+    recalculate_vmx_msr_policy(d);
+}
+
 int init_domain_msr_policy(struct domain *d)
 {
     struct msr_domain_policy *dp;
@@ -303,6 +337,7 @@ int init_domain_msr_policy(struct domain *d)
     }
 
     d->arch.msr = dp;
+    recalculate_msr_policy(d);
 
     return 0;
 }
diff --git a/xen/include/asm-x86/msr.h b/xen/include/asm-x86/msr.h
index 5fdf82860e..41433fea94 100644
--- a/xen/include/asm-x86/msr.h
+++ b/xen/include/asm-x86/msr.h
@@ -641,6 +641,9 @@ int init_vcpu_msr_policy(struct vcpu *v);
 int guest_rdmsr(const struct vcpu *v, uint32_t msr, uint64_t *val);
 int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val);
 
+/* Updates availability of MSRs based on CPUID policy */
+void recalculate_msr_policy(struct domain *d);
+
 #endif /* !__ASSEMBLY__ */
 
 #endif /* __ASM_MSR_H */
-- 
2.14.1


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v6 5/5] x86/msr: handle VMX MSRs with guest_rd/wrmsr()
  2018-03-22 11:32 [PATCH v6 0/5] VMX MSRs policy for Nested Virt: part 1 Sergey Dyasli
                   ` (3 preceding siblings ...)
  2018-03-22 11:32 ` [PATCH v6 4/5] x86/msr: update domain policy on CPUID policy changes Sergey Dyasli
@ 2018-03-22 11:32 ` Sergey Dyasli
  2018-04-30 15:46 ` [PATCH v6 0/5] VMX MSRs policy for Nested Virt: part 1 Jan Beulich
  5 siblings, 0 replies; 10+ messages in thread
From: Sergey Dyasli @ 2018-03-22 11:32 UTC (permalink / raw)
  To: xen-devel
  Cc: Andrew Cooper, Kevin Tian, Jan Beulich, Jun Nakajima, Sergey Dyasli

Now that each domain has a correct view of VMX MSRs in it's per-domain
MSR policy, it's possible to handle guest's RD/WRMSR with the new
handlers. Do it and remove the old nvmx_msr_read_intercept() and
associated bits.

There is no functional change to what a guest sees in its VMX MSRs.

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
v5 --> v6:
- Moved VMX MSRs case to the read-only block in guest_wrmsr()
- Added Reviewed-by
---
 xen/arch/x86/hvm/vmx/vmx.c         |   6 --
 xen/arch/x86/hvm/vmx/vvmx.c        | 178 -------------------------------------
 xen/arch/x86/msr.c                 |  32 +++++++
 xen/include/asm-x86/hvm/vmx/vvmx.h |   2 -
 4 files changed, 32 insertions(+), 186 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 847c314a08..ba5b78a9c2 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2869,10 +2869,6 @@ static int vmx_msr_read_intercept(unsigned int msr, uint64_t *msr_content)
         if ( nestedhvm_enabled(curr->domain) )
             *msr_content |= IA32_FEATURE_CONTROL_ENABLE_VMXON_OUTSIDE_SMX;
         break;
-    case MSR_IA32_VMX_BASIC...MSR_IA32_VMX_VMFUNC:
-        if ( !nvmx_msr_read_intercept(msr, msr_content) )
-            goto gp_fault;
-        break;
     case MSR_IA32_MISC_ENABLE:
         rdmsrl(MSR_IA32_MISC_ENABLE, *msr_content);
         /* Debug Trace Store is not supported. */
@@ -3126,8 +3122,6 @@ static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content)
         break;
     }
     case MSR_IA32_FEATURE_CONTROL:
-    case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMFUNC:
-        /* None of these MSRs are writeable. */
         goto gp_fault;
 
     case MSR_P6_PERFCTR(0)...MSR_P6_PERFCTR(7):
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 43f7297c04..5a1d9c8fc5 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1980,184 +1980,6 @@ int nvmx_handle_invvpid(struct cpu_user_regs *regs)
     return X86EMUL_OKAY;
 }
 
-#define __emul_value(enable1, default1) \
-    ((enable1 | default1) << 32 | (default1))
-
-#define gen_vmx_msr(enable1, default1, host_value) \
-    (((__emul_value(enable1, default1) & host_value) & (~0ul << 32)) | \
-    ((uint32_t)(__emul_value(enable1, default1) | host_value)))
-
-/*
- * Capability reporting
- */
-int nvmx_msr_read_intercept(unsigned int msr, u64 *msr_content)
-{
-    struct vcpu *v = current;
-    struct domain *d = v->domain;
-    u64 data = 0, host_data = 0;
-    int r = 1;
-
-    /* VMX capablity MSRs are available only when guest supports VMX. */
-    if ( !nestedhvm_enabled(d) || !d->arch.cpuid->basic.vmx )
-        return 0;
-
-    /*
-     * These MSRs are only available when flags in other MSRs are set.
-     * These prerequisites are listed in the Intel 64 and IA-32
-     * Architectures Software Developer’s Manual, Vol 3, Appendix A.
-     */
-    switch ( msr )
-    {
-    case MSR_IA32_VMX_PROCBASED_CTLS2:
-        if ( !cpu_has_vmx_secondary_exec_control )
-            return 0;
-        break;
-
-    case MSR_IA32_VMX_EPT_VPID_CAP:
-        if ( !(cpu_has_vmx_ept || cpu_has_vmx_vpid) )
-            return 0;
-        break;
-
-    case MSR_IA32_VMX_TRUE_PINBASED_CTLS:
-    case MSR_IA32_VMX_TRUE_PROCBASED_CTLS:
-    case MSR_IA32_VMX_TRUE_EXIT_CTLS:
-    case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
-        if ( !(vmx_basic_msr & VMX_BASIC_DEFAULT1_ZERO) )
-            return 0;
-        break;
-
-    case MSR_IA32_VMX_VMFUNC:
-        if ( !cpu_has_vmx_vmfunc )
-            return 0;
-        break;
-    }
-
-    rdmsrl(msr, host_data);
-
-    /*
-     * Remove unsupport features from n1 guest capability MSR
-     */
-    switch (msr) {
-    case MSR_IA32_VMX_BASIC:
-    {
-        const struct vmcs_struct *vmcs =
-            map_domain_page(_mfn(PFN_DOWN(v->arch.hvm_vmx.vmcs_pa)));
-
-        data = (host_data & (~0ul << 32)) |
-               (vmcs->vmcs_revision_id & 0x7fffffff);
-        unmap_domain_page(vmcs);
-        break;
-    }
-    case MSR_IA32_VMX_PINBASED_CTLS:
-    case MSR_IA32_VMX_TRUE_PINBASED_CTLS:
-        /* 1-settings */
-        data = PIN_BASED_EXT_INTR_MASK |
-               PIN_BASED_NMI_EXITING |
-               PIN_BASED_PREEMPT_TIMER;
-        data = gen_vmx_msr(data, VMX_PINBASED_CTLS_DEFAULT1, host_data);
-        break;
-    case MSR_IA32_VMX_PROCBASED_CTLS:
-    case MSR_IA32_VMX_TRUE_PROCBASED_CTLS:
-    {
-        u32 default1_bits = VMX_PROCBASED_CTLS_DEFAULT1;
-        /* 1-settings */
-        data = CPU_BASED_HLT_EXITING |
-               CPU_BASED_VIRTUAL_INTR_PENDING |
-               CPU_BASED_CR8_LOAD_EXITING |
-               CPU_BASED_CR8_STORE_EXITING |
-               CPU_BASED_INVLPG_EXITING |
-               CPU_BASED_CR3_LOAD_EXITING |
-               CPU_BASED_CR3_STORE_EXITING |
-               CPU_BASED_MONITOR_EXITING |
-               CPU_BASED_MWAIT_EXITING |
-               CPU_BASED_MOV_DR_EXITING |
-               CPU_BASED_ACTIVATE_IO_BITMAP |
-               CPU_BASED_USE_TSC_OFFSETING |
-               CPU_BASED_UNCOND_IO_EXITING |
-               CPU_BASED_RDTSC_EXITING |
-               CPU_BASED_MONITOR_TRAP_FLAG |
-               CPU_BASED_VIRTUAL_NMI_PENDING |
-               CPU_BASED_ACTIVATE_MSR_BITMAP |
-               CPU_BASED_PAUSE_EXITING |
-               CPU_BASED_RDPMC_EXITING |
-               CPU_BASED_TPR_SHADOW |
-               CPU_BASED_ACTIVATE_SECONDARY_CONTROLS;
-
-        if ( msr == MSR_IA32_VMX_TRUE_PROCBASED_CTLS )
-            default1_bits &= ~(CPU_BASED_CR3_LOAD_EXITING |
-                               CPU_BASED_CR3_STORE_EXITING |
-                               CPU_BASED_INVLPG_EXITING);
-
-        data = gen_vmx_msr(data, default1_bits, host_data);
-        break;
-    }
-    case MSR_IA32_VMX_PROCBASED_CTLS2:
-        /* 1-settings */
-        data = SECONDARY_EXEC_DESCRIPTOR_TABLE_EXITING |
-               SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
-               SECONDARY_EXEC_ENABLE_VPID |
-               SECONDARY_EXEC_UNRESTRICTED_GUEST |
-               SECONDARY_EXEC_ENABLE_EPT;
-        data = gen_vmx_msr(data, 0, host_data);
-        break;
-    case MSR_IA32_VMX_EXIT_CTLS:
-    case MSR_IA32_VMX_TRUE_EXIT_CTLS:
-        /* 1-settings */
-        data = VM_EXIT_ACK_INTR_ON_EXIT |
-               VM_EXIT_IA32E_MODE |
-               VM_EXIT_SAVE_PREEMPT_TIMER |
-               VM_EXIT_SAVE_GUEST_PAT |
-               VM_EXIT_LOAD_HOST_PAT |
-               VM_EXIT_SAVE_GUEST_EFER |
-               VM_EXIT_LOAD_HOST_EFER |
-               VM_EXIT_LOAD_PERF_GLOBAL_CTRL;
-        data = gen_vmx_msr(data, VMX_EXIT_CTLS_DEFAULT1, host_data);
-        break;
-    case MSR_IA32_VMX_ENTRY_CTLS:
-    case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
-        /* 1-settings */
-        data = VM_ENTRY_LOAD_GUEST_PAT |
-               VM_ENTRY_LOAD_GUEST_EFER |
-               VM_ENTRY_LOAD_PERF_GLOBAL_CTRL |
-               VM_ENTRY_IA32E_MODE;
-        data = gen_vmx_msr(data, VMX_ENTRY_CTLS_DEFAULT1, host_data);
-        break;
-
-    case MSR_IA32_VMX_VMCS_ENUM:
-        /* The max index of VVMCS encoding is 0x1f. */
-        data = 0x1f << 1;
-        break;
-    case MSR_IA32_VMX_CR0_FIXED0:
-        /* PG, PE bits must be 1 in VMX operation */
-        data = X86_CR0_PE | X86_CR0_PG;
-        break;
-    case MSR_IA32_VMX_CR0_FIXED1:
-        /* allow 0-settings for all bits */
-        data = 0xffffffff;
-        break;
-    case MSR_IA32_VMX_CR4_FIXED0:
-        /* VMXE bit must be 1 in VMX operation */
-        data = X86_CR4_VMXE;
-        break;
-    case MSR_IA32_VMX_CR4_FIXED1:
-        data = hvm_cr4_guest_valid_bits(d, false);
-        break;
-    case MSR_IA32_VMX_MISC:
-        /* Do not support CR3-target feature now */
-        data = host_data & ~VMX_MISC_CR3_TARGET;
-        break;
-    case MSR_IA32_VMX_EPT_VPID_CAP:
-        data = nept_get_ept_vpid_cap();
-        break;
-    default:
-        r = 0;
-        break;
-    }
-
-    *msr_content = data;
-    return r;
-}
-
 /* This function uses L2_gpa to walk the P2M page table in L1. If the
  * walk is successful, the translated value is returned in
  * L1_gpa. The result value tells what to do next.
diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index 26d987098b..9feab453b3 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -366,6 +366,7 @@ int init_vcpu_msr_policy(struct vcpu *v)
 
 int guest_rdmsr(const struct vcpu *v, uint32_t msr, uint64_t *val)
 {
+    const struct domain *d = v->domain;
     const struct cpuid_policy *cp = v->domain->arch.cpuid;
     const struct msr_domain_policy *dp = v->domain->arch.msr;
     const struct msr_vcpu_policy *vp = v->arch.msr;
@@ -402,6 +403,36 @@ int guest_rdmsr(const struct vcpu *v, uint32_t msr, uint64_t *val)
                _MSR_MISC_FEATURES_CPUID_FAULTING;
         break;
 
+    case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMCS_ENUM:
+        if ( !nestedhvm_enabled(d) || !cp->basic.vmx )
+            goto gp_fault;
+        *val = dp->vmx.raw[msr - MSR_IA32_VMX_BASIC];
+        break;
+
+    case MSR_IA32_VMX_PROCBASED_CTLS2:
+        if ( !vmx_procbased_ctls2_available(dp) )
+            goto gp_fault;
+        *val = dp->vmx_procbased_ctls2.raw;
+        break;
+
+    case MSR_IA32_VMX_EPT_VPID_CAP:
+        if ( !vmx_ept_vpid_cap_available(dp) )
+            goto gp_fault;
+        *val = dp->vmx_ept_vpid_cap.raw;
+        break;
+
+    case MSR_IA32_VMX_TRUE_PINBASED_CTLS ... MSR_IA32_VMX_TRUE_ENTRY_CTLS:
+        if ( !vmx_true_ctls_available(dp) )
+            goto gp_fault;
+        *val = dp->vmx_true_ctls.raw[msr - MSR_IA32_VMX_TRUE_PINBASED_CTLS];
+        break;
+
+    case MSR_IA32_VMX_VMFUNC:
+        if ( !vmx_vmfunc_available(dp) )
+            goto gp_fault;
+        *val = dp->vmx_vmfunc.raw;
+        break;
+
     default:
         return X86EMUL_UNHANDLEABLE;
     }
@@ -424,6 +455,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
     {
     case MSR_INTEL_PLATFORM_INFO:
     case MSR_ARCH_CAPABILITIES:
+    case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMFUNC:
         /* Read-only */
         goto gp_fault;
 
diff --git a/xen/include/asm-x86/hvm/vmx/vvmx.h b/xen/include/asm-x86/hvm/vmx/vvmx.h
index 9ea35eb795..d8ae93e61a 100644
--- a/xen/include/asm-x86/hvm/vmx/vvmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vvmx.h
@@ -200,8 +200,6 @@ int nvmx_handle_vmresume(struct cpu_user_regs *regs);
 int nvmx_handle_vmlaunch(struct cpu_user_regs *regs);
 int nvmx_handle_invept(struct cpu_user_regs *regs);
 int nvmx_handle_invvpid(struct cpu_user_regs *regs);
-int nvmx_msr_read_intercept(unsigned int msr,
-                                u64 *msr_content);
 
 void nvmx_update_exec_control(struct vcpu *v, u32 value);
 void nvmx_update_secondary_exec_control(struct vcpu *v,
-- 
2.14.1


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v6 1/5] x86/msr: add VMX MSRs definitions and populate Raw domain policy
  2018-03-22 11:32 ` [PATCH v6 1/5] x86/msr: add VMX MSRs definitions and populate Raw domain policy Sergey Dyasli
@ 2018-04-02  2:11   ` Tian, Kevin
  0 siblings, 0 replies; 10+ messages in thread
From: Tian, Kevin @ 2018-04-02  2:11 UTC (permalink / raw)
  To: Sergey Dyasli, xen-devel; +Cc: Andrew Cooper, Jan Beulich, Nakajima, Jun

> From: Sergey Dyasli [mailto:sergey.dyasli@citrix.com]
> Sent: Thursday, March 22, 2018 7:33 PM
> 
> New definitions provide a convenient way of accessing contents of
> VMX MSRs. They are separated into 5 logical blocks based on the
> availability conditions of MSRs in the each block:
> 
>     1. vmx: [VMX_BASIC, VMX_VMCS_ENUM]
>     2. VMX_PROCBASED_CTLS2
>     3. VMX_EPT_VPID_CAP
>     4. vmx_true_ctls: [VMX_TRUE_PINBASED_CTLS, VMX_TRUE_ENTRY_CTLS]
>     5. VMX_VMFUNC
> 
> Every bit value is accessible by its name and bit names match existing
> Xen's definitions as close as possible. There is a "raw" 64-bit field
> for each MSR as well as "raw" arrays for vmx and vmx_true_ctls blocks.
> 
> Add calculate_raw_vmx_policy() which fills Raw policy with H/W values
> of VMX MSRs. Host policy will contain a copy of these values (for now).
> 
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
> ---
> v5 --> v6:
> - Removed "_bits" and "_based" from union names
> - Removed "_exiting" suffixes from control bit names
> - Various shortenings of control bit names
> ---
>  xen/arch/x86/msr.c              | 118 ++++++++++++++
>  xen/include/asm-x86/msr.h       | 330
> ++++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-x86/x86-defns.h |  54 +++++++
>  3 files changed, 502 insertions(+)
> 
> diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
> index 369b4754ce..87239e151e 100644
> --- a/xen/arch/x86/msr.c
> +++ b/xen/arch/x86/msr.c
> @@ -34,10 +34,65 @@ struct msr_domain_policy __read_mostly
> raw_msr_domain_policy,
>  struct msr_vcpu_policy __read_mostly hvm_max_msr_vcpu_policy,
>                         __read_mostly  pv_max_msr_vcpu_policy;
> 
> +static bool vmx_procbased_ctls2_available(const struct
> msr_domain_policy *dp)
> +{
> +    return dp->vmx.procbased_ctls.allowed_1.secondary;
> +}
> +
> +static bool vmx_ept_vpid_cap_available(const struct msr_domain_policy
> *dp)
> +{
> +    return dp->vmx_procbased_ctls2.allowed_1.ept ||
> +           dp->vmx_procbased_ctls2.allowed_1.vpid;
> +}
> +
> +static bool vmx_true_ctls_available(const struct msr_domain_policy *dp)
> +{
> +    return dp->vmx.basic.default1_zero;
> +}
> +
> +static bool vmx_vmfunc_available(const struct msr_domain_policy *dp)
> +{
> +    return dp->vmx_procbased_ctls2.allowed_1.vmfunc;
> +}
> +
> +static void __init calculate_raw_vmx_policy(struct msr_domain_policy *dp)
> +{
> +    unsigned int i, start_msr, end_msr;
> +
> +    if ( !cpu_has_vmx )
> +        return;
> +
> +    start_msr = MSR_IA32_VMX_BASIC;
> +    end_msr = MSR_IA32_VMX_VMCS_ENUM;
> +    for ( i = start_msr; i <= end_msr; i++ )
> +        rdmsrl(i, dp->vmx.raw[i - start_msr]);
> +
> +    if ( vmx_procbased_ctls2_available(dp) )
> +        rdmsrl(MSR_IA32_VMX_PROCBASED_CTLS2, dp-
> >vmx_procbased_ctls2.raw);
> +
> +    if ( vmx_ept_vpid_cap_available(dp) )
> +        rdmsrl(MSR_IA32_VMX_EPT_VPID_CAP, dp->vmx_ept_vpid_cap.raw);
> +
> +    if ( vmx_true_ctls_available(dp) )
> +    {
> +        start_msr = MSR_IA32_VMX_TRUE_PINBASED_CTLS;
> +        end_msr = MSR_IA32_VMX_TRUE_ENTRY_CTLS;
> +        for ( i = start_msr; i <= end_msr; i++ )
> +            rdmsrl(i, dp->vmx_true_ctls.raw[i - start_msr]);
> +    }
> +
> +    if ( vmx_vmfunc_available(dp) )
> +        rdmsrl(MSR_IA32_VMX_VMFUNC, dp->vmx_vmfunc.raw);
> +}
> +
>  static void __init calculate_raw_policy(void)
>  {
> +    struct msr_domain_policy *dp = &raw_msr_domain_policy;
> +
>      /* 0x000000ce  MSR_INTEL_PLATFORM_INFO */
>      /* Was already added by probe_cpuid_faulting() */

could be more specific on what above comment is for otherwise 
it looks decorating the below one though inserted with a blank
line.

> +
> +    calculate_raw_vmx_policy(dp);
>  }
> 
>  static void __init calculate_host_policy(void)
> @@ -284,6 +339,69 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr,
> uint64_t val)
>      return X86EMUL_EXCEPTION;
>  }
> 
> +static void __init __maybe_unused build_assertions(void)
> +{
> +    struct msr_domain_policy dp;
> +
> +    BUILD_BUG_ON(sizeof(dp.vmx.basic) !=
> +                 sizeof(dp.vmx.basic.raw));
> +    BUILD_BUG_ON(sizeof(dp.vmx.pinbased_ctls) !=
> +                 sizeof(dp.vmx.pinbased_ctls.raw));
> +    BUILD_BUG_ON(sizeof(dp.vmx.procbased_ctls) !=
> +                 sizeof(dp.vmx.procbased_ctls.raw));
> +    BUILD_BUG_ON(sizeof(dp.vmx.exit_ctls) !=
> +                 sizeof(dp.vmx.exit_ctls.raw));
> +    BUILD_BUG_ON(sizeof(dp.vmx.entry_ctls) !=
> +                 sizeof(dp.vmx.entry_ctls.raw));
> +    BUILD_BUG_ON(sizeof(dp.vmx.misc) !=
> +                 sizeof(dp.vmx.misc.raw));
> +    BUILD_BUG_ON(sizeof(dp.vmx.cr0_fixed0) !=
> +                 sizeof(dp.vmx.cr0_fixed0.raw));
> +    BUILD_BUG_ON(sizeof(dp.vmx.cr0_fixed1) !=
> +                 sizeof(dp.vmx.cr0_fixed1.raw));
> +    BUILD_BUG_ON(sizeof(dp.vmx.cr4_fixed0) !=
> +                 sizeof(dp.vmx.cr4_fixed0.raw));
> +    BUILD_BUG_ON(sizeof(dp.vmx.cr4_fixed1) !=
> +                 sizeof(dp.vmx.cr4_fixed1.raw));
> +    BUILD_BUG_ON(sizeof(dp.vmx.vmcs_enum) !=
> +                 sizeof(dp.vmx.vmcs_enum.raw));
> +    BUILD_BUG_ON(sizeof(dp.vmx.raw) !=
> +                     sizeof(dp.vmx.basic) +
> +                     sizeof(dp.vmx.pinbased_ctls) +
> +                     sizeof(dp.vmx.procbased_ctls) +
> +                     sizeof(dp.vmx.exit_ctls) +
> +                     sizeof(dp.vmx.entry_ctls) +
> +                     sizeof(dp.vmx.misc) +
> +                     sizeof(dp.vmx.cr0_fixed0) +
> +                     sizeof(dp.vmx.cr0_fixed1) +
> +                     sizeof(dp.vmx.cr4_fixed0) +
> +                     sizeof(dp.vmx.cr4_fixed1) +
> +                     sizeof(dp.vmx.vmcs_enum));
> +
> +    BUILD_BUG_ON(sizeof(dp.vmx_procbased_ctls2) !=
> +                 sizeof(dp.vmx_procbased_ctls2.raw));
> +
> +    BUILD_BUG_ON(sizeof(dp.vmx_ept_vpid_cap) !=
> +                 sizeof(dp.vmx_ept_vpid_cap.raw));
> +
> +    BUILD_BUG_ON(sizeof(dp.vmx_true_ctls.pinbased) !=
> +                 sizeof(dp.vmx_true_ctls.pinbased.raw));
> +    BUILD_BUG_ON(sizeof(dp.vmx_true_ctls.procbased) !=
> +                 sizeof(dp.vmx_true_ctls.procbased.raw));
> +    BUILD_BUG_ON(sizeof(dp.vmx_true_ctls.exit) !=
> +                 sizeof(dp.vmx_true_ctls.exit.raw));
> +    BUILD_BUG_ON(sizeof(dp.vmx_true_ctls.entry) !=
> +                 sizeof(dp.vmx_true_ctls.entry.raw));
> +    BUILD_BUG_ON(sizeof(dp.vmx_true_ctls.raw) !=
> +                     sizeof(dp.vmx_true_ctls.pinbased) +
> +                     sizeof(dp.vmx_true_ctls.procbased) +
> +                     sizeof(dp.vmx_true_ctls.exit) +
> +                     sizeof(dp.vmx_true_ctls.entry));
> +
> +    BUILD_BUG_ON(sizeof(dp.vmx_vmfunc) !=
> +                 sizeof(dp.vmx_vmfunc.raw));
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/include/asm-x86/msr.h b/xen/include/asm-x86/msr.h
> index f14f265aa5..5fdf82860e 100644
> --- a/xen/include/asm-x86/msr.h
> +++ b/xen/include/asm-x86/msr.h
> @@ -10,6 +10,7 @@
>  #include <xen/errno.h>
>  #include <asm/asm_defns.h>
>  #include <asm/cpufeature.h>
> +#include <asm/x86-defns.h>
> 
>  #define rdmsr(msr,val1,val2) \
>       __asm__ __volatile__("rdmsr" \
> @@ -257,6 +258,127 @@ static inline void wrmsr_tsc_aux(uint32_t val)
>      }
>  }
> 
> +union vmx_pin_exec_control {
> +    uint32_t raw;
> +    struct {
> +        bool         ext_intr:1;
> +        uint32_t             :2;  /* 1:2 reserved */
> +        bool              nmi:1;
> +        uint32_t             :1;  /* 4 reserved */
> +        bool            vnmis:1;
> +        bool    preempt_timer:1;
> +        bool posted_interrupt:1;

posted_intr

> +        uint32_t             :24; /* 8:31 reserved */
> +    };
> +};
> +
> +union vmx_cpu_exec_control {

vmx_proc_exec_control to follow SDM

> +    uint32_t raw;
> +    struct {
> +        uint32_t                 :2;  /* 0:1 reserved */
> +        bool virtual_intr_pending:1;

better use same name as SDM, i.e. intr_window

> +        bool           tsc_offset:1;
> +        uint32_t                 :3;  /* 4:6 reserved */
> +        bool                  hlt:1;
> +        uint32_t                 :1;  /* 8 reserved */
> +        bool               invlpg:1;
> +        bool                mwait:1;
> +        bool                rdpmc:1;
> +        bool                rdtsc:1;
> +        uint32_t                 :2;  /* 13:14 reserved */
> +        bool             cr3_load:1;
> +        bool            cr3_store:1;
> +        uint32_t                 :2;  /* 17:18 reserved */
> +        bool             cr8_load:1;
> +        bool            cr8_store:1;
> +        bool           tpr_shadow:1;
> +        bool         vnmi_pending:1;

nmi_window

> +        bool               mov_dr:1;
> +        bool            uncond_io:1;
> +        bool            io_bitmap:1;
> +        uint32_t                 :1;  /* 26 reserved */
> +        bool                  mtf:1;
> +        bool           msr_bitmap:1;
> +        bool              monitor:1;
> +        bool                pause:1;
> +        bool            secondary:1;
> +    };
> +};
> +
> +union vmx_vmexit_control {
> +    uint32_t raw;
> +    struct {
> +        uint32_t               :2;  /* 0:1 reserved */
> +        bool   save_guest_debug:1;
> +        uint32_t               :6;  /* 3:8 reserved */
> +        bool         ia32e_mode:1;
> +        uint32_t               :2;  /* 10:11 reserved */
> +        bool     load_perf_ctrl:1;
> +        uint32_t               :2;  /* 13:14 reserved */
> +        bool           ack_intr:1;
> +        uint32_t               :2;  /* 16:17 reserved */
> +        bool     save_guest_pat:1;
> +        bool      load_host_pat:1;
> +        bool    save_guest_efer:1;
> +        bool     load_host_efer:1;
> +        bool save_preempt_timer:1;
> +        bool      clear_bndcfgs:1;
> +        bool    conceal_from_pt:1;
> +        uint32_t               :7;  /* 25:31 reserved */
> +    };

Is it really useful to spell guest/host explicitly? for vmexit
save implies guest state and load implies host state...

> +};
> +
> +union vmx_vmentry_control {
> +    uint32_t raw;
> +    struct {
> +        uint32_t                    :2;  /* 0:1 reserved */
> +        bool         load_debug_ctls:1;

and looks your naming here is inconsistent with
exit_control

> +        uint32_t                    :6;  /* 3:8 reserved */
> +        bool              ia32e_mode:1;
> +        bool                     smm:1;
> +        bool deactivate_dual_monitor:1;
> +        uint32_t                    :1;  /* 12 reserved */
> +        bool   load_perf_global_ctrl:1;
> +        bool          load_guest_pat:1;
> +        bool         load_guest_efer:1;
> +        bool            load_bndcfgs:1;
> +        bool conceal_entries_from_pt:1;
> +        uint32_t                    :14; /* 18:31 reserved */
> +    };
> +};
> +
> +union vmx_secondary_exec_control {

can move close to primary control?

> +    uint32_t raw;
> +    struct {
> +        bool           apic_accesses:1;

vapic_accesses

> +        bool                     ept:1;
> +        bool              desc_table:1;
> +        bool                  rdtscp:1;
> +        bool  virtualize_x2apic_mode:1;

vx2apic_mode

> +        bool                    vpid:1;
> +        bool                  wbinvd:1;
> +        bool            unrestricted:1;
> +        bool      apic_register_virt:1;

vapic_reg

> +        bool   virtual_intr_delivery:1;

vintr_delivery

> +        bool                     ple:1;
> +        bool                  rdrand:1;
> +        bool                 invpcid:1;
> +        bool                  vmfunc:1;
> +        bool          vmcs_shadowing:1;
> +        bool                   encls:1;
> +        bool                  rdseed:1;
> +        bool                     pml:1;
> +        bool                      ve:1;

ept_ve

> +        bool conceal_nonroot_from_pt:1;
> +        bool                  xsaves:1;
> +        uint32_t                    :1;  /* 21 reserved */
> +        bool ept_mode_based_exec_ctl:1;

remove _based_

> +        uint32_t                    :2;  /* 23:24 reserved */
> +        bool             tsc_scaling:1;
> +        uint32_t                    :6;  /* 26:31 reserved */
> +    };
> +};
> +
>  /* MSR policy object for shared per-domain MSRs */
>  struct msr_domain_policy
>  {
> @@ -265,6 +387,214 @@ struct msr_domain_policy
>          bool available; /* This MSR is non-architectural */
>          bool cpuid_faulting;
>      } plaform_info;
> +
> +    union {
> +        uint64_t raw[MSR_IA32_VMX_VMCS_ENUM - MSR_IA32_VMX_BASIC
> + 1];
> +
> +        struct {
> +            /* 0x00000480  MSR_IA32_VMX_BASIC */
> +            union {
> +                uint64_t raw;
> +                struct {
> +                    uint32_t      vmcs_rev_id:31;
> +                    bool                  mbz:1;  /* 31 always zero */
> +                    uint32_t vmcs_region_size:13;
> +                    uint32_t                 :3;  /* 45:47 reserved */
> +                    bool      addresses_32bit:1;

32bit_addresses

> +                    bool         dual_monitor:1;
> +                    uint32_t      memory_type:4;
> +                    bool         ins_out_info:1;
> +                    bool        default1_zero:1;
> +                    uint32_t                 :8;  /* 56:63 reserved */
> +                };
> +            } basic;
> +
> +            /* 0x00000481  MSR_IA32_VMX_PINBASED_CTLS */
> +            union {
> +                uint64_t raw;
> +                struct {
> +                    union vmx_pin_exec_control allowed_0;
> +                    union vmx_pin_exec_control allowed_1;
> +                };
> +            } pinbased_ctls;
> +
> +            /* 0x00000482  MSR_IA32_VMX_PROCBASED_CTLS */
> +            union {
> +                uint64_t raw;
> +                struct {
> +                    union vmx_cpu_exec_control allowed_0;
> +                    union vmx_cpu_exec_control allowed_1;
> +                };
> +            } procbased_ctls;
> +
> +            /* 0x00000483  MSR_IA32_VMX_EXIT_CTLS */
> +            union {
> +                uint64_t raw;
> +                struct {
> +                    union vmx_vmexit_control allowed_0;
> +                    union vmx_vmexit_control allowed_1;
> +                };
> +            } exit_ctls;
> +
> +            /* 0x00000484  MSR_IA32_VMX_ENTRY_CTLS */
> +            union {
> +                uint64_t raw;
> +                struct {
> +                    union vmx_vmentry_control allowed_0;
> +                    union vmx_vmentry_control allowed_1;
> +                };
> +            } entry_ctls;
> +
> +            /* 0x00000485  MSR_IA32_VMX_MISC */
> +            union {
> +                uint64_t raw;
> +                struct {
> +                    uint32_t   preempt_timer_scale:5;
> +                    bool         vmexit_stores_lma:1;
> +                    bool                 hlt_state:1;
> +                    bool            shutdown_state:1;
> +                    bool       wait_for_sipi_state:1;

remove '_state'

> +                    uint32_t                      :5;  /* 9:13 reserved */
> +                    bool                 pt_in_vmx:1;
> +                    bool               ia32_smbase:1;
> +                    uint32_t            cr3_target:9;

cr3_target_num

> +                    uint32_t    max_msr_load_count:3;
> +                    bool ia32_smm_monitor_ctl_bit2:1;

ia32_ could be removed

> +                    bool               vmwrite_all:1;
> +                    bool        inject_ilen0_event:1;
> +                    uint32_t                      :1;  /* 31 reserved */
> +                    uint32_t      mseg_revision_id;
> +                };
> +            } misc;
> +
> +            /* 0x00000486  MSR_IA32_VMX_CR0_FIXED0 */
> +            union {
> +                uint64_t raw;
> +                union cr0_bits allowed_0;
> +            } cr0_fixed0;
> +
> +            /* 0x00000487  MSR_IA32_VMX_CR0_FIXED1 */
> +            union {
> +                uint64_t raw;
> +                union cr0_bits allowed_1;
> +            } cr0_fixed1;
> +
> +            /* 0x00000488  MSR_IA32_VMX_CR4_FIXED0 */
> +            union {
> +                uint64_t raw;
> +                union cr4_bits allowed_0;
> +            } cr4_fixed0;
> +
> +            /* 0x00000489  MSR_IA32_VMX_CR4_FIXED1 */
> +            union {
> +                uint64_t raw;
> +                union cr4_bits allowed_1;
> +            } cr4_fixed1;
> +
> +            /* 0x0000048A  MSR_IA32_VMX_VMCS_ENUM */
> +            union {
> +                uint64_t raw;
> +                struct {
> +                    uint32_t             :1;  /* 0 reserved */
> +                    uint32_t max_encoding:9;
> +                    uint64_t             :54; /* 10:63 reserved */
> +                };
> +            } vmcs_enum;
> +        };
> +    } vmx;
> +
> +    /* 0x0000048B  MSR_IA32_VMX_PROCBASED_CTLS2 */
> +    union {
> +        uint64_t raw;
> +        struct {
> +            union vmx_secondary_exec_control allowed_0;
> +            union vmx_secondary_exec_control allowed_1;
> +        };
> +    } vmx_procbased_ctls2;
> +
> +    /* 0x0000048C  MSR_IA32_VMX_EPT_VPID_CAP */
> +    union {
> +        uint64_t raw;
> +        struct {
> +            bool                   x_only:1;
> +            uint32_t                     :5;  /* 1:5 reserved */
> +            bool                      wl4:1;
> +            uint32_t                     :1;  /* 7 reserved */
> +            bool                       uc:1;
> +            uint32_t                     :5;  /* 9:13 reserved */
> +            bool                       wb:1;
> +            uint32_t                     :1;  /* 15 reserved */
> +            bool                 page_2mb:1;
> +            bool                 page_1gb:1;
> +            uint32_t                     :2;  /* 18:19 reserved */
> +            bool                   invept:1;
> +            bool                   ad_bit:1;
> +            bool  advanced_ept_violations:1;
> +            uint32_t                     :2;  /* 23:24 reserved */
> +            bool            invept_single:1;
> +            bool               invept_all:1;
> +            uint32_t                     :5;  /* 27:31 reserved */
> +            bool                  invvpid:1;
> +            uint32_t                     :7;  /* 33:39 reserved */
> +            bool  invvpid_individual_addr:1;
> +            bool   invvpid_single_context:1;
> +            bool      invvpid_all_context:1;
> +            bool invvpid_single_nonglobal:1;
> +            uint32_t                    :20; /* 44:63 reserved */
> +        };
> +    } vmx_ept_vpid_cap;
> +
> +    union {
> +        uint64_t raw[MSR_IA32_VMX_TRUE_ENTRY_CTLS -
> +                     MSR_IA32_VMX_TRUE_PINBASED_CTLS + 1];
> +
> +        struct {
> +            /* 0x0000048D  MSR_IA32_VMX_TRUE_PINBASED_CTLS */
> +            union {
> +                uint64_t raw;
> +                struct {
> +                    union vmx_pin_exec_control allowed_0;
> +                    union vmx_pin_exec_control allowed_1;
> +                };
> +            } pinbased;
> +
> +            /* 0x0000048E  MSR_IA32_VMX_TRUE_PROCBASED_CTLS */
> +            union {
> +                uint64_t raw;
> +                struct {
> +                    union vmx_cpu_exec_control allowed_0;
> +                    union vmx_cpu_exec_control allowed_1;
> +                };
> +            } procbased;
> +
> +            /* 0x0000048F  MSR_IA32_VMX_TRUE_EXIT_CTLS */
> +            union {
> +                uint64_t raw;
> +                struct {
> +                    union vmx_vmexit_control allowed_0;
> +                    union vmx_vmexit_control allowed_1;
> +                };
> +            } exit;
> +
> +            /* 0x00000490  MSR_IA32_VMX_TRUE_ENTRY_CTLS */
> +            union {
> +                uint64_t raw;
> +                struct {
> +                    union vmx_vmentry_control allowed_0;
> +                    union vmx_vmentry_control allowed_1;
> +                };
> +            } entry;
> +        };
> +    } vmx_true_ctls;
> +
> +    /* 0x00000491  MSR_IA32_VMX_VMFUNC */
> +    union {
> +        uint64_t raw;
> +        struct {
> +            bool eptp_switching:1;
> +            uint64_t           :63; /* 1:63 reserved */
> +        };
> +    } vmx_vmfunc;
>  };
> 
>  /* RAW msr domain policy: contains the actual values from H/W MSRs */
> diff --git a/xen/include/asm-x86/x86-defns.h b/xen/include/asm-x86/x86-
> defns.h
> index ff8d66be3c..1d26328520 100644
> --- a/xen/include/asm-x86/x86-defns.h
> +++ b/xen/include/asm-x86/x86-defns.h
> @@ -42,6 +42,28 @@
>  #define X86_CR0_CD              0x40000000 /* Cache Disable            (RW) */
>  #define X86_CR0_PG              0x80000000 /* Paging                   (RW) */
> 
> +#ifndef __ASSEMBLY__
> +union cr0_bits {
> +    uint64_t raw;
> +    struct {
> +        bool     pe:1;
> +        bool     mp:1;
> +        bool     em:1;
> +        bool     ts:1;
> +        bool     et:1;
> +        bool     ne:1;
> +        uint32_t   :10; /* 6:15 reserved */
> +        bool     wp:1;
> +        uint32_t   :1;  /* 17 reserved */
> +        bool     am:1;
> +        uint32_t   :10; /* 19:28 reserved */
> +        bool     nw:1;
> +        bool     cd:1;
> +        bool     pg:1;
> +    };
> +};
> +#endif /* #ifndef __ASSEMBLY__ */
> +
>  /*
>   * Intel CPU flags in CR3
>   */
> @@ -71,6 +93,38 @@
>  #define X86_CR4_SMAP       0x00200000 /* enable SMAP */
>  #define X86_CR4_PKE        0x00400000 /* enable PKE */
> 
> +#ifndef __ASSEMBLY__
> +union cr4_bits {
> +    uint64_t raw;
> +    struct {
> +        bool        vme:1;
> +        bool        pvi:1;
> +        bool        tsd:1;
> +        bool         de:1;
> +        bool        pse:1;
> +        bool        pae:1;
> +        bool        mce:1;
> +        bool        pge:1;
> +        bool        pce:1;
> +        bool     osfxsr:1;
> +        bool osxmmexcpt:1;
> +        bool       umip:1;
> +        uint32_t       :1;  /* 12 reserved */
> +        bool       vmxe:1;
> +        bool       smxe:1;
> +        uint32_t       :1;  /* 15 reserved */
> +        bool   fsgsbase:1;
> +        bool      pcide:1;
> +        bool    osxsave:1;
> +        uint32_t       :1;  /* 19 reserved */
> +        bool       smep:1;
> +        bool       smap:1;
> +        bool        pke:1;
> +        uint32_t       :9;  /* 23:31 reserved */
> +    };
> +};
> +#endif /* #ifndef __ASSEMBLY__ */
> +
>  /*
>   * XSTATE component flags in XCR0
>   */
> --
> 2.14.1


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v6 2/5] x86/msr: add VMX MSRs into HVM_max domain policy
  2018-03-22 11:32 ` [PATCH v6 2/5] x86/msr: add VMX MSRs into HVM_max " Sergey Dyasli
@ 2018-04-02  2:14   ` Tian, Kevin
  0 siblings, 0 replies; 10+ messages in thread
From: Tian, Kevin @ 2018-04-02  2:14 UTC (permalink / raw)
  To: Sergey Dyasli, xen-devel; +Cc: Andrew Cooper, Jan Beulich, Nakajima, Jun

> From: Sergey Dyasli [mailto:sergey.dyasli@citrix.com]
> Sent: Thursday, March 22, 2018 7:33 PM
> 
> Currently, when nested virt is enabled, the set of L1 VMX features
> is fixed and calculated by nvmx_msr_read_intercept() as an intersection
> between the full set of Xen's supported L1 VMX features, the set of
> actual H/W features and, for MSR_IA32_VMX_EPT_VPID_CAP, the set of
> features that Xen uses.
> 
> Add calculate_hvm_max_vmx_policy() which will save the end result of
> nvmx_msr_read_intercept() on current H/W into HVM_max domain policy.
> There will be no functional change to what L1 sees in VMX MSRs. But the
> actual use of HVM_max domain policy will happen later, when VMX MSRs
> are handled by guest_rd/wrmsr().

It's not good to directly touch vmx detail in such common file...

> 
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
> ---
> v5 --> v6:
> - Replaced !cpu_has_vmx check with !hvm_max_cpuid_policy.basic.vmx
> - Added a TODO reminder
> - Added brackets around bit or expressions
> ---
>  xen/arch/x86/msr.c | 135
> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 135 insertions(+)
> 
> diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
> index 87239e151e..01a5b52f95 100644
> --- a/xen/arch/x86/msr.c
> +++ b/xen/arch/x86/msr.c
> @@ -106,6 +106,139 @@ static void __init calculate_host_policy(void)
>      dp->plaform_info.cpuid_faulting = cpu_has_cpuid_faulting;
>  }
> 
> +static void vmx_clear_policy(struct msr_domain_policy *dp)
> +{
> +    memset(dp->vmx.raw, 0, sizeof(dp->vmx.raw));
> +    dp->vmx_procbased_ctls2.raw = 0;
> +    dp->vmx_ept_vpid_cap.raw = 0;
> +    memset(dp->vmx_true_ctls.raw, 0, sizeof(dp->vmx_true_ctls.raw));
> +    dp->vmx_vmfunc.raw = 0;
> +}
> +
> +static void __init calculate_hvm_max_vmx_policy(struct
> msr_domain_policy *dp)
> +{
> +    const struct msr_domain_policy *hp = &host_msr_domain_policy;
> +    uint32_t supported;
> +
> +    if ( !hvm_max_cpuid_policy.basic.vmx )
> +        return;
> +
> +    vmx_clear_policy(dp);
> +
> +     /* TODO: actually make vmx features selection sane */
> +    dp->vmx.basic.raw = hp->vmx.basic.raw;
> +
> +    dp->vmx.pinbased_ctls.allowed_0.raw =
> VMX_PINBASED_CTLS_DEFAULT1;
> +    dp->vmx.pinbased_ctls.allowed_1.raw =
> VMX_PINBASED_CTLS_DEFAULT1;
> +    supported = (PIN_BASED_EXT_INTR_MASK |
> +                 PIN_BASED_NMI_EXITING   |
> +                 PIN_BASED_PREEMPT_TIMER);
> +    dp->vmx.pinbased_ctls.allowed_1.raw |= supported;
> +    dp->vmx.pinbased_ctls.allowed_1.raw &= hp-
> >vmx.pinbased_ctls.allowed_1.raw;
> +
> +    dp->vmx.procbased_ctls.allowed_0.raw =
> VMX_PROCBASED_CTLS_DEFAULT1;
> +    dp->vmx.procbased_ctls.allowed_1.raw =
> VMX_PROCBASED_CTLS_DEFAULT1;
> +    supported = (CPU_BASED_HLT_EXITING          |
> +                 CPU_BASED_VIRTUAL_INTR_PENDING |
> +                 CPU_BASED_CR8_LOAD_EXITING     |
> +                 CPU_BASED_CR8_STORE_EXITING    |
> +                 CPU_BASED_INVLPG_EXITING       |
> +                 CPU_BASED_MONITOR_EXITING      |
> +                 CPU_BASED_MWAIT_EXITING        |
> +                 CPU_BASED_MOV_DR_EXITING       |
> +                 CPU_BASED_ACTIVATE_IO_BITMAP   |
> +                 CPU_BASED_USE_TSC_OFFSETING    |
> +                 CPU_BASED_UNCOND_IO_EXITING    |
> +                 CPU_BASED_RDTSC_EXITING        |
> +                 CPU_BASED_MONITOR_TRAP_FLAG    |
> +                 CPU_BASED_VIRTUAL_NMI_PENDING  |
> +                 CPU_BASED_ACTIVATE_MSR_BITMAP  |
> +                 CPU_BASED_PAUSE_EXITING        |
> +                 CPU_BASED_RDPMC_EXITING        |
> +                 CPU_BASED_TPR_SHADOW           |
> +                 CPU_BASED_ACTIVATE_SECONDARY_CONTROLS);
> +    dp->vmx.procbased_ctls.allowed_1.raw |= supported;
> +    dp->vmx.procbased_ctls.allowed_1.raw &=
> +        hp->vmx.procbased_ctls.allowed_1.raw;
> +
> +    dp->vmx.exit_ctls.allowed_0.raw = VMX_EXIT_CTLS_DEFAULT1;
> +    dp->vmx.exit_ctls.allowed_1.raw = VMX_EXIT_CTLS_DEFAULT1;
> +    supported = (VM_EXIT_ACK_INTR_ON_EXIT   |
> +                 VM_EXIT_IA32E_MODE         |
> +                 VM_EXIT_SAVE_PREEMPT_TIMER |
> +                 VM_EXIT_SAVE_GUEST_PAT     |
> +                 VM_EXIT_LOAD_HOST_PAT      |
> +                 VM_EXIT_SAVE_GUEST_EFER    |
> +                 VM_EXIT_LOAD_HOST_EFER     |
> +                 VM_EXIT_LOAD_PERF_GLOBAL_CTRL);
> +    dp->vmx.exit_ctls.allowed_1.raw |= supported;
> +    dp->vmx.exit_ctls.allowed_1.raw &= hp->vmx.exit_ctls.allowed_1.raw;
> +
> +    dp->vmx.entry_ctls.allowed_0.raw = VMX_ENTRY_CTLS_DEFAULT1;
> +    dp->vmx.entry_ctls.allowed_1.raw = VMX_ENTRY_CTLS_DEFAULT1;
> +    supported = (VM_ENTRY_LOAD_GUEST_PAT        |
> +                 VM_ENTRY_LOAD_GUEST_EFER       |
> +                 VM_ENTRY_LOAD_PERF_GLOBAL_CTRL |
> +                 VM_ENTRY_IA32E_MODE);
> +    dp->vmx.entry_ctls.allowed_1.raw |= supported;
> +    dp->vmx.entry_ctls.allowed_1.raw &= hp-
> >vmx.entry_ctls.allowed_1.raw;
> +
> +    dp->vmx.misc.raw = hp->vmx.misc.raw;
> +    /* Do not support CR3-target feature now */
> +    dp->vmx.misc.cr3_target = false;
> +
> +    /* PG, PE bits must be 1 in VMX operation */
> +    dp->vmx.cr0_fixed0.allowed_0.pe = true;
> +    dp->vmx.cr0_fixed0.allowed_0.pg = true;
> +
> +    /* allow 0-settings for all bits */
> +    dp->vmx.cr0_fixed1.allowed_1.raw = 0xffffffff;
> +
> +    /* VMXE bit must be 1 in VMX operation */
> +    dp->vmx.cr4_fixed0.allowed_0.vmxe = true;
> +
> +    /*
> +     * Allowed CR4 bits will be updated during domain creation by
> +     * hvm_cr4_guest_valid_bits()
> +     */
> +    dp->vmx.cr4_fixed1.allowed_1.raw = hp-
> >vmx.cr4_fixed1.allowed_1.raw;
> +
> +    /* The max index of VVMCS encoding is 0x1f. */
> +    dp->vmx.vmcs_enum.max_encoding = 0x1f;
> +
> +    if ( vmx_procbased_ctls2_available(dp) )
> +    {
> +        supported = (SECONDARY_EXEC_DESCRIPTOR_TABLE_EXITING |
> +                     SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
> +                     SECONDARY_EXEC_ENABLE_VPID              |
> +                     SECONDARY_EXEC_UNRESTRICTED_GUEST       |
> +                     SECONDARY_EXEC_ENABLE_EPT);
> +        dp->vmx_procbased_ctls2.allowed_1.raw |= supported;
> +        dp->vmx_procbased_ctls2.allowed_1.raw &=
> +            hp->vmx_procbased_ctls2.allowed_1.raw;
> +    }
> +
> +    if ( vmx_ept_vpid_cap_available(dp) )
> +        dp->vmx_ept_vpid_cap.raw = nept_get_ept_vpid_cap();
> +
> +    if ( vmx_true_ctls_available(dp) )
> +    {
> +        dp->vmx_true_ctls.pinbased.raw = dp->vmx.pinbased_ctls.raw;
> +
> +        dp->vmx_true_ctls.procbased.raw = dp->vmx.procbased_ctls.raw;
> +        supported = (CPU_BASED_CR3_LOAD_EXITING |
> +                     CPU_BASED_CR3_STORE_EXITING);
> +        dp->vmx_true_ctls.procbased.raw |= supported;
> +        dp->vmx_true_ctls.procbased.raw &= hp-
> >vmx_true_ctls.procbased.raw;
> +
> +        dp->vmx_true_ctls.exit.raw = dp->vmx.exit_ctls.raw;
> +
> +        dp->vmx_true_ctls.entry.raw = dp->vmx.entry_ctls.raw;
> +    }
> +
> +    /* MSR_IA32_VMX_VMFUNC is N/A */
> +}
> +
>  static void __init calculate_hvm_max_policy(void)
>  {
>      struct msr_domain_policy *dp = &hvm_max_msr_domain_policy;
> @@ -127,6 +260,8 @@ static void __init calculate_hvm_max_policy(void)
> 
>      /* 0x00000140  MSR_INTEL_MISC_FEATURES_ENABLES */
>      vp->misc_features_enables.available = dp->plaform_info.cpuid_faulting;
> +
> +    calculate_hvm_max_vmx_policy(dp);
>  }
> 
>  static void __init calculate_pv_max_policy(void)
> --
> 2.14.1


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v6 3/5] x86/cpuid: update signature of hvm_cr4_guest_valid_bits()
  2018-03-22 11:32 ` [PATCH v6 3/5] x86/cpuid: update signature of hvm_cr4_guest_valid_bits() Sergey Dyasli
@ 2018-04-02  2:15   ` Tian, Kevin
  0 siblings, 0 replies; 10+ messages in thread
From: Tian, Kevin @ 2018-04-02  2:15 UTC (permalink / raw)
  To: Sergey Dyasli, xen-devel; +Cc: Andrew Cooper, Jan Beulich, Nakajima, Jun

> From: Sergey Dyasli [mailto:sergey.dyasli@citrix.com]
> Sent: Thursday, March 22, 2018 7:33 PM
> 
> With the new cpuid infrastructure there is a domain-wide struct cpuid
> policy and there is no need to pass a separate struct vcpu * into
> hvm_cr4_guest_valid_bits() anymore. Make the function accept struct
> domain * instead and update callers.
> 
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

> ---
> v5 --> v6:
> - Added brackets to expression in vmx.c and replaced 0 with false
> - Added Reviewed-by
> ---
>  xen/arch/x86/hvm/domain.c       | 3 ++-
>  xen/arch/x86/hvm/hvm.c          | 7 +++----
>  xen/arch/x86/hvm/svm/svmdebug.c | 4 ++--
>  xen/arch/x86/hvm/vmx/vmx.c      | 4 ++--
>  xen/arch/x86/hvm/vmx/vvmx.c     | 2 +-
>  xen/include/asm-x86/hvm/hvm.h   | 2 +-
>  6 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/domain.c b/xen/arch/x86/hvm/domain.c
> index 60474649de..ce15ce0470 100644
> --- a/xen/arch/x86/hvm/domain.c
> +++ b/xen/arch/x86/hvm/domain.c
> @@ -111,6 +111,7 @@ static int check_segment(struct segment_register
> *reg, enum x86_segment seg)
>  /* Called by VCPUOP_initialise for HVM guests. */
>  int arch_set_info_hvm_guest(struct vcpu *v, const vcpu_hvm_context_t
> *ctx)
>  {
> +    const struct domain *d = v->domain;
>      struct cpu_user_regs *uregs = &v->arch.user_regs;
>      struct segment_register cs, ds, ss, es, tr;
>      const char *errstr;
> @@ -272,7 +273,7 @@ int arch_set_info_hvm_guest(struct vcpu *v, const
> vcpu_hvm_context_t *ctx)
>      if ( v->arch.hvm_vcpu.guest_efer & EFER_LME )
>          v->arch.hvm_vcpu.guest_efer |= EFER_LMA;
> 
> -    if ( v->arch.hvm_vcpu.guest_cr[4] & ~hvm_cr4_guest_valid_bits(v, 0) )
> +    if ( v->arch.hvm_vcpu.guest_cr[4] & ~hvm_cr4_guest_valid_bits(d,
> false) )
>      {
>          gprintk(XENLOG_ERR, "Bad CR4 value: %#016lx\n",
>                  v->arch.hvm_vcpu.guest_cr[4]);
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 5759c73dd4..fe253034f2 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -931,9 +931,8 @@ const char *hvm_efer_valid(const struct vcpu *v,
> uint64_t value,
>          X86_CR0_CD | X86_CR0_PG)))
> 
>  /* These bits in CR4 can be set by the guest. */
> -unsigned long hvm_cr4_guest_valid_bits(const struct vcpu *v, bool restore)
> +unsigned long hvm_cr4_guest_valid_bits(const struct domain *d, bool
> restore)
>  {
> -    const struct domain *d = v->domain;
>      const struct cpuid_policy *p;
>      bool mce, vmxe;
> 
> @@ -1000,7 +999,7 @@ static int hvm_load_cpu_ctxt(struct domain *d,
> hvm_domain_context_t *h)
>          return -EINVAL;
>      }
> 
> -    if ( ctxt.cr4 & ~hvm_cr4_guest_valid_bits(v, 1) )
> +    if ( ctxt.cr4 & ~hvm_cr4_guest_valid_bits(d, true) )
>      {
>          printk(XENLOG_G_ERR "HVM%d restore: bad CR4 %#" PRIx64 "\n",
>                 d->domain_id, ctxt.cr4);
> @@ -2350,7 +2349,7 @@ int hvm_set_cr4(unsigned long value, bool_t
> may_defer)
>      struct vcpu *v = current;
>      unsigned long old_cr;
> 
> -    if ( value & ~hvm_cr4_guest_valid_bits(v, 0) )
> +    if ( value & ~hvm_cr4_guest_valid_bits(v->domain, false) )
>      {
>          HVM_DBG_LOG(DBG_LEVEL_1,
>                      "Guest attempts to set reserved bit in CR4: %lx",
> diff --git a/xen/arch/x86/hvm/svm/svmdebug.c
> b/xen/arch/x86/hvm/svm/svmdebug.c
> index 091c58fa1b..6c215d19fe 100644
> --- a/xen/arch/x86/hvm/svm/svmdebug.c
> +++ b/xen/arch/x86/hvm/svm/svmdebug.c
> @@ -121,9 +121,9 @@ bool svm_vmcb_isvalid(const char *from, const
> struct vmcb_struct *vmcb,
>             (cr3 >> v->domain->arch.cpuid->extd.maxphysaddr))) )
>          PRINTF("CR3: MBZ bits are set (%#"PRIx64")\n", cr3);
> 
> -    if ( cr4 & ~hvm_cr4_guest_valid_bits(v, false) )
> +    if ( cr4 & ~hvm_cr4_guest_valid_bits(v->domain, false) )
>          PRINTF("CR4: invalid bits are set (%#"PRIx64", valid: %#"PRIx64")\n",
> -               cr4, hvm_cr4_guest_valid_bits(v, false));
> +               cr4, hvm_cr4_guest_valid_bits(v->domain, false));
> 
>      if ( vmcb_get_dr6(vmcb) >> 32 )
>          PRINTF("DR6: bits [63:32] are not zero (%#"PRIx64")\n",
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index c5cc96339e..847c314a08 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -1598,8 +1598,8 @@ static void vmx_update_guest_cr(struct vcpu *v,
> unsigned int cr,
>               * Update CR4 host mask to only trap when the guest tries to set
>               * bits that are controlled by the hypervisor.
>               */
> -            v->arch.hvm_vmx.cr4_host_mask = HVM_CR4_HOST_MASK |
> X86_CR4_PKE |
> -                                            ~hvm_cr4_guest_valid_bits(v, 0);
> +            v->arch.hvm_vmx.cr4_host_mask = (HVM_CR4_HOST_MASK |
> X86_CR4_PKE |
> +                                   ~hvm_cr4_guest_valid_bits(v->domain, false));
>              v->arch.hvm_vmx.cr4_host_mask |= v-
> >arch.hvm_vmx.vmx_realmode ?
>                                               X86_CR4_VME : 0;
>              v->arch.hvm_vmx.cr4_host_mask |= !hvm_paging_enabled(v) ?
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c
> b/xen/arch/x86/hvm/vmx/vvmx.c
> index dcd3b28f86..43f7297c04 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -2140,7 +2140,7 @@ int nvmx_msr_read_intercept(unsigned int msr,
> u64 *msr_content)
>          data = X86_CR4_VMXE;
>          break;
>      case MSR_IA32_VMX_CR4_FIXED1:
> -        data = hvm_cr4_guest_valid_bits(v, 0);
> +        data = hvm_cr4_guest_valid_bits(d, false);
>          break;
>      case MSR_IA32_VMX_MISC:
>          /* Do not support CR3-target feature now */
> diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-
> x86/hvm/hvm.h
> index 2376ed6912..bfd42b065d 100644
> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -621,7 +621,7 @@ static inline bool altp2m_vcpu_emulate_ve(struct
> vcpu *v)
>  /* Check CR4/EFER values */
>  const char *hvm_efer_valid(const struct vcpu *v, uint64_t value,
>                             signed int cr0_pg);
> -unsigned long hvm_cr4_guest_valid_bits(const struct vcpu *v, bool
> restore);
> +unsigned long hvm_cr4_guest_valid_bits(const struct domain *d, bool
> restore);
> 
>  /*
>   * This must be defined as a macro instead of an inline function,
> --
> 2.14.1


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v6 0/5] VMX MSRs policy for Nested Virt: part 1
  2018-03-22 11:32 [PATCH v6 0/5] VMX MSRs policy for Nested Virt: part 1 Sergey Dyasli
                   ` (4 preceding siblings ...)
  2018-03-22 11:32 ` [PATCH v6 5/5] x86/msr: handle VMX MSRs with guest_rd/wrmsr() Sergey Dyasli
@ 2018-04-30 15:46 ` Jan Beulich
  5 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2018-04-30 15:46 UTC (permalink / raw)
  To: Sergey Dyasli; +Cc: Andrew Cooper, Kevin Tian, Jun Nakajima, xen-devel

>>> On 22.03.18 at 12:32, <sergey.dyasli@citrix.com> wrote:
> The end goal of having VMX MSRs policy is to be able to manage
> L1 VMX features. This patch series is the first part of this work.
> There is no functional change to what L1 sees in VMX MSRs at this
> point. But each domain will have a policy object which allows to
> sensibly query what VMX features the domain has. This will unblock
> some other nested virtualization work items.
> 
> Currently, when nested virt is enabled, the set of L1 VMX features
> is fixed and calculated by nvmx_msr_read_intercept() as an intersection
> between the full set of Xen's supported L1 VMX features, the set of
> actual H/W features and, for MSR_IA32_VMX_EPT_VPID_CAP, the set of
> features that Xen uses.
> 
> The above makes L1 VMX feature set inconsistent between different H/W
> and there is no ability to control what features are available to L1.
> The overall set of issues has much in common with CPUID policy.
> 
> Part 1 adds VMX MSRs into struct msr_domain_policy and initializes them
> during domain creation based on CPUID policy. In the future it should be
> possible to independently configure values of VMX MSRs for each domain.
> 
> v5 --> v6:
> - Various shortenings of control bit names
> - Added Reviewed-by: Andrew Cooper to pathes 3,4 and 5
> - Other changes are provided on per-patch basis
> 
> Sergey Dyasli (5):
>   x86/msr: add VMX MSRs definitions and populate Raw domain policy
>   x86/msr: add VMX MSRs into HVM_max domain policy
>   x86/cpuid: update signature of hvm_cr4_guest_valid_bits()
>   x86/msr: update domain policy on CPUID policy changes
>   x86/msr: handle VMX MSRs with guest_rd/wrmsr()

Provided there's going to be a R-b by one of the VMX maintainers,
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan



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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-04-30 15:46 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-22 11:32 [PATCH v6 0/5] VMX MSRs policy for Nested Virt: part 1 Sergey Dyasli
2018-03-22 11:32 ` [PATCH v6 1/5] x86/msr: add VMX MSRs definitions and populate Raw domain policy Sergey Dyasli
2018-04-02  2:11   ` Tian, Kevin
2018-03-22 11:32 ` [PATCH v6 2/5] x86/msr: add VMX MSRs into HVM_max " Sergey Dyasli
2018-04-02  2:14   ` Tian, Kevin
2018-03-22 11:32 ` [PATCH v6 3/5] x86/cpuid: update signature of hvm_cr4_guest_valid_bits() Sergey Dyasli
2018-04-02  2:15   ` Tian, Kevin
2018-03-22 11:32 ` [PATCH v6 4/5] x86/msr: update domain policy on CPUID policy changes Sergey Dyasli
2018-03-22 11:32 ` [PATCH v6 5/5] x86/msr: handle VMX MSRs with guest_rd/wrmsr() Sergey Dyasli
2018-04-30 15:46 ` [PATCH v6 0/5] VMX MSRs policy for Nested Virt: part 1 Jan Beulich

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.