[Qemu-devel] [PATCH QEMU] Patches for new AMD CPU bits.

[Qemu-devel] [PATCH QEMU] Patches for new AMD CPU bits.

[Konrad Rzeszutek Wilk]

Hi!


I was reading the AMD whitepaper on SSBD and noticed that they have added
two new bits in the 8000_0008 CPUID. EBX:
 1) Bit[26] - similar to Intel's SSB_NO not needed anymore.
 2) Bit[24] - use SPEC_CTRL MSR (0x48) instead of VIRT SPEC_CTRL MSR
    (0xC001_011f).

See 124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf
A copy of this document is available at
        https://bugzilla.kernel.org/show_bug.cgi?id=199889

These two patches along with the kernel ones allow us to expose those
two bits to the guest.

Thank you!

 target/i386/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


Konrad Rzeszutek Wilk (2):
      i386: define the AMD 'amd-ssbd' CPUID feature bit
      i386: Define AMD's no SSB mitigation needed.

[Qemu-devel] [PATCH 1/2] i386: define the AMD 'amd-ssbd' CPUID feature bit

[Konrad Rzeszutek Wilk]

AMD future CPUs expose _two_ ways to utilize the Intel equivalant
of the Speculative Store Bypass Disable. The first is via
the virtualized VIRT_SPEC CTRL MSR (0xC001_011f) and the second
is via the SPEC_CTRL MSR (0x48). The document titled:
124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf

gives priority of SPEC CTRL MSR over the VIRT SPEC CTRL MSR.

A copy of this document is available at
      https://bugzilla.kernel.org/show_bug.cgi?id=199889

Anyhow, this means that on future AMD CPUs there will be  _two_ ways to
deal with SSBD.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 target/i386/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 52d334a..f91990c 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -490,7 +490,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
             "ibpb", NULL, NULL, NULL,
             NULL, NULL, NULL, NULL,
             NULL, NULL, NULL, NULL,
-            NULL, "virt-ssbd", NULL, NULL,
+            "amd-ssbd", "virt-ssbd", NULL, NULL,
             NULL, NULL, NULL, NULL,
         },
         .cpuid_eax = 0x80000008,
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/2] i386: Define AMD's no SSB mitigation needed.

[Konrad Rzeszutek Wilk]

AMD future CPUs expose a mechanism to tell the guest that the
Speculative Store Bypass Disable is not needed and that the
CPU is all good.

This is exposed via the CPUID 8000_0008.EBX[26] bit.

See 124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf

A copy of this document is available at
        https://bugzilla.kernel.org/show_bug.cgi?id=199889

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 target/i386/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index f91990c..43b1532 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -490,7 +490,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
             "ibpb", NULL, NULL, NULL,
             NULL, NULL, NULL, NULL,
             NULL, NULL, NULL, NULL,
-            "amd-ssbd", "virt-ssbd", NULL, NULL,
+            "amd-ssbd", "virt-ssbd", "amd-no-ssb", NULL,
             NULL, NULL, NULL, NULL,
         },
         .cpuid_eax = 0x80000008,
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 1/2] i386: define the AMD 'amd-ssbd' CPUID feature bit

[Daniel P. Berrangé]

On Fri, Jun 01, 2018 at 11:38:08AM -0400, Konrad Rzeszutek Wilk wrote:
> AMD future CPUs expose _two_ ways to utilize the Intel equivalant
> of the Speculative Store Bypass Disable. The first is via
> the virtualized VIRT_SPEC CTRL MSR (0xC001_011f) and the second
> is via the SPEC_CTRL MSR (0x48). The document titled:
> 124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf
> 
> gives priority of SPEC CTRL MSR over the VIRT SPEC CTRL MSR.
> 
> A copy of this document is available at
>       https://bugzilla.kernel.org/show_bug.cgi?id=199889
> 
> Anyhow, this means that on future AMD CPUs there will be  _two_ ways to
> deal with SSBD.

Oh what fun ;-)

Unless I'm mistaken the current Linux kernel doesn't know about these
new amd-ssbd / amd-no-ssb flags either. Will you also be sending patches
for that half of the problem ?

> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  target/i386/cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 52d334a..f91990c 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -490,7 +490,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>              "ibpb", NULL, NULL, NULL,
>              NULL, NULL, NULL, NULL,
>              NULL, NULL, NULL, NULL,
> -            NULL, "virt-ssbd", NULL, NULL,
> +            "amd-ssbd", "virt-ssbd", NULL, NULL,
>              NULL, NULL, NULL, NULL,
>          },
>          .cpuid_eax = 0x80000008,
> -- 
> 1.8.3.1
> 
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 1/2] i386: define the AMD 'amd-ssbd' CPUID feature bit

[Eduardo Habkost]

