Received-SPF: pass (google.com: domain of linux-kernel+bounces-28438-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <18f3d0e9-316a-49ec-87fd-3fd16828c7f8@intel.com>
Date: Wed, 17 Jan 2024 09:43:18 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v8 24/26] KVM: x86: Enable CET virtualization for VMX and
 advertise to userspace
Content-Language: en-US
To: Yuan Yao <yuan.yao@linux.intel.com>
CC: <seanjc@google.com>, <pbonzini@redhat.com>, <dave.hansen@intel.com>,
	<kvm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<peterz@infradead.org>, <chao.gao@intel.com>, <rick.p.edgecombe@intel.com>,
	<mlevitsk@redhat.com>, <john.allen@amd.com>
References: <20231221140239.4349-1-weijiang.yang@intel.com>
 <20231221140239.4349-25-weijiang.yang@intel.com>
 <20240116072555.jnj74yairx7add6i@yy-desk-7060>
From: "Yang, Weijiang" <weijiang.yang@intel.com>
In-Reply-To: <20240116072555.jnj74yairx7add6i@yy-desk-7060>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RFE4TTV1LzVaa1dXcVZxVWZLc3FFZWI3b2V6aEdjR2R2S3BHdi81dzJLZDJN?=
 =?utf-8?B?U3MxS1BITUh1WFRONUJ4YTdyMGlTMzhoZ243a1kwRkduYVVwSmt3T2VESG5J?=
 =?utf-8?B?cGN0NVVkWjNTVUJWUTZyQ1RkVEMyUVMxbXNrVlZ6U2ttdDM0cnVCUWxpa1Uv?=
 =?utf-8?B?eEdzdlQyRXdBZGozeFVhMVQ3ZWdzekJ2NmFROHU4WDNqUFVHVUM3SEpPVDBq?=
 =?utf-8?B?Vmx0NUhrdkluUFZFczR4Yk5vYjV6MFR0bVE4eFNjek1qMU1PUXEzZFZJM3Nl?=
 =?utf-8?B?OWY2bC80eklYcDQ4K05YWFpwdTRzT0dyVUJSQ3hQNkF2MkxxdHVEcWhSUWx4?=
 =?utf-8?B?NHluYkt5OFJ4VmRzMGFVLy9xZmFQOUhXQkdhWkErVW1lbVQ2Wkh3RDY3U2cw?=
 =?utf-8?B?N2hvejdnR3p1TWZGU0dNNGthSE43RlpVUXFoU1o0amRtam16eU1McFNMOHlV?=
 =?utf-8?B?OE9uZmJNY0tzSmVSTDZmZ09DN0NmekF1Sktpb1VKSm5ERVJJd3EyMkkrOFNS?=
 =?utf-8?B?YlVHU0w2djY3SW92QlkyY1pOdmREbnRHVlpFQW50cGRvMGtCMkZFWG5xaDlC?=
 =?utf-8?B?VWp2aFlSNWZydlVxYWQ5VVNRYm9NQ05EeWsyOUozVlE4QzVRV3pxK01oTVI2?=
 =?utf-8?B?Rzg0Wk1Ga2kxN3lCdzlMM3NKMXY2cWN5My9Hc3VxRUd0dnRGSldxNmJsUnNK?=
 =?utf-8?B?VHlHNzg0aVNrQmZFU0xlMFNPWUFYcGJITGJQT0FIYlIrbmd6UklCUjI5VDMv?=
 =?utf-8?B?ekFNQjBxRXN5WmFBbXRrWWI1c0VoSDlXUktrYU1vNWgwcENvaDRmVHZ4M21G?=
 =?utf-8?B?aTM2LzhkYVZFOCt2R1VCUllMZ3VxMzlTRDdkSXg1T3FEcDNRamRyeXlvSjVy?=
 =?utf-8?B?ZVlRZHArUGFmaDdyNnBMNThMYkxla3FlWWRjQVBrWWZGSGl6WU0zSElvditu?=
 =?utf-8?B?UGtvZjRPMjR3djdKZER0MktGRnZtcVJVMk1Zdm9HOWx3QXZONW1id3kzVDhv?=
 =?utf-8?B?NUJYMzRaZ1J5ZmtqbGVrcWFBUUxsVFEzb0FnZXdmRDdxUGZ0NmdIemttZTkw?=
 =?utf-8?B?b09aaUg0dlBBY1QxYlJVc2daZWc5Y2haT0RkZ1FhNmdZVGZBTk5CMmtTcnhn?=
 =?utf-8?B?UW9GZDc1c0xvS0lNVWZrTllwYkxxYWdhcVkvVjc5MTM5WG9qRkxqYnlCSGls?=
 =?utf-8?B?MXNUSnhLMFBWcHZsYTRMdWgrbExBN1FCYmovb0xub3NocGh0OUhvUEJFR2Nq?=
 =?utf-8?B?Z1FCRXZ1ejg4RTZqb1RNSXdwYTYxVmpIZG5wcFl6U21naGlEa3M0Zis3UU5S?=
 =?utf-8?B?MDgrYmZpY2x0RHFSYnh2ZU4wZDFYQXZlQmRLWVFKU0ZHOGExRHVLa1ZwdFNW?=
 =?utf-8?B?UU85UDlZVmpyTnZRTkliR0FWcFpYNXFQajkrNWxZVUdyNDJmVWdjS2FVNUNj?=
 =?utf-8?B?dmRNSTF3OFlPUmZRaU5TbS9ZOGFkTGg0T09qVkpCMHUvbTJkbVlxVmNITUVY?=
 =?utf-8?B?Unk0MVlqN25ZQ0ZBUGZ5VE54Smdja1Rmckk2YThmc2ZNWlBya0wxbW9xNzli?=
 =?utf-8?B?eHIzNGYvQWY5Y2RSUUlkN0gzSW81dWw4NGc0ZDMwVG1hQWxLeW1rVkRqcU9o?=
 =?utf-8?B?V3cyZnF6bVZXNjczcVljRUJmY0NOdDFoZU9RVkVwdis5WVF6NFUvakNmTWFv?=
 =?utf-8?B?dm00NVZYcTNKc0JQRzhmelg0alZhb0JuZEZCZ3RNaCtGM0RFeTUyR2RSSTRo?=
 =?utf-8?B?ZWJ1ampocHhkWloxK2FqY0FUcUpKbEtYRVBnQkZsL24vOHEzZVI1aDduQ0Q4?=
 =?utf-8?B?NkpMd0FMbm5HcThQeVNNeXg3N0I2Mk9VZnBKU0xyZVVxU0dMNE1rdVJkbGta?=
 =?utf-8?B?YVQvbnFRWm1WNyszUy9JR0xBUm4yOEhFYnJqcDN2Y2FTek1YS0VtOTQ2dDdi?=
 =?utf-8?B?TDVZMTVwVngwMlhFNFBDTldScHA4b2NvbkJvR0JRbHArY1FDWGs4cElnalVy?=
 =?utf-8?B?QlVEN0hmN2Z3M1ZnYzZNdlhoVXB6bERiUFdEdnhsZnZrREJ6ei9NdGxvMmdy?=
 =?utf-8?B?UXVwREFLNjRBalZZODk4RTNHVWYra3crekt6ZlBrMm00NndjSlp6T2xocVFI?=
 =?utf-8?B?ZXE4UUF3UnRsWWV4b0NZa2NFS043SUI5VWVkQnplekoyUW1KSnovK2RaTjNB?=
 =?utf-8?B?MVE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 3654d3ad-fdf9-48bf-2936-08dc16fdb1c9
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4965.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Jan 2024 01:43:24.7909
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: mAxGUshdiQ/QqJgDsaWgIBGvwSYg0R17zLRPm70OOvdSO3U45BaH+7/dNowdsaBhtyrPVYIDRA4xd9I3WC1evg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB7571
X-OriginatorOrg: intel.com

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@intel.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
>>
>>