[PATCH 0/2] Fix Cooperlake CPU model

[PATCH 0/2] Fix Cooperlake CPU model

[Xiaoyao Li]

Current Cooperlake CPU model lacks VMX features which are introduced by Paolo
several months ago, and it also lacks 2 security features in
MSR_IA32_ARCH_CAPABILITIES disclosed recently.

Xiaoyao Li (2):
  target/i386: Add new bit definitions of MSR_IA32_ARCH_CAPABILITIES
  target/i386: Add missed features to Cooperlake CPU model

 target/i386/cpu.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-
 target/i386/cpu.h | 13 +++++++-----
 2 files changed, 58 insertions(+), 6 deletions(-)

-- 
2.19.1

[PATCH 1/2] target/i386: Add new bit definitions of MSR_IA32_ARCH_CAPABILITIES

[Xiaoyao Li]

The bit 6, 7 and 8 of MSR_IA32_ARCH_CAPABILITIES are recently disclosed
for some security issues. Add the definitions for them to be used by named
CPU models.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 target/i386/cpu.h | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index af282936a785..594326a79467 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -835,12 +835,15 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS];
 #define CPUID_TOPOLOGY_LEVEL_DIE      (5U << 8)
 
 /* MSR Feature Bits */
-#define MSR_ARCH_CAP_RDCL_NO    (1U << 0)
-#define MSR_ARCH_CAP_IBRS_ALL   (1U << 1)
-#define MSR_ARCH_CAP_RSBA       (1U << 2)
+#define MSR_ARCH_CAP_RDCL_NO            (1U << 0)
+#define MSR_ARCH_CAP_IBRS_ALL           (1U << 1)
+#define MSR_ARCH_CAP_RSBA               (1U << 2)
 #define MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY (1U << 3)
-#define MSR_ARCH_CAP_SSB_NO     (1U << 4)
-#define MSR_ARCH_CAP_MDS_NO     (1U << 5)
+#define MSR_ARCH_CAP_SSB_NO             (1U << 4)
+#define MSR_ARCH_CAP_MDS_NO             (1U << 5)
+#define MSR_ARCH_CAP_PSCHANGE_MC_NO     (1U << 6)
+#define MSR_ARCH_CAP_TSX_CTRL_MSR       (1U << 7)
+#define MSR_ARCH_CAP_TAA_NO             (1U << 8)
 
 #define MSR_CORE_CAP_SPLIT_LOCK_DETECT  (1U << 5)
 
-- 
2.19.1

[PATCH 2/2] target/i386: Add missed features to Cooperlake CPU model

[Xiaoyao Li]

It lacks VMX features and two security feature bits (disclosed recently) in
MSR_IA32_ARCH_CAPABILITIES in current Cooperlake CPU model, so add them.

Fixes: 22a866b6166d ("i386: Add new CPU model Cooperlake")
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 target/i386/cpu.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index e1eb9f473989..c9798ac8652b 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3198,7 +3198,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_7_0_EDX_SPEC_CTRL_SSBD | CPUID_7_0_EDX_ARCH_CAPABILITIES,
         .features[FEAT_ARCH_CAPABILITIES] =
             MSR_ARCH_CAP_RDCL_NO | MSR_ARCH_CAP_IBRS_ALL |