On Fri, Jun 01, 2018 at 11:38:08AM -0400, Konrad Rzeszutek Wilk wrote:
> AMD future CPUs expose _two_ ways to utilize the Intel equivalant
> of the Speculative Store Bypass Disable. The first is via
> the virtualized VIRT_SPEC CTRL MSR (0xC001_011f) and the second
> is via the SPEC_CTRL MSR (0x48). The document titled:
> 124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf
> 
> gives priority of SPEC CTRL MSR over the VIRT SPEC CTRL MSR.
> 
> A copy of this document is available at
>       https://bugzilla.kernel.org/show_bug.cgi?id=199889
> 
> Anyhow, this means that on future AMD CPUs there will be  _two_ ways to
> deal with SSBD.

Does anybody know if there are AMD CPUs where virt-ssbd won't
work and would require amd-ssbd to mitigate vulnerabilities?

Also, do we have kernel arch/x86/kvm/cpuid.c patches, already?
I prefer to add new CPUID flag names only after the flag name is
already agreed upon on the kernel side.


> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  target/i386/cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 52d334a..f91990c 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -490,7 +490,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>              "ibpb", NULL, NULL, NULL,
>              NULL, NULL, NULL, NULL,
>              NULL, NULL, NULL, NULL,
> -            NULL, "virt-ssbd", NULL, NULL,
> +            "amd-ssbd", "virt-ssbd", NULL, NULL,
>              NULL, NULL, NULL, NULL,
>          },
>          .cpuid_eax = 0x80000008,
> -- 
> 1.8.3.1
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 1/2] i386: define the AMD 'amd-ssbd' CPUID feature bit

[Konrad Rzeszutek Wilk]

On Mon, Jun 04, 2018 at 09:54:40AM +0100, Daniel P. Berrangé wrote:
> On Fri, Jun 01, 2018 at 11:38:08AM -0400, Konrad Rzeszutek Wilk wrote:
> > AMD future CPUs expose _two_ ways to utilize the Intel equivalant
> > of the Speculative Store Bypass Disable. The first is via
> > the virtualized VIRT_SPEC CTRL MSR (0xC001_011f) and the second
> > is via the SPEC_CTRL MSR (0x48). The document titled:
> > 124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf
> > 
> > gives priority of SPEC CTRL MSR over the VIRT SPEC CTRL MSR.
> > 
> > A copy of this document is available at
> >       https://bugzilla.kernel.org/show_bug.cgi?id=199889
> > 
> > Anyhow, this means that on future AMD CPUs there will be  _two_ ways to
> > deal with SSBD.
> 
> Oh what fun ;-)
> 
> Unless I'm mistaken the current Linux kernel doesn't know about these
> new amd-ssbd / amd-no-ssb flags either. Will you also be sending patches
> for that half of the problem ?

I sent them as well. But forgot to CC qemu-devel :-(

Re: [Qemu-devel] [PATCH 1/2] i386: define the AMD 'amd-ssbd' CPUID feature bit

[Konrad Rzeszutek Wilk]

On Mon, Jun 04, 2018 at 05:07:01PM -0300, Eduardo Habkost wrote:
> On Fri, Jun 01, 2018 at 11:38:08AM -0400, Konrad Rzeszutek Wilk wrote:
> > AMD future CPUs expose _two_ ways to utilize the Intel equivalant
> > of the Speculative Store Bypass Disable. The first is via
> > the virtualized VIRT_SPEC CTRL MSR (0xC001_011f) and the second
> > is via the SPEC_CTRL MSR (0x48). The document titled:
> > 124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf
> > 
> > gives priority of SPEC CTRL MSR over the VIRT SPEC CTRL MSR.
> > 
> > A copy of this document is available at
> >       https://bugzilla.kernel.org/show_bug.cgi?id=199889
> > 
> > Anyhow, this means that on future AMD CPUs there will be  _two_ ways to
> > deal with SSBD.
> 
> Does anybody know if there are AMD CPUs where virt-ssbd won't
> work and would require amd-ssbd to mitigate vulnerabilities?
> 
> Also, do we have kernel arch/x86/kvm/cpuid.c patches, already?

Not yet. They are being discussed right now. I figured I would send
these patches out as a 'Hey, coming at you!', but failed to change
the title to be 'RFC'.

> I prefer to add new CPUID flag names only after the flag name is
> already agreed upon on the kernel side.

Of course. I will respin once that discussion has calmed down.

> 
> 
> > 
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > ---
> >  target/i386/cpu.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 52d334a..f91990c 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -490,7 +490,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> >              "ibpb", NULL, NULL, NULL,
> >              NULL, NULL, NULL, NULL,
> >              NULL, NULL, NULL, NULL,
> > -            NULL, "virt-ssbd", NULL, NULL,
> > +            "amd-ssbd", "virt-ssbd", NULL, NULL,
> >              NULL, NULL, NULL, NULL,
> >          },
> >          .cpuid_eax = 0x80000008,
> > -- 
> > 1.8.3.1
> > 
> > 
> 
> -- 
> Eduardo

Re: [Qemu-devel] [PATCH 1/2] i386: define the AMD 'amd-ssbd' CPUID feature bit

[Eduardo Habkost]

