LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* Re: [PATCH] KVM: x86: Expose Predictive Store Forwarding Disable
       [not found] <163244601049.30292.5855870305350227855.stgit@bmoger-ubuntu>
@ 2021-09-27 10:59 ` Borislav Petkov
  2021-09-27 11:13   ` Paolo Bonzini
  2021-09-28 16:04 ` Paolo Bonzini
  1 sibling, 1 reply; 13+ messages in thread
From: Borislav Petkov @ 2021-09-27 10:59 UTC (permalink / raw)
  To: Babu Moger
  Cc: tglx, mingo, x86, pbonzini, hpa, seanjc, vkuznets, wanpengli,
	jmattson, joro, tony.luck, peterz, kyung.min.park, wei.huang2,
	jgross, andrew.cooper3, linux-kernel, kvm

On Thu, Sep 23, 2021 at 08:15:28PM -0500, Babu Moger wrote:
> Linux kernel does not have the interface to enable/disable PSFD yet. Plan
> here is to expose the PSFD technology to KVM so that the guest kernel can
> make use of it if they wish to.

Why should the guest kernel expose it if we said that for now we want to
disable it with the SSBD control?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] KVM: x86: Expose Predictive Store Forwarding Disable
  2021-09-27 10:59 ` [PATCH] KVM: x86: Expose Predictive Store Forwarding Disable Borislav Petkov
@ 2021-09-27 11:13   ` Paolo Bonzini
  2021-09-27 12:06     ` Borislav Petkov
  0 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2021-09-27 11:13 UTC (permalink / raw)
  To: Borislav Petkov, Babu Moger
  Cc: tglx, mingo, x86, hpa, seanjc, vkuznets, wanpengli, jmattson,
	joro, tony.luck, peterz, kyung.min.park, wei.huang2, jgross,
	andrew.cooper3, linux-kernel, kvm

On 27/09/21 12:59, Borislav Petkov wrote:
> On Thu, Sep 23, 2021 at 08:15:28PM -0500, Babu Moger wrote:
>> Linux kernel does not have the interface to enable/disable PSFD yet. Plan
>> here is to expose the PSFD technology to KVM so that the guest kernel can
>> make use of it if they wish to.
> 
> Why should the guest kernel expose it if we said that for now we want to
> disable it with the SSBD control?

Because the guest kernel needs to know which MSRs to write when you 
touch the SSBD prctl, so that PSFD is properly disabled *inside the guest*.

Paolo


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] KVM: x86: Expose Predictive Store Forwarding Disable
  2021-09-27 11:13   ` Paolo Bonzini
@ 2021-09-27 12:06     ` Borislav Petkov
  2021-09-27 12:14       ` Paolo Bonzini
  0 siblings, 1 reply; 13+ messages in thread
From: Borislav Petkov @ 2021-09-27 12:06 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Babu Moger, tglx, mingo, x86, hpa, seanjc, vkuznets, wanpengli,
	jmattson, joro, tony.luck, peterz, kyung.min.park, wei.huang2,
	jgross, andrew.cooper3, linux-kernel, kvm

On Mon, Sep 27, 2021 at 01:13:26PM +0200, Paolo Bonzini wrote:
> Because the guest kernel needs to know which MSRs to write when you touch
> the SSBD prctl, so that PSFD is properly disabled *inside the guest*.

It already knows which - the same one which disables SSB. PSF is
disabled *together* with SSB, for now...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] KVM: x86: Expose Predictive Store Forwarding Disable
  2021-09-27 12:06     ` Borislav Petkov
@ 2021-09-27 12:14       ` Paolo Bonzini
  2021-09-27 12:28         ` Borislav Petkov
  0 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2021-09-27 12:14 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Babu Moger, tglx, mingo, x86, hpa, seanjc, vkuznets, wanpengli,
	jmattson, joro, tony.luck, peterz, kyung.min.park, wei.huang2,
	jgross, andrew.cooper3, linux-kernel, kvm

