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

2018-06-13 Thread 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 1/2] i386: define the AMD 'amd-ssbd' CPUID feature bit

2018-06-13 Thread 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

2018-06-13 Thread 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

2018-06-13 Thread 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

2018-06-13 Thread 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

2018-06-13 Thread 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

2018-06-08 Thread 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

2018-06-06 Thread 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

2018-06-05 Thread 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

2018-06-05 Thread 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 
> >> ---
> >>  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 = 0x8008,

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

2018-06-05 Thread 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 
>> ---
>>  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 = 0x8008,
>> -- 
>> 1.8.3.1
>>
>>
> 



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

2018-06-04 Thread 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

2018-06-04 Thread 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 
> > ---
> >  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 = 0x8008,
> > -- 
> > 1.8.3.1
> > 
> > 
> 
> -- 
> Eduardo



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

2018-06-04 Thread 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

2018-06-04 Thread 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 
> ---
>  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 = 0x8008,
> -- 
> 1.8.3.1
> 
> 

-- 
Eduardo



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

2018-06-04 Thread 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 
> ---
>  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 = 0x8008,
> -- 
> 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 :|