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.
