[MODERATED] Re: [PATCH v4 8/8] [PATCH v4 8/8] Linux Patch #8

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -194,6 +194,14 @@ module_param(ple_window_max, uint, 0444)
>  
>  extern const ulong vmx_return;
>  
> +static DEFINE_STATIC_KEY_FALSE(vmx_l1d_should_flush);
> +
> +enum vmx_l1d_flush_state {
> +	VMENTER_L1D_FLUSH_NEVER,
> +	VMENTER_L1D_FLUSH_COND,
> +	VMENTER_L1D_FLUSH_ALWAYS,
> +};
> +
>  struct kvm_vmx {
>  	struct kvm kvm;
>  
> @@ -2653,38 +2661,10 @@ static void vmx_prepare_guest_switch(str
>  {
>  	vmx_save_host_state(vcpu);
>  
> -	if (!enable_ept || static_cpu_has(X86_FEATURE_HYPERVISOR) ||
> -	    !static_cpu_has(X86_BUG_L1TF)) {
> -		vcpu->arch.flush_cache_req = false;
> -		return;
> -	}
> +	if (static_branch_unlikely(&vmx_l1d_should_flush)) {
> +		bool force = vmentry_l1d_flush == VMENTER_L1D_FLUSH_ALWAYS;
>  
> -	switch (vmentry_l1d_flush) {
> -	case 0:
> -		vcpu->arch.flush_cache_req = false;
> -		break;
> -	case 1:
> -		/*
> -		 * If vmentry_l1d_flush is 1, each vmexit handler is responsible for
> -		 * setting vcpu->arch.vcpu_unconfined.  Currently this happens in the
> -		 * following cases:
> -		 * - vmlaunch/vmresume: we do not want the cache to be cleared by a
> -		 *   nested hypervisor *and* by KVM on bare metal, so we just do it
> -		 *   on every nested entry.  Nested hypervisors do not bother clearing
> -		 *   the cache.
> -		 * - anything that runs the emulator (the slow paths for EPT misconfig
> -		 *   or I/O instruction)
> -		 * - anything that can cause get_user_pages (EPT violation, and again
> -		 *   the slow paths for EPT misconfig or I/O instruction)
> -		 * - anything that can run code outside KVM (external interrupt,
> -		 *   which can run interrupt handlers or irqs; or the sched_in
> -		 *   preempt notifier)
> -		 */
> -		break;
> -	case 2:
> -	default:
> -		vcpu->arch.flush_cache_req = true;
> -		break;
> +		vcpu->arch.flush_cache_req = force;

This ought to be:

		if (force)
			vcpu->arch.flush_cache_req = force;

or perhaps:

		if (vmentry_l1d_flush == VMENTER_L1D_FLUSH_ALWAYS)
			vcpu->arch.flush_cache_req = true;

The problem is that if we are in 'vmentry_l1d_flush=1' we should
not modify vcpu->arch.flush_cache_req as the vcpu_enter_guest does:

7464                                                                                 
7465         vcpu->arch.flush_cache_req = vcpu->arch.vcpu_unconfined;                 <===
7466         kvm_x86_ops->prepare_guest_switch(vcpu);                                
7467         vcpu->arch.vcpu_unconfined = false;                                     
7468         if (vcpu->arch.flush_cache_req)                                         
7469                 vcpu->stat.l1d_flush++;      

Rethinking this, maybe rip the above from vcpu_enter_guest and in this function
will do:

		vcpu->arch.flush_cache_req = (vmentry_l1d_flush == VMENTER_L1D_FLUSH_ALWAYS) ? true : vcpu->arch.vcpu_unconfined;

or so?

Let me try that out.