Re: [RFC PATCH 6/9] kvm/x86: Add ARCH_CAP_VIRTUAL_ENUM for guest MSR_IA32_ARCH_CAPABILITIES

["Yang, Weijiang" <weijiang.yang@intel.com>]

On 12/29/2022 3:41 PM, Zhang, Chen wrote:
>
>> -----Original Message-----
>> From: Yang, Weijiang <weijiang.yang@intel.com>
>> Sent: Thursday, December 29, 2022 3:03 PM
>> To: Zhang, Chen <chen.zhang@intel.com>
>> Cc: Gao, Chao <chao.gao@intel.com>; Pawan Gupta
>> <pawan.kumar.gupta@linux.intel.com>; Paolo Bonzini
>> <pbonzini@redhat.com>; Christopherson,, Sean <seanjc@google.com>; H.
>> Peter Anvin <hpa@zytor.com>; Dave Hansen <dave.hansen@linux.intel.com>;
>> Borislav Petkov <bp@alien8.de>; Ingo Molnar <mingo@redhat.com>; Thomas
>> Gleixner <tglx@linutronix.de>; x86@kernel.org; linux-kernel@vger.kernel.org;
>> kvm@vger.kernel.org
>> Subject: Re: [RFC PATCH 6/9] kvm/x86: Add ARCH_CAP_VIRTUAL_ENUM for
>> guest MSR_IA32_ARCH_CAPABILITIES
>>
>>
>> On 12/29/2022 10:58 AM, Zhang, Chen wrote:
>>
>> [...]
>>
>>>> \
>>>>> +	 ARCH_CAP_VIRTUAL_ENUM)
>>>>>
>>>>>     static u64 kvm_get_arch_capabilities(void)
>>>>>     {
>>>>> @@ -1607,6 +1611,13 @@ static u64 kvm_get_arch_capabilities(void)
>>>>>     	 */
>>>>>     	data |= ARCH_CAP_PSCHANGE_MC_NO;
>>>>>
>>>>> +	/*
>>>>> +	 * Virtual MSRs can allow guests to notify VMM whether or not
>>>>> +	 * they are using specific software mitigation, allowing a VMM
>>>>> +	 * to enable there hardware control only where necessary.
>>>>> +	 */
>>>>> +	data |= ARCH_CAP_VIRTUAL_ENUM;
>>>> IMO, this is:  data &= ARCH_CAP_VIRTUAL_ENUM; because it requires
>>>> platform support.
>>> Intel defined the virtual MSRs for software mitigations for all platforms.
>>> KVM should be unconditionally opened it for the software mitigation in
>> migration pools.
>>> For example migration from the old platform to the new platform.
>>> Please check the Software Mitigations in Migration Pools section in
>> documents:
>>> https://www.intel.com/content/www/us/en/developer/articles/technical/s
>>> oftware-security-guidance/technical-documentation/branch-history-injec
>>> tion.html
>> If this series running on old platforms, how VMM can set specific vmcs fields,
>>
>> e.g., “virtualize IA32_SPEC_CTRL” VM-execution control, to mitigate guest
>> issues?
> Enable the virtual MSRs does not means to enable the “virtualize IA32_SPEC_CTRL”.
> KVM will check "cpu_has_virt_spec_ctrl()" before set specific VMCS.


What I'm concerned is, if the feature is not supported on some 
platforms, unconditionally

exposing to guest would make it wrongly "think"  the feature is 
supported on HW, but actually

WMM can do nothing to mitigate issues. Read through the patch set, looks 
like guest cannot

even get any errors during feature configuration in this case. So I 
suggest add more

sanity checks. Did I  miss something?


>
> Thanks
> Chen
>
>>
>>>>> +
>>>>>     	/*
>>>>>     	 * If we're doing cache flushes (either "always" or "cond")
>>>>>     	 * we will do one whenever the guest does a vmlaunch/vmresume.
>>>>> @@ -1657,6 +1668,9 @@ static int kvm_get_msr_feature(struct
>>>> kvm_msr_entry *msr)
>>>>>     	case MSR_IA32_UCODE_REV:
>>>>>     		rdmsrl_safe(msr->index, &msr->data);
>>>>>     		break;
>>>>> +	case MSR_VIRTUAL_ENUMERATION:
>>>>> +		msr->data = VIRT_ENUM_MITIGATION_CTRL_SUPPORT;
>>>> Need to check bit 63 of host MSR_ARCH_CAPABILITIES before expose the
>>>> feature.
>>> Refer to the above comments.
>>>
>>> Thanks
>>> Chen
>>>
>>>>> +		break;
>>>>>     	default:
>>>>>     		return static_call(kvm_x86_get_msr_feature)(msr);
>>>>>     	}