-            MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY | MSR_ARCH_CAP_MDS_NO,
+            MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY | MSR_ARCH_CAP_MDS_NO |
+            MSR_ARCH_CAP_PSCHANGE_MC_NO | MSR_ARCH_CAP_TAA_NO,
         .features[FEAT_7_1_EAX] =
             CPUID_7_1_EAX_AVX512_BF16,
         /*
@@ -3213,6 +3214,54 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_XSAVE_XGETBV1,
         .features[FEAT_6_EAX] =
             CPUID_6_EAX_ARAT,
+        /* Missing: Mode-based execute control (XS/XU), processor tracing, TSC scaling */
+        .features[FEAT_VMX_BASIC] = MSR_VMX_BASIC_INS_OUTS |
+             MSR_VMX_BASIC_TRUE_CTLS,
+        .features[FEAT_VMX_ENTRY_CTLS] = VMX_VM_ENTRY_IA32E_MODE |
+             VMX_VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL | VMX_VM_ENTRY_LOAD_IA32_PAT |
+             VMX_VM_ENTRY_LOAD_DEBUG_CONTROLS | VMX_VM_ENTRY_LOAD_IA32_EFER,
+        .features[FEAT_VMX_EPT_VPID_CAPS] = MSR_VMX_EPT_EXECONLY |
+             MSR_VMX_EPT_PAGE_WALK_LENGTH_4 | MSR_VMX_EPT_WB | MSR_VMX_EPT_2MB |
+             MSR_VMX_EPT_1GB | MSR_VMX_EPT_INVEPT |
+             MSR_VMX_EPT_INVEPT_SINGLE_CONTEXT | MSR_VMX_EPT_INVEPT_ALL_CONTEXT |
+             MSR_VMX_EPT_INVVPID | MSR_VMX_EPT_INVVPID_SINGLE_ADDR |
+             MSR_VMX_EPT_INVVPID_SINGLE_CONTEXT | MSR_VMX_EPT_INVVPID_ALL_CONTEXT |
+             MSR_VMX_EPT_INVVPID_SINGLE_CONTEXT_NOGLOBALS | MSR_VMX_EPT_AD_BITS,
+        .features[FEAT_VMX_EXIT_CTLS] =
+             VMX_VM_EXIT_ACK_INTR_ON_EXIT | VMX_VM_EXIT_SAVE_DEBUG_CONTROLS |
+             VMX_VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL |
+             VMX_VM_EXIT_LOAD_IA32_PAT | VMX_VM_EXIT_LOAD_IA32_EFER |
+             VMX_VM_EXIT_SAVE_IA32_PAT | VMX_VM_EXIT_SAVE_IA32_EFER |
+             VMX_VM_EXIT_SAVE_VMX_PREEMPTION_TIMER,
+        .features[FEAT_VMX_MISC] = MSR_VMX_MISC_ACTIVITY_HLT |
+             MSR_VMX_MISC_STORE_LMA | MSR_VMX_MISC_VMWRITE_VMEXIT,
+        .features[FEAT_VMX_PINBASED_CTLS] = VMX_PIN_BASED_EXT_INTR_MASK |
+             VMX_PIN_BASED_NMI_EXITING | VMX_PIN_BASED_VIRTUAL_NMIS |
+             VMX_PIN_BASED_VMX_PREEMPTION_TIMER | VMX_PIN_BASED_POSTED_INTR,
+        .features[FEAT_VMX_PROCBASED_CTLS] = VMX_CPU_BASED_VIRTUAL_INTR_PENDING |
+             VMX_CPU_BASED_USE_TSC_OFFSETING | VMX_CPU_BASED_HLT_EXITING |
+             VMX_CPU_BASED_INVLPG_EXITING | VMX_CPU_BASED_MWAIT_EXITING |
+             VMX_CPU_BASED_RDPMC_EXITING | VMX_CPU_BASED_RDTSC_EXITING |
+             VMX_CPU_BASED_CR8_LOAD_EXITING | VMX_CPU_BASED_CR8_STORE_EXITING |
+             VMX_CPU_BASED_TPR_SHADOW | VMX_CPU_BASED_MOV_DR_EXITING |
+             VMX_CPU_BASED_UNCOND_IO_EXITING | VMX_CPU_BASED_USE_IO_BITMAPS |
+             VMX_CPU_BASED_MONITOR_EXITING | VMX_CPU_BASED_PAUSE_EXITING |
+             VMX_CPU_BASED_VIRTUAL_NMI_PENDING | VMX_CPU_BASED_USE_MSR_BITMAPS |
+             VMX_CPU_BASED_CR3_LOAD_EXITING | VMX_CPU_BASED_CR3_STORE_EXITING |
+             VMX_CPU_BASED_MONITOR_TRAP_FLAG |
+             VMX_CPU_BASED_ACTIVATE_SECONDARY_CONTROLS,
+        .features[FEAT_VMX_SECONDARY_CTLS] =
+             VMX_SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
+             VMX_SECONDARY_EXEC_WBINVD_EXITING | VMX_SECONDARY_EXEC_ENABLE_EPT |
+             VMX_SECONDARY_EXEC_DESC | VMX_SECONDARY_EXEC_RDTSCP |
+             VMX_SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
+             VMX_SECONDARY_EXEC_ENABLE_VPID | VMX_SECONDARY_EXEC_UNRESTRICTED_GUEST |
+             VMX_SECONDARY_EXEC_APIC_REGISTER_VIRT |
+             VMX_SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
+             VMX_SECONDARY_EXEC_RDRAND_EXITING | VMX_SECONDARY_EXEC_ENABLE_INVPCID |
+             VMX_SECONDARY_EXEC_ENABLE_VMFUNC | VMX_SECONDARY_EXEC_SHADOW_VMCS |
+             VMX_SECONDARY_EXEC_RDSEED_EXITING | VMX_SECONDARY_EXEC_ENABLE_PML,
+        .features[FEAT_VMX_VMFUNC] = MSR_VMX_VMFUNC_EPT_SWITCHING,
         .xlevel = 0x80000008,
         .model_id = "Intel Xeon Processor (Cooperlake)",
     },
