* [PATCH v2 0/5] VMX MSRs policy for Nested Virt: part 1
@ 2017-07-24 13:47 Sergey Dyasli
2017-07-24 13:47 ` [PATCH v2 1/5] x86/vmx: add struct vmx_msr_policy Sergey Dyasli
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Sergey Dyasli @ 2017-07-24 13:47 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 introduces struct vmx_msr_policy and the following instances:
* Raw policy (raw_vmx_msr_policy) -- the actual contents of H/W VMX MSRs
* VVMX max policy (vvmx_max_msr_policy) -- the end result of
nvmx_msr_read_intercept() on current H/W
* Per-domain policy (d->arch.vmx_msr) -- the copy of VVMX max policy
(for now)
In the future it should be possible to independently configure the VMX
policy for each domain using some new domctl.
There is no "Host policy" object because Xen already has a set of
variables (vmx_pin_based_exec_control and others) which represent
the set of VMX features that Xen uses. There are features that Xen
doesn't use (e.g. CPU_BASED_PAUSE_EXITING) but they are available to L1.
This makes it not worthy to introduce "Host policy" at this stage.
v1 --> v2:
- Rebased to the latest master
- hvm_max_vmx_msr_policy is renamed to vvmx_max_msr_policy
- Dropped the debug patch
- Other changes are available on a per-patch basis
Sergey Dyasli (5):
x86/vmx: add struct vmx_msr_policy
x86/vmx: add raw_vmx_msr_policy
x86/vmx: refactor vmx_init_vmcs_config()
x86/vvmx: add vvmx_max_msr_policy
x86/vvmx: add per domain vmx msr policy
xen/arch/x86/domain.c | 6 +
xen/arch/x86/hvm/vmx/vmcs.c | 269 +++++++++++++++++---------
xen/arch/x86/hvm/vmx/vmx.c | 2 +
xen/arch/x86/hvm/vmx/vvmx.c | 296 ++++++++++++++--------------
xen/include/asm-x86/domain.h | 2 +
xen/include/asm-x86/hvm/vmx/vmcs.h | 383 +++++++++++++++++++++++++++++++++++++
xen/include/asm-x86/hvm/vmx/vvmx.h | 3 +
xen/include/asm-x86/msr-index.h | 1 +
8 files changed, 722 insertions(+), 240 deletions(-)
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/5] x86/vmx: add struct vmx_msr_policy
2017-07-24 13:47 [PATCH v2 0/5] VMX MSRs policy for Nested Virt: part 1 Sergey Dyasli
@ 2017-07-24 13:47 ` Sergey Dyasli
2017-07-27 8:11 ` Tian, Kevin
2017-07-24 13:47 ` [PATCH v2 2/5] x86/vmx: add raw_vmx_msr_policy Sergey Dyasli
` (4 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Sergey Dyasli @ 2017-07-24 13:47 UTC (permalink / raw)
To: xen-devel
Cc: Andrew Cooper, Kevin Tian, Jan Beulich, Jun Nakajima, Sergey Dyasli
This structure provides a convenient way of accessing contents of
VMX MSRs: every bit value is accessible by its name. Bit names match
existing Xen's definitions as close as possible. The structure also
contains the bitmap of available MSRs since not all of them may be
available on a particular H/W.
A set of helper functions is introduced to provide a simple way of
interacting with the new structure.
Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
---
v1 --> v2:
- Replaced MSR indices with MSR names in struct vmx_msr_policy's comments
- Named "always zero bit" 31 of basic msr as mbz
- Added placeholder bits into union vmfunc
- Added structures cr0_bits and cr4_bits
- Added MSR_IA32_VMX_LAST define to use instead of MSR_IA32_VMX_VMFUNC
- vmx_msr_available() now uses pointer to const struct vmx_msr_policy
- build_assertions() now uses local struct vmx_msr_policy
- Added BUILD_BUG_ON to check that width of vmx_msr_policy::available
bitmap is enough for all existing VMX MSRs
- Helpers get_vmx_msr_val(), get_vmx_msr_ptr() and gen_vmx_msr_mask()
are added
xen/arch/x86/hvm/vmx/vmcs.c | 78 ++++++++
xen/include/asm-x86/hvm/vmx/vmcs.h | 380 +++++++++++++++++++++++++++++++++++++
xen/include/asm-x86/msr-index.h | 1 +
3 files changed, 459 insertions(+)
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 8103b20d29..33715748f0 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -144,6 +144,40 @@ static void __init vmx_display_features(void)
printk(" - none\n");
}
+bool vmx_msr_available(const struct vmx_msr_policy *p, uint32_t msr)
+{
+ if ( msr < MSR_IA32_VMX_BASIC || msr > MSR_IA32_VMX_LAST )
+ return 0;
+
+ return p->available & (1u << (msr - MSR_IA32_VMX_BASIC));
+}
+
+uint64_t get_vmx_msr_val(const struct vmx_msr_policy *p, uint32_t msr)
+{
+ if ( !vmx_msr_available(p, msr))
+ return 0;
+
+ return p->msr[msr - MSR_IA32_VMX_BASIC];
+}
+
+uint64_t *get_vmx_msr_ptr(struct vmx_msr_policy *p, uint32_t msr)
+{
+ if ( !vmx_msr_available(p, msr))
+ return NULL;
+
+ return &p->msr[msr - MSR_IA32_VMX_BASIC];
+}
+
+uint32_t gen_vmx_msr_mask(uint32_t start_msr, uint32_t end_msr)
+{
+ if ( start_msr < MSR_IA32_VMX_BASIC || start_msr > MSR_IA32_VMX_LAST ||
+ end_msr < MSR_IA32_VMX_BASIC || end_msr > MSR_IA32_VMX_LAST )
+ return 0;
+
+ return ((1u << (end_msr - start_msr + 1)) - 1) <<
+ (start_msr - MSR_IA32_VMX_BASIC);
+}
+
static u32 adjust_vmx_controls(
const char *name, u32 ctl_min, u32 ctl_opt, u32 msr, bool_t *mismatch)
{
@@ -1956,6 +1990,50 @@ void __init setup_vmcs_dump(void)
register_keyhandler('v', vmcs_dump, "dump VT-x VMCSs", 1);
}
+static void __init __maybe_unused build_assertions(void)
+{
+ struct vmx_msr_policy policy;
+
+ BUILD_BUG_ON(sizeof(policy.basic) !=
+ sizeof(policy.basic.raw));
+ BUILD_BUG_ON(sizeof(policy.pinbased_ctls) !=
+ sizeof(policy.pinbased_ctls.raw));
+ BUILD_BUG_ON(sizeof(policy.procbased_ctls) !=
+ sizeof(policy.procbased_ctls.raw));
+ BUILD_BUG_ON(sizeof(policy.exit_ctls) !=
+ sizeof(policy.exit_ctls.raw));
+ BUILD_BUG_ON(sizeof(policy.entry_ctls) !=
+ sizeof(policy.entry_ctls.raw));
+ BUILD_BUG_ON(sizeof(policy.misc) !=
+ sizeof(policy.misc.raw));
+ BUILD_BUG_ON(sizeof(policy.cr0_fixed_0) !=
+ sizeof(policy.cr0_fixed_0.raw));
+ BUILD_BUG_ON(sizeof(policy.cr0_fixed_1) !=
+ sizeof(policy.cr0_fixed_1.raw));
+ BUILD_BUG_ON(sizeof(policy.cr4_fixed_0) !=
+ sizeof(policy.cr4_fixed_0.raw));
+ BUILD_BUG_ON(sizeof(policy.cr4_fixed_1) !=
+ sizeof(policy.cr4_fixed_1.raw));
+ BUILD_BUG_ON(sizeof(policy.vmcs_enum) !=
+ sizeof(policy.vmcs_enum.raw));
+ BUILD_BUG_ON(sizeof(policy.procbased_ctls2) !=
+ sizeof(policy.procbased_ctls2.raw));
+ BUILD_BUG_ON(sizeof(policy.ept_vpid_cap) !=
+ sizeof(policy.ept_vpid_cap.raw));
+ BUILD_BUG_ON(sizeof(policy.true_pinbased_ctls) !=
+ sizeof(policy.true_pinbased_ctls.raw));
+ BUILD_BUG_ON(sizeof(policy.true_procbased_ctls) !=
+ sizeof(policy.true_procbased_ctls.raw));
+ BUILD_BUG_ON(sizeof(policy.true_exit_ctls) !=
+ sizeof(policy.true_exit_ctls.raw));
+ BUILD_BUG_ON(sizeof(policy.true_entry_ctls) !=
+ sizeof(policy.true_entry_ctls.raw));
+ BUILD_BUG_ON(sizeof(policy.vmfunc) !=
+ sizeof(policy.vmfunc.raw));
+
+ BUILD_BUG_ON(MSR_IA32_VMX_LAST - MSR_IA32_VMX_BASIC + 1 >
+ sizeof(policy.available) * 8);
+}
/*
* Local variables:
diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h
index e3cdfdf576..c6ff3fe0b8 100644
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
@@ -562,6 +562,386 @@ void vmx_domain_flush_pml_buffers(struct domain *d);
void vmx_domain_update_eptp(struct domain *d);
+union vmx_pin_based_exec_control_bits {
+ uint32_t raw;
+ struct {
+ bool ext_intr_exiting:1;
+ uint32_t :2; /* 1:2 reserved */
+ bool nmi_exiting:1;
+ uint32_t :1; /* 4 reserved */
+ bool virtual_nmis:1;
+ bool preempt_timer:1;
+ bool posted_interrupt:1;
+ uint32_t :24; /* 8:31 reserved */
+ };
+};
+
+union vmx_cpu_based_exec_control_bits {
+ uint32_t raw;
+ struct {
+ uint32_t :2; /* 0:1 reserved */
+ bool virtual_intr_pending:1;
+ bool use_tsc_offseting:1;
+ uint32_t :3; /* 4:6 reserved */
+ bool hlt_exiting:1;
+ uint32_t :1; /* 8 reserved */
+ bool invlpg_exiting:1;
+ bool mwait_exiting:1;
+ bool rdpmc_exiting:1;
+ bool rdtsc_exiting:1;
+ uint32_t :2; /* 13:14 reserved */
+ bool cr3_load_exiting:1;
+ bool cr3_store_exiting:1;
+ uint32_t :2; /* 17:18 reserved */
+ bool cr8_load_exiting:1;
+ bool cr8_store_exiting:1;
+ bool tpr_shadow_0:1;
+ bool virtual_nmi_pending:1;
+ bool mov_dr_exiting:1;
+ bool uncond_io_exiting:1;
+ bool activate_io_bitmap:1;
+ uint32_t :1; /* 26 reserved */
+ bool monitor_trap_flag:1;
+ bool activate_msr_bitmap:1;
+ bool monitor_exiting:1;
+ bool pause_exiting:1;
+ bool activate_secondary_controls:1;
+ };
+};
+
+union vmx_vmexit_control_bits {
+ uint32_t raw;
+ struct {
+ uint32_t :2; /* 0:1 reserved */
+ bool save_debug_cntrls:1;
+ uint32_t :6; /* 3:8 reserved */
+ bool ia32e_mode:1;
+ uint32_t :2; /* 10:11 reserved */
+ bool load_perf_global_ctrl:1;
+ uint32_t :2; /* 13:14 reserved */
+ bool ack_intr_on_exit: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_vmexits_from_pt:1;
+ uint32_t :7; /* 25:31 reserved */
+ };
+};
+
+union vmx_vmentry_control_bits {
+ uint32_t raw;
+ struct {
+ uint32_t :2; /* 0:1 reserved */
+ bool load_debug_cntrls:1;
+ uint32_t :6; /* 3:8 reserved */
+ bool ia32e_mode:1;
+ bool smm:1;
+ bool deact_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_vmentries_from_pt:1;
+ uint32_t :14; /* 18:31 reserved */
+ };
+};
+
+union vmx_secondary_exec_control_bits {
+ uint32_t raw;
+ struct {
+ bool virtualize_apic_accesses:1;
+ bool enable_ept:1;
+ bool descriptor_table_exiting:1;
+ bool enable_rdtscp:1;
+ bool virtualize_x2apic_mode:1;
+ bool enable_vpid:1;
+ bool wbinvd_exiting:1;
+ bool unrestricted_guest:1;
+ bool apic_register_virt:1;
+ bool virtual_intr_delivery:1;
+ bool pause_loop_exiting:1;
+ bool rdrand_exiting:1;
+ bool enable_invpcid:1;
+ bool enable_vm_functions:1;
+ bool enable_vmcs_shadowing:1;
+ bool encls_exiting:1;
+ bool rdseed_exiting:1;
+ bool enable_pml:1;
+ bool enable_virt_exceptions:1;
+ bool conceal_vmx_nonroot_from_pt:1;
+ bool xsaves:1;
+ uint32_t :1; /* 21 reserved */
+ bool ept_mode_based_exec_cntrl:1;
+ uint32_t :2; /* 23:24 reserved */
+ bool tsc_scaling:1;
+ uint32_t :6; /* 26:31 reserved */
+ };
+};
+
+struct cr0_bits {
+ 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;
+};
+
+struct cr4_bits {
+ 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 */
+};
+
+struct vmx_msr_policy
+{
+ /*
+ * Bitmap of readable MSRs, starting from MSR_IA32_VMX_BASIC,
+ * derived from contents of MSRs in this structure.
+ */
+ uint32_t available;
+
+ union {
+ uint64_t msr[MSR_IA32_VMX_LAST - MSR_IA32_VMX_BASIC + 1];
+
+ struct {
+ /* MSR_IA32_VMX_BASIC */
+ union {
+ uint64_t raw;
+ struct {
+ uint32_t vmcs_revision_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;
+
+ /* MSR_IA32_VMX_PINBASED_CTLS */
+ union {
+ uint64_t raw;
+ struct {
+ union vmx_pin_based_exec_control_bits allowed_0;
+ union vmx_pin_based_exec_control_bits allowed_1;
+ };
+ } pinbased_ctls;
+
+ /* MSR_IA32_VMX_PROCBASED_CTLS */
+ union {
+ uint64_t raw;
+ struct {
+ union vmx_cpu_based_exec_control_bits allowed_0;
+ union vmx_cpu_based_exec_control_bits allowed_1;
+ };
+ } procbased_ctls;
+
+ /* MSR_IA32_VMX_EXIT_CTLS */
+ union {
+ uint64_t raw;
+ struct {
+ union vmx_vmexit_control_bits allowed_0;
+ union vmx_vmexit_control_bits allowed_1;
+ };
+ } exit_ctls;
+
+ /* MSR_IA32_VMX_ENTRY_CTLS */
+ union {
+ uint64_t raw;
+ struct {
+ union vmx_vmentry_control_bits allowed_0;
+ union vmx_vmentry_control_bits allowed_1;
+ };
+ } entry_ctls;
+
+ /* MSR_IA32_VMX_MISC */
+ union {
+ uint64_t raw;
+ struct {
+ uint32_t preempt_timer_scale:5;
+ bool vmexit_stores_lma:1;
+ bool hlt_activity_state:1;
+ bool shutdown_activity_state:1;
+ bool wait_for_sipi_activity_state:1;
+ uint32_t :5; /* 9:13 reserved */
+ bool pt_in_vmx:1;
+ bool ia32_smbase_support: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;
+
+ /* MSR_IA32_VMX_CR0_FIXED0 */
+ union {
+ uint64_t raw;
+ struct cr0_bits allowed_0;
+ } cr0_fixed_0;
+
+ /* MSR_IA32_VMX_CR0_FIXED1 */
+ union {
+ uint64_t raw;
+ struct cr0_bits allowed_1;
+ } cr0_fixed_1;
+
+ /* MSR_IA32_VMX_CR4_FIXED0 */
+ union {
+ uint64_t raw;
+ struct cr4_bits allowed_0;
+ } cr4_fixed_0;
+
+ /* MSR_IA32_VMX_CR4_FIXED1 */
+ union {
+ uint64_t raw;
+ struct cr4_bits allowed_1;
+ } cr4_fixed_1;
+
+ /* MSR_IA32_VMX_VMCS_ENUM */
+ union {
+ uint64_t raw;
+ struct {
+ uint32_t :1; /* 0 reserved */
+ uint32_t vmcs_encoding_max_idx:9;
+ uint64_t :54; /* 10:63 reserved */
+ };
+ } vmcs_enum;
+
+ /* MSR_IA32_VMX_PROCBASED_CTLS2 */
+ union {
+ uint64_t raw;
+ struct {
+ union vmx_secondary_exec_control_bits allowed_0;
+ union vmx_secondary_exec_control_bits allowed_1;
+ };
+ } procbased_ctls2;
+
+ /* MSR_IA32_VMX_EPT_VPID_CAP */
+ union {
+ uint64_t raw;
+ struct {
+ bool exec_only_supported:1;
+ uint32_t :5; /* 1:5 reserved */
+ bool walk_length_4_supported:1;
+ uint32_t :1; /* 7 reserved */
+ bool memory_type_uc:1;
+ uint32_t :5; /* 9:13 reserved */
+ bool memory_type_wb:1;
+ uint32_t :1; /* 15 reserved */
+ bool superpage_2mb:1;
+ bool superpage_1gb:1;
+ uint32_t :2; /* 18:19 reserved */
+ bool invept_instruction:1;
+ bool ad_bit:1;
+ bool advanced_ept_violations:1;
+ uint32_t :2; /* 23:24 reserved */
+ bool invept_single_context:1;
+ bool invept_all_context:1;
+ uint32_t :5; /* 27:31 reserved */
+ bool invvpid_instruction: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_context_retaining_global:1;
+ uint32_t :20; /* 44:63 reserved */
+ };
+ } ept_vpid_cap;
+
+ /* MSR_IA32_VMX_TRUE_PINBASED_CTLS */
+ union {
+ uint64_t raw;
+ struct {
+ union vmx_pin_based_exec_control_bits allowed_0;
+ union vmx_pin_based_exec_control_bits allowed_1;
+ };
+ } true_pinbased_ctls;
+
+ /* MSR_IA32_VMX_TRUE_PROCBASED_CTLS */
+ union {
+ uint64_t raw;
+ struct {
+ union vmx_cpu_based_exec_control_bits allowed_0;
+ union vmx_cpu_based_exec_control_bits allowed_1;
+ };
+ } true_procbased_ctls;
+
+ /* MSR_IA32_VMX_TRUE_EXIT_CTLS */
+ union {
+ uint64_t raw;
+ struct {
+ union vmx_vmexit_control_bits allowed_0;
+ union vmx_vmexit_control_bits allowed_1;
+ };
+ } true_exit_ctls;
+
+ /* MSR_IA32_VMX_TRUE_ENTRY_CTLS */
+ union {
+ uint64_t raw;
+ struct {
+ union vmx_vmentry_control_bits allowed_0;
+ union vmx_vmentry_control_bits allowed_1;
+ };
+ } true_entry_ctls;
+
+ /* MSR_IA32_VMX_VMFUNC */
+ union {
+ uint64_t raw;
+ struct {
+ bool eptp_switching:1;
+ uint64_t :63; /* 1:63 reserved */
+ };
+ } vmfunc;
+ };
+ };
+};
+
+bool vmx_msr_available(const struct vmx_msr_policy *p, uint32_t msr);
+uint64_t get_vmx_msr_val(const struct vmx_msr_policy *p, uint32_t msr);
+uint64_t *get_vmx_msr_ptr(struct vmx_msr_policy *p, uint32_t msr);
+uint32_t gen_vmx_msr_mask(uint32_t start_msr, uint32_t end_msr);
+
#endif /* ASM_X86_HVM_VMX_VMCS_H__ */
/*
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index 756b23d19e..90b23e6952 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -139,6 +139,7 @@
#define MSR_IA32_VMX_TRUE_EXIT_CTLS 0x48f
#define MSR_IA32_VMX_TRUE_ENTRY_CTLS 0x490
#define MSR_IA32_VMX_VMFUNC 0x491
+#define MSR_IA32_VMX_LAST MSR_IA32_VMX_VMFUNC
/* K7/K8 MSRs. Not complete. See the architecture manual for a more
complete list. */
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/5] x86/vmx: add raw_vmx_msr_policy
2017-07-24 13:47 [PATCH v2 0/5] VMX MSRs policy for Nested Virt: part 1 Sergey Dyasli
2017-07-24 13:47 ` [PATCH v2 1/5] x86/vmx: add struct vmx_msr_policy Sergey Dyasli
@ 2017-07-24 13:47 ` Sergey Dyasli
2017-07-27 8:21 ` Tian, Kevin
2017-07-24 13:47 ` [PATCH v2 3/5] x86/vmx: refactor vmx_init_vmcs_config() Sergey Dyasli
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Sergey Dyasli @ 2017-07-24 13:47 UTC (permalink / raw)
To: xen-devel
Cc: Andrew Cooper, Kevin Tian, Jan Beulich, Jun Nakajima, Sergey Dyasli
Add calculate_vmx_raw_policy() which fills the raw_vmx_msr_policy
object (the actual contents of H/W VMX MSRs) on the boot CPU. On
secondary CPUs, this function checks that contents of VMX MSRs match
the boot CPU's contents.
Remove lesser version of same-contents-check from vmx_init_vmcs_config().
Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
---
v1 --> v2:
- calculate_raw_policy() is renamed to calculate_vmx_raw_policy()
to avoid clash with the same-name function in cpuid.c
- Declaration of calculate_vmx_raw_policy() is removed from vmx.c
and added to vmcs.h
- msr variable is now unsigned in calculate_vmx_raw_policy()
- "\n" moved to the same line as the printk format string
- Replaced magic constants for available bitmap with gen_vmx_msr_mask()
- get_vmx_msr_ptr() and get_vmx_msr_val() helpers are used instead of
accessing MSR array directly
xen/arch/x86/hvm/vmx/vmcs.c | 134 +++++++++++++++++++++----------------
xen/arch/x86/hvm/vmx/vmx.c | 2 +
xen/include/asm-x86/hvm/vmx/vmcs.h | 3 +
3 files changed, 82 insertions(+), 57 deletions(-)
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 33715748f0..8070ed21c8 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -144,6 +144,8 @@ static void __init vmx_display_features(void)
printk(" - none\n");
}
+struct vmx_msr_policy __read_mostly raw_vmx_msr_policy;
+
bool vmx_msr_available(const struct vmx_msr_policy *p, uint32_t msr)
{
if ( msr < MSR_IA32_VMX_BASIC || msr > MSR_IA32_VMX_LAST )
@@ -178,6 +180,78 @@ uint32_t gen_vmx_msr_mask(uint32_t start_msr, uint32_t end_msr)
(start_msr - MSR_IA32_VMX_BASIC);
}
+int calculate_vmx_raw_policy(bool bsp)
+{
+ struct vmx_msr_policy policy;
+ struct vmx_msr_policy *p = &policy;
+ unsigned int msr;
+
+ /* Raw policy is filled only on boot CPU */
+ if ( bsp )
+ p = &raw_vmx_msr_policy;
+ else
+ memset(&policy, 0, sizeof(policy));
+
+ p->available = gen_vmx_msr_mask(MSR_IA32_VMX_BASIC, MSR_IA32_VMX_VMCS_ENUM);
+ for ( msr = MSR_IA32_VMX_BASIC; msr <= MSR_IA32_VMX_VMCS_ENUM; msr++ )
+ rdmsrl(msr, *get_vmx_msr_ptr(p, msr));
+
+ if ( p->basic.default1_zero )
+ {
+ p->available |= gen_vmx_msr_mask(MSR_IA32_VMX_TRUE_PINBASED_CTLS,
+ MSR_IA32_VMX_TRUE_ENTRY_CTLS);
+ for ( msr = MSR_IA32_VMX_TRUE_PINBASED_CTLS;
+ msr <= MSR_IA32_VMX_TRUE_ENTRY_CTLS; msr++ )
+ rdmsrl(msr, *get_vmx_msr_ptr(p, msr));
+ }
+
+ if ( p->procbased_ctls.allowed_1.activate_secondary_controls )
+ {
+ p->available |= gen_vmx_msr_mask(MSR_IA32_VMX_PROCBASED_CTLS2,
+ MSR_IA32_VMX_PROCBASED_CTLS2);
+ msr = MSR_IA32_VMX_PROCBASED_CTLS2;
+ rdmsrl(msr, *get_vmx_msr_ptr(p, msr));
+
+ if ( p->procbased_ctls2.allowed_1.enable_ept ||
+ p->procbased_ctls2.allowed_1.enable_vpid )
+ {
+ p->available |= gen_vmx_msr_mask(MSR_IA32_VMX_EPT_VPID_CAP,
+ MSR_IA32_VMX_EPT_VPID_CAP);
+ msr = MSR_IA32_VMX_EPT_VPID_CAP;
+ rdmsrl(msr, *get_vmx_msr_ptr(p, msr));
+ }
+
+ if ( p->procbased_ctls2.allowed_1.enable_vm_functions )
+ {
+ p->available |= gen_vmx_msr_mask(MSR_IA32_VMX_VMFUNC,
+ MSR_IA32_VMX_VMFUNC);
+ msr = MSR_IA32_VMX_VMFUNC;
+ rdmsrl(msr, *get_vmx_msr_ptr(p, msr));
+ }
+ }
+
+ /* Check that secondary CPUs have exactly the same bits in VMX MSRs */
+ if ( !bsp && memcmp(p, &raw_vmx_msr_policy, sizeof(*p)) != 0 )
+ {
+ for ( msr = MSR_IA32_VMX_BASIC; msr <= MSR_IA32_VMX_LAST; msr++ )
+ {
+ if ( get_vmx_msr_val(p, msr) !=
+ get_vmx_msr_val(&raw_vmx_msr_policy, msr) )
+ {
+ printk("VMX msr %#x: saw 0x%016"PRIx64" expected 0x%016"PRIx64"\n",
+ msr, get_vmx_msr_val(p, msr),
+ get_vmx_msr_val(&raw_vmx_msr_policy, msr));
+ }
+ }
+
+ printk("VMX: Capabilities fatally differ between CPU%d and boot CPU\n",
+ smp_processor_id());
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static u32 adjust_vmx_controls(
const char *name, u32 ctl_min, u32 ctl_opt, u32 msr, bool_t *mismatch)
{
@@ -199,13 +273,6 @@ static u32 adjust_vmx_controls(
return ctl;
}
-static bool_t cap_check(const char *name, u32 expected, u32 saw)
-{
- if ( saw != expected )
- printk("VMX %s: saw %#x expected %#x\n", name, saw, expected);
- return saw != expected;
-}
-
static int vmx_init_vmcs_config(void)
{
u32 vmx_basic_msr_low, vmx_basic_msr_high, min, opt;
@@ -438,56 +505,6 @@ static int vmx_init_vmcs_config(void)
return -EINVAL;
}
}
- else
- {
- /* Globals are already initialised: re-check them. */
- mismatch |= cap_check(
- "VMCS revision ID",
- vmcs_revision_id, vmx_basic_msr_low & VMX_BASIC_REVISION_MASK);
- mismatch |= cap_check(
- "Pin-Based Exec Control",
- vmx_pin_based_exec_control, _vmx_pin_based_exec_control);
- mismatch |= cap_check(
- "CPU-Based Exec Control",
- vmx_cpu_based_exec_control, _vmx_cpu_based_exec_control);
- mismatch |= cap_check(
- "Secondary Exec Control",
- vmx_secondary_exec_control, _vmx_secondary_exec_control);
- mismatch |= cap_check(
- "VMExit Control",
- vmx_vmexit_control, _vmx_vmexit_control);
- mismatch |= cap_check(
- "VMEntry Control",
- vmx_vmentry_control, _vmx_vmentry_control);
- mismatch |= cap_check(
- "EPT and VPID Capability",
- vmx_ept_vpid_cap, _vmx_ept_vpid_cap);
- mismatch |= cap_check(
- "VMFUNC Capability",
- vmx_vmfunc, _vmx_vmfunc);
- if ( cpu_has_vmx_ins_outs_instr_info !=
- !!(vmx_basic_msr_high & (VMX_BASIC_INS_OUT_INFO >> 32)) )
- {
- printk("VMX INS/OUTS Instruction Info: saw %d expected %d\n",
- !!(vmx_basic_msr_high & (VMX_BASIC_INS_OUT_INFO >> 32)),
- cpu_has_vmx_ins_outs_instr_info);
- mismatch = 1;
- }
- if ( (vmx_basic_msr_high & (VMX_BASIC_VMCS_SIZE_MASK >> 32)) !=
- ((vmx_basic_msr & VMX_BASIC_VMCS_SIZE_MASK) >> 32) )
- {
- printk("VMX: CPU%d unexpected VMCS size %Lu\n",
- smp_processor_id(),
- vmx_basic_msr_high & (VMX_BASIC_VMCS_SIZE_MASK >> 32));
- mismatch = 1;
- }
- if ( mismatch )
- {
- printk("VMX: Capabilities fatally differ between CPU%d and CPU0\n",
- smp_processor_id());
- return -EINVAL;
- }
- }
/* IA-32 SDM Vol 3B: 64-bit CPUs always have VMX_BASIC_MSR[48]==0. */
if ( vmx_basic_msr_high & (VMX_BASIC_32BIT_ADDRESSES >> 32) )
@@ -637,6 +654,9 @@ int vmx_cpu_up(void)
BUG_ON(!(read_cr4() & X86_CR4_VMXE));
+ if ( (rc = calculate_vmx_raw_policy(false)) != 0 )
+ return rc;
+
/*
* Ensure the current processor operating mode meets
* the requred CRO fixed bits in VMX operation.
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 69ce3aae25..ce6537d96f 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2440,6 +2440,8 @@ const struct hvm_function_table * __init start_vmx(void)
{
set_in_cr4(X86_CR4_VMXE);
+ calculate_vmx_raw_policy(true);
+
if ( vmx_cpu_up() )
{
printk("VMX: failed to initialise.\n");
diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h
index c6ff3fe0b8..25f84308dc 100644
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
@@ -942,6 +942,9 @@ uint64_t get_vmx_msr_val(const struct vmx_msr_policy *p, uint32_t msr);
uint64_t *get_vmx_msr_ptr(struct vmx_msr_policy *p, uint32_t msr);
uint32_t gen_vmx_msr_mask(uint32_t start_msr, uint32_t end_msr);
+extern struct vmx_msr_policy raw_vmx_msr_policy;
+int calculate_vmx_raw_policy(bool bsp);
+
#endif /* ASM_X86_HVM_VMX_VMCS_H__ */
/*
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/5] x86/vmx: refactor vmx_init_vmcs_config()
2017-07-24 13:47 [PATCH v2 0/5] VMX MSRs policy for Nested Virt: part 1 Sergey Dyasli
2017-07-24 13:47 ` [PATCH v2 1/5] x86/vmx: add struct vmx_msr_policy Sergey Dyasli
2017-07-24 13:47 ` [PATCH v2 2/5] x86/vmx: add raw_vmx_msr_policy Sergey Dyasli
@ 2017-07-24 13:47 ` Sergey Dyasli
2017-07-27 8:24 ` Tian, Kevin
2017-07-24 13:47 ` [PATCH v2 4/5] x86/vvmx: add vvmx_max_msr_policy Sergey Dyasli
` (2 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Sergey Dyasli @ 2017-07-24 13:47 UTC (permalink / raw)
To: xen-devel
Cc: Andrew Cooper, Kevin Tian, Jan Beulich, Jun Nakajima, Sergey Dyasli
1. Remove RDMSRs of VMX MSRs since all values are already available in
raw_vmx_msr_policy.
2. Replace bit operations involving VMX bitmasks with accessing VMX
features by name and using vmx_msr_available() where appropriate.
Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
---
v1 --> v2:
- get_vmx_msr_val() is used instead of accessing policy's msr array
directly
xen/arch/x86/hvm/vmx/vmcs.c | 56 +++++++++++++++++++++------------------------
1 file changed, 26 insertions(+), 30 deletions(-)
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 8070ed21c8..bd36b6e12a 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -257,7 +257,8 @@ static u32 adjust_vmx_controls(
{
u32 vmx_msr_low, vmx_msr_high, ctl = ctl_min | ctl_opt;
- rdmsr(msr, vmx_msr_low, vmx_msr_high);
+ vmx_msr_low = get_vmx_msr_val(&raw_vmx_msr_policy, msr);
+ vmx_msr_high = get_vmx_msr_val(&raw_vmx_msr_policy, msr) >> 32;
ctl &= vmx_msr_high; /* bit == 0 in high word ==> must be zero */
ctl |= vmx_msr_low; /* bit == 1 in low word ==> must be one */
@@ -275,19 +276,16 @@ static u32 adjust_vmx_controls(
static int vmx_init_vmcs_config(void)
{
- u32 vmx_basic_msr_low, vmx_basic_msr_high, min, opt;
+ u32 min, opt;
u32 _vmx_pin_based_exec_control;
u32 _vmx_cpu_based_exec_control;
u32 _vmx_secondary_exec_control = 0;
u64 _vmx_ept_vpid_cap = 0;
- u64 _vmx_misc_cap = 0;
u32 _vmx_vmexit_control;
u32 _vmx_vmentry_control;
u64 _vmx_vmfunc = 0;
bool_t mismatch = 0;
- rdmsr(MSR_IA32_VMX_BASIC, vmx_basic_msr_low, vmx_basic_msr_high);
-
min = (PIN_BASED_EXT_INTR_MASK |
PIN_BASED_NMI_EXITING);
opt = (PIN_BASED_VIRTUAL_NMIS |
@@ -321,7 +319,7 @@ static int vmx_init_vmcs_config(void)
_vmx_cpu_based_exec_control &=
~(CPU_BASED_CR8_LOAD_EXITING | CPU_BASED_CR8_STORE_EXITING);
- if ( _vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS )
+ if ( vmx_msr_available(&raw_vmx_msr_policy, MSR_IA32_VMX_PROCBASED_CTLS2) )
{
min = 0;
opt = (SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
@@ -335,8 +333,7 @@ static int vmx_init_vmcs_config(void)
SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS |
SECONDARY_EXEC_XSAVES |
SECONDARY_EXEC_TSC_SCALING);
- rdmsrl(MSR_IA32_VMX_MISC, _vmx_misc_cap);
- if ( _vmx_misc_cap & VMX_MISC_VMWRITE_ALL )
+ if ( raw_vmx_msr_policy.misc.vmwrite_all )
opt |= SECONDARY_EXEC_ENABLE_VMCS_SHADOWING;
if ( opt_vpid_enabled )
opt |= SECONDARY_EXEC_ENABLE_VPID;
@@ -361,10 +358,9 @@ static int vmx_init_vmcs_config(void)
}
/* The IA32_VMX_EPT_VPID_CAP MSR exists only when EPT or VPID available */
- if ( _vmx_secondary_exec_control & (SECONDARY_EXEC_ENABLE_EPT |
- SECONDARY_EXEC_ENABLE_VPID) )
+ if ( vmx_msr_available(&raw_vmx_msr_policy, MSR_IA32_VMX_EPT_VPID_CAP) )
{
- rdmsrl(MSR_IA32_VMX_EPT_VPID_CAP, _vmx_ept_vpid_cap);
+ _vmx_ept_vpid_cap = raw_vmx_msr_policy.ept_vpid_cap.raw;
if ( !opt_ept_ad )
_vmx_ept_vpid_cap &= ~VMX_EPT_AD_BIT;
@@ -409,10 +405,14 @@ static int vmx_init_vmcs_config(void)
* To use EPT we expect to be able to clear certain intercepts.
* We check VMX_BASIC_MSR[55] to correctly handle default controls.
*/
- uint32_t must_be_one, must_be_zero, msr = MSR_IA32_VMX_PROCBASED_CTLS;
- if ( vmx_basic_msr_high & (VMX_BASIC_DEFAULT1_ZERO >> 32) )
- msr = MSR_IA32_VMX_TRUE_PROCBASED_CTLS;
- rdmsr(msr, must_be_one, must_be_zero);
+ uint32_t must_be_one = raw_vmx_msr_policy.procbased_ctls.allowed_0.raw;
+ uint32_t must_be_zero = raw_vmx_msr_policy.procbased_ctls.allowed_1.raw;
+ if ( vmx_msr_available(&raw_vmx_msr_policy,
+ MSR_IA32_VMX_TRUE_PROCBASED_CTLS) )
+ {
+ must_be_one = raw_vmx_msr_policy.true_procbased_ctls.allowed_0.raw;
+ must_be_zero = raw_vmx_msr_policy.true_procbased_ctls.allowed_1.raw;
+ }
if ( must_be_one & (CPU_BASED_INVLPG_EXITING |
CPU_BASED_CR3_LOAD_EXITING |
CPU_BASED_CR3_STORE_EXITING) )
@@ -453,9 +453,9 @@ static int vmx_init_vmcs_config(void)
_vmx_pin_based_exec_control &= ~ PIN_BASED_POSTED_INTERRUPT;
/* The IA32_VMX_VMFUNC MSR exists only when VMFUNC is available */
- if ( _vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VM_FUNCTIONS )
+ if ( vmx_msr_available(&raw_vmx_msr_policy, MSR_IA32_VMX_VMFUNC) )
{
- rdmsrl(MSR_IA32_VMX_VMFUNC, _vmx_vmfunc);
+ _vmx_vmfunc = raw_vmx_msr_policy.vmfunc.raw;
/*
* VMFUNC leaf 0 (EPTP switching) must be supported.
@@ -481,33 +481,31 @@ static int vmx_init_vmcs_config(void)
if ( !vmx_pin_based_exec_control )
{
/* First time through. */
- vmcs_revision_id = vmx_basic_msr_low & VMX_BASIC_REVISION_MASK;
+ vmcs_revision_id = raw_vmx_msr_policy.basic.vmcs_revision_id;
vmx_pin_based_exec_control = _vmx_pin_based_exec_control;
vmx_cpu_based_exec_control = _vmx_cpu_based_exec_control;
vmx_secondary_exec_control = _vmx_secondary_exec_control;
vmx_ept_vpid_cap = _vmx_ept_vpid_cap;
vmx_vmexit_control = _vmx_vmexit_control;
vmx_vmentry_control = _vmx_vmentry_control;
- vmx_basic_msr = ((u64)vmx_basic_msr_high << 32) |
- vmx_basic_msr_low;
+ vmx_basic_msr = raw_vmx_msr_policy.basic.raw;
vmx_vmfunc = _vmx_vmfunc;
vmx_virt_exception = !!(_vmx_secondary_exec_control &
SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS);
vmx_display_features();
/* IA-32 SDM Vol 3B: VMCS size is never greater than 4kB. */
- if ( (vmx_basic_msr_high & (VMX_BASIC_VMCS_SIZE_MASK >> 32)) >
- PAGE_SIZE )
+ if ( raw_vmx_msr_policy.basic.vmcs_region_size > PAGE_SIZE )
{
- printk("VMX: CPU%d VMCS size is too big (%Lu bytes)\n",
+ printk("VMX: CPU%d VMCS size is too big (%d bytes)\n",
smp_processor_id(),
- vmx_basic_msr_high & (VMX_BASIC_VMCS_SIZE_MASK >> 32));
+ raw_vmx_msr_policy.basic.vmcs_region_size);
return -EINVAL;
}
}
/* IA-32 SDM Vol 3B: 64-bit CPUs always have VMX_BASIC_MSR[48]==0. */
- if ( vmx_basic_msr_high & (VMX_BASIC_32BIT_ADDRESSES >> 32) )
+ if ( raw_vmx_msr_policy.basic.addresses_32bit )
{
printk("VMX: CPU%d limits VMX structure pointers to 32 bits\n",
smp_processor_id());
@@ -515,9 +513,7 @@ static int vmx_init_vmcs_config(void)
}
/* Require Write-Back (WB) memory type for VMCS accesses. */
- opt = (vmx_basic_msr_high & (VMX_BASIC_MEMORY_TYPE_MASK >> 32)) /
- ((VMX_BASIC_MEMORY_TYPE_MASK & -VMX_BASIC_MEMORY_TYPE_MASK) >> 32);
- if ( opt != MTRR_TYPE_WRBACK )
+ if ( raw_vmx_msr_policy.basic.memory_type != MTRR_TYPE_WRBACK )
{
printk("VMX: CPU%d has unexpected VMCS access type %u\n",
smp_processor_id(), opt);
@@ -662,8 +658,8 @@ int vmx_cpu_up(void)
* the requred CRO fixed bits in VMX operation.
*/
cr0 = read_cr0();
- rdmsrl(MSR_IA32_VMX_CR0_FIXED0, vmx_cr0_fixed0);
- rdmsrl(MSR_IA32_VMX_CR0_FIXED1, vmx_cr0_fixed1);
+ vmx_cr0_fixed0 = raw_vmx_msr_policy.cr0_fixed_0.raw;
+ vmx_cr0_fixed1 = raw_vmx_msr_policy.cr0_fixed_1.raw;
if ( (~cr0 & vmx_cr0_fixed0) || (cr0 & ~vmx_cr0_fixed1) )
{
printk("CPU%d: some settings of host CR0 are "
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 4/5] x86/vvmx: add vvmx_max_msr_policy
2017-07-24 13:47 [PATCH v2 0/5] VMX MSRs policy for Nested Virt: part 1 Sergey Dyasli
` (2 preceding siblings ...)
2017-07-24 13:47 ` [PATCH v2 3/5] x86/vmx: refactor vmx_init_vmcs_config() Sergey Dyasli
@ 2017-07-24 13:47 ` Sergey Dyasli
2017-07-24 13:47 ` [PATCH v2 5/5] x86/vvmx: add per domain vmx msr policy Sergey Dyasli
2017-07-27 7:48 ` [PATCH v2 0/5] VMX MSRs policy for Nested Virt: part 1 Tian, Kevin
5 siblings, 0 replies; 11+ messages in thread
From: Sergey Dyasli @ 2017-07-24 13:47 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 vvmx_max_msr_policy object which represents the end result of
nvmx_msr_read_intercept() on current H/W. Most of the code is moved
from nvmx_msr_read_intercept() to calculate_vvmx_max_policy() which is
called only once during the startup.
There is no functional change to what L1 sees in VMX MSRs.
Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
---
v1 --> v2:
- Renamed hvm_max_vmx_msr_policy to vvmx_max_msr_policy and made it
static
- calculate_hvm_max_policy() is renamed to calculate_vvmx_max_policy()
- Declaration of calculate_vvmx_max_policy() is removed from vmcs.c
and added to vvmx.h
- Removed comment "XXX: vmcs_revision_id for nested virt"
- nvmx_msr_read_intercept() now uses const struct vmx_msr_policy *
- Shortened "msr = *msr &" to "*msr &="
- Removed usage of "data" as an intermediate variable for 4 MSRs
- Replaced magic constant for disabling MSR_IA32_VMX_VMFUNC with
gen_vmx_msr_mask()
- get_vmx_msr_ptr() and get_vmx_msr_val() helpers are used instead of
accessing MSR array directly
xen/arch/x86/hvm/vmx/vmcs.c | 1 +
xen/arch/x86/hvm/vmx/vvmx.c | 284 +++++++++++++++++--------------------
xen/include/asm-x86/hvm/vmx/vvmx.h | 2 +
3 files changed, 134 insertions(+), 153 deletions(-)
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index bd36b6e12a..67077cc41a 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -493,6 +493,7 @@ static int vmx_init_vmcs_config(void)
vmx_virt_exception = !!(_vmx_secondary_exec_control &
SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS);
vmx_display_features();
+ calculate_vvmx_max_policy();
/* IA-32 SDM Vol 3B: VMCS size is never greater than 4kB. */
if ( raw_vmx_msr_policy.basic.vmcs_region_size > PAGE_SIZE )
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 2c8cf637a8..e71728f356 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1941,6 +1941,8 @@ int nvmx_handle_invvpid(struct cpu_user_regs *regs)
return X86EMUL_OKAY;
}
+static struct vmx_msr_policy __read_mostly vvmx_max_msr_policy;
+
#define __emul_value(enable1, default1) \
((enable1 | default1) << 32 | (default1))
@@ -1948,6 +1950,128 @@ int nvmx_handle_invvpid(struct cpu_user_regs *regs)
(((__emul_value(enable1, default1) & host_value) & (~0ul << 32)) | \
((uint32_t)(__emul_value(enable1, default1) | host_value)))
+void __init calculate_vvmx_max_policy(void)
+{
+ struct vmx_msr_policy *p = &vvmx_max_msr_policy;
+ uint64_t data, *msr;
+ u32 default1_bits;
+
+ *p = raw_vmx_msr_policy;
+
+ /* Pinbased controls 1-settings */
+ data = PIN_BASED_EXT_INTR_MASK |
+ PIN_BASED_NMI_EXITING |
+ PIN_BASED_PREEMPT_TIMER;
+
+ msr = get_vmx_msr_ptr(p, MSR_IA32_VMX_PINBASED_CTLS);
+ *msr = gen_vmx_msr(data, VMX_PINBASED_CTLS_DEFAULT1, *msr);
+ msr = get_vmx_msr_ptr(p, MSR_IA32_VMX_TRUE_PINBASED_CTLS);
+ *msr = gen_vmx_msr(data, VMX_PINBASED_CTLS_DEFAULT1, *msr);
+
+ /* Procbased controls 1-settings */
+ default1_bits = VMX_PROCBASED_CTLS_DEFAULT1;
+ 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;
+
+ msr = get_vmx_msr_ptr(p, MSR_IA32_VMX_PROCBASED_CTLS);
+ *msr = gen_vmx_msr(data, default1_bits, *msr);
+
+ default1_bits &= ~(CPU_BASED_CR3_LOAD_EXITING |
+ CPU_BASED_CR3_STORE_EXITING |
+ CPU_BASED_INVLPG_EXITING);
+
+ msr = get_vmx_msr_ptr(p, MSR_IA32_VMX_TRUE_PROCBASED_CTLS);
+ *msr = gen_vmx_msr(data, default1_bits, *msr);
+
+ /* Procbased-2 controls 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;
+ msr = get_vmx_msr_ptr(p, MSR_IA32_VMX_PROCBASED_CTLS2);
+ *msr = gen_vmx_msr(data, 0, *msr);
+
+ /* Vmexit controls 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;
+ msr = get_vmx_msr_ptr(p, MSR_IA32_VMX_EXIT_CTLS);
+ *msr = gen_vmx_msr(data, VMX_EXIT_CTLS_DEFAULT1, *msr);
+ msr = get_vmx_msr_ptr(p, MSR_IA32_VMX_TRUE_EXIT_CTLS);
+ *msr = gen_vmx_msr(data, VMX_EXIT_CTLS_DEFAULT1, *msr);
+
+ /* Vmentry controls 1-settings */
+ data = VM_ENTRY_LOAD_GUEST_PAT |
+ VM_ENTRY_LOAD_GUEST_EFER |
+ VM_ENTRY_LOAD_PERF_GLOBAL_CTRL |
+ VM_ENTRY_IA32E_MODE;
+ msr = get_vmx_msr_ptr(p, MSR_IA32_VMX_ENTRY_CTLS);
+ *msr = gen_vmx_msr(data, VMX_ENTRY_CTLS_DEFAULT1, *msr);
+ msr = get_vmx_msr_ptr(p, MSR_IA32_VMX_TRUE_ENTRY_CTLS);
+ *msr = gen_vmx_msr(data, VMX_ENTRY_CTLS_DEFAULT1, *msr);
+
+ /* MSR_IA32_VMX_VMCS_ENUM */
+ /* The max index of VVMCS encoding is 0x1f. */
+ msr = get_vmx_msr_ptr(p, MSR_IA32_VMX_VMCS_ENUM);
+ *msr = 0x1f << 1;
+
+ /* MSR_IA32_VMX_CR0_FIXED0 */
+ /* PG, PE bits must be 1 in VMX operation */
+ msr = get_vmx_msr_ptr(p, MSR_IA32_VMX_CR0_FIXED0);
+ *msr = X86_CR0_PE | X86_CR0_PG;
+
+ /* MSR_IA32_VMX_CR0_FIXED1 */
+ /* allow 0-settings for all bits */
+ msr = get_vmx_msr_ptr(p, MSR_IA32_VMX_CR0_FIXED1);
+ *msr = 0xffffffff;
+
+ /* MSR_IA32_VMX_CR4_FIXED0 */
+ /* VMXE bit must be 1 in VMX operation */
+ msr = get_vmx_msr_ptr(p, MSR_IA32_VMX_CR4_FIXED0);
+ *msr = X86_CR4_VMXE;
+
+ /* MSR_IA32_VMX_CR4_FIXED1 */
+ /* Treated dynamically */
+
+ /* MSR_IA32_VMX_MISC */
+ /* Do not support CR3-target feature now */
+ msr = get_vmx_msr_ptr(p, MSR_IA32_VMX_MISC);
+ *msr &= ~VMX_MISC_CR3_TARGET;
+
+ /* MSR_IA32_VMX_EPT_VPID_CAP */
+ msr = get_vmx_msr_ptr(p, MSR_IA32_VMX_EPT_VPID_CAP);
+ *msr = nept_get_ept_vpid_cap();
+
+ /* MSR_IA32_VMX_VMFUNC is N/A */
+ p->available &= ~gen_vmx_msr_mask(MSR_IA32_VMX_VMFUNC,
+ MSR_IA32_VMX_VMFUNC);
+}
+
/*
* Capability reporting
*/
@@ -1955,167 +2079,21 @@ 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;
+ const struct vmx_msr_policy *p = &vvmx_max_msr_policy;
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;
+ if ( !vmx_msr_available(p, msr) )
+ return 0;
- 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(v, 0);
- 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;
- }
+ if ( msr == MSR_IA32_VMX_CR4_FIXED1 )
+ *msr_content = hvm_cr4_guest_valid_bits(v, 0);
+ else
+ *msr_content = get_vmx_msr_val(p, msr);
- *msr_content = data;
return r;
}
diff --git a/xen/include/asm-x86/hvm/vmx/vvmx.h b/xen/include/asm-x86/hvm/vmx/vvmx.h
index 3285b03bbb..150124f3a3 100644
--- a/xen/include/asm-x86/hvm/vmx/vvmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vvmx.h
@@ -244,5 +244,7 @@ int nept_translate_l2ga(struct vcpu *v, paddr_t l2ga,
uint64_t *exit_qual, uint32_t *exit_reason);
int nvmx_cpu_up_prepare(unsigned int cpu);
void nvmx_cpu_dead(unsigned int cpu);
+
+void calculate_vvmx_max_policy(void);
#endif /* __ASM_X86_HVM_VVMX_H__ */
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 5/5] x86/vvmx: add per domain vmx msr policy
2017-07-24 13:47 [PATCH v2 0/5] VMX MSRs policy for Nested Virt: part 1 Sergey Dyasli
` (3 preceding siblings ...)
2017-07-24 13:47 ` [PATCH v2 4/5] x86/vvmx: add vvmx_max_msr_policy Sergey Dyasli
@ 2017-07-24 13:47 ` Sergey Dyasli
2017-07-30 10:05 ` Jan Beulich
2017-07-27 7:48 ` [PATCH v2 0/5] VMX MSRs policy for Nested Virt: part 1 Tian, Kevin
5 siblings, 1 reply; 11+ messages in thread
From: Sergey Dyasli @ 2017-07-24 13:47 UTC (permalink / raw)
To: xen-devel
Cc: Andrew Cooper, Kevin Tian, Jan Beulich, Jun Nakajima, Sergey Dyasli
Having a policy per domain allows to sensibly query what VMX features
the domain has, which unblocks some other nested virt work items.
For now, make policy for each domain equal to vvmx_max_msr_policy.
In the future it should be possible to independently configure
the policy for each domain.
Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
v1 --> v2:
- nvmx_msr_read_intercept() now uses const struct vmx_msr_policy *
(starting from patch #4)
- Added Reviewed-by: Jan Beulich <jbeulich@suse.com>
xen/arch/x86/domain.c | 6 ++++++
xen/arch/x86/hvm/vmx/vvmx.c | 14 +++++++++++++-
xen/include/asm-x86/domain.h | 2 ++
xen/include/asm-x86/hvm/vmx/vvmx.h | 1 +
4 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index dd8bf1302f..e72f17c593 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -425,6 +425,7 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
{
d->arch.emulation_flags = 0;
d->arch.cpuid = ZERO_BLOCK_PTR; /* Catch stray misuses. */
+ d->arch.vmx_msr = ZERO_BLOCK_PTR;
}
else
{
@@ -470,6 +471,9 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
if ( (rc = init_domain_cpuid_policy(d)) )
goto fail;
+ if ( (rc = init_domain_vmx_msr_policy(d)) )
+ goto fail;
+
d->arch.ioport_caps =
rangeset_new(d, "I/O Ports", RANGESETF_prettyprint_hex);
rc = -ENOMEM;
@@ -541,6 +545,7 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
cleanup_domain_irq_mapping(d);
free_xenheap_page(d->shared_info);
xfree(d->arch.cpuid);
+ xfree(d->arch.vmx_msr);
if ( paging_initialised )
paging_final_teardown(d);
free_perdomain_mappings(d);
@@ -555,6 +560,7 @@ void arch_domain_destroy(struct domain *d)
xfree(d->arch.e820);
xfree(d->arch.cpuid);
+ xfree(d->arch.vmx_msr);
free_domain_pirqs(d);
if ( !is_idle_domain(d) )
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index e71728f356..9a19e7a7c0 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -2072,6 +2072,18 @@ void __init calculate_vvmx_max_policy(void)
MSR_IA32_VMX_VMFUNC);
}
+int init_domain_vmx_msr_policy(struct domain *d)
+{
+ d->arch.vmx_msr = xmalloc(struct vmx_msr_policy);
+
+ if ( !d->arch.vmx_msr )
+ return -ENOMEM;
+
+ *d->arch.vmx_msr = vvmx_max_msr_policy;
+
+ return 0;
+}
+
/*
* Capability reporting
*/
@@ -2079,7 +2091,7 @@ int nvmx_msr_read_intercept(unsigned int msr, u64 *msr_content)
{
struct vcpu *v = current;
struct domain *d = v->domain;
- const struct vmx_msr_policy *p = &vvmx_max_msr_policy;
+ const struct vmx_msr_policy *p = d->arch.vmx_msr;
int r = 1;
/* VMX capablity MSRs are available only when guest supports VMX. */
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index c10522b7f5..430188c1fa 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -359,6 +359,8 @@ struct arch_domain
/* CPUID Policy. */
struct cpuid_policy *cpuid;
+ struct vmx_msr_policy *vmx_msr;
+
struct PITState vpit;
/* TSC management (emulation, pv, scaling, stats) */
diff --git a/xen/include/asm-x86/hvm/vmx/vvmx.h b/xen/include/asm-x86/hvm/vmx/vvmx.h
index 150124f3a3..0f5e44ae94 100644
--- a/xen/include/asm-x86/hvm/vmx/vvmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vvmx.h
@@ -246,5 +246,6 @@ int nvmx_cpu_up_prepare(unsigned int cpu);
void nvmx_cpu_dead(unsigned int cpu);
void calculate_vvmx_max_policy(void);
+int init_domain_vmx_msr_policy(struct domain *d);
#endif /* __ASM_X86_HVM_VVMX_H__ */
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/5] VMX MSRs policy for Nested Virt: part 1
2017-07-24 13:47 [PATCH v2 0/5] VMX MSRs policy for Nested Virt: part 1 Sergey Dyasli
` (4 preceding siblings ...)
2017-07-24 13:47 ` [PATCH v2 5/5] x86/vvmx: add per domain vmx msr policy Sergey Dyasli
@ 2017-07-27 7:48 ` Tian, Kevin
5 siblings, 0 replies; 11+ messages in thread
From: Tian, Kevin @ 2017-07-27 7:48 UTC (permalink / raw)
To: Sergey Dyasli, xen-devel; +Cc: Andrew Cooper, Jan Beulich, Nakajima, Jun
> From: Sergey Dyasli [mailto:sergey.dyasli@citrix.com]
> Sent: Monday, July 24, 2017 9:48 PM
>
> 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 introduces struct vmx_msr_policy and the following instances:
>
> * Raw policy (raw_vmx_msr_policy) -- the actual contents of H/W VMX MSRs
> * VVMX max policy (vvmx_max_msr_policy) -- the end result of
> nvmx_msr_read_intercept() on current H/W
it's clearer to call it max_vvmx_msr_policy
> * Per-domain policy (d->arch.vmx_msr) -- the copy of VVMX max policy
> (for now)
>
> In the future it should be possible to independently configure the VMX
> policy for each domain using some new domctl.
>
> There is no "Host policy" object because Xen already has a set of
> variables (vmx_pin_based_exec_control and others) which represent
> the set of VMX features that Xen uses. There are features that Xen
> doesn't use (e.g. CPU_BASED_PAUSE_EXITING) but they are available to L1.
> This makes it not worthy to introduce "Host policy" at this stage.
>
> v1 --> v2:
> - Rebased to the latest master
> - hvm_max_vmx_msr_policy is renamed to vvmx_max_msr_policy
> - Dropped the debug patch
> - Other changes are available on a per-patch basis
>
> Sergey Dyasli (5):
> x86/vmx: add struct vmx_msr_policy
> x86/vmx: add raw_vmx_msr_policy
> x86/vmx: refactor vmx_init_vmcs_config()
> x86/vvmx: add vvmx_max_msr_policy
> x86/vvmx: add per domain vmx msr policy
>
> xen/arch/x86/domain.c | 6 +
> xen/arch/x86/hvm/vmx/vmcs.c | 269 +++++++++++++++++---------
> xen/arch/x86/hvm/vmx/vmx.c | 2 +
> xen/arch/x86/hvm/vmx/vvmx.c | 296 ++++++++++++++--------------
> xen/include/asm-x86/domain.h | 2 +
> xen/include/asm-x86/hvm/vmx/vmcs.h | 383
> +++++++++++++++++++++++++++++++++++++
> xen/include/asm-x86/hvm/vmx/vvmx.h | 3 +
> xen/include/asm-x86/msr-index.h | 1 +
> 8 files changed, 722 insertions(+), 240 deletions(-)
>
> --
> 2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/5] x86/vmx: add struct vmx_msr_policy
2017-07-24 13:47 ` [PATCH v2 1/5] x86/vmx: add struct vmx_msr_policy Sergey Dyasli
@ 2017-07-27 8:11 ` Tian, Kevin
0 siblings, 0 replies; 11+ messages in thread
From: Tian, Kevin @ 2017-07-27 8: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: Monday, July 24, 2017 9:48 PM
>
> This structure provides a convenient way of accessing contents of
> VMX MSRs: every bit value is accessible by its name. Bit names match
> existing Xen's definitions as close as possible. The structure also
> contains the bitmap of available MSRs since not all of them may be
> available on a particular H/W.
>
> A set of helper functions is introduced to provide a simple way of
> interacting with the new structure.
>
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
> ---
> v1 --> v2:
> - Replaced MSR indices with MSR names in struct vmx_msr_policy's
> comments
> - Named "always zero bit" 31 of basic msr as mbz
> - Added placeholder bits into union vmfunc
> - Added structures cr0_bits and cr4_bits
> - Added MSR_IA32_VMX_LAST define to use instead of
> MSR_IA32_VMX_VMFUNC
> - vmx_msr_available() now uses pointer to const struct vmx_msr_policy
> - build_assertions() now uses local struct vmx_msr_policy
> - Added BUILD_BUG_ON to check that width of vmx_msr_policy::available
> bitmap is enough for all existing VMX MSRs
> - Helpers get_vmx_msr_val(), get_vmx_msr_ptr() and gen_vmx_msr_mask()
> are added
>
> xen/arch/x86/hvm/vmx/vmcs.c | 78 ++++++++
> xen/include/asm-x86/hvm/vmx/vmcs.h | 380
> +++++++++++++++++++++++++++++++++++++
> xen/include/asm-x86/msr-index.h | 1 +
> 3 files changed, 459 insertions(+)
>
> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> index 8103b20d29..33715748f0 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -144,6 +144,40 @@ static void __init vmx_display_features(void)
> printk(" - none\n");
> }
>
> +bool vmx_msr_available(const struct vmx_msr_policy *p, uint32_t msr)
regarding to naming, many functions use vmx_ as prefix
with later strings represent the actual purpose e.g.
vmx_msr_read_intercept which is about general msr read
interception. while here your intention is whether "vmx_msr"
is available, which is for specific VMX MSRs. similar for
vmx_msr_policy which reads more general than the real
intention here. Can we find a way to differentiate? SDM
calls this category as "VMX CAPABILITY REPORTING FACILITY",
maybe using vmxcap_msr?
> +{
> + if ( msr < MSR_IA32_VMX_BASIC || msr > MSR_IA32_VMX_LAST )
> + return 0;
then why not also introducing MSR_IA32_VMX_FIRST for
better readability?
> +
> + return p->available & (1u << (msr - MSR_IA32_VMX_BASIC));
> +}
> +
> +uint64_t get_vmx_msr_val(const struct vmx_msr_policy *p, uint32_t msr)
> +{
> + if ( !vmx_msr_available(p, msr))
> + return 0;
0 is a valid MSR value. better return value in a pointer parameter and
then return error number here.
> +
> + return p->msr[msr - MSR_IA32_VMX_BASIC];
> +}
Thanks
Kevin
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/5] x86/vmx: add raw_vmx_msr_policy
2017-07-24 13:47 ` [PATCH v2 2/5] x86/vmx: add raw_vmx_msr_policy Sergey Dyasli
@ 2017-07-27 8:21 ` Tian, Kevin
0 siblings, 0 replies; 11+ messages in thread
From: Tian, Kevin @ 2017-07-27 8:21 UTC (permalink / raw)
To: Sergey Dyasli, xen-devel; +Cc: Andrew Cooper, Jan Beulich, Nakajima, Jun
> From: Sergey Dyasli [mailto:sergey.dyasli@citrix.com]
> Sent: Monday, July 24, 2017 9:48 PM
>
> Add calculate_vmx_raw_policy() which fills the raw_vmx_msr_policy
> object (the actual contents of H/W VMX MSRs) on the boot CPU. On
> secondary CPUs, this function checks that contents of VMX MSRs match
> the boot CPU's contents.
>
> Remove lesser version of same-contents-check from vmx_init_vmcs_config().
>
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
> ---
> v1 --> v2:
> - calculate_raw_policy() is renamed to calculate_vmx_raw_policy()
> to avoid clash with the same-name function in cpuid.c
> - Declaration of calculate_vmx_raw_policy() is removed from vmx.c
> and added to vmcs.h
> - msr variable is now unsigned in calculate_vmx_raw_policy()
> - "\n" moved to the same line as the printk format string
> - Replaced magic constants for available bitmap with gen_vmx_msr_mask()
> - get_vmx_msr_ptr() and get_vmx_msr_val() helpers are used instead of
> accessing MSR array directly
>
> xen/arch/x86/hvm/vmx/vmcs.c | 134 +++++++++++++++++++++------------
> ----
> xen/arch/x86/hvm/vmx/vmx.c | 2 +
> xen/include/asm-x86/hvm/vmx/vmcs.h | 3 +
> 3 files changed, 82 insertions(+), 57 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> index 33715748f0..8070ed21c8 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -144,6 +144,8 @@ static void __init vmx_display_features(void)
> printk(" - none\n");
> }
>
> +struct vmx_msr_policy __read_mostly raw_vmx_msr_policy;
> +
> bool vmx_msr_available(const struct vmx_msr_policy *p, uint32_t msr)
> {
> if ( msr < MSR_IA32_VMX_BASIC || msr > MSR_IA32_VMX_LAST )
> @@ -178,6 +180,78 @@ uint32_t gen_vmx_msr_mask(uint32_t start_msr,
> uint32_t end_msr)
> (start_msr - MSR_IA32_VMX_BASIC);
> }
>
> +int calculate_vmx_raw_policy(bool bsp)
calculate_raw_vmxcap_policy
> +{
> + struct vmx_msr_policy policy;
> + struct vmx_msr_policy *p = &policy;
> + unsigned int msr;
> +
> + /* Raw policy is filled only on boot CPU */
> + if ( bsp )
> + p = &raw_vmx_msr_policy;
> + else
> + memset(&policy, 0, sizeof(policy));
memset(p, 0, sizeof(*p));
> +
> + p->available = gen_vmx_msr_mask(MSR_IA32_VMX_BASIC,
> MSR_IA32_VMX_VMCS_ENUM);
> + for ( msr = MSR_IA32_VMX_BASIC; msr <= MSR_IA32_VMX_VMCS_ENUM;
> msr++ )
> + rdmsrl(msr, *get_vmx_msr_ptr(p, msr));
move above into a function since quite some duplication below.
Thanks
Kevin
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/5] x86/vmx: refactor vmx_init_vmcs_config()
2017-07-24 13:47 ` [PATCH v2 3/5] x86/vmx: refactor vmx_init_vmcs_config() Sergey Dyasli
@ 2017-07-27 8:24 ` Tian, Kevin
0 siblings, 0 replies; 11+ messages in thread
From: Tian, Kevin @ 2017-07-27 8:24 UTC (permalink / raw)
To: Sergey Dyasli, xen-devel; +Cc: Andrew Cooper, Jan Beulich, Nakajima, Jun
> From: Sergey Dyasli [mailto:sergey.dyasli@citrix.com]
> Sent: Monday, July 24, 2017 9:48 PM
>
> 1. Remove RDMSRs of VMX MSRs since all values are already available in
> raw_vmx_msr_policy.
> 2. Replace bit operations involving VMX bitmasks with accessing VMX
> features by name and using vmx_msr_available() where appropriate.
>
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 5/5] x86/vvmx: add per domain vmx msr policy
2017-07-24 13:47 ` [PATCH v2 5/5] x86/vvmx: add per domain vmx msr policy Sergey Dyasli
@ 2017-07-30 10:05 ` Jan Beulich
0 siblings, 0 replies; 11+ messages in thread
From: Jan Beulich @ 2017-07-30 10:05 UTC (permalink / raw)
To: sergey.dyasli; +Cc: Andrew.Cooper3, kevin.tian, jun.nakajima, xen-devel
>>> Sergey Dyasli <sergey.dyasli@citrix.com> 07/24/17 3:48 PM >>>
>@@ -470,6 +471,9 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
>if ( (rc = init_domain_cpuid_policy(d)) )
>goto fail;
>
>+ if ( (rc = init_domain_vmx_msr_policy(d)) )
>+ goto fail;
There should not be a direct call from here to VMX-specific code - an
intermediate HVM layer is needed.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-07-30 10:05 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-24 13:47 [PATCH v2 0/5] VMX MSRs policy for Nested Virt: part 1 Sergey Dyasli
2017-07-24 13:47 ` [PATCH v2 1/5] x86/vmx: add struct vmx_msr_policy Sergey Dyasli
2017-07-27 8:11 ` Tian, Kevin
2017-07-24 13:47 ` [PATCH v2 2/5] x86/vmx: add raw_vmx_msr_policy Sergey Dyasli
2017-07-27 8:21 ` Tian, Kevin
2017-07-24 13:47 ` [PATCH v2 3/5] x86/vmx: refactor vmx_init_vmcs_config() Sergey Dyasli
2017-07-27 8:24 ` Tian, Kevin
2017-07-24 13:47 ` [PATCH v2 4/5] x86/vvmx: add vvmx_max_msr_policy Sergey Dyasli
2017-07-24 13:47 ` [PATCH v2 5/5] x86/vvmx: add per domain vmx msr policy Sergey Dyasli
2017-07-30 10:05 ` Jan Beulich
2017-07-27 7:48 ` [PATCH v2 0/5] VMX MSRs policy for Nested Virt: part 1 Tian, Kevin
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.