| Message ID | 1530098844-236851-3-git-send-email-robert.hu@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 27, 2018 at 07:27:21PM +0800, Robert Hoo wrote: > Support of IA32_PRED_CMD MSR already be enumerated by same CPUID bit as > SPEC_CTRL. > > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> Based on kernel commit 1eaafe91, it looks like we must always set IA32_ARCH_CAPABILITIES.RSBA[bit 2] unless we're really sure the VM will not be migrated to a vulnerable processor. Considering this, I'd like to make "+arch-capabilities" set IA32_ARCH_CAPABILITIES.RSBA by default, unless RSBA is explicitly disabled by management software. > --- > target/i386/cpu.c | 2 +- > target/i386/cpu.h | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index e6c2f8a..953098c 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -1002,7 +1002,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { > NULL, NULL, NULL, NULL, > NULL, NULL, NULL, NULL, > NULL, NULL, "spec-ctrl", NULL, > - NULL, NULL, NULL, "ssbd", > + NULL, "arch-capabilities", NULL, "ssbd", > }, > .cpuid_eax = 7, > .cpuid_needs_ecx = true, .cpuid_ecx = 0, > diff --git a/target/i386/cpu.h b/target/i386/cpu.h > index 734a73e..1ef2040 100644 > --- a/target/i386/cpu.h > +++ b/target/i386/cpu.h > @@ -688,6 +688,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS]; > #define CPUID_7_0_EDX_AVX512_4VNNIW (1U << 2) /* AVX512 Neural Network Instructions */ > #define CPUID_7_0_EDX_AVX512_4FMAPS (1U << 3) /* AVX512 Multiply Accumulation Single Precision */ > #define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */ > +#define CPUID_7_0_EDX_ARCH_CAPABILITIES (1U << 29) /*Arch Capabilities of RDCL_NO and IBRS_ALL*/ > #define CPUID_7_0_EDX_SPEC_CTRL_SSBD (1U << 31) /* Speculative Store Bypass Disable */ > > #define CPUID_8000_0008_EBX_IBPB (1U << 12) /* Indirect Branch Prediction Barrier */ > -- > 1.8.3.1 > >
On Thu, 2018-06-28 at 15:28 -0300, Eduardo Habkost wrote: > On Wed, Jun 27, 2018 at 07:27:21PM +0800, Robert Hoo wrote: > > Support of IA32_PRED_CMD MSR already be enumerated by same CPUID bit as > > SPEC_CTRL. > > > > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> > > Based on kernel commit 1eaafe91, it looks like we must always set > IA32_ARCH_CAPABILITIES.RSBA[bit 2] unless we're really sure the > VM will not be migrated to a vulnerable processor. > > Considering this, I'd like to make "+arch-capabilities" set > IA32_ARCH_CAPABILITIES.RSBA by default, unless RSBA is explicitly > disabled by management software. > Agree. But this seems beyond Icelake CPU model scope. How about I think about this carefully and compose another patch (set) for this? And you'd like to set IA32_ARCH_CAPABILITIES.RSBA by default in qemu or kvm layer? > > --- > > target/i386/cpu.c | 2 +- > > target/i386/cpu.h | 1 + > > 2 files changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > > index e6c2f8a..953098c 100644 > > --- a/target/i386/cpu.c > > +++ b/target/i386/cpu.c > > @@ -1002,7 +1002,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { > > NULL, NULL, NULL, NULL, > > NULL, NULL, NULL, NULL, > > NULL, NULL, "spec-ctrl", NULL, > > - NULL, NULL, NULL, "ssbd", > > + NULL, "arch-capabilities", NULL, "ssbd", > > }, > > .cpuid_eax = 7, > > .cpuid_needs_ecx = true, .cpuid_ecx = 0, > > diff --git a/target/i386/cpu.h b/target/i386/cpu.h > > index 734a73e..1ef2040 100644 > > --- a/target/i386/cpu.h > > +++ b/target/i386/cpu.h > > @@ -688,6 +688,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS]; > > #define CPUID_7_0_EDX_AVX512_4VNNIW (1U << 2) /* AVX512 Neural Network Instructions */ > > #define CPUID_7_0_EDX_AVX512_4FMAPS (1U << 3) /* AVX512 Multiply Accumulation Single Precision */ > > #define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */ > > +#define CPUID_7_0_EDX_ARCH_CAPABILITIES (1U << 29) /*Arch Capabilities of RDCL_NO and IBRS_ALL*/ > > #define CPUID_7_0_EDX_SPEC_CTRL_SSBD (1U << 31) /* Speculative Store Bypass Disable */ > > > > #define CPUID_8000_0008_EBX_IBPB (1U << 12) /* Indirect Branch Prediction Barrier */ > > -- > > 1.8.3.1 > > > > >
On Tue, Jul 03, 2018 at 03:35:13PM +0800, Robert Hoo wrote: > On Thu, 2018-06-28 at 15:28 -0300, Eduardo Habkost wrote: > > On Wed, Jun 27, 2018 at 07:27:21PM +0800, Robert Hoo wrote: > > > Support of IA32_PRED_CMD MSR already be enumerated by same CPUID bit as > > > SPEC_CTRL. > > > > > > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> > > > > Based on kernel commit 1eaafe91, it looks like we must always set > > IA32_ARCH_CAPABILITIES.RSBA[bit 2] unless we're really sure the > > VM will not be migrated to a vulnerable processor. > > > > Considering this, I'd like to make "+arch-capabilities" set > > IA32_ARCH_CAPABILITIES.RSBA by default, unless RSBA is explicitly > > disabled by management software. > > > Agree. But this seems beyond Icelake CPU model scope. How about I think > about this carefully and compose another patch (set) for this? This plan makes sense to me, as I don't want to make this decision block IceLake from being in QEMU 3.0. However, enabling CPUID_7_0_EDX_ARCH_CAPABILITIES in IceLake but setting the MSR to 0 seems pointless. I think we should add IceLake without CPUID_7_0_EDX_ARCH_CAPABILITIES first, and later (after deciding on a reasonable default value for MSR_IA32_ARCH_CAPABILITIES), enable the CPUID bit on IceLake (hopefully in time for QEMU 3.0). > And you'd like to set IA32_ARCH_CAPABILITIES.RSBA by default in qemu or > kvm layer? Probably we need to make this decision in QEMU. If KVM set RSBA automatically on .get_msr_feature(), QEMU won't be able to differentiate a host with RSBA set from a host with RSBA unset.
On Tue, 2018-07-03 at 08:00 -0300, Eduardo Habkost wrote: > On Tue, Jul 03, 2018 at 03:35:13PM +0800, Robert Hoo wrote: > > On Thu, 2018-06-28 at 15:28 -0300, Eduardo Habkost wrote: > > > On Wed, Jun 27, 2018 at 07:27:21PM +0800, Robert Hoo wrote: > > > > Support of IA32_PRED_CMD MSR already be enumerated by same CPUID bit as > > > > SPEC_CTRL. > > > > > > > > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> > > > > > > Based on kernel commit 1eaafe91, it looks like we must always set > > > IA32_ARCH_CAPABILITIES.RSBA[bit 2] unless we're really sure the > > > VM will not be migrated to a vulnerable processor. > > > > > > Considering this, I'd like to make "+arch-capabilities" set > > > IA32_ARCH_CAPABILITIES.RSBA by default, unless RSBA is explicitly > > > disabled by management software. > > > > > Agree. But this seems beyond Icelake CPU model scope. How about I think > > about this carefully and compose another patch (set) for this? > > This plan makes sense to me, as I don't want to make this > decision block IceLake from being in QEMU 3.0. > > However, enabling CPUID_7_0_EDX_ARCH_CAPABILITIES in IceLake but > setting the MSR to 0 seems pointless. > > I think we should add IceLake without > CPUID_7_0_EDX_ARCH_CAPABILITIES first, and later (after deciding > on a reasonable default value for MSR_IA32_ARCH_CAPABILITIES), > enable the CPUID bit on IceLake (hopefully in time for QEMU 3.0). > > > > And you'd like to set IA32_ARCH_CAPABILITIES.RSBA by default in qemu or > > kvm layer? > > Probably we need to make this decision in QEMU. If KVM set RSBA > automatically on .get_msr_feature(), QEMU won't be able to > differentiate a host with RSBA set from a host with RSBA unset. > What's the default value for MSR IA32_ARCH_CAPABILITIES? is it clear now?
On 12/07/2018 11:18, Robert Hoo wrote: >>> And you'd like to set IA32_ARCH_CAPABILITIES.RSBA by default in qemu or >>> kvm layer? >> Probably we need to make this decision in QEMU. If KVM set RSBA >> automatically on .get_msr_feature(), QEMU won't be able to >> differentiate a host with RSBA set from a host with RSBA unset. >> > What's the default value for MSR IA32_ARCH_CAPABILITIES? is it clear > now? > It's the host value. However, QEMU never sets the CPUID bit so it doesn't matter if you are using QEMU. Paolo
diff --git a/target/i386/cpu.c b/target/i386/cpu.c index e6c2f8a..953098c 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -1002,7 +1002,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, "spec-ctrl", NULL, - NULL, NULL, NULL, "ssbd", + NULL, "arch-capabilities", NULL, "ssbd", }, .cpuid_eax = 7, .cpuid_needs_ecx = true, .cpuid_ecx = 0, diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 734a73e..1ef2040 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -688,6 +688,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS]; #define CPUID_7_0_EDX_AVX512_4VNNIW (1U << 2) /* AVX512 Neural Network Instructions */ #define CPUID_7_0_EDX_AVX512_4FMAPS (1U << 3) /* AVX512 Multiply Accumulation Single Precision */ #define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */ +#define CPUID_7_0_EDX_ARCH_CAPABILITIES (1U << 29) /*Arch Capabilities of RDCL_NO and IBRS_ALL*/ #define CPUID_7_0_EDX_SPEC_CTRL_SSBD (1U << 31) /* Speculative Store Bypass Disable */ #define CPUID_8000_0008_EBX_IBPB (1U << 12) /* Indirect Branch Prediction Barrier */
Support of IA32_PRED_CMD MSR already be enumerated by same CPUID bit as SPEC_CTRL. Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> --- target/i386/cpu.c | 2 +- target/i386/cpu.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)