linux-kernel - Re: [PATCH v8 24/26] KVM: x86: Enable CET virtualization for VMX and advertise to userspace

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 17 Jan 2024 09:43:18 +0800
From: "Yang, Weijiang" <weijiang.yang@...el.com>
To: Yuan Yao <yuan.yao@...ux.intel.com>
CC: <seanjc@...gle.com>, <pbonzini@...hat.com>, <dave.hansen@...el.com>,
	<kvm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<peterz@...radead.org>, <chao.gao@...el.com>, <rick.p.edgecombe@...el.com>,
	<mlevitsk@...hat.com>, <john.allen@....com>
Subject: Re: [PATCH v8 24/26] KVM: x86: Enable CET virtualization for VMX and
 advertise to userspace

On 1/16/2024 3:25 PM, Yuan Yao wrote:
> On Thu, Dec 21, 2023 at 09:02:37AM -0500, Yang Weijiang wrote:
>> Expose CET features to guest if KVM/host can support them, clear CPUID
>> feature bits if KVM/host cannot support.
>>
>> Set CPUID feature bits so that CET features are available in guest CPUID.
>> Add CR4.CET bit support in order to allow guest set CET master control
>> bit.
>>
>> Disable KVM CET feature if unrestricted_guest is unsupported/disabled as
>> KVM does not support emulating CET.
>>
>> The CET load-bits in VM_ENTRY/VM_EXIT control fields should be set to make
>> guest CET xstates isolated from host's.
>>
>> On platforms with VMX_BASIC[bit56] == 0, inject #CP at VMX entry with error
>> code will fail, and if VMX_BASIC[bit56] == 1, #CP injection with or without
>> error code is allowed. Disable CET feature bits if the MSR bit is cleared
>> so that nested VMM can inject #CP if and only if VMX_BASIC[bit56] == 1.
>>
>> Don't expose CET feature if either of {U,S}_CET xstate bits is cleared
>> in host XSS or if XSAVES isn't supported.
>>
>> CET MSR contents after reset, power-up and INIT are set to 0s, clears the
>> guest fpstate fields so that the guest MSRs are reset to 0s after the events.
>>
>> Signed-off-by: Yang Weijiang <weijiang.yang@...el.com>
>> ---
>>   arch/x86/include/asm/kvm_host.h  |  2 +-
>>   arch/x86/include/asm/msr-index.h |  1 +
>>   arch/x86/kvm/cpuid.c             | 19 +++++++++++++++++--
>>   arch/x86/kvm/vmx/capabilities.h  |  6 ++++++
>>   arch/x86/kvm/vmx/vmx.c           | 29 ++++++++++++++++++++++++++++-
>>   arch/x86/kvm/vmx/vmx.h           |  6 ++++--
>>   arch/x86/kvm/x86.c               | 31 +++++++++++++++++++++++++++++--
>>   arch/x86/kvm/x86.h               |  3 +++
>>   8 files changed, 89 insertions(+), 8 deletions(-)
> ...
>> -#define KVM_SUPPORTED_XSS     0
>> +#define KVM_SUPPORTED_XSS	(XFEATURE_MASK_CET_USER | \
>> +				 XFEATURE_MASK_CET_KERNEL)
>>
>>   u64 __read_mostly host_efer;
>>   EXPORT_SYMBOL_GPL(host_efer);
>> @@ -9921,6 +9922,20 @@ static int __kvm_x86_vendor_init(struct kvm_x86_init_ops *ops)
>>   	if (!kvm_cpu_cap_has(X86_FEATURE_XSAVES))
>>   		kvm_caps.supported_xss = 0;
>>
>> +	if (!kvm_cpu_cap_has(X86_FEATURE_SHSTK) &&
>> +	    !kvm_cpu_cap_has(X86_FEATURE_IBT))
>> +		kvm_caps.supported_xss &= ~(XFEATURE_CET_USER |
>> +					    XFEATURE_CET_KERNEL);
> Looks should be XFEATURE_MASK_xxx.

Good catch! Thanks!

>
>> +
>> +	if ((kvm_caps.supported_xss & (XFEATURE_MASK_CET_USER |
>> +	     XFEATURE_MASK_CET_KERNEL)) !=
>> +	    (XFEATURE_MASK_CET_USER | XFEATURE_MASK_CET_KERNEL)) {
>> +		kvm_cpu_cap_clear(X86_FEATURE_SHSTK);
>> +		kvm_cpu_cap_clear(X86_FEATURE_IBT);
>> +		kvm_caps.supported_xss &= ~(XFEATURE_CET_USER |
>> +					    XFEATURE_CET_KERNEL);
> Ditto.

Yes.

>
>> +	}
>> +
>>   #define __kvm_cpu_cap_has(UNUSED_, f) kvm_cpu_cap_has(f)
>>   	cr4_reserved_bits = __cr4_reserved_bits(__kvm_cpu_cap_has, UNUSED_);
>>   #undef __kvm_cpu_cap_has
>> @@ -12392,7 +12407,9 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
>>
>>   static inline bool is_xstate_reset_needed(void)
>>   {
>> -	return kvm_cpu_cap_has(X86_FEATURE_MPX);
>> +	return kvm_cpu_cap_has(X86_FEATURE_MPX) ||
>> +	       kvm_cpu_cap_has(X86_FEATURE_SHSTK) ||
>> +	       kvm_cpu_cap_has(X86_FEATURE_IBT);
>>   }
>>
>>   void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>> @@ -12469,6 +12486,16 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>>   						       XFEATURE_BNDCSR);
>>   		}
>>
>> +		if (kvm_cpu_cap_has(X86_FEATURE_SHSTK)) {
>> +			fpstate_clear_xstate_component(fpstate,
>> +						       XFEATURE_CET_USER);
>> +			fpstate_clear_xstate_component(fpstate,
>> +						       XFEATURE_CET_KERNEL);
>> +		} else if (kvm_cpu_cap_has(X86_FEATURE_IBT)) {
>> +			fpstate_clear_xstate_component(fpstate,
>> +						       XFEATURE_CET_USER);
>> +		}
>> +
>>   		if (init_event)
>>   			kvm_load_guest_fpu(vcpu);
>>   	}
>> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
>> index 656107e64c93..cc585051d24b 100644
>> --- a/arch/x86/kvm/x86.h
>> +++ b/arch/x86/kvm/x86.h
>> @@ -533,6 +533,9 @@ bool kvm_msr_allowed(struct kvm_vcpu *vcpu, u32 index, u32 type);
>>   		__reserved_bits |= X86_CR4_PCIDE;       \
>>   	if (!__cpu_has(__c, X86_FEATURE_LAM))           \
>>   		__reserved_bits |= X86_CR4_LAM_SUP;     \
>> +	if (!__cpu_has(__c, X86_FEATURE_SHSTK) &&       \
>> +	    !__cpu_has(__c, X86_FEATURE_IBT))           \
>> +		__reserved_bits |= X86_CR4_CET;         \
>>   	__reserved_bits;                                \
>>   })
>>
>> --
>> 2.39.3
>>
>>