[Qemu-devel] [PATCH 0/2] Add MSR based features on Cascadelake CPU model

[Qemu-devel] [PATCH 0/2] Add MSR based features on Cascadelake CPU model

[Tao Xu]

These patches update the stepping and add some MSR based features 
enumerated by ARCH_CAPABILITIES on Cascadelake-Server CPU model.

Tao Xu (2):
  i386: Update stepping of Cascadelake-Server
  i386: Add some MSR based features on Cascadelake-Server CPU model

 include/hw/i386/pc.h | 20 ++++++++++++++++++++
 target/i386/cpu.c    |  8 ++++++--
 2 files changed, 26 insertions(+), 2 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH 1/2] i386: Update stepping of Cascadelake-Server

[Tao Xu]

Update the stepping from 5 to 6, in order that
the Cascadelake-Server CPU model can support AVX512VNNI
and MSR based features exposed by ARCH_CAPABILITIES.

Signed-off-by: Tao Xu <tao3.xu@intel.com>
---
 include/hw/i386/pc.h | 4 ++++
 target/i386/cpu.c    | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index c7c0c944e8..95453968db 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -300,6 +300,10 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
         .driver   = "intel-iommu",\
         .property = "dma-drain",\
         .value    = "off",\
+    },{\
+        .driver   = "Cascadelake-Server" "-" TYPE_X86_CPU,\
+        .property = "stepping",\
+        .value    = "5",\
     },
 
 #define PC_COMPAT_3_0 \
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 677a3bd5fb..09706ad51a 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2466,7 +2466,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
         .model = 85,
-        .stepping = 5,
+        .stepping = 6,
         .features[FEAT_1_EDX] =
             CPUID_VME | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
             CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
-- 
2.17.1

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

[Tao Xu]

As noted in
http://lists.gnu.org/archive/html/qemu-devel/2018-09/msg02212.html
Because MSR based feature has been supported by QEMU,
we add CPUID_7_0_EDX_ARCH_CAPABILITIES on Cascadelake-Server CPU
model, and add IA32_ARCH_CAPABILITIES MSR based features (RDCL_NO,
IBRS_ALL and SKIP_L1DFL_VMENTRY).

Note:
RSBA and SSBD_NO are not supported by native machines on stepping
6 of Cascadelake-Server, so we will keep the CPU model updated.

Signed-off-by: Tao Xu <tao3.xu@intel.com>
---
 include/hw/i386/pc.h | 16 ++++++++++++++++
 target/i386/cpu.c    |  6 +++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 95453968db..741ceefa5b 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -304,6 +304,22 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
         .driver   = "Cascadelake-Server" "-" TYPE_X86_CPU,\
         .property = "stepping",\
         .value    = "5",\