-- 
2.19.1

Re: [PATCH 0/2] Fix Cooperlake CPU model

[Paolo Bonzini]

On 25/12/19 07:30, Xiaoyao Li wrote:
> Current Cooperlake CPU model lacks VMX features which are introduced by Paolo
> several months ago, and it also lacks 2 security features in
> MSR_IA32_ARCH_CAPABILITIES disclosed recently.
> 
> Xiaoyao Li (2):
>   target/i386: Add new bit definitions of MSR_IA32_ARCH_CAPABILITIES
>   target/i386: Add missed features to Cooperlake CPU model
> 
>  target/i386/cpu.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-
>  target/i386/cpu.h | 13 +++++++-----
>  2 files changed, 58 insertions(+), 6 deletions(-)
> 

Queued, thanks.

Paolo

Re: [PATCH 0/2] Fix Cooperlake CPU model

[Zhang, Cathy]

On 1/7/2020 9:31 PM, Paolo Bonzini wrote:
> On 25/12/19 07:30, Xiaoyao Li wrote:
>> Current Cooperlake CPU model lacks VMX features which are introduced by Paolo
>> several months ago, and it also lacks 2 security features in
>> MSR_IA32_ARCH_CAPABILITIES disclosed recently.
>>
>> Xiaoyao Li (2):
>>    target/i386: Add new bit definitions of MSR_IA32_ARCH_CAPABILITIES
>>    target/i386: Add missed features to Cooperlake CPU model
>>
>>   target/i386/cpu.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-
>>   target/i386/cpu.h | 13 +++++++-----
>>   2 files changed, 58 insertions(+), 6 deletions(-)
>>
> Queued, thanks.
>
> Paolo

Hi Paolo,

Can I ask one question that will you put all the patches for Cooper Lake 
Cpu model into QEMU v5.0-rc0?

Re: [PATCH 0/2] Fix Cooperlake CPU model

[Paolo Bonzini]

On 16/03/20 02:39, Zhang, Cathy wrote:
> On 1/7/2020 9:31 PM, Paolo Bonzini wrote:
>> On 25/12/19 07:30, Xiaoyao Li wrote:
>>> Current Cooperlake CPU model lacks VMX features which are introduced
>>> by Paolo
>>> several months ago, and it also lacks 2 security features in
>>> MSR_IA32_ARCH_CAPABILITIES disclosed recently.
>>>
>>> Xiaoyao Li (2):
>>>    target/i386: Add new bit definitions of MSR_IA32_ARCH_CAPABILITIES
>>>    target/i386: Add missed features to Cooperlake CPU model
>>>
>>>   target/i386/cpu.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-
>>>   target/i386/cpu.h | 13 +++++++-----
>>>   2 files changed, 58 insertions(+), 6 deletions(-)
>>>
>> Queued, thanks.
>>
>> Paolo
> 
> Hi Paolo,
> 
> Can I ask one question that will you put all the patches for Cooper Lake
> Cpu model into QEMU v5.0-rc0?

These are included already:

