| Message ID | 20200326081847.5870-5-weijiang.yang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce support for guest CET feature | expand |
On Thu, Mar 26, 2020 at 04:18:41PM +0800, Yang Weijiang wrote: > CR4.CET is master control bit for CET function. > There're mutual constrains between CR0.WP and CR4.CET, so need > to check the dependent bit while changing the control registers. > > Note: > 1)The processor does not allow CR4.CET to be set if CR0.WP = 0, > similarly, it does not allow CR0.WP to be cleared while > CR4.CET = 1. In either case, KVM would inject #GP to guest. Nit: the CET vs. WP dependency and #GP belongs in the "main" part of the changelog, as it's the crux of the patch. Item (2) below is more along the lines of "note" material. > > 2)SHSTK and IBT features share one control MSR: > MSR_IA32_{U,S}_CET, which means it's difficult to hide > one feature from another in the case of SHSTK != IBT, > after discussed in community, it's agreed to allow guest > control two features independently as it won't introduce > security hole. > > Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> > --- > arch/x86/kvm/vmx/vmx.c | 4 ++++ > arch/x86/kvm/x86.c | 3 +++ > 2 files changed, 7 insertions(+) > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index bd7cd175fd81..87f101750746 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -3089,6 +3089,10 @@ int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4) > return 1; > } > > + if ((cr4 & X86_CR4_CET) && (!is_cet_supported(vcpu) || > + !(kvm_read_cr0(vcpu) & X86_CR0_WP))) > + return 1; > + > if (vmx->nested.vmxon && !nested_cr4_valid(vcpu, cr4)) > return 1; > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 830afe5038d1..90acdbbb8a5a 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -804,6 +804,9 @@ int kvm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0) > if (!(cr0 & X86_CR0_PG) && kvm_read_cr4_bits(vcpu, X86_CR4_PCIDE)) > return 1; > > + if (!(cr0 & X86_CR0_WP) && kvm_read_cr4_bits(vcpu, X86_CR4_CET)) > + return 1; > + > kvm_x86_ops->set_cr0(vcpu, cr0); > > if ((cr0 ^ old_cr0) & X86_CR0_PG) { > -- > 2.17.2 >
On Thu, Apr 23, 2020 at 10:20:32AM -0700, Sean Christopherson wrote: > On Thu, Mar 26, 2020 at 04:18:41PM +0800, Yang Weijiang wrote: > > CR4.CET is master control bit for CET function. > > There're mutual constrains between CR0.WP and CR4.CET, so need > > to check the dependent bit while changing the control registers. > > > > Note: > > 1)The processor does not allow CR4.CET to be set if CR0.WP = 0, > > similarly, it does not allow CR0.WP to be cleared while > > CR4.CET = 1. In either case, KVM would inject #GP to guest. > > Nit: the CET vs. WP dependency and #GP belongs in the "main" part of the > changelog, as it's the crux of the patch. Item (2) below is more along > the lines of "note" material. > OK, will change it, thank you! > > > > 2)SHSTK and IBT features share one control MSR: > > MSR_IA32_{U,S}_CET, which means it's difficult to hide > > one feature from another in the case of SHSTK != IBT, > > after discussed in community, it's agreed to allow guest > > control two features independently as it won't introduce > > security hole. > > >
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index bd7cd175fd81..87f101750746 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -3089,6 +3089,10 @@ int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4) return 1; } + if ((cr4 & X86_CR4_CET) && (!is_cet_supported(vcpu) || + !(kvm_read_cr0(vcpu) & X86_CR0_WP))) + return 1; + if (vmx->nested.vmxon && !nested_cr4_valid(vcpu, cr4)) return 1; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 830afe5038d1..90acdbbb8a5a 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -804,6 +804,9 @@ int kvm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0) if (!(cr0 & X86_CR0_PG) && kvm_read_cr4_bits(vcpu, X86_CR4_PCIDE)) return 1; + if (!(cr0 & X86_CR0_WP) && kvm_read_cr4_bits(vcpu, X86_CR4_CET)) + return 1; + kvm_x86_ops->set_cr0(vcpu, cr0); if ((cr0 ^ old_cr0) & X86_CR0_PG) {
CR4.CET is master control bit for CET function. There're mutual constrains between CR0.WP and CR4.CET, so need to check the dependent bit while changing the control registers. Note: 1)The processor does not allow CR4.CET to be set if CR0.WP = 0, similarly, it does not allow CR0.WP to be cleared while CR4.CET = 1. In either case, KVM would inject #GP to guest. 2)SHSTK and IBT features share one control MSR: MSR_IA32_{U,S}_CET, which means it's difficult to hide one feature from another in the case of SHSTK != IBT, after discussed in community, it's agreed to allow guest control two features independently as it won't introduce security hole. Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> --- arch/x86/kvm/vmx/vmx.c | 4 ++++ arch/x86/kvm/x86.c | 3 +++ 2 files changed, 7 insertions(+)