On 27/09/21 14:06, Borislav Petkov wrote:
>> Because the guest kernel needs to know which MSRs to write when you touch
>> the SSBD prctl, so that PSFD is properly disabled*inside the guest*.
> It already knows which - the same one which disables SSB. PSF is
> disabled*together*  with SSB, for now...

Right, not which MSR to write but which value to write.  It doesn't know 
that the PSF disable bit is valid unless the corresponding CPUID bit is set.

Paolo


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] KVM: x86: Expose Predictive Store Forwarding Disable
  2021-09-27 12:14       ` Paolo Bonzini
@ 2021-09-27 12:28         ` Borislav Petkov
  2021-09-27 12:54           ` Paolo Bonzini
  0 siblings, 1 reply; 13+ messages in thread
From: Borislav Petkov @ 2021-09-27 12:28 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Babu Moger, tglx, mingo, x86, hpa, seanjc, vkuznets, wanpengli,
	jmattson, joro, tony.luck, peterz, kyung.min.park, wei.huang2,
	jgross, andrew.cooper3, linux-kernel, kvm

On Mon, Sep 27, 2021 at 02:14:52PM +0200, Paolo Bonzini wrote:
> Right, not which MSR to write but which value to write.  It doesn't know
> that the PSF disable bit is valid unless the corresponding CPUID bit is set.

There's no need for the separate PSF CPUID bit yet. We have decided for
now to not control PSF separately but disable it through SSB. Please
follow this thread:

https://lore.kernel.org/all/20210904172334.lfjyqi4qfzvbxef7@treble/T/#u

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] KVM: x86: Expose Predictive Store Forwarding Disable
  2021-09-27 12:28         ` Borislav Petkov
@ 2021-09-27 12:54           ` Paolo Bonzini
  2021-09-27 13:38             ` Borislav Petkov
  2021-09-27 15:47             ` Babu Moger
  0 siblings, 2 replies; 13+ messages in thread
From: Paolo Bonzini @ 2021-09-27 12:54 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Babu Moger, tglx, mingo, x86, hpa, seanjc, vkuznets, wanpengli,
	jmattson, joro, tony.luck, peterz, kyung.min.park, wei.huang2,
	jgross, andrew.cooper3, linux-kernel, kvm

On 27/09/21 14:28, Borislav Petkov wrote:
> On Mon, Sep 27, 2021 at 02:14:52PM +0200, Paolo Bonzini wrote:
>> Right, not which MSR to write but which value to write.  It doesn't know
>> that the PSF disable bit is valid unless the corresponding CPUID bit is set.
> 
> There's no need for the separate PSF CPUID bit yet. We have decided for
> now to not control PSF separately but disable it through SSB. Please
> follow this thread:

There are other guests than Linux.  This patch is just telling userspace 
that KVM knows what the PSFD bit is.  It is also possible to expose the 
bit in KVM without having any #define in cpufeatures.h or without the 
kernel using it.  For example KVM had been exposing FSGSBASE long before 
Linux supported it.

That said, the patch is incomplete because it should also add the new 
CPUID bit to guest_has_spec_ctrl_msr (what KVM *really* cares about is 
not the individual bits, only whether SPEC_CTRL exists at all).

Paolo


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] KVM: x86: Expose Predictive Store Forwarding Disable
  2021-09-27 12:54           ` Paolo Bonzini
@ 2021-09-27 13:38             ` Borislav Petkov
  2021-09-27 15:47             ` Babu Moger
  1 sibling, 0 replies; 13+ messages in thread
From: Borislav Petkov @ 2021-09-27 13:38 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Babu Moger, tglx, mingo, x86, hpa, seanjc, vkuznets, wanpengli,
	jmattson, joro, tony.luck, peterz, kyung.min.park, wei.huang2,
	jgross, andrew.cooper3, linux-kernel, kvm

On Mon, Sep 27, 2021 at 02:54:08PM +0200, Paolo Bonzini wrote:
> There are other guests than Linux.  This patch is just telling userspace
> that KVM knows what the PSFD bit is.  It is also possible to expose the bit
> in KVM without having any #define in cpufeatures.h