On Mon, Jun 04, 2018 at 04:22:05PM -0400, Konrad Rzeszutek Wilk wrote:
> On Mon, Jun 04, 2018 at 05:07:01PM -0300, Eduardo Habkost wrote:
> > On Fri, Jun 01, 2018 at 11:38:08AM -0400, Konrad Rzeszutek Wilk wrote:
> > > AMD future CPUs expose _two_ ways to utilize the Intel equivalant
> > > of the Speculative Store Bypass Disable. The first is via
> > > the virtualized VIRT_SPEC CTRL MSR (0xC001_011f) and the second
> > > is via the SPEC_CTRL MSR (0x48). The document titled:
> > > 124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf
> > > 
> > > gives priority of SPEC CTRL MSR over the VIRT SPEC CTRL MSR.
> > > 
> > > A copy of this document is available at
> > >       https://bugzilla.kernel.org/show_bug.cgi?id=199889
> > > 
> > > Anyhow, this means that on future AMD CPUs there will be  _two_ ways to
> > > deal with SSBD.
> > 
> > Does anybody know if there are AMD CPUs where virt-ssbd won't
> > work and would require amd-ssbd to mitigate vulnerabilities?
> > 
> > Also, do we have kernel arch/x86/kvm/cpuid.c patches, already?
> 
> Not yet. They are being discussed right now. I figured I would send
> these patches out as a 'Hey, coming at you!', but failed to change
> the title to be 'RFC'.

OK.  I was queueing them on x86-next, but I'm going drop them by
now.


> 
> > I prefer to add new CPUID flag names only after the flag name is
> > already agreed upon on the kernel side.
> 
> Of course. I will respin once that discussion has calmed down.

Thanks!

BTW, it looks like the patch on LKML[1] will make bit 26 appear
on /proc/cpuinfo as "amd_ssb_no", is that correct?  If that's the
case, I'd prefer to make the QEMU flag to match the name used by
Linux, and be called "amd-ssb-no" (which sounds weird to me, but
at least it will be consistent with /proc/cpuinfo).

[1] https://patchwork.kernel.org/patch/10443689/

-- 
Eduardo

Re: [Qemu-devel] [PATCH 1/2] i386: define the AMD 'amd-ssbd' CPUID feature bit

[Tom Lendacky]

On 6/4/2018 3:07 PM, Eduardo Habkost wrote:
> On Fri, Jun 01, 2018 at 11:38:08AM -0400, Konrad Rzeszutek Wilk wrote:
>> AMD future CPUs expose _two_ ways to utilize the Intel equivalant
>> of the Speculative Store Bypass Disable. The first is via
>> the virtualized VIRT_SPEC CTRL MSR (0xC001_011f) and the second
>> is via the SPEC_CTRL MSR (0x48). The document titled:
>> 124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf
>>
>> gives priority of SPEC CTRL MSR over the VIRT SPEC CTRL MSR.
>>
>> A copy of this document is available at
>>       https://bugzilla.kernel.org/show_bug.cgi?id=199889
>>
>> Anyhow, this means that on future AMD CPUs there will be  _two_ ways to
>> deal with SSBD.
> 
> Does anybody know if there are AMD CPUs where virt-ssbd won't
> work and would require amd-ssbd to mitigate vulnerabilities?

The idea behind virt-ssbd was to provide an architectural method for
a guest to do SSBD when amd-ssbd isn't present.  The amd-ssbd feature
will use SPEC_CTRL which is intended to not be intercepted and
will be fast.  The use of virt-ssbd will always be intercepted and
therefore will not be as fast.  So a guest should be presented with
amd-ssbd, if available, in preference to virt-ssbd.

Thanks,
Tom

> 
> Also, do we have kernel arch/x86/kvm/cpuid.c patches, already?
> I prefer to add new CPUID flag names only after the flag name is
> already agreed upon on the kernel side.
> 
> 
>>
>> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> ---
>>  target/i386/cpu.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index 52d334a..f91990c 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -490,7 +490,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>>              "ibpb", NULL, NULL, NULL,
>>              NULL, NULL, NULL, NULL,
>>              NULL, NULL, NULL, NULL,
>> -            NULL, "virt-ssbd", NULL, NULL,
>> +            "amd-ssbd", "virt-ssbd", NULL, NULL,
>>              NULL, NULL, NULL, NULL,
>>          },
>>          .cpuid_eax = 0x80000008,
>> -- 
>> 1.8.3.1
>>
>>
> 

Re: [Qemu-devel] [PATCH 1/2] i386: define the AMD 'amd-ssbd' CPUID feature bit

[Daniel P. Berrangé]

