* [PATCH v5 1/5] x86/msr: add VMX MSRs definitions and populate Raw domain policy
2018-02-28 16:09 [PATCH v5 0/5] VMX MSRs policy for Nested Virt: part 1 Sergey Dyasli
@ 2018-02-28 16:09 ` Sergey Dyasli
2018-03-21 19:52 ` Andrew Cooper
2018-02-28 16:09 ` [PATCH v5 2/5] x86/msr: add VMX MSRs into HVM_max " Sergey Dyasli
` (3 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Sergey Dyasli @ 2018-02-28 16:09 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>
---
v4 --> v5:
- Clarified the reason for splitting MSRs into 5 blocks
- Added raw field into cr0/4_bits
- Moved cr0/4_bits definitions into asm-x86/x86-defns.h
- Added msr availability helpers
---
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 8ae3b4e616..43607b5107 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.activate_secondary_controls;
+}
+
+static bool vmx_ept_vpid_cap_available(const struct msr_domain_policy *dp)
+{
+ return dp->vmx_procbased_ctls2.allowed_1.enable_ept ||
+ dp->vmx_procbased_ctls2.allowed_1.enable_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.enable_vm_functions;
+}
+
+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)
@@ -260,6 +315,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 a5072a2d5e..419ab6f8a7 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" \
@@ -222,6 +223,127 @@ static inline void wrmsr_tsc_aux(uint32_t val)
}
}
+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: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 */
+ };
+};
+
/* MSR policy object for shared per-domain MSRs */
struct msr_domain_policy
{
@@ -230,6 +352,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_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;
+
+ /* 0x00000481 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;
+
+ /* 0x00000482 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;
+
+ /* 0x00000483 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;
+
+ /* 0x00000484 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;
+
+ /* 0x00000485 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;
+
+ /* 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 vmcs_encoding_max_idx: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_bits allowed_0;
+ union vmx_secondary_exec_control_bits allowed_1;
+ };
+ } vmx_procbased_ctls2;
+
+ /* 0x0000048C 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 */
+ };
+ } 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_based_exec_control_bits allowed_0;
+ union vmx_pin_based_exec_control_bits allowed_1;
+ };
+ } pinbased;
+
+ /* 0x0000048E 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;
+ };
+ } procbased;
+
+ /* 0x0000048F 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;
+ };
+ } exit;
+
+ /* 0x00000490 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;
+ };
+ } 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 70453e8dfb..ff2f59e732 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 features in CR4
*/
@@ -66,4 +88,36 @@
#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__ */
+
#endif /* __XEN_X86_DEFNS_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] 15+ messages in thread* Re: [PATCH v5 1/5] x86/msr: add VMX MSRs definitions and populate Raw domain policy
2018-02-28 16:09 ` [PATCH v5 1/5] x86/msr: add VMX MSRs definitions and populate Raw domain policy Sergey Dyasli
@ 2018-03-21 19:52 ` Andrew Cooper
2018-03-22 9:12 ` Jan Beulich
2018-03-22 9:39 ` Sergey Dyasli
0 siblings, 2 replies; 15+ messages in thread
From: Andrew Cooper @ 2018-03-21 19:52 UTC (permalink / raw)
To: Sergey Dyasli, xen-devel; +Cc: Kevin Tian, Jan Beulich, Jun Nakajima
On 28/02/18 16:09, Sergey Dyasli wrote:
> 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>
Overall, I think this is good. However, I'd like to take this
opportunity to make the names shorter, because there is a huge quantity
of unnecessary code volume in these names. Some suggestions inline.
> ---
> v4 --> v5:
> - Clarified the reason for splitting MSRs into 5 blocks
> - Added raw field into cr0/4_bits
> - Moved cr0/4_bits definitions into asm-x86/x86-defns.h
> - Added msr availability helpers
> ---
> 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 8ae3b4e616..43607b5107 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.activate_secondary_controls;
> +}
> +
> +static bool vmx_ept_vpid_cap_available(const struct msr_domain_policy *dp)
> +{
> + return dp->vmx_procbased_ctls2.allowed_1.enable_ept ||
> + dp->vmx_procbased_ctls2.allowed_1.enable_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.enable_vm_functions;
> +}
> +
> +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)
> @@ -260,6 +315,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 a5072a2d5e..419ab6f8a7 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" \
> @@ -222,6 +223,127 @@ static inline void wrmsr_tsc_aux(uint32_t val)
> }
> }
>
> +union vmx_pin_based_exec_control_bits {
I don't think you need a _bits suffix on these union names, and I don't
think you need based in the middle of any of them. I know that is how
the manual refers to them, but "pin controls" is equally as meaningful
as "pin based controls".
> + uint32_t raw;
> + struct {
> + bool ext_intr_exiting:1;
I don't think we need any _exiting suffixes. Enough context is
available from the control name.
> + uint32_t :2; /* 1:2 reserved */
> + bool nmi_exiting:1;
> + uint32_t :1; /* 4 reserved */
> + bool virtual_nmis:1;
vnmis ?
> + 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;
tsc_offset ?
> + 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:1;
> + bool virtual_nmi_pending:1;
vnmi_pending ?
> + bool mov_dr_exiting:1;
> + bool uncond_io_exiting:1;
> + bool activate_io_bitmap:1;
io_bitmap ?
> + uint32_t :1; /* 26 reserved */
> + bool monitor_trap_flag:1;
mtf is an abbreviation already used in the vm_event infrastructure.
> + bool activate_msr_bitmap:1;
msr_bitmap ?
> + bool monitor_exiting:1;
> + bool pause_exiting:1;
> + bool activate_secondary_controls:1;
Simply just secondary? It is the only name like this.
> + };
> +};
> +
> +union vmx_vmexit_control_bits {
> + uint32_t raw;
> + struct {
> + uint32_t :2; /* 0:1 reserved */
> + bool save_debug_cntrls:1;
save_guest_debug ? matches the later conventions.
> + uint32_t :6; /* 3:8 reserved */
> + bool ia32e_mode:1;
> + uint32_t :2; /* 10:11 reserved */
> + bool load_perf_global_ctrl:1;
the global in the middle is a bit redundant.
> + uint32_t :2; /* 13:14 reserved */
> + bool ack_intr_on_exit:1;
just ack_intr. These are already the exit controls.
> + 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;
conceal_from_pt?
> + 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;
deact isn't an obvious abbreviation. Here, I'd actually go for the
longer "deactivate_dual_monitor".
OTOH, I wonder whether we should include any of the dual monitor
controls. Xen isn't plausibly going to make use of them, although I
suppose we probably do need them to audit guest settings.
> + 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;
This is annoying because I can't suggest an obviously better shorter
alternative. We can probably drop the leading virtualise_ as these
fields are always in the context of the exec controls.
> + bool enable_ept:1;
ept.
> + bool descriptor_table_exiting:1;
desc_table which again is an abbreviation already used by the monitor
subsystem.
> + bool enable_rdtscp:1;
> + bool virtualize_x2apic_mode:1;
> + bool enable_vpid:1;
> + bool wbinvd_exiting:1;
> + bool unrestricted_guest:1;
Can probably get away with just unrestricted. This particular field is
quite unique.
> + bool apic_register_virt:1;
> + bool virtual_intr_delivery:1;
> + bool pause_loop_exiting:1;
ple ?
> + bool rdrand_exiting:1;
> + bool enable_invpcid:1;
> + bool enable_vm_functions:1;
vmfunc.
> + bool enable_vmcs_shadowing:1;
> + bool encls_exiting:1;
> + bool rdseed_exiting:1;
> + bool enable_pml:1;
> + bool enable_virt_exceptions:1;
ve.
> + 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 */
> + };
> +};
> +
> /* MSR policy object for shared per-domain MSRs */
> struct msr_domain_policy
> {
> @@ -230,6 +352,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_revision_id:31;
vmcs_rev_id
> + bool mbz:1; /* 31 always zero */
Is this really mbz? Isn't this the shadow identifier bit for shadow vmcs's?
> + 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_based_exec_control_bits allowed_0;
> + union vmx_pin_based_exec_control_bits allowed_1;
> + };
> + } pinbased_ctls;
> +
> + /* 0x00000482 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;
> +
> + /* 0x00000483 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;
> +
> + /* 0x00000484 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;
> +
> + /* 0x00000485 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;
> +
> + /* 0x00000486 MSR_IA32_VMX_CR0_FIXED0 */
> + union {
> + uint64_t raw;
> + union cr0_bits allowed_0;
Hmm - it is rather unfortunate to have this allowed_0 infix, but I can't
think of a clean way of removing it.
The best I can think of is to have the contents of cr0_bits be a
define. See CPUID_BITFIELD_ and DECL_BITFIELD(), although there is
probably no need to automatically generate the cr information.
> + } 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 vmcs_encoding_max_idx:9;
max_encoding ?
> + 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_bits allowed_0;
> + union vmx_secondary_exec_control_bits allowed_1;
> + };
> + } vmx_procbased_ctls2;
> +
> + /* 0x0000048C MSR_IA32_VMX_EPT_VPID_CAP */
> + union {
> + uint64_t raw;
> + struct {
> + bool exec_only_supported:1;
x_only ?
> + uint32_t :5; /* 1:5 reserved */
> + bool walk_length_4_supported:1;
wl4 ?
> + uint32_t :1; /* 7 reserved */
> + bool memory_type_uc:1;
uc
> + uint32_t :5; /* 9:13 reserved */
> + bool memory_type_wb:1;
wb
> + uint32_t :1; /* 15 reserved */
> + bool superpage_2mb:1;
> + bool superpage_1gb:1;
_2mb and _1gb respectively.
> + uint32_t :2; /* 18:19 reserved */
> + bool invept_instruction:1;
the _instruction suffix doesn't add anything.
> + 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;
The _context suffixes probably aren't helpful here.
> + 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;
invvpid_single_nonglobal?
~Andrew
> + 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_based_exec_control_bits allowed_0;
> + union vmx_pin_based_exec_control_bits allowed_1;
> + };
> + } pinbased;
> +
> + /* 0x0000048E 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;
> + };
> + } procbased;
> +
> + /* 0x0000048F 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;
> + };
> + } exit;
> +
> + /* 0x00000490 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;
> + };
> + } 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 70453e8dfb..ff2f59e732 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 features in CR4
> */
> @@ -66,4 +88,36 @@
> #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__ */
> +
> #endif /* __XEN_X86_DEFNS_H__ */
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v5 1/5] x86/msr: add VMX MSRs definitions and populate Raw domain policy
2018-03-21 19:52 ` Andrew Cooper
@ 2018-03-22 9:12 ` Jan Beulich
2018-03-22 9:39 ` Sergey Dyasli
1 sibling, 0 replies; 15+ messages in thread
From: Jan Beulich @ 2018-03-22 9:12 UTC (permalink / raw)
To: Andrew Cooper, Sergey Dyasli, xen-devel; +Cc: Kevin Tian, Jun Nakajima
>>> On 21.03.18 at 20:52, <andrew.cooper3@citrix.com> wrote:
> On 28/02/18 16:09, Sergey Dyasli wrote:
>> + uint32_t :1; /* 15 reserved */
>> + bool superpage_2mb:1;
>> + bool superpage_1gb:1;
>
> _2mb and _1gb respectively.
That'll be a name space violation again. Just drop "super" perhaps?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 1/5] x86/msr: add VMX MSRs definitions and populate Raw domain policy
2018-03-21 19:52 ` Andrew Cooper
2018-03-22 9:12 ` Jan Beulich
@ 2018-03-22 9:39 ` Sergey Dyasli
1 sibling, 0 replies; 15+ messages in thread
From: Sergey Dyasli @ 2018-03-22 9:39 UTC (permalink / raw)
To: Andrew Cooper, xen-devel@lists.xen.org
Cc: Sergey Dyasli, Kevin Tian, jun.nakajima@intel.com,
jbeulich@suse.com
On Wed, 2018-03-21 at 19:52 +0000, Andrew Cooper wrote:
> On 28/02/18 16:09, Sergey Dyasli wrote:
> >
> > + struct {
> > + /* 0x00000480 MSR_IA32_VMX_BASIC */
> > + union {
> > + uint64_t raw;
> > + struct {
> > + uint32_t vmcs_revision_id:31;
>
> vmcs_rev_id
>
> > + bool mbz:1; /* 31 always zero */
>
> Is this really mbz? Isn't this the shadow identifier bit for shadow vmcs's?
Yes, in vmcs itself it's the shadow bit. However, it is always zero in
MSR since the job of MSR is to report vmcs revision identifier.
--
Thanks,
Sergey
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v5 2/5] x86/msr: add VMX MSRs into HVM_max domain policy
2018-02-28 16:09 [PATCH v5 0/5] VMX MSRs policy for Nested Virt: part 1 Sergey Dyasli
2018-02-28 16:09 ` [PATCH v5 1/5] x86/msr: add VMX MSRs definitions and populate Raw domain policy Sergey Dyasli
@ 2018-02-28 16:09 ` Sergey Dyasli
2018-03-21 20:46 ` Andrew Cooper
2018-02-28 16:09 ` [PATCH v5 3/5] x86/cpuid: update signature of hvm_cr4_guest_valid_bits() Sergey Dyasli
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Sergey Dyasli @ 2018-02-28 16:09 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>
---
v4 --> v5:
- Macros are removed and now supported bitmask is used to derive policy
- Added vmx_clear_policy() helper
---
xen/arch/x86/msr.c | 134 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 134 insertions(+)
diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index 43607b5107..f700e05570 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -106,6 +106,138 @@ 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 ( !cpu_has_vmx )
+ return;
+
+ vmx_clear_policy(dp);
+
+ 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.vmcs_encoding_max_idx = 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 +259,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] 15+ messages in thread* Re: [PATCH v5 2/5] x86/msr: add VMX MSRs into HVM_max domain policy
2018-02-28 16:09 ` [PATCH v5 2/5] x86/msr: add VMX MSRs into HVM_max " Sergey Dyasli
@ 2018-03-21 20:46 ` Andrew Cooper
2018-03-22 10:14 ` Sergey Dyasli
0 siblings, 1 reply; 15+ messages in thread
From: Andrew Cooper @ 2018-03-21 20:46 UTC (permalink / raw)
To: Sergey Dyasli, xen-devel; +Cc: Kevin Tian, Jan Beulich, Jun Nakajima
On 28/02/2018 16:09, Sergey Dyasli wrote:
> 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>
> ---
> v4 --> v5:
> - Macros are removed and now supported bitmask is used to derive policy
> - Added vmx_clear_policy() helper
> ---
> xen/arch/x86/msr.c | 134 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 134 insertions(+)
>
> diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
> index 43607b5107..f700e05570 100644
> --- a/xen/arch/x86/msr.c
> +++ b/xen/arch/x86/msr.c
> @@ -106,6 +106,138 @@ 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)
It might be worth leaving a /* TODO - actually make this feature
selection sane. */ ? For one, it should be built up as a whitelist of
features (bounded by hardware support), rather than the mixed
blacklist/whitelist that it currently is.
Either way, changing the selection logic is work for a later series, not
this one.
> +{
> + const struct msr_domain_policy *hp = &host_msr_domain_policy;
> + uint32_t supported;
> +
> + if ( !cpu_has_vmx )
> + return;
This should probably be hvm_max_cpuid_policy->basic.vmx. i.e. if by
some mechanism a user has elected to not offer the vmx feature even in
the max policy, we shouldn't offer the MSRs even if they are actually
available on the hardware.
> +
> + vmx_clear_policy(dp);
> +
> + 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;
Please have a single set of brackets around the entire or statement, so
editors will indent new changes correctly.
> + 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.vmcs_encoding_max_idx = 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) )
> + {
I wonder. Whether the true ctls are available or not, we have the
fields to use (and wouldn't leak them into guests because of this
predicate).
Would it be an idea to calculate reality in the true ctls, then have a
helper to sync the true ctls into the legacy MSRs? That way, we don't
risk the guests view getting out of sync. (Again - not a suggestion for
this series.)
~Andrew
> + 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 +259,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)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v5 2/5] x86/msr: add VMX MSRs into HVM_max domain policy
2018-03-21 20:46 ` Andrew Cooper
@ 2018-03-22 10:14 ` Sergey Dyasli
2018-03-22 10:22 ` Andrew Cooper
0 siblings, 1 reply; 15+ messages in thread
From: Sergey Dyasli @ 2018-03-22 10:14 UTC (permalink / raw)
To: Andrew Cooper, xen-devel@lists.xen.org
Cc: Sergey Dyasli, Kevin Tian, jun.nakajima@intel.com,
jbeulich@suse.com
On Wed, 2018-03-21 at 20:46 +0000, Andrew Cooper wrote:
> On 28/02/2018 16:09, Sergey Dyasli wrote:
> > +
> > + 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;
>
> Please have a single set of brackets around the entire or statement, so
> editors will indent new changes correctly.
Which editors? My editor is doing it fine. Anyway, is this what you are
asking for?
supported = (PIN_BASED_EXT_INTR_MASK |
PIN_BASED_NMI_EXITING |
PIN_BASED_PREEMPT_TIMER);
--
Thanks,
Sergey
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v5 2/5] x86/msr: add VMX MSRs into HVM_max domain policy
2018-03-22 10:14 ` Sergey Dyasli
@ 2018-03-22 10:22 ` Andrew Cooper
0 siblings, 0 replies; 15+ messages in thread
From: Andrew Cooper @ 2018-03-22 10:22 UTC (permalink / raw)
To: Sergey Dyasli, xen-devel@lists.xen.org
Cc: Kevin Tian, jun.nakajima@intel.com, jbeulich@suse.com
On 22/03/2018 10:14, Sergey Dyasli wrote:
> On Wed, 2018-03-21 at 20:46 +0000, Andrew Cooper wrote:
>> On 28/02/2018 16:09, Sergey Dyasli wrote:
>>> +
>>> + 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;
>> Please have a single set of brackets around the entire or statement, so
>> editors will indent new changes correctly.
> Which editors?
Any editor which can read the file annotation stating BSD style, which
results in
supported = PIN_BASED_EXT_INTR_MASK |
PIN_BASED_NMI_EXITING |
by default.
> My editor is doing it fine. Anyway, is this what you are
> asking for?
>
> supported = (PIN_BASED_EXT_INTR_MASK |
> PIN_BASED_NMI_EXITING |
> PIN_BASED_PREEMPT_TIMER);
>
Yes. That's great thanks.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v5 3/5] x86/cpuid: update signature of hvm_cr4_guest_valid_bits()
2018-02-28 16:09 [PATCH v5 0/5] VMX MSRs policy for Nested Virt: part 1 Sergey Dyasli
2018-02-28 16:09 ` [PATCH v5 1/5] x86/msr: add VMX MSRs definitions and populate Raw domain policy Sergey Dyasli
2018-02-28 16:09 ` [PATCH v5 2/5] x86/msr: add VMX MSRs into HVM_max " Sergey Dyasli
@ 2018-02-28 16:09 ` Sergey Dyasli
2018-03-21 20:49 ` Andrew Cooper
2018-02-28 16:09 ` [PATCH v5 4/5] x86/msr: update domain policy on CPUID policy changes Sergey Dyasli
2018-02-28 16:09 ` [PATCH v5 5/5] x86/msr: handle VMX MSRs with guest_rd/wrmsr() Sergey Dyasli
4 siblings, 1 reply; 15+ messages in thread
From: Sergey Dyasli @ 2018-02-28 16:09 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>
---
v4 --> v5: rebased to the latest staging
---
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 | 2 +-
xen/arch/x86/hvm/vmx/vvmx.c | 2 +-
xen/include/asm-x86/hvm/hvm.h | 2 +-
6 files changed, 10 insertions(+), 10 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 0539551851..c96e166952 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -932,9 +932,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;
@@ -1001,7 +1000,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);
@@ -2344,7 +2343,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 aa0505036b..4856ad7c24 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1699,7 +1699,7 @@ static void vmx_update_guest_cr(struct vcpu *v, unsigned int cr)
* 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);
+ ~hvm_cr4_guest_valid_bits(v->domain, 0);
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 8176736e8f..c8c168b7d0 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -2135,7 +2135,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 dd3dd5f0ee..13bb9b42fe 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -611,7 +611,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] 15+ messages in thread* Re: [PATCH v5 3/5] x86/cpuid: update signature of hvm_cr4_guest_valid_bits()
2018-02-28 16:09 ` [PATCH v5 3/5] x86/cpuid: update signature of hvm_cr4_guest_valid_bits() Sergey Dyasli
@ 2018-03-21 20:49 ` Andrew Cooper
0 siblings, 0 replies; 15+ messages in thread
From: Andrew Cooper @ 2018-03-21 20:49 UTC (permalink / raw)
To: Sergey Dyasli, xen-devel; +Cc: Kevin Tian, Jan Beulich, Jun Nakajima
On 28/02/2018 16:09, Sergey Dyasli wrote:
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index aa0505036b..4856ad7c24 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -1699,7 +1699,7 @@ static void vmx_update_guest_cr(struct vcpu *v, unsigned int cr)
> * 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);
> + ~hvm_cr4_guest_valid_bits(v->domain, 0);
Indentation here looks fishy. Please add some brackets around the
expression, and 0 => false. Alternatively, reflow the expression to
have a linebreak after the =.
Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 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) ?
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v5 4/5] x86/msr: update domain policy on CPUID policy changes
2018-02-28 16:09 [PATCH v5 0/5] VMX MSRs policy for Nested Virt: part 1 Sergey Dyasli
` (2 preceding siblings ...)
2018-02-28 16:09 ` [PATCH v5 3/5] x86/cpuid: update signature of hvm_cr4_guest_valid_bits() Sergey Dyasli
@ 2018-02-28 16:09 ` Sergey Dyasli
2018-03-21 20:53 ` Andrew Cooper
2018-02-28 16:09 ` [PATCH v5 5/5] x86/msr: handle VMX MSRs with guest_rd/wrmsr() Sergey Dyasli
4 siblings, 1 reply; 15+ messages in thread
From: Sergey Dyasli @ 2018-02-28 16:09 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 per-domain
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>
---
v4 --> v5:
- Removed _domain from function names
- Added vmx_copy_policy() helper
- recalculate_vmx_msr_policy() was rewritten
---
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 f700e05570..9114b8f53b 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);
@@ -282,6 +283,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;
@@ -302,6 +336,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 419ab6f8a7..4747572871 100644
--- a/xen/include/asm-x86/msr.h
+++ b/xen/include/asm-x86/msr.h
@@ -606,6 +606,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);
+/* Update availability of per-domain 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] 15+ messages in thread* Re: [PATCH v5 4/5] x86/msr: update domain policy on CPUID policy changes
2018-02-28 16:09 ` [PATCH v5 4/5] x86/msr: update domain policy on CPUID policy changes Sergey Dyasli
@ 2018-03-21 20:53 ` Andrew Cooper
0 siblings, 0 replies; 15+ messages in thread
From: Andrew Cooper @ 2018-03-21 20:53 UTC (permalink / raw)
To: Sergey Dyasli, xen-devel; +Cc: Kevin Tian, Jan Beulich, Jun Nakajima
On 28/02/2018 16:09, Sergey Dyasli wrote:
> diff --git a/xen/include/asm-x86/msr.h b/xen/include/asm-x86/msr.h
> index 419ab6f8a7..4747572871 100644
> --- a/xen/include/asm-x86/msr.h
> +++ b/xen/include/asm-x86/msr.h
> @@ -606,6 +606,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);
>
> +/* Update availability of per-domain MSRs based on CPUID policy */
> +void recalculate_msr_policy(struct domain *d);
Either this needs naming to be specifically the domain policy, or the
comment shouldn't specify per-domain only. As we build up the policies,
I expect we will end up needing to tweak the vcpu policies as well.
Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> (with
some full stops at the end of comments, seeing as the series needs
another spin.)
> +
> #endif /* !__ASSEMBLY__ */
>
> #endif /* __ASM_MSR_H */
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v5 5/5] x86/msr: handle VMX MSRs with guest_rd/wrmsr()
2018-02-28 16:09 [PATCH v5 0/5] VMX MSRs policy for Nested Virt: part 1 Sergey Dyasli
` (3 preceding siblings ...)
2018-02-28 16:09 ` [PATCH v5 4/5] x86/msr: update domain policy on CPUID policy changes Sergey Dyasli
@ 2018-02-28 16:09 ` Sergey Dyasli
2018-03-21 20:28 ` Andrew Cooper
4 siblings, 1 reply; 15+ messages in thread
From: Sergey Dyasli @ 2018-02-28 16:09 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>
---
v4 --> v5:
- New msr availability helpers are used
---
xen/arch/x86/hvm/vmx/vmx.c | 6 --
xen/arch/x86/hvm/vmx/vvmx.c | 178 -------------------------------------
xen/arch/x86/msr.c | 35 ++++++++
xen/include/asm-x86/hvm/vmx/vvmx.h | 2 -
4 files changed, 35 insertions(+), 186 deletions(-)
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 4856ad7c24..e850ef913f 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2934,10 +2934,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. */
@@ -3160,8 +3156,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 c8c168b7d0..8f4a68cf9a 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1975,184 +1975,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 9114b8f53b..ee2ff94fcd 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -365,6 +365,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;
@@ -399,6 +400,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;
}
@@ -474,6 +505,10 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
break;
}
+ case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMFUNC:
+ /* None of these MSRs are writeable. */
+ goto gp_fault;
+
default:
return X86EMUL_UNHANDLEABLE;
}
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] 15+ messages in thread* Re: [PATCH v5 5/5] x86/msr: handle VMX MSRs with guest_rd/wrmsr()
2018-02-28 16:09 ` [PATCH v5 5/5] x86/msr: handle VMX MSRs with guest_rd/wrmsr() Sergey Dyasli
@ 2018-03-21 20:28 ` Andrew Cooper
0 siblings, 0 replies; 15+ messages in thread
From: Andrew Cooper @ 2018-03-21 20:28 UTC (permalink / raw)
To: Sergey Dyasli, xen-devel; +Cc: Kevin Tian, Jan Beulich, Jun Nakajima
On 28/02/2018 16:09, Sergey Dyasli wrote:
> @@ -474,6 +505,10 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
> break;
> }
>
> + case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMFUNC:
> + /* None of these MSRs are writeable. */
> + goto gp_fault;
There is a block at the top of the switch statement for read-only MSRs
(so they can be more easily removed when we finally get rid of the
legacy MSR paths). With this moved, Reviewed-by: Andrew Cooper
<andrew.cooper3@citrix.com>
> +
> default:
> return X86EMUL_UNHANDLEABLE;
> }
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread