| Message ID | 20170830103433.6605-3-sergey.dyasli@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> From: Sergey Dyasli [mailto:sergey.dyasli@citrix.com] > Sent: Wednesday, August 30, 2017 6:35 PM > > The new structure contains information about guest's MSRs that are > unique to each vCPU. It starts with only 1 MSR: > > MSR_INTEL_MISC_FEATURES_ENABLES > > Which currently has only 1 usable bit: cpuid_faulting. > > Add 2 global policy objects: hvm_max and pv_max that are inited during > boot up. Availability of MSR_INTEL_MISC_FEATURES_ENABLES depends on > availability of MSR_INTEL_PLATFORM_INFO. > > Add init_vcpu_msr_policy() which sets initial MSR policy for every vCPU > during domain creation with a special case for Dom0. > > Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com> with similar cosmetic comments: Reviewed-by: Kevin Tian <kevin.tian@intel.com>
>>> On 30.08.17 at 12:34, <sergey.dyasli@citrix.com> wrote: > @@ -40,11 +44,15 @@ static void __init calculate_hvm_max_policy(void) > dp->plaform_info.available = true; > dp->plaform_info.cpuid_faulting = true; > } > + > + /* 0x00000140 MSR_INTEL_MISC_FEATURES_ENABLES */ > + vp->misc_features_enables.available = dp->plaform_info.available; > } > > static void __init calculate_pv_max_policy(void) > { > struct msr_domain_policy *dp = &pv_max_msr_domain_policy; > + struct msr_vcpu_policy *vp = &pv_max_msr_vcpu_policy; > > /* 0x000000ce MSR_INTEL_PLATFORM_INFO */ > if ( cpu_has_cpuid_faulting ) > @@ -52,6 +60,9 @@ static void __init calculate_pv_max_policy(void) > dp->plaform_info.available = true; > dp->plaform_info.cpuid_faulting = true; > } > + > + /* 0x00000140 MSR_INTEL_MISC_FEATURES_ENABLES */ > + vp->misc_features_enables.available = dp->plaform_info.available; > } The similarity of the two changes makes me wonder whether down the road it wouldn't be better to have a function doing things which are uniform between HVM and PV. > @@ -84,6 +95,28 @@ int init_domain_msr_policy(struct domain *d) > return 0; > } > > +int init_vcpu_msr_policy(struct vcpu *v) > +{ > + struct domain *d = v->domain; > + struct msr_vcpu_policy *vp; > + > + vp = xmalloc(struct msr_vcpu_policy); > + > + if ( !vp ) > + return -ENOMEM; > + > + *vp = is_pv_domain(d) ? pv_max_msr_vcpu_policy : > + hvm_max_msr_vcpu_policy; > + > + /* See comment in intel_ctxt_switch_levelling() */ > + if ( is_control_domain(d) ) > + vp->misc_features_enables.available = false; It's CPUID faulting that's meant to be suppressed, not the MSR's presence. > --- a/xen/include/asm-x86/msr.h > +++ b/xen/include/asm-x86/msr.h > @@ -212,8 +212,19 @@ struct msr_domain_policy > } plaform_info; > }; > > +/* MSR policy object for per-vCPU MSRs */ > +struct msr_vcpu_policy > +{ > + /* 0x00000140 MSR_INTEL_MISC_FEATURES_ENABLES */ > + struct { > + bool available; /* This MSR is non-architectural */ > + bool cpuid_faulting; Here as well as in patch 1 I think down the road we want these to be single-bit bitfields, to limit the growth of the structure. In any event, none of the comments are meant to prevent the series from going in as-is - in fact I'm preparing to commit it as I'm going over the patches. Follow-up changes for some of the items pointed out would be nice post-4.10. Jan
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index 620666b33a..1667d2ad57 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -344,13 +344,27 @@ int vcpu_initialise(struct vcpu *v) /* Idle domain */ v->arch.cr3 = __pa(idle_pg_table); rc = 0; + v->arch.msr = ZERO_BLOCK_PTR; /* Catch stray misuses */ } if ( rc ) - vcpu_destroy_fpu(v); - else if ( !is_idle_domain(v->domain) ) + goto fail; + + if ( !is_idle_domain(v->domain) ) + { vpmu_initialise(v); + if ( (rc = init_vcpu_msr_policy(v)) ) + goto fail; + } + + return rc; + + fail: + vcpu_destroy_fpu(v); + xfree(v->arch.msr); + v->arch.msr = NULL; + return rc; } diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c index eac50ec897..b5ad97d3c8 100644 --- a/xen/arch/x86/msr.c +++ b/xen/arch/x86/msr.c @@ -27,9 +27,13 @@ struct msr_domain_policy __read_mostly hvm_max_msr_domain_policy, __read_mostly pv_max_msr_domain_policy; +struct msr_vcpu_policy __read_mostly hvm_max_msr_vcpu_policy, + __read_mostly pv_max_msr_vcpu_policy; + static void __init calculate_hvm_max_policy(void) { struct msr_domain_policy *dp = &hvm_max_msr_domain_policy; + struct msr_vcpu_policy *vp = &hvm_max_msr_vcpu_policy; if ( !hvm_enabled ) return; @@ -40,11 +44,15 @@ static void __init calculate_hvm_max_policy(void) dp->plaform_info.available = true; dp->plaform_info.cpuid_faulting = true; } + + /* 0x00000140 MSR_INTEL_MISC_FEATURES_ENABLES */ + vp->misc_features_enables.available = dp->plaform_info.available; } static void __init calculate_pv_max_policy(void) { struct msr_domain_policy *dp = &pv_max_msr_domain_policy; + struct msr_vcpu_policy *vp = &pv_max_msr_vcpu_policy; /* 0x000000ce MSR_INTEL_PLATFORM_INFO */ if ( cpu_has_cpuid_faulting ) @@ -52,6 +60,9 @@ static void __init calculate_pv_max_policy(void) dp->plaform_info.available = true; dp->plaform_info.cpuid_faulting = true; } + + /* 0x00000140 MSR_INTEL_MISC_FEATURES_ENABLES */ + vp->misc_features_enables.available = dp->plaform_info.available; } void __init init_guest_msr_policy(void) @@ -84,6 +95,28 @@ int init_domain_msr_policy(struct domain *d) return 0; } +int init_vcpu_msr_policy(struct vcpu *v) +{ + struct domain *d = v->domain; + struct msr_vcpu_policy *vp; + + vp = xmalloc(struct msr_vcpu_policy); + + if ( !vp ) + return -ENOMEM; + + *vp = is_pv_domain(d) ? pv_max_msr_vcpu_policy : + hvm_max_msr_vcpu_policy; + + /* See comment in intel_ctxt_switch_levelling() */ + if ( is_control_domain(d) ) + vp->misc_features_enables.available = false; + + v->arch.msr = vp; + + return 0; +} + /* * Local variables: * mode: C diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h index f08ede3a05..866a03b508 100644 --- a/xen/include/asm-x86/domain.h +++ b/xen/include/asm-x86/domain.h @@ -575,6 +575,8 @@ struct arch_vcpu struct arch_vm_event *vm_event; + struct msr_vcpu_policy *msr; + struct { bool next_interrupt_enabled; } monitor; diff --git a/xen/include/asm-x86/msr.h b/xen/include/asm-x86/msr.h index 5cf7be1821..7c8395b9b3 100644 --- a/xen/include/asm-x86/msr.h +++ b/xen/include/asm-x86/msr.h @@ -212,8 +212,19 @@ struct msr_domain_policy } plaform_info; }; +/* MSR policy object for per-vCPU MSRs */ +struct msr_vcpu_policy +{ + /* 0x00000140 MSR_INTEL_MISC_FEATURES_ENABLES */ + struct { + bool available; /* This MSR is non-architectural */ + bool cpuid_faulting; + } misc_features_enables; +}; + void init_guest_msr_policy(void); int init_domain_msr_policy(struct domain *d); +int init_vcpu_msr_policy(struct vcpu *v); #endif /* !__ASSEMBLY__ */
The new structure contains information about guest's MSRs that are unique to each vCPU. It starts with only 1 MSR: MSR_INTEL_MISC_FEATURES_ENABLES Which currently has only 1 usable bit: cpuid_faulting. Add 2 global policy objects: hvm_max and pv_max that are inited during boot up. Availability of MSR_INTEL_MISC_FEATURES_ENABLES depends on availability of MSR_INTEL_PLATFORM_INFO. Add init_vcpu_msr_policy() which sets initial MSR policy for every vCPU during domain creation with a special case for Dom0. Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com> --- xen/arch/x86/domain.c | 18 ++++++++++++++++-- xen/arch/x86/msr.c | 33 +++++++++++++++++++++++++++++++++ xen/include/asm-x86/domain.h | 2 ++ xen/include/asm-x86/msr.h | 11 +++++++++++ 4 files changed, 62 insertions(+), 2 deletions(-)