On Tue, Jun 05, 2018 at 08:31:41AM -0500, Tom Lendacky wrote:
> On 6/4/2018 3:07 PM, Eduardo Habkost wrote:
> > On Fri, Jun 01, 2018 at 11:38:08AM -0400, Konrad Rzeszutek Wilk wrote:
> >> AMD future CPUs expose _two_ ways to utilize the Intel equivalant
> >> of the Speculative Store Bypass Disable. The first is via
> >> the virtualized VIRT_SPEC CTRL MSR (0xC001_011f) and the second
> >> is via the SPEC_CTRL MSR (0x48). The document titled:
> >> 124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf
> >>
> >> gives priority of SPEC CTRL MSR over the VIRT SPEC CTRL MSR.
> >>
> >> A copy of this document is available at
> >>       https://bugzilla.kernel.org/show_bug.cgi?id=199889
> >>
> >> Anyhow, this means that on future AMD CPUs there will be  _two_ ways to
> >> deal with SSBD.
> > 
> > Does anybody know if there are AMD CPUs where virt-ssbd won't
> > work and would require amd-ssbd to mitigate vulnerabilities?
> 
> The idea behind virt-ssbd was to provide an architectural method for
> a guest to do SSBD when amd-ssbd isn't present.  The amd-ssbd feature
> will use SPEC_CTRL which is intended to not be intercepted and
> will be fast.  The use of virt-ssbd will always be intercepted and
> therefore will not be as fast.  So a guest should be presented with
> amd-ssbd, if available, in preference to virt-ssbd.

Thanks, that's useful info.

Can you say whether amd-ssbd is going to become available for existing
CPUs via microcode updates, or will it only be present in future CPUs ?

> > Also, do we have kernel arch/x86/kvm/cpuid.c patches, already?
> > I prefer to add new CPUID flag names only after the flag name is
> > already agreed upon on the kernel side.
> > 
> > 
> >>
> >> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >> ---
> >>  target/i386/cpu.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> >> index 52d334a..f91990c 100644
> >> --- a/target/i386/cpu.c
> >> +++ b/target/i386/cpu.c
> >> @@ -490,7 +490,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> >>              "ibpb", NULL, NULL, NULL,
> >>              NULL, NULL, NULL, NULL,
> >>              NULL, NULL, NULL, NULL,
> >> -            NULL, "virt-ssbd", NULL, NULL,
> >> +            "amd-ssbd", "virt-ssbd", NULL, NULL,
> >>              NULL, NULL, NULL, NULL,
> >>          },
> >>          .cpuid_eax = 0x80000008,

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 1/2] i386: define the AMD 'amd-ssbd' CPUID feature bit

[Konrad Rzeszutek Wilk]

On Mon, Jun 04, 2018 at 06:15:09PM -0300, Eduardo Habkost wrote:
> On Mon, Jun 04, 2018 at 04:22:05PM -0400, Konrad Rzeszutek Wilk wrote:
> > On Mon, Jun 04, 2018 at 05:07:01PM -0300, Eduardo Habkost wrote:
> > > On Fri, Jun 01, 2018 at 11:38:08AM -0400, Konrad Rzeszutek Wilk wrote:
> > > > AMD future CPUs expose _two_ ways to utilize the Intel equivalant
> > > > of the Speculative Store Bypass Disable. The first is via
> > > > the virtualized VIRT_SPEC CTRL MSR (0xC001_011f) and the second
> > > > is via the SPEC_CTRL MSR (0x48). The document titled:
> > > > 124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf
> > > > 
> > > > gives priority of SPEC CTRL MSR over the VIRT SPEC CTRL MSR.
> > > > 
> > > > A copy of this document is available at
> > > >       https://bugzilla.kernel.org/show_bug.cgi?id=199889
> > > > 
> > > > Anyhow, this means that on future AMD CPUs there will be  _two_ ways to
> > > > deal with SSBD.
> > > 
> > > Does anybody know if there are AMD CPUs where virt-ssbd won't
> > > work and would require amd-ssbd to mitigate vulnerabilities?
> > > 
> > > Also, do we have kernel arch/x86/kvm/cpuid.c patches, already?
> > 
> > Not yet. They are being discussed right now. I figured I would send
> > these patches out as a 'Hey, coming at you!', but failed to change
> > the title to be 'RFC'.
> 
> OK.  I was queueing them on x86-next, but I'm going drop them by
> now.
> 
> 
> > 
> > > I prefer to add new CPUID flag names only after the flag name is
> > > already agreed upon on the kernel side.
> > 
> > Of course. I will respin once that discussion has calmed down.
> 
> Thanks!
> 
> BTW, it looks like the patch on LKML[1] will make bit 26 appear
> on /proc/cpuinfo as "amd_ssb_no", is that correct?  If that's the
> case, I'd prefer to make the QEMU flag to match the name used by
> Linux, and be called "amd-ssb-no" (which sounds weird to me, but
> at least it will be consistent with /proc/cpuinfo).

The "" in the comment section makes sure to hide it. That is only
CPU features without the "" are exposed in /proc/cpuinfo

You got me worried there for a minute :-)
> 
> [1] https://patchwork.kernel.org/patch/10443689/
> 
> -- 
> Eduardo

Re: [Qemu-devel] [PATCH 1/2] i386: define the AMD 'amd-ssbd' CPUID feature bit

[Daniel P. Berrangé]