commit b952544fe8a061f0c0cccfd50a58220bc6ac94da
Merge: dc65a5bdc9 083b266f69
Author: Peter Maydell <peter.maydell@linaro.org>
Date:   Fri Jan 10 17:16:49 2020 +0000

    Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging
    
    * Compat machines fix (Denis)
    * Command line parsing fixes (Michal, Peter, Xiaoyao)
    * Cooperlake CPU model fixes (Xiaoyao)
    * i386 gdb fix (mkdolata)
    * IOEventHandler cleanup (Philippe)
    * icount fix (Pavel)
    * RR support for random number sources (Pavel)
    * Kconfig fixes (Philippe)
    
Paolo

Re: [PATCH 0/2] Fix Cooperlake CPU model

[Zhang, Cathy]

On 3/16/2020 4:41 PM, Paolo Bonzini wrote:
> On 16/03/20 02:39, Zhang, Cathy wrote:
>> On 1/7/2020 9:31 PM, Paolo Bonzini wrote:
>>> On 25/12/19 07:30, Xiaoyao Li wrote:
>>>> Current Cooperlake CPU model lacks VMX features which are introduced
>>>> by Paolo
>>>> several months ago, and it also lacks 2 security features in
>>>> MSR_IA32_ARCH_CAPABILITIES disclosed recently.
>>>>
>>>> Xiaoyao Li (2):
>>>>     target/i386: Add new bit definitions of MSR_IA32_ARCH_CAPABILITIES
>>>>     target/i386: Add missed features to Cooperlake CPU model
>>>>
>>>>    target/i386/cpu.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-
>>>>    target/i386/cpu.h | 13 +++++++-----
>>>>    2 files changed, 58 insertions(+), 6 deletions(-)
>>>>
>>> Queued, thanks.
>>>
>>> Paolo
>> Hi Paolo,
>>
>> Can I ask one question that will you put all the patches for Cooper Lake
>> Cpu model into QEMU v5.0-rc0?
> These are included already:
>
> commit b952544fe8a061f0c0cccfd50a58220bc6ac94da
> Merge: dc65a5bdc9 083b266f69
> Author: Peter Maydell <peter.maydell@linaro.org>
> Date:   Fri Jan 10 17:16:49 2020 +0000
>
>      Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging
>      
>      * Compat machines fix (Denis)
>      * Command line parsing fixes (Michal, Peter, Xiaoyao)
>      * Cooperlake CPU model fixes (Xiaoyao)
>      * i386 gdb fix (mkdolata)
>      * IOEventHandler cleanup (Philippe)
>      * icount fix (Pavel)
>      * RR support for random number sources (Pavel)
>      * Kconfig fixes (Philippe)
>      
> Paolo
Yes, I see they are already in master, but not in v4.2 yet, so will they 
be in the next release v5.0?

Re: [PATCH 0/2] Fix Cooperlake CPU model

[Paolo Bonzini]

On 16/03/20 11:19, Zhang, Cathy wrote:
> Yes, I see they are already in master, but not in v4.2 yet, so will they
> be in the next release v5.0?

Yes, that's what master will become.

Paolo

Re: [PATCH 0/2] Fix Cooperlake CPU model

[Zhang, Cathy]

On 3/16/2020 6:24 PM, Paolo Bonzini wrote:
> On 16/03/20 11:19, Zhang, Cathy wrote:
>> Yes, I see they are already in master, but not in v4.2 yet, so will they
>> be in the next release v5.0?
> Yes, that's what master will become.
>
> Paolo
That's great! Thanks for your response!

Re: [PATCH 2/2] target/i386: Add missed features to Cooperlake CPU model

[Eduardo Habkost]

