diff mbox series

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

Message ID 20191225063018.20038-3-xiaoyao.li@intel.com (mailing list archive)
State New, archived
Headers show
Series Fix Cooperlake CPU model | expand

Commit Message

Xiaoyao Li Dec. 25, 2019, 6:30 a.m. UTC 
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(-)

Comments

Eduardo Habkost Aug. 24, 2020, 10:07 p.m. UTC | #1 
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?
Xiaoyao Li Aug. 25, 2020, 12:20 a.m. UTC | #2 
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?
Eduardo Habkost Aug. 25, 2020, 2:01 p.m. UTC | #3 
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?
Xiaoyao Li Aug. 26, 2020, 2:43 a.m. UTC | #4 
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.
diff mbox series

Patch

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)",
     },