On Tue, Jun 05, 2018 at 08:31:41AM -0500, Tom Lendacky wrote:
> On 6/4/2018 3:07 PM, Eduardo Habkost wrote:
> > On Fri, Jun 01, 2018 at 11:38:08AM -0400, Konrad Rzeszutek Wilk wrote:
> >> AMD future CPUs expose _two_ ways to utilize the Intel equivalant
> >> of the Speculative Store Bypass Disable. The first is via
> >> the virtualized VIRT_SPEC CTRL MSR (0xC001_011f) and the second
> >> is via the SPEC_CTRL MSR (0x48). The document titled:
> >> 124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf
> >>
> >> gives priority of SPEC CTRL MSR over the VIRT SPEC CTRL MSR.
> >>
> >> A copy of this document is available at
> >>       https://bugzilla.kernel.org/show_bug.cgi?id=199889
> >>
> >> Anyhow, this means that on future AMD CPUs there will be  _two_ ways to
> >> deal with SSBD.
> > 
> > Does anybody know if there are AMD CPUs where virt-ssbd won't
> > work and would require amd-ssbd to mitigate vulnerabilities?
> 
> The idea behind virt-ssbd was to provide an architectural method for
> a guest to do SSBD when amd-ssbd isn't present.  The amd-ssbd feature
> will use SPEC_CTRL which is intended to not be intercepted and
> will be fast.  The use of virt-ssbd will always be intercepted and
> therefore will not be as fast.  So a guest should be presented with
> amd-ssbd, if available, in preference to virt-ssbd.

Can you clarify whether 'amd-ssbd' is also an architectural method
or not ?  ie is it safe to use 'amd-ssbd' in a guest which can be
live migrated between different generations/families of AMD CPU,
or must be use virt-ssbd in that case ?


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 1/2] i386: define the AMD 'amd-ssbd' CPUID feature bit

[Tom Lendacky]

On 6/6/2018 9:20 AM, Daniel P. Berrangé wrote:
> On Tue, Jun 05, 2018 at 08:31:41AM -0500, Tom Lendacky wrote:
>> On 6/4/2018 3:07 PM, Eduardo Habkost wrote:
>>> On Fri, Jun 01, 2018 at 11:38:08AM -0400, Konrad Rzeszutek Wilk wrote:
>>>> AMD future CPUs expose _two_ ways to utilize the Intel equivalant
>>>> of the Speculative Store Bypass Disable. The first is via
>>>> the virtualized VIRT_SPEC CTRL MSR (0xC001_011f) and the second
>>>> is via the SPEC_CTRL MSR (0x48). The document titled:
>>>> 124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf
>>>>
>>>> gives priority of SPEC CTRL MSR over the VIRT SPEC CTRL MSR.
>>>>
>>>> A copy of this document is available at
>>>>       https://bugzilla.kernel.org/show_bug.cgi?id=199889
>>>>
>>>> Anyhow, this means that on future AMD CPUs there will be  _two_ ways to
>>>> deal with SSBD.
>>>
>>> Does anybody know if there are AMD CPUs where virt-ssbd won't
>>> work and would require amd-ssbd to mitigate vulnerabilities?
>>
>> The idea behind virt-ssbd was to provide an architectural method for
>> a guest to do SSBD when amd-ssbd isn't present.  The amd-ssbd feature
>> will use SPEC_CTRL which is intended to not be intercepted and
>> will be fast.  The use of virt-ssbd will always be intercepted and
>> therefore will not be as fast.  So a guest should be presented with
>> amd-ssbd, if available, in preference to virt-ssbd.
> 
> Can you clarify whether 'amd-ssbd' is also an architectural method

Yes, amd-ssbd is architectural - it is a defined CPUID bit.

Thanks,
Tom

> or not ?  ie is it safe to use 'amd-ssbd' in a guest which can be
> live migrated between different generations/families of AMD CPU,
> or must be use virt-ssbd in that case ?
> 
> 
> Regards,
> Daniel
> 

Re: [Qemu-devel] [PATCH 1/2] i386: define the AMD 'amd-ssbd' CPUID feature bit

[Daniel P. Berrangé]

On Mon, Jun 04, 2018 at 04:22:05PM -0400, Konrad Rzeszutek Wilk wrote:
> On Mon, Jun 04, 2018 at 05:07:01PM -0300, Eduardo Habkost wrote:
> > On Fri, Jun 01, 2018 at 11:38:08AM -0400, Konrad Rzeszutek Wilk wrote:
> > > AMD future CPUs expose _two_ ways to utilize the Intel equivalant
> > > of the Speculative Store Bypass Disable. The first is via
> > > the virtualized VIRT_SPEC CTRL MSR (0xC001_011f) and the second
> > > is via the SPEC_CTRL MSR (0x48). The document titled:
> > > 124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf
> > > 
> > > gives priority of SPEC CTRL MSR over the VIRT SPEC CTRL MSR.
> > > 
> > > A copy of this document is available at
> > >       https://bugzilla.kernel.org/show_bug.cgi?id=199889
> > > 
> > > Anyhow, this means that on future AMD CPUs there will be  _two_ ways to
> > > deal with SSBD.
> > 
> > Does anybody know if there are AMD CPUs where virt-ssbd won't
> > work and would require amd-ssbd to mitigate vulnerabilities?
> > 
> > Also, do we have kernel arch/x86/kvm/cpuid.c patches, already?
> 
> Not yet. They are being discussed right now. I figured I would send
> these patches out as a 'Hey, coming at you!', but failed to change
> the title to be 'RFC'.
> 
> > I prefer to add new CPUID flag names only after the flag name is
> > already agreed upon on the kernel side.
> 
> Of course. I will respin once that discussion has calmed down.