+    },{\
+        .driver   = "Cascadelake-Server" "-" TYPE_X86_CPU,\
+        .property = "arch-capabilities",\
+        .value    = "off",\
+    },{\
+        .driver   = "Cascadelake-Server" "-" TYPE_X86_CPU,\
+        .property = "rdctl-no",\
+        .value    = "off",\
+    },{\
+        .driver   = "Cascadelake-Server" "-" TYPE_X86_CPU,\
+        .property = "ibrs-all",\
+        .value    = "off",\
+    },{\
+        .driver   = "Cascadelake-Server" "-" TYPE_X86_CPU,\
+        .property = "skip-l1dfl-vmentry",\
+        .value    = "off",\
     },
 
 #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,
         /* Missing: XSAVES (not supported by some Linux versions,
                 * including v4.1 to v4.12).
                 * KVM doesn't yet expose any XSAVES state save component,
@@ -2511,6 +2512,9 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_XSAVE_XGETBV1,
         .features[FEAT_6_EAX] =
             CPUID_6_EAX_ARAT,
+        .features[FEAT_ARCH_CAPABILITIES] =
+            MSR_ARCH_CAP_RDCL_NO | MSR_ARCH_CAP_IBRS_ALL |
+            MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY,
         .xlevel = 0x80000008,
         .model_id = "Intel Xeon Processor (Cascadelake)",
     },
-- 
2.17.1

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

[Tao Xu]

Hi Eduardo/Paolo,

Do you have any comments about these patches ?

Thanks
Tao

On 12/27/18 10:43 AM, Tao Xu wrote:
> These patches update the stepping and add some MSR based features
> enumerated by ARCH_CAPABILITIES on Cascadelake-Server CPU model.
>
> Tao Xu (2):
>    i386: Update stepping of Cascadelake-Server
>    i386: Add some MSR based features on Cascadelake-Server CPU model
>
>   include/hw/i386/pc.h | 20 ++++++++++++++++++++
>   target/i386/cpu.c    |  8 ++++++--
>   2 files changed, 26 insertions(+), 2 deletions(-)
>

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

[Tao Xu]

Hi Eduardo/Paolo,

Do you have any comments about these patches?

Thanks
Tao

On 12/27/18 10:43 AM, Tao Xu wrote:
> These patches update the stepping and add some MSR based features
> enumerated by ARCH_CAPABILITIES on Cascadelake-Server CPU model.
>
> Tao Xu (2):
>    i386: Update stepping of Cascadelake-Server
>    i386: Add some MSR based features on Cascadelake-Server CPU model
>
>   include/hw/i386/pc.h | 20 ++++++++++++++++++++
>   target/i386/cpu.c    |  8 ++++++--
>   2 files changed, 26 insertions(+), 2 deletions(-)
>

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

[Eduardo Habkost]

On Thu, Dec 27, 2018 at 10:43:02AM +0800, Tao Xu wrote:
> These patches update the stepping and add some MSR based features 
> enumerated by ARCH_CAPABILITIES on Cascadelake-Server CPU model.

Queued, thanks.

-- 
Eduardo

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

[Eduardo Habkost]

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.

Also, why are you setting this only on Cascadelake-Server and not
on all the other Intel CPUs?

I'm queueing only patch 1/2 until we sort this out.

-- 
Eduardo

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

[Tao Xu]

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?

> Also, why are you setting this only on Cascadelake-Server and not
> on all the other Intel CPUs?
> 
Thank you for your notice. I reviewed the git log of KVM.
"MSR_IA32_ARCH_CAPABILITIES is emulated in kvm, there is no dependency 
on hardware support for this feature". So do you mean we should also add 
features based on ARCH_CAPABILITIES in former CPU(such as skylake)?

> I'm queueing only patch 1/2 until we sort this out.
>

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

[Eduardo Habkost]

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.


> 
> > Also, why are you setting this only on Cascadelake-Server and not
> > on all the other Intel CPUs?
> > 
> Thank you for your notice. I reviewed the git log of KVM.
> "MSR_IA32_ARCH_CAPABILITIES is emulated in kvm, there is no dependency on
> hardware support for this feature". So do you mean we should also add
> features based on ARCH_CAPABILITIES in former CPU(such as skylake)?

It doesn't depend on host hardware features, but it does depend
on host KVM code added in Linux v4.17, which makes this more
tricky.

We don't want a QEMU upgrade to make an existing VM configuration
to stop running under Linux v4.16.  This will probably require
versioned CPU models[1] to work well.

[1] https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg08422.html

> 
> > I'm queueing only patch 1/2 until we sort this out.
> > 
> 
> 

-- 
Eduardo

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

[Tao Xu]

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.
> 
>>
>>> Also, why are you setting this only on Cascadelake-Server and not
>>> on all the other Intel CPUs?
>>>
>> Thank you for your notice. I reviewed the git log of KVM.
>> "MSR_IA32_ARCH_CAPABILITIES is emulated in kvm, there is no dependency on
>> hardware support for this feature". So do you mean we should also add
>> features based on ARCH_CAPABILITIES in former CPU(such as skylake)?
> 
> It doesn't depend on host hardware features, but it does depend
> on host KVM code added in Linux v4.17, which makes this more
> tricky.
> 
> We don't want a QEMU upgrade to make an existing VM configuration
> to stop running under Linux v4.16.  This will probably require
> versioned CPU models[1] to work well.
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg08422.html
> 
Hi Eduardo, I think the versioned CPU models solution is great. But I 
haven't seen further patches in the mail list. What is the recent 
progress of this work? Need me to do something?

Looking forward to your reply.
Tao

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

[Eduardo Habkost]

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.


> > 
> > > 
> > > > Also, why are you setting this only on Cascadelake-Server and not
> > > > on all the other Intel CPUs?
> > > > 
> > > Thank you for your notice. I reviewed the git log of KVM.
> > > "MSR_IA32_ARCH_CAPABILITIES is emulated in kvm, there is no dependency on
> > > hardware support for this feature". So do you mean we should also add
> > > features based on ARCH_CAPABILITIES in former CPU(such as skylake)?
> > 
> > It doesn't depend on host hardware features, but it does depend
> > on host KVM code added in Linux v4.17, which makes this more
> > tricky.
> > 
> > We don't want a QEMU upgrade to make an existing VM configuration
> > to stop running under Linux v4.16.  This will probably require
> > versioned CPU models[1] to work well.
> > 
> > [1] https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg08422.html
> > 
> Hi Eduardo, I think the versioned CPU models solution is great. But I
> haven't seen further patches in the mail list. What is the recent progress
> of this work? Need me to do something?

Nothing was done to implement this yet.  Before implementing it,
I'd like to get decent automated test coverage for CPUID
compatibility.  Keeping compatibility with non-versioned CPU
models is already too error-prone today, and I don't want to make
the situation worse.  I have an old branch where I was working on
it, at:
https://github.com/ehabkost/qemu-hacks/commits/work/guest-abi-tests

Unfortunately I'll be away from work for the next 4 weeks, so
I'll need to get back to this discussion after I'm back.

-- 
Eduardo

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

[Tao Xu]

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.

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

[Eduardo Habkost]

Hi,

Sorry for taking so long to reply.  I was away from work for a
few weeks and I'm still catching up on old threads.  Reply below:

On Tue, Jan 29, 2019 at 04:55:20PM +0800, Tao Xu wrote:
> 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
                                                  ^^^^

Sorry, I meant "4.16" above.

> > 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?

This is not a harmless warning if you need live migration.  Any
management software that supports live migration must ensure the
chosen CPU model is fully supported by the host (by using
query-cpu-* commands or using the "enforce" flag), and existing
VM configurations using Cascadelake CPU model would stop running
on < v4.16 hosts if Cascadelake starts requiring
arch-capabilities.

> And other feature will have the same problem?

We had similar issues with other features, and we have tried
different solutions:

SPEC_CTRL requires a host microcode update + Linux >= v4.16, and
we have added new CPU models for it (the -IBRS variants).

Some Opteron CPU models didn't have RDTSCP, and the feature
requires Linux >= v4.5.  We could add RDTSCP to the existing CPU
models because we documented the minimum host kernel version on
qemu-doc.texi.

SSBD requires a host microcode update + Linux >= v4.17.  We
decided to not add new -SSBD CPU models, and let management
software enable the flag when it knows it's safe.


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

It's too late to implement that in QEMU 4.0, but we could aim to
do it in 4.1.

Note that this doesn't block people from using the feature.  The
features above are already enabled automatically by management
software using libvirt's "host-model" mode.  Versioned CPU models
will just make things more convenient for management software and
people running QEMU from the command-line.


-- 
Eduardo