Ok, then there's no need for the cpufeatures.h hunk.

> or without the kernel using it. For example KVM had been exposing
> FSGSBASE long before Linux supported it.

Ok, please do that for now then, if you want to expose it to other
guests. I'm sceptical they will have a use case for it either but I'm
always open to suggestions.

For the same reason as for baremetal, though, I wouldn't do that and do
that solely through the SSBD control but that's your call.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] KVM: x86: Expose Predictive Store Forwarding Disable
  2021-09-27 12:54           ` Paolo Bonzini
  2021-09-27 13:38             ` Borislav Petkov
@ 2021-09-27 15:47             ` Babu Moger
  2021-09-27 15:48               ` Borislav Petkov
  1 sibling, 1 reply; 13+ messages in thread
From: Babu Moger @ 2021-09-27 15:47 UTC (permalink / raw)
  To: Paolo Bonzini, Borislav Petkov
  Cc: tglx, mingo, x86, hpa, seanjc, vkuznets, wanpengli, jmattson,
	joro, tony.luck, peterz, kyung.min.park, wei.huang2, jgross,
	andrew.cooper3, linux-kernel, kvm



On 9/27/21 7:54 AM, Paolo Bonzini wrote:
> On 27/09/21 14:28, Borislav Petkov wrote:
>> On Mon, Sep 27, 2021 at 02:14:52PM +0200, Paolo Bonzini wrote:
>>> Right, not which MSR to write but which value to write.  It doesn't know
>>> that the PSF disable bit is valid unless the corresponding CPUID bit is
>>> set.
>>
>> There's no need for the separate PSF CPUID bit yet. We have decided for
>> now to not control PSF separately but disable it through SSB. Please
>> follow this thread:
> 
> There are other guests than Linux.  This patch is just telling userspace

Yes, That is the reason for this patch.

> that KVM knows what the PSFD bit is.  It is also possible to expose the
> bit in KVM without having any #define in cpufeatures.h or without the
> kernel using it.  For example KVM had been exposing FSGSBASE long before
> Linux supported it.
> 
> That said, the patch is incomplete because it should also add the new
> CPUID bit to guest_has_spec_ctrl_msr (what KVM *really* cares about is not
> the individual bits, only whether SPEC_CTRL exists at all).

Yea, I missed that. Will add it in next revision,
Thanks
Babu

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] KVM: x86: Expose Predictive Store Forwarding Disable
  2021-09-27 15:47             ` Babu Moger
@ 2021-09-27 15:48               ` Borislav Petkov
  0 siblings, 0 replies; 13+ messages in thread
From: Borislav Petkov @ 2021-09-27 15:48 UTC (permalink / raw)
  To: Babu Moger
  Cc: Paolo Bonzini, tglx, mingo, x86, hpa, seanjc, vkuznets,
	wanpengli, jmattson, joro, tony.luck, peterz, kyung.min.park,
	wei.huang2, jgross, andrew.cooper3, linux-kernel, kvm

On Mon, Sep 27, 2021 at 10:47:01AM -0500, Babu Moger wrote:
> > There are other guests than Linux.  This patch is just telling userspace
> 
> Yes, That is the reason for this patch.