Looks like the kernel side has merged now, and we'll need to rename
the 2nd CPU bit from what I see.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 1/2] i386: define the AMD 'amd-ssbd' CPUID feature bit

[Konrad Rzeszutek Wilk]

On Wed, Jun 13, 2018 at 11:19:49AM +0100, Daniel P. Berrangé wrote:
> On Mon, Jun 04, 2018 at 04:22:05PM -0400, Konrad Rzeszutek Wilk wrote:
> > On Mon, Jun 04, 2018 at 05:07:01PM -0300, Eduardo Habkost wrote:
> > > On Fri, Jun 01, 2018 at 11:38:08AM -0400, Konrad Rzeszutek Wilk wrote:
> > > > AMD future CPUs expose _two_ ways to utilize the Intel equivalant
> > > > of the Speculative Store Bypass Disable. The first is via
> > > > the virtualized VIRT_SPEC CTRL MSR (0xC001_011f) and the second
> > > > is via the SPEC_CTRL MSR (0x48). The document titled:
> > > > 124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf
> > > > 
> > > > gives priority of SPEC CTRL MSR over the VIRT SPEC CTRL MSR.
> > > > 
> > > > A copy of this document is available at
> > > >       https://bugzilla.kernel.org/show_bug.cgi?id=199889
> > > > 
> > > > Anyhow, this means that on future AMD CPUs there will be  _two_ ways to
> > > > deal with SSBD.
> > > 
> > > Does anybody know if there are AMD CPUs where virt-ssbd won't
> > > work and would require amd-ssbd to mitigate vulnerabilities?
> > > 
> > > Also, do we have kernel arch/x86/kvm/cpuid.c patches, already?
> > 
> > Not yet. They are being discussed right now. I figured I would send
> > these patches out as a 'Hey, coming at you!', but failed to change
> > the title to be 'RFC'.
> > 
> > > I prefer to add new CPUID flag names only after the flag name is
> > > already agreed upon on the kernel side.
> > 
> > Of course. I will respin once that discussion has calmed down.
> 
> Looks like the kernel side has merged now, and we'll need to rename
> the 2nd CPU bit from what I see.

What name did you have in mind?
> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
> 

Re: [Qemu-devel] [PATCH 1/2] i386: define the AMD 'amd-ssbd' CPUID feature bit

[Daniel P. Berrangé]

On Wed, Jun 13, 2018 at 12:09:59PM -0400, Konrad Rzeszutek Wilk wrote:
> On Wed, Jun 13, 2018 at 11:19:49AM +0100, Daniel P. Berrangé wrote:
> > On Mon, Jun 04, 2018 at 04:22:05PM -0400, Konrad Rzeszutek Wilk wrote:
> > > On Mon, Jun 04, 2018 at 05:07:01PM -0300, Eduardo Habkost wrote:
> > > > On Fri, Jun 01, 2018 at 11:38:08AM -0400, Konrad Rzeszutek Wilk wrote:
> > > > > AMD future CPUs expose _two_ ways to utilize the Intel equivalant
> > > > > of the Speculative Store Bypass Disable. The first is via
> > > > > the virtualized VIRT_SPEC CTRL MSR (0xC001_011f) and the second
> > > > > is via the SPEC_CTRL MSR (0x48). The document titled:
> > > > > 124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf
> > > > > 
> > > > > gives priority of SPEC CTRL MSR over the VIRT SPEC CTRL MSR.
> > > > > 
> > > > > A copy of this document is available at
> > > > >       https://bugzilla.kernel.org/show_bug.cgi?id=199889
> > > > > 
> > > > > Anyhow, this means that on future AMD CPUs there will be  _two_ ways to
> > > > > deal with SSBD.
> > > > 
> > > > Does anybody know if there are AMD CPUs where virt-ssbd won't
> > > > work and would require amd-ssbd to mitigate vulnerabilities?
> > > > 
> > > > Also, do we have kernel arch/x86/kvm/cpuid.c patches, already?
> > > 
> > > Not yet. They are being discussed right now. I figured I would send
> > > these patches out as a 'Hey, coming at you!', but failed to change
> > > the title to be 'RFC'.
> > > 
> > > > I prefer to add new CPUID flag names only after the flag name is
> > > > already agreed upon on the kernel side.
> > > 
> > > Of course. I will respin once that discussion has calmed down.
> > 
> > Looks like the kernel side has merged now, and we'll need to rename
> > the 2nd CPU bit from what I see.
> 
> What name did you have in mind?

IIUC from the kernel patches, it will be reported as 'amd-ssbd' and
'amd-ssb-no' in /proc/cpuinfo, so only your second patch needs a simple
tweak to match that naming.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 1/2] i386: define the AMD 'amd-ssbd' CPUID feature bit

[Konrad Rzeszutek Wilk]

