[MODERATED] [PATCH 2/2] Guest L1D flush

[MODERATED] [PATCH 2/2] Guest L1D flush

[Jim Mattson]

[patch 2/2] kvm: x86: Expose X86_FEATURE_FLUSH_L1D to kvm guests

If this feature is available on the host, it can be exposed to a kvm
guest.

Signed-off-by: Jim Mattson <jmattson@google.com>
Reviewed-by: Ben Serebrin <serebrin@google.com>
Reviewed-by: Peter Shier <pshier@google.com>
---
 arch/x86/kvm/cpuid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 7e042e3d47fd5..2a62270d82b69 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -411,7 +411,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 	/* cpuid 7.0.edx*/
 	const u32 kvm_cpuid_7_0_edx_x86_features =
 		F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) |
-		F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES);
+		F(SPEC_CTRL_SSBD) | F(FLUSH_L1D) | F(ARCH_CAPABILITIES);
 
 	/* all calls to cpuid_count() should be made on the same cpu */
 	get_cpu();
-- 
2.18.0.597.ga71716f1ad-goog

[MODERATED] Re: [PATCH 2/2] Guest L1D flush

[Konrad Rzeszutek Wilk]

On Wed, Aug 08, 2018 at 12:15:06PM -0700, speck for Jim Mattson wrote:
> [patch 2/2] kvm: x86: Expose X86_FEATURE_FLUSH_L1D to kvm guests
> 
> If this feature is available on the host, it can be exposed to a kvm
> guest.

Are you sure? I was reading the docs and nothing in it said to do this.

In fact the doc suggests that this should not be done in the first place
and instead the ARCH_CAPABILITIES Bit 3 is to be exposed.

> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> Reviewed-by: Ben Serebrin <serebrin@google.com>
> Reviewed-by: Peter Shier <pshier@google.com>
> ---
>  arch/x86/kvm/cpuid.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 7e042e3d47fd5..2a62270d82b69 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -411,7 +411,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  	/* cpuid 7.0.edx*/
>  	const u32 kvm_cpuid_7_0_edx_x86_features =
>  		F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) |
> -		F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES);
> +		F(SPEC_CTRL_SSBD) | F(FLUSH_L1D) | F(ARCH_CAPABILITIES);
>  
>  	/* all calls to cpuid_count() should be made on the same cpu */
>  	get_cpu();
> -- 
> 2.18.0.597.ga71716f1ad-goog
> 
> 

[MODERATED] Re: [PATCH 2/2] Guest L1D flush

[Jim Mattson]

2018-08-08 14:21 GMT-07:00 speck for Konrad Rzeszutek Wilk <speck@linutronix.de>:
>On Wed, Aug 08, 2018 at 12:15:06PM -0700, speck for Jim Mattson wrote:
>> [patch 2/2] kvm: x86: Expose X86_FEATURE_FLUSH_L1D to kvm guests
>> 
>> If this feature is available on the host, it can be exposed to a kvm
>> guest.
>
>Are you sure? I was reading the docs and nothing in it said to do this.
>
>In fact the doc suggests that this should not be done in the first place
>and instead the ARCH_CAPABILITIES Bit 3 is to be exposed.

Intel's white paper, "Description and mitigation overview for L1
Terminal Fault" (337866-000) says:

  Each VMM can choose which CPU capabilities to make available to the
  guests under its control.

...and...

  A nested VMM that finds IA32_FLUSH_CMD is enumerated should check
  whether IA32_ARCH_CAPABILITIES bit 3 (SKIP_L1DFL_VMENTRY) is set,
  which indicates that it is not required to flush L1D on VMENTER.

My reading of this is that:

A) Exposing IA32_FLUSH_CMD to a guest is at the whim of the VMM, and
B) There is actually no reason for a VMM to query
   IA32_ARCH_CAPABILITIES.SKIP_L1DFL_VMENTRY[bit 3] *unless*
   IA32_FLUSH_CMD is enumerated.

[MODERATED] Re: [PATCH 2/2] Guest L1D flush

[Andrew Cooper]

[-- Attachment #1: Type: text/plain, Size: 2223 bytes --]

On 08/08/2018 23:20, speck for Jim Mattson wrote:
> 2018-08-08 14:21 GMT-07:00 speck for Konrad Rzeszutek Wilk <speck@linutronix.de>:
>> On Wed, Aug 08, 2018 at 12:15:06PM -0700, speck for Jim Mattson wrote:
>>> [patch 2/2] kvm: x86: Expose X86_FEATURE_FLUSH_L1D to kvm guests
>>>
>>> If this feature is available on the host, it can be exposed to a kvm
>>> guest.
>> Are you sure? I was reading the docs and nothing in it said to do this.
>>
>> In fact the doc suggests that this should not be done in the first place
>> and instead the ARCH_CAPABILITIES Bit 3 is to be exposed.
> Intel's white paper, "Description and mitigation overview for L1
> Terminal Fault" (337866-000) says:
>
>   Each VMM can choose which CPU capabilities to make available to the
>   guests under its control.
>
> ...and...
>
>   A nested VMM that finds IA32_FLUSH_CMD is enumerated should check
>   whether IA32_ARCH_CAPABILITIES bit 3 (SKIP_L1DFL_VMENTRY) is set,
>   which indicates that it is not required to flush L1D on VMENTER.
>
> My reading of this is that:
>
> A) Exposing IA32_FLUSH_CMD to a guest is at the whim of the VMM, and
> B) There is actually no reason for a VMM to query
>    IA32_ARCH_CAPABILITIES.SKIP_L1DFL_VMENTRY[bit 3] *unless*
>    IA32_FLUSH_CMD is enumerated.

Per the current understanding, MSR_FLUSH_CMD functionality is only
needed by hypervisors (nested or otherwise).  When SKIP_L1DFL is set,
nested hypervisors shouldn't need it themselves.  However, there is
still a gaping HT-sized hole in the guaranteed-safety of this argument.

The absolute cost of using this MSR isn't terribly high (dispatch
serialising, and ~400 cycles to recover after), and yes - a guest can
waste time flushing its own L1D, but it can waste far more time by
needlessly VMExiting.

The important questions are 1) what is the cost of implementing the
functionality, and 2) how much are you willing to bet that it won't
become necessary for other reasons in the future.

I've implemented support in Xen for offering this functionality to
guests, because doing so is a whole 10 lines of code (inc. feature
levelling migration safety), and I'm not willing to take that bet.

~Andrew