| Message ID | 20191225063018.20038-3-xiaoyao.li@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix Cooperlake CPU model | expand |
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?
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?
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?
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 --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)", },
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(-)