On Wed, Jun 13, 2018 at 05:21:29PM +0100, Daniel P. Berrangé wrote:
> On Wed, Jun 13, 2018 at 12:09:59PM -0400, Konrad Rzeszutek Wilk wrote:
> > On Wed, Jun 13, 2018 at 11:19:49AM +0100, Daniel P. Berrangé wrote:
> > > On Mon, Jun 04, 2018 at 04:22:05PM -0400, Konrad Rzeszutek Wilk wrote:
> > > > On Mon, Jun 04, 2018 at 05:07:01PM -0300, Eduardo Habkost wrote:
> > > > > On Fri, Jun 01, 2018 at 11:38:08AM -0400, Konrad Rzeszutek Wilk wrote:
> > > > > > AMD future CPUs expose _two_ ways to utilize the Intel equivalant
> > > > > > of the Speculative Store Bypass Disable. The first is via
> > > > > > the virtualized VIRT_SPEC CTRL MSR (0xC001_011f) and the second
> > > > > > is via the SPEC_CTRL MSR (0x48). The document titled:
> > > > > > 124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf
> > > > > > 
> > > > > > gives priority of SPEC CTRL MSR over the VIRT SPEC CTRL MSR.
> > > > > > 
> > > > > > A copy of this document is available at
> > > > > >       https://bugzilla.kernel.org/show_bug.cgi?id=199889
> > > > > > 
> > > > > > Anyhow, this means that on future AMD CPUs there will be  _two_ ways to
> > > > > > deal with SSBD.
> > > > > 
> > > > > Does anybody know if there are AMD CPUs where virt-ssbd won't
> > > > > work and would require amd-ssbd to mitigate vulnerabilities?
> > > > > 
> > > > > Also, do we have kernel arch/x86/kvm/cpuid.c patches, already?
> > > > 
> > > > Not yet. They are being discussed right now. I figured I would send
> > > > these patches out as a 'Hey, coming at you!', but failed to change
> > > > the title to be 'RFC'.
> > > > 
> > > > > I prefer to add new CPUID flag names only after the flag name is
> > > > > already agreed upon on the kernel side.
> > > > 
> > > > Of course. I will respin once that discussion has calmed down.
> > > 
> > > Looks like the kernel side has merged now, and we'll need to rename
> > > the 2nd CPU bit from what I see.
> > 
> > What name did you have in mind?
> 
> IIUC from the kernel patches, it will be reported as 'amd-ssbd' and
> 'amd-ssb-no' in /proc/cpuinfo, so only your second patch needs a simple
> tweak to match that naming.

It will only report 'ssbd' but not 'amd-ssb-no' nor 'amd-ssbd'.

If the cpufeature.h has "" in the comment section then it is hidden. That is:

#define X86_FEATURE_MSR_SPEC_CTRL       ( 7*32+16) /* "" MSR SPEC_CTRL is implemented */
..sniup..
+#define X86_FEATURE_AMD_SSBD           (13*32+24) /* "" Speculative Store Bypass Disable */
+#define X86_FEATURE_AMD_SSB_NO         (13*32+26) /* "" Speculative Store Bypass is fixed in hardware. */

are hidden ones, while:
#define X86_FEATURE_SSBD                ( 7*32+17) /* Speculative Store Bypass Disable */

is visible.

The code that finds the AMD_SSBD and sets the 'ssbd' is:

+       if (cpu_has(c, X86_FEATURE_AMD_SSBD)) {
+               set_cpu_cap(c, X86_FEATURE_SSBD);
+               set_cpu_cap(c, X86_FEATURE_MSR_SPEC_CTRL);
+               clear_cpu_cap(c, X86_FEATURE_VIRT_SSBD);
+       }

Meaning the 'ssbd' will show up in /proc/cpuinfo 


> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 1/2] i386: define the AMD 'amd-ssbd' CPUID feature bit

[Daniel P. Berrangé]

