Re: [Qemu-devel] [PATCH 2/2] i386: Add some MSR based features on Cascadelake-Server CPU model

[Tao Xu <tao3.xu@intel.com>]

On 1/28/2019 10:48 PM, Eduardo Habkost wrote:
> On Mon, Jan 28, 2019 at 04:33:40PM +0800, Tao Xu wrote:
>> On 1/24/2019 3:15 AM, Eduardo Habkost wrote:
>>> On Mon, Jan 21, 2019 at 05:29:32PM +0800, Tao Xu wrote:
>>>> On 1/15/2019 2:35 AM, Eduardo Habkost wrote:
>>>>> Sorry, we do have a problem here:
>>>>>
>>>>> On Thu, Dec 27, 2018 at 10:43:04AM +0800, Tao Xu wrote:
>>>>> [...]
>>>>>>     #define PC_COMPAT_3_0 \
>>>>>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>>>>>> index 09706ad51a..5296c73cd5 100644
>>>>>> --- a/target/i386/cpu.c
>>>>>> +++ b/target/i386/cpu.c
>>>>>> @@ -2499,7 +2499,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
>>>>>>                 CPUID_7_0_ECX_PKU | CPUID_7_0_ECX_OSPKE |
>>>>>>                 CPUID_7_0_ECX_AVX512VNNI,
>>>>>>             .features[FEAT_7_0_EDX] =
>>>>>> -            CPUID_7_0_EDX_SPEC_CTRL | CPUID_7_0_EDX_SPEC_CTRL_SSBD,
>>>>>> +            CPUID_7_0_EDX_SPEC_CTRL | CPUID_7_0_EDX_SPEC_CTRL_SSBD |
>>>>>> +            CPUID_7_0_EDX_ARCH_CAPABILITIES,
>>>>>
>>>>> CPUID_7_0_EDX_ARCH_CAPABILITIES is still set on
>>>>> unmigratable_flags.  We need to make it migratable before adding
>>>>> it by default to a named CPU model.
>>>>>
>>>> Hi Eduardo,
>>>>
>>>> Do you mean I need to remove CPUID_7_0_EDX_ARCH_CAPABILITIES
>>>> from .migratable_flags? Or CPUID_7_0_EDX_ARCH_CAPABILITIES can not
>>>> support migration now?
>>>
>>> We need to remove it from .unmigratable_flags, but only after
>>> confirm it is really migration-safe.  To make it migration-safe,
>>> the MSR value seen by the guest must have a predictable value
>>> that is 100% independent from host hardware or host software
>>> version.
>>>
>>> This is easy to ensure if MSR_IA32_ARCH_CAPABILITIES is in
>>> KVM_GET_MSR_INDEX_LIST (meaning QEMU can actually configure the
>>> MSR value seen by the guest).  If MSR_IA32_ARCH_CAPABILITIES is
>>> not on KVM_GET_MSR_INDEX_LIST, arch-capabilities must not be
>>> returned by x86_cpu_get_supported_feature_word() when
>>> migratable_only is true.
>>>
>> Thank you. I have seen your patch to migratable it:
>>
>> [PATCH 0/2] i386: arch_capabilities fixes + migratability
>> http://lists.nongnu.org/archive/html/qemu-devel/2019-01/msg06701.html
>>
>> So now can arch-capabilities and features exposed by it be added in
>> Cascadelake CPU model? Because Cascadelake CPU model can support it in
>> hardware. And for Icelake CPU model, Robert will add in the future.
> 
> Even if there's host hardware support for the MSR, the feature
> still depends on a KVM commit added in Linux v6.16 (commit
> 28c1c9fabf48 "KVM/VMX: Emulate MSR_IA32_ARCH_CAPABILITIES").  We
> would still have the kernel version dependency described below.
> 
Sorry, we are still confused about it. I read the kernel commit 
28c1c9fabf48 again. This commit indeed enabling ARCH_CAPABILITIES and 
emulate it in the platform which doesn't support it in hardware. But 
with your patch, the ARCH_CAPABILITIES is like other CPU new features. 
When the guest start up with old host kernel, it will just warning 
"warning: host doesn't support requested feature...". Beside the 
warnings, will this has other further problems? And other feature will 
have the same problem?

I am sorry for so many questions and this is really a tricky issue. 
Indeed, the versioned CPU model will solve the problem of user using the 
old version of kernel. But maybe it will have a long time to in-placement.