Re: [RFC PATCH v2 08/11] KVM: VMX: Advertise MITI_ENUM_RETPOLINE_S_SUPPORT

[Chao Gao]

On Thu, May 18, 2023 at 06:25:30PM +0800, Xiaoyao Li wrote:
>> @@ -2173,7 +2173,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>>   	struct vmx_uret_msr *msr;
>>   	int ret = 0;
>>   	u32 msr_index = msr_info->index;
>> -	u64 data = msr_info->data, spec_ctrl_mask;
>> +	u64 data = msr_info->data, arch_msr = 0, spec_ctrl_mask = 0;
>
>Sugget to make arch_msr and spec_ctrl_mask as local variables of each case {}
>block

Sure. Will do

>
>>   	u32 index;
>>   	switch (msr_index) {
>> @@ -2488,6 +2488,24 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>>   		if (data & ~MITI_CTRL_VALID_BITS)
>>   			return 1;
>> +		if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES))
>> +			rdmsrl(MSR_IA32_ARCH_CAPABILITIES, arch_msr);
>> +
>> +		if (data & MITI_CTRL_RETPOLINE_S_USED &&
>> +		    kvm_cpu_cap_has(X86_FEATURE_RRSBA_CTRL) &&
>
>why kvm_cpu_cap_has() is used here? it means whether KVM supports expose this
>feature to guest. But what we need here is whether host supports this
>feature. Though they might get the same result, we'd better use
>boot_cpu_has() or even read CPUID directly (since cpuid info can be changed
>by clearcpuid magic) to avoid confusion.

OK. This makes sense. I will use boot_cpu_has(). clearcpuid sometimes is
helpful for debugging. I prefer to honor it.

Thanks.