| Message ID | 20160616060531.30028-2-haozhong.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jun 16, 2016 at 02:05:29PM +0800, Haozhong Zhang wrote: > msr_ia32_feature_control will be used for LMCE and not depend only on > nested anymore, so move it from struct nested_vmx to struct vcpu_vmx. > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > --- > arch/x86/kvm/vmx.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 57ec6a4..6b63f2d 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -421,7 +421,6 @@ struct nested_vmx { > struct pi_desc *pi_desc; > bool pi_pending; > u16 posted_intr_nv; > - u64 msr_ia32_feature_control; > > struct hrtimer preemption_timer; > bool preemption_timer_expired; > @@ -602,6 +601,8 @@ struct vcpu_vmx { > bool guest_pkru_valid; > u32 guest_pkru; > u32 host_pkru; > + > + u64 msr_ia32_feature_control; > }; > > enum segment_cache_field { > @@ -2907,7 +2908,7 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > case MSR_IA32_FEATURE_CONTROL: > if (!nested_vmx_allowed(vcpu)) > return 1; > - msr_info->data = to_vmx(vcpu)->nested.msr_ia32_feature_control; > + msr_info->data = to_vmx(vcpu)->msr_ia32_feature_control; Since this moves out of struct nested_vmx, that check above it: if (!nested_vmx_allowed(vcpu)) should not influence it anymore, no? Ditto for the rest.
On 16/06/2016 13:49, Borislav Petkov wrote: >> > enum segment_cache_field { >> > @@ -2907,7 +2908,7 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) >> > case MSR_IA32_FEATURE_CONTROL: >> > if (!nested_vmx_allowed(vcpu)) >> > return 1; >> > - msr_info->data = to_vmx(vcpu)->nested.msr_ia32_feature_control; >> > + msr_info->data = to_vmx(vcpu)->msr_ia32_feature_control; > Since this moves out of struct nested_vmx, that check above it: > > if (!nested_vmx_allowed(vcpu)) > > should not influence it anymore, no? For get, yes, this "if" should go. For set, you need to check that the guest doesn't write to reserved bits. So as of this patch the "if" remains tied to VMX, but the next patch changes it to be generic. Paolo > Ditto for the rest. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/16/16 13:49, Borislav Petkov wrote: > On Thu, Jun 16, 2016 at 02:05:29PM +0800, Haozhong Zhang wrote: > > msr_ia32_feature_control will be used for LMCE and not depend only on > > nested anymore, so move it from struct nested_vmx to struct vcpu_vmx. > > > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > > --- > > arch/x86/kvm/vmx.c | 11 ++++++----- > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > > index 57ec6a4..6b63f2d 100644 > > --- a/arch/x86/kvm/vmx.c > > +++ b/arch/x86/kvm/vmx.c > > @@ -421,7 +421,6 @@ struct nested_vmx { > > struct pi_desc *pi_desc; > > bool pi_pending; > > u16 posted_intr_nv; > > - u64 msr_ia32_feature_control; > > > > struct hrtimer preemption_timer; > > bool preemption_timer_expired; > > @@ -602,6 +601,8 @@ struct vcpu_vmx { > > bool guest_pkru_valid; > > u32 guest_pkru; > > u32 host_pkru; > > + > > + u64 msr_ia32_feature_control; > > }; > > > > enum segment_cache_field { > > @@ -2907,7 +2908,7 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > > case MSR_IA32_FEATURE_CONTROL: > > if (!nested_vmx_allowed(vcpu)) > > return 1; > > - msr_info->data = to_vmx(vcpu)->nested.msr_ia32_feature_control; > > + msr_info->data = to_vmx(vcpu)->msr_ia32_feature_control; > > Since this moves out of struct nested_vmx, that check above it: > > if (!nested_vmx_allowed(vcpu)) > > should not influence it anymore, no? > > Ditto for the rest. > The same check in the set case still makes sense. I can remove the check here and leave it in the set case. Thanks, Haozhong -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 57ec6a4..6b63f2d 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -421,7 +421,6 @@ struct nested_vmx { struct pi_desc *pi_desc; bool pi_pending; u16 posted_intr_nv; - u64 msr_ia32_feature_control; struct hrtimer preemption_timer; bool preemption_timer_expired; @@ -602,6 +601,8 @@ struct vcpu_vmx { bool guest_pkru_valid; u32 guest_pkru; u32 host_pkru; + + u64 msr_ia32_feature_control; }; enum segment_cache_field { @@ -2907,7 +2908,7 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) case MSR_IA32_FEATURE_CONTROL: if (!nested_vmx_allowed(vcpu)) return 1; - msr_info->data = to_vmx(vcpu)->nested.msr_ia32_feature_control; + msr_info->data = to_vmx(vcpu)->msr_ia32_feature_control; break; case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMFUNC: if (!nested_vmx_allowed(vcpu)) @@ -2999,10 +3000,10 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) break; case MSR_IA32_FEATURE_CONTROL: if (!nested_vmx_allowed(vcpu) || - (to_vmx(vcpu)->nested.msr_ia32_feature_control & + (to_vmx(vcpu)->msr_ia32_feature_control & FEATURE_CONTROL_LOCKED && !msr_info->host_initiated)) return 1; - vmx->nested.msr_ia32_feature_control = data; + vmx->msr_ia32_feature_control = data; if (msr_info->host_initiated && data == 0) vmx_leave_nested(vcpu); break; @@ -6859,7 +6860,7 @@ static int handle_vmon(struct kvm_vcpu *vcpu) return 1; } - if ((vmx->nested.msr_ia32_feature_control & VMXON_NEEDED_FEATURES) + if ((vmx->msr_ia32_feature_control & VMXON_NEEDED_FEATURES) != VMXON_NEEDED_FEATURES) { kvm_inject_gp(vcpu, 0); return 1;
msr_ia32_feature_control will be used for LMCE and not depend only on nested anymore, so move it from struct nested_vmx to struct vcpu_vmx. Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> --- arch/x86/kvm/vmx.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)