On Wed, Dec 25, 2019 at 02:30:18PM +0800, Xiaoyao Li wrote:
> It lacks VMX features and two security feature bits (disclosed recently) in
> MSR_IA32_ARCH_CAPABILITIES in current Cooperlake CPU model, so add them.
> 
> Fixes: 22a866b6166d ("i386: Add new CPU model Cooperlake")
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>  target/i386/cpu.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index e1eb9f473989..c9798ac8652b 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -3198,7 +3198,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_7_0_EDX_SPEC_CTRL_SSBD | CPUID_7_0_EDX_ARCH_CAPABILITIES,
>          .features[FEAT_ARCH_CAPABILITIES] =
>              MSR_ARCH_CAP_RDCL_NO | MSR_ARCH_CAP_IBRS_ALL |
> -            MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY | MSR_ARCH_CAP_MDS_NO,
> +            MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY | MSR_ARCH_CAP_MDS_NO |
> +            MSR_ARCH_CAP_PSCHANGE_MC_NO | MSR_ARCH_CAP_TAA_NO,

This seems to break on some Cooperlake hosts, see:

https://bugzilla.redhat.com/show_bug.cgi?id=1860743

Are all Cooperlake hosts supposed to have TAA_NO set?  Are there
hosts where this requires a microcode update to be installed?

-- 
Eduardo

Re: [PATCH 2/2] target/i386: Add missed features to Cooperlake CPU model

[Xiaoyao Li]

On 8/25/2020 6:07 AM, Eduardo Habkost wrote:
> On Wed, Dec 25, 2019 at 02:30:18PM +0800, Xiaoyao Li wrote:
>> It lacks VMX features and two security feature bits (disclosed recently) in
>> MSR_IA32_ARCH_CAPABILITIES in current Cooperlake CPU model, so add them.
>>
>> Fixes: 22a866b6166d ("i386: Add new CPU model Cooperlake")
>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>> ---
>>   target/i386/cpu.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 50 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index e1eb9f473989..c9798ac8652b 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -3198,7 +3198,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
>>               CPUID_7_0_EDX_SPEC_CTRL_SSBD | CPUID_7_0_EDX_ARCH_CAPABILITIES,
>>           .features[FEAT_ARCH_CAPABILITIES] =
>>               MSR_ARCH_CAP_RDCL_NO | MSR_ARCH_CAP_IBRS_ALL |
>> -            MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY | MSR_ARCH_CAP_MDS_NO,
>> +            MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY | MSR_ARCH_CAP_MDS_NO |
>> +            MSR_ARCH_CAP_PSCHANGE_MC_NO | MSR_ARCH_CAP_TAA_NO,
> 
> This seems to break on some Cooperlake hosts, see:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1860743
> 
> Are all Cooperlake hosts supposed to have TAA_NO set?  Are there
> hosts where this requires a microcode update to be installed?
> 

All the production CPX in market should have IAA_NO bit. We can check it 
directly with rdmsr(0x10a).