On Wed, Jun 13, 2018 at 12:34:21PM -0400, Konrad Rzeszutek Wilk wrote:
> On Wed, Jun 13, 2018 at 05:21:29PM +0100, Daniel P. Berrangé wrote:
> > On Wed, Jun 13, 2018 at 12:09:59PM -0400, Konrad Rzeszutek Wilk wrote:
> > > On Wed, Jun 13, 2018 at 11:19:49AM +0100, Daniel P. Berrangé wrote:
> > > > On Mon, Jun 04, 2018 at 04:22:05PM -0400, Konrad Rzeszutek Wilk wrote:
> > > > > On Mon, Jun 04, 2018 at 05:07:01PM -0300, Eduardo Habkost wrote:
> > > > > > On Fri, Jun 01, 2018 at 11:38:08AM -0400, Konrad Rzeszutek Wilk wrote:
> > > > > > > AMD future CPUs expose _two_ ways to utilize the Intel equivalant
> > > > > > > of the Speculative Store Bypass Disable. The first is via
> > > > > > > the virtualized VIRT_SPEC CTRL MSR (0xC001_011f) and the second
> > > > > > > is via the SPEC_CTRL MSR (0x48). The document titled:
> > > > > > > 124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf
> > > > > > > 
> > > > > > > gives priority of SPEC CTRL MSR over the VIRT SPEC CTRL MSR.
> > > > > > > 
> > > > > > > A copy of this document is available at
> > > > > > >       https://bugzilla.kernel.org/show_bug.cgi?id=199889
> > > > > > > 
> > > > > > > Anyhow, this means that on future AMD CPUs there will be  _two_ ways to
> > > > > > > deal with SSBD.
> > > > > > 
> > > > > > Does anybody know if there are AMD CPUs where virt-ssbd won't
> > > > > > work and would require amd-ssbd to mitigate vulnerabilities?
> > > > > > 
> > > > > > Also, do we have kernel arch/x86/kvm/cpuid.c patches, already?
> > > > > 
> > > > > Not yet. They are being discussed right now. I figured I would send
> > > > > these patches out as a 'Hey, coming at you!', but failed to change
> > > > > the title to be 'RFC'.
> > > > > 
> > > > > > I prefer to add new CPUID flag names only after the flag name is
> > > > > > already agreed upon on the kernel side.
> > > > > 
> > > > > Of course. I will respin once that discussion has calmed down.
> > > > 
> > > > Looks like the kernel side has merged now, and we'll need to rename
> > > > the 2nd CPU bit from what I see.
> > > 
> > > What name did you have in mind?
> > 
> > IIUC from the kernel patches, it will be reported as 'amd-ssbd' and
> > 'amd-ssb-no' in /proc/cpuinfo, so only your second patch needs a simple
> > tweak to match that naming.
> 
> It will only report 'ssbd' but not 'amd-ssb-no' nor 'amd-ssbd'.
> 
> If the cpufeature.h has "" in the comment section then it is hidden. That is:
> 
> #define X86_FEATURE_MSR_SPEC_CTRL       ( 7*32+16) /* "" MSR SPEC_CTRL is implemented */
> ..sniup..
> +#define X86_FEATURE_AMD_SSBD           (13*32+24) /* "" Speculative Store Bypass Disable */
> +#define X86_FEATURE_AMD_SSB_NO         (13*32+26) /* "" Speculative Store Bypass is fixed in hardware. */
> 
> are hidden ones, while:
> #define X86_FEATURE_SSBD                ( 7*32+17) /* Speculative Store Bypass Disable */
> 
> is visible.

Ah, thanks for explaining that ! 


> The code that finds the AMD_SSBD and sets the 'ssbd' is:
> 
> +       if (cpu_has(c, X86_FEATURE_AMD_SSBD)) {
> +               set_cpu_cap(c, X86_FEATURE_SSBD);
> +               set_cpu_cap(c, X86_FEATURE_MSR_SPEC_CTRL);
> +               clear_cpu_cap(c, X86_FEATURE_VIRT_SSBD);
> +       }
> 
> Meaning the 'ssbd' will show up in /proc/cpuinfo

Given that, there's no exposed kernel naming we need to align with.

So personally I'd be fine with the current patches that exist, but
I'll defer to Eduardo for the final say, wrt amd-ssb-no vs amd-no-ssb.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 1/2] i386: define the AMD 'amd-ssbd' CPUID feature bit

[Eduardo Habkost]

On Wed, Jun 13, 2018 at 05:39:50PM +0100, Daniel P. Berrangé wrote:
[...]
> > The code that finds the AMD_SSBD and sets the 'ssbd' is:
> > 
> > +       if (cpu_has(c, X86_FEATURE_AMD_SSBD)) {
> > +               set_cpu_cap(c, X86_FEATURE_SSBD);
> > +               set_cpu_cap(c, X86_FEATURE_MSR_SPEC_CTRL);
> > +               clear_cpu_cap(c, X86_FEATURE_VIRT_SSBD);
> > +       }
> > 
> > Meaning the 'ssbd' will show up in /proc/cpuinfo
> 
> Given that, there's no exposed kernel naming we need to align with.
> 
> So personally I'd be fine with the current patches that exist, but
> I'll defer to Eduardo for the final say, wrt amd-ssb-no vs amd-no-ssb.

I prefer amd-no-ssb, so I plan to apply these patches as is.

-- 
Eduardo

Re: [Qemu-devel] [PATCH QEMU] Patches for new AMD CPU bits.

[Eduardo Habkost]

On Fri, Jun 01, 2018 at 11:38:07AM -0400, Konrad Rzeszutek Wilk wrote:
> Hi!
> 
> 
> I was reading the AMD whitepaper on SSBD and noticed that they have added
> two new bits in the 8000_0008 CPUID. EBX:
>  1) Bit[26] - similar to Intel's SSB_NO not needed anymore.
>  2) Bit[24] - use SPEC_CTRL MSR (0x48) instead of VIRT SPEC_CTRL MSR
>     (0xC001_011f).
> 
> See 124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf
> A copy of this document is available at
>         https://bugzilla.kernel.org/show_bug.cgi?id=199889
> 
> These two patches along with the kernel ones allow us to expose those
> two bits to the guest.

Queued on x86-next, thanks!

-- 
Eduardo