And can you share, per chance, what their use case is?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] KVM: x86: Expose Predictive Store Forwarding Disable
       [not found] <163244601049.30292.5855870305350227855.stgit@bmoger-ubuntu>
  2021-09-27 10:59 ` [PATCH] KVM: x86: Expose Predictive Store Forwarding Disable Borislav Petkov
@ 2021-09-28 16:04 ` Paolo Bonzini
  2021-09-29 20:27   ` Babu Moger
  1 sibling, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2021-09-28 16:04 UTC (permalink / raw)
  To: Babu Moger, tglx, mingo, bp, x86
  Cc: hpa, seanjc, vkuznets, wanpengli, jmattson, joro, tony.luck,
	peterz, kyung.min.park, wei.huang2, jgross, andrew.cooper3,
	linux-kernel, kvm

On 24/09/21 03:15, Babu Moger wrote:
>   arch/x86/include/asm/cpufeatures.h |    1 +
>   arch/x86/kvm/cpuid.c               |    2 +-
>   2 files changed, 2 insertions(+), 1 deletion(-)

Queued, with a private #define instead of the one in cpufeatures.h:

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index fe03bd978761..343a01a05058 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -53,9 +53,16 @@ static u32 xstate_required_size(u64 xstate_bv, bool compacted)
  	return ret;
  }
  
+/*
+ * This one is tied to SSB in the user API, and not
+ * visible in /proc/cpuinfo.
+ */
+#define KVM_X86_FEATURE_PSFD		(13*32+28) /* Predictive Store Forwarding Disable */
+
  #define F feature_bit
  #define SF(name) (boot_cpu_has(X86_FEATURE_##name) ? F(name) : 0)
  
+
  static inline struct kvm_cpuid_entry2 *cpuid_entry2_find(
  	struct kvm_cpuid_entry2 *entries, int nent, u32 function, u32 index)
  {
@@ -500,7 +507,8 @@ void kvm_set_cpu_caps(void)
  	kvm_cpu_cap_mask(CPUID_8000_0008_EBX,
  		F(CLZERO) | F(XSAVEERPTR) |
  		F(WBNOINVD) | F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | F(VIRT_SSBD) |
-		F(AMD_SSB_NO) | F(AMD_STIBP) | F(AMD_STIBP_ALWAYS_ON)
+		F(AMD_SSB_NO) | F(AMD_STIBP) | F(AMD_STIBP_ALWAYS_ON) |
+		__feature_bit(KVM_X86_FEATURE_PSFD)
  	);
  
  	/*

Paolo


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] KVM: x86: Expose Predictive Store Forwarding Disable
  2021-09-28 16:04 ` Paolo Bonzini
@ 2021-09-29 20:27   ` Babu Moger
  2021-09-30 13:41     ` Paolo Bonzini
  0 siblings, 1 reply; 13+ messages in thread
From: Babu Moger @ 2021-09-29 20:27 UTC (permalink / raw)
  To: Paolo Bonzini, tglx, mingo, bp, x86
  Cc: hpa, seanjc, vkuznets, wanpengli, jmattson, joro, tony.luck,
	peterz, kyung.min.park, wei.huang2, jgross, andrew.cooper3,
	linux-kernel, kvm



On 9/28/21 11:04 AM, Paolo Bonzini wrote:
> On 24/09/21 03:15, Babu Moger wrote:
>>   arch/x86/include/asm/cpufeatures.h |    1 +
>>   arch/x86/kvm/cpuid.c               |    2 +-
>>   2 files changed, 2 insertions(+), 1 deletion(-)
> 
> Queued, with a private #define instead of the one in cpufeatures.h:

Thanks Paolo. Don't we need change in guest_has_spec_ctrl_msr?

> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index fe03bd978761..343a01a05058 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -53,9 +53,16 @@ static u32 xstate_required_size(u64 xstate_bv, bool
> compacted)
>      return ret;
>  }
>  
> +/*
> + * This one is tied to SSB in the user API, and not
> + * visible in /proc/cpuinfo.
> + */
> +#define KVM_X86_FEATURE_PSFD        (13*32+28) /* Predictive Store
> Forwarding Disable */
> +
>  #define F feature_bit
>  #define SF(name) (boot_cpu_has(X86_FEATURE_##name) ? F(name) : 0)
>  
> +
>  static inline struct kvm_cpuid_entry2 *cpuid_entry2_find(
>      struct kvm_cpuid_entry2 *entries, int nent, u32 function, u32 index)
>  {
> @@ -500,7 +507,8 @@ void kvm_set_cpu_caps(void)
>      kvm_cpu_cap_mask(CPUID_8000_0008_EBX,
>          F(CLZERO) | F(XSAVEERPTR) |
>          F(WBNOINVD) | F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) |
> F(VIRT_SSBD) |
> -        F(AMD_SSB_NO) | F(AMD_STIBP) | F(AMD_STIBP_ALWAYS_ON)
> +        F(AMD_SSB_NO) | F(AMD_STIBP) | F(AMD_STIBP_ALWAYS_ON) |
> +        __feature_bit(KVM_X86_FEATURE_PSFD)
>      );
>  
>      /*
> 
> Paolo
> 

-- 
Thanks
Babu Moger

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] KVM: x86: Expose Predictive Store Forwarding Disable
  2021-09-29 20:27   ` Babu Moger
@ 2021-09-30 13:41     ` Paolo Bonzini
  2021-09-30 14:12       ` Babu Moger
  0 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2021-09-30 13:41 UTC (permalink / raw)
  To: Babu Moger, tglx, mingo, bp, x86
  Cc: hpa, seanjc, vkuznets, wanpengli, jmattson, joro, tony.luck,
	peterz, kyung.min.park, wei.huang2, jgross, andrew.cooper3,
	linux-kernel, kvm

On 29/09/21 22:27, Babu Moger wrote:
> 
> On 9/28/21 11:04 AM, Paolo Bonzini wrote:
>> On 24/09/21 03:15, Babu Moger wrote:
>>>    arch/x86/include/asm/cpufeatures.h |    1 +
>>>    arch/x86/kvm/cpuid.c               |    2 +-
>>>    2 files changed, 2 insertions(+), 1 deletion(-)
>> Queued, with a private #define instead of the one in cpufeatures.h:
> Thanks Paolo. Don't we need change in guest_has_spec_ctrl_msr?

Not strictly necessary unless you expect processors to have PSFD and not 
SSBD; but yes it's cleaner.

Paolo


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] KVM: x86: Expose Predictive Store Forwarding Disable
  2021-09-30 13:41     ` Paolo Bonzini
@ 2021-09-30 14:12       ` Babu Moger
  0 siblings, 0 replies; 13+ messages in thread
From: Babu Moger @ 2021-09-30 14:12 UTC (permalink / raw)
  To: Paolo Bonzini, tglx, mingo, bp, x86
  Cc: hpa, seanjc, vkuznets, wanpengli, jmattson, joro, tony.luck,
	peterz, kyung.min.park, wei.huang2, jgross, andrew.cooper3,
	linux-kernel, kvm



On 9/30/21 8:41 AM, Paolo Bonzini wrote:
> On 29/09/21 22:27, Babu Moger wrote:
>>
>> On 9/28/21 11:04 AM, Paolo Bonzini wrote:
>>> On 24/09/21 03:15, Babu Moger wrote:
>>>>    arch/x86/include/asm/cpufeatures.h |    1 +
>>>>    arch/x86/kvm/cpuid.c               |    2 +-
>>>>    2 files changed, 2 insertions(+), 1 deletion(-)
>>> Queued, with a private #define instead of the one in cpufeatures.h:
>> Thanks Paolo. Don't we need change in guest_has_spec_ctrl_msr?
> 
> Not strictly necessary unless you expect processors to have PSFD and not
> SSBD; but yes it's cleaner.

It is always both SSBD and PSFD together. We are good.Thanks

Tested-By: Babu Moger <babu.moger@amd.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-09-30 14:13 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <163244601049.30292.5855870305350227855.stgit@bmoger-ubuntu>
2021-09-27 10:59 ` [PATCH] KVM: x86: Expose Predictive Store Forwarding Disable Borislav Petkov
2021-09-27 11:13   ` Paolo Bonzini
2021-09-27 12:06     ` Borislav Petkov
2021-09-27 12:14       ` Paolo Bonzini
2021-09-27 12:28         ` Borislav Petkov
2021-09-27 12:54           ` Paolo Bonzini
2021-09-27 13:38             ` Borislav Petkov
2021-09-27 15:47             ` Babu Moger
2021-09-27 15:48               ` Borislav Petkov
2021-09-28 16:04 ` Paolo Bonzini
2021-09-29 20:27   ` Babu Moger
2021-09-30 13:41     ` Paolo Bonzini
2021-09-30 14:12       ` Babu Moger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).