The problem of this issue is due to commit db616173d787 ("x86/tsx: Add 
config options to set tsx=on|off|auto"), which sets the default to "off" 
for 100% safety. However, default to off may cause noticeable 
regressions on TSX safe platform, e.g., CPX.

Maybe we need to set CONFIG_X86_INTEL_TSX_MODE_AUTO=y for OSV released 
kernel?

Re: [PATCH 2/2] target/i386: Add missed features to Cooperlake CPU model

[Eduardo Habkost]

On Tue, Aug 25, 2020 at 08:20:35AM +0800, Xiaoyao Li wrote:
> On 8/25/2020 6:07 AM, Eduardo Habkost wrote:
> > On Wed, Dec 25, 2019 at 02:30:18PM +0800, Xiaoyao Li wrote:
> > > It lacks VMX features and two security feature bits (disclosed recently) in
> > > MSR_IA32_ARCH_CAPABILITIES in current Cooperlake CPU model, so add them.
> > > 
> > > Fixes: 22a866b6166d ("i386: Add new CPU model Cooperlake")
> > > Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> > > ---
> > >   target/i386/cpu.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-
> > >   1 file changed, 50 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > > index e1eb9f473989..c9798ac8652b 100644
> > > --- a/target/i386/cpu.c
> > > +++ b/target/i386/cpu.c
> > > @@ -3198,7 +3198,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
> > >               CPUID_7_0_EDX_SPEC_CTRL_SSBD | CPUID_7_0_EDX_ARCH_CAPABILITIES,
> > >           .features[FEAT_ARCH_CAPABILITIES] =
> > >               MSR_ARCH_CAP_RDCL_NO | MSR_ARCH_CAP_IBRS_ALL |
> > > -            MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY | MSR_ARCH_CAP_MDS_NO,
> > > +            MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY | MSR_ARCH_CAP_MDS_NO |
> > > +            MSR_ARCH_CAP_PSCHANGE_MC_NO | MSR_ARCH_CAP_TAA_NO,
> > 
> > This seems to break on some Cooperlake hosts, see:
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1860743
> > 
> > Are all Cooperlake hosts supposed to have TAA_NO set?  Are there
> > hosts where this requires a microcode update to be installed?
> > 
> 
> All the production CPX in market should have IAA_NO bit. We can check it
> directly with rdmsr(0x10a).
> 
> The problem of this issue is due to commit db616173d787 ("x86/tsx: Add
> config options to set tsx=on|off|auto"), which sets the default to "off" for
> 100% safety. However, default to off may cause noticeable regressions on TSX
> safe platform, e.g., CPX.
> 
> Maybe we need to set CONFIG_X86_INTEL_TSX_MODE_AUTO=y for OSV released
> kernel?

Considering that disabling TSX is a policy decision likely to be
taken by the OS vendor or by the system administrator, we could
at least make the CPU model easier to use on those cases.

Maybe we should provide a version of Cooperlake without TSX, like
we already do for the other CPU models?

-- 
Eduardo

Re: [PATCH 2/2] target/i386: Add missed features to Cooperlake CPU model

[Xiaoyao Li]

On 8/25/2020 10:01 PM, Eduardo Habkost wrote:
> On Tue, Aug 25, 2020 at 08:20:35AM +0800, Xiaoyao Li wrote:
>> On 8/25/2020 6:07 AM, Eduardo Habkost wrote:
>>> On Wed, Dec 25, 2019 at 02:30:18PM +0800, Xiaoyao Li wrote:
>>>> It lacks VMX features and two security feature bits (disclosed recently) in
>>>> MSR_IA32_ARCH_CAPABILITIES in current Cooperlake CPU model, so add them.
>>>>
>>>> Fixes: 22a866b6166d ("i386: Add new CPU model Cooperlake")
>>>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>>>> ---
>>>>    target/i386/cpu.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-
>>>>    1 file changed, 50 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>>>> index e1eb9f473989..c9798ac8652b 100644
>>>> --- a/target/i386/cpu.c
>>>> +++ b/target/i386/cpu.c
>>>> @@ -3198,7 +3198,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
>>>>                CPUID_7_0_EDX_SPEC_CTRL_SSBD | CPUID_7_0_EDX_ARCH_CAPABILITIES,
>>>>            .features[FEAT_ARCH_CAPABILITIES] =
>>>>                MSR_ARCH_CAP_RDCL_NO | MSR_ARCH_CAP_IBRS_ALL |
>>>> -            MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY | MSR_ARCH_CAP_MDS_NO,
>>>> +            MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY | MSR_ARCH_CAP_MDS_NO |
>>>> +            MSR_ARCH_CAP_PSCHANGE_MC_NO | MSR_ARCH_CAP_TAA_NO,
>>>
>>> This seems to break on some Cooperlake hosts, see:
>>>
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1860743
>>>
>>> Are all Cooperlake hosts supposed to have TAA_NO set?  Are there
>>> hosts where this requires a microcode update to be installed?
>>>
>>
>> All the production CPX in market should have IAA_NO bit. We can check it
>> directly with rdmsr(0x10a).
>>
>> The problem of this issue is due to commit db616173d787 ("x86/tsx: Add
>> config options to set tsx=on|off|auto"), which sets the default to "off" for
>> 100% safety. However, default to off may cause noticeable regressions on TSX
>> safe platform, e.g., CPX.
>>
>> Maybe we need to set CONFIG_X86_INTEL_TSX_MODE_AUTO=y for OSV released
>> kernel?
> 
> Considering that disabling TSX is a policy decision likely to be
> taken by the OS vendor or by the system administrator, we could
> at least make the CPU model easier to use on those cases.
> 
> Maybe we should provide a version of Cooperlake without TSX, like
> we already do for the other CPU models?
> 

sure we can do it.