Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <5202ac86-7438-eeae-61d7-23bf10b475f5@intel.com>
Date:   Wed, 9 Mar 2022 12:56:26 -0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Content-Language: en-US
To:     "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
        tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
        luto@kernel.org, peterz@infradead.org
Cc:     sathyanarayanan.kuppuswamy@linux.intel.com, aarcange@redhat.com,
        ak@linux.intel.com, dan.j.williams@intel.com, david@redhat.com,
        hpa@zytor.com, jgross@suse.com, jmattson@google.com,
        joro@8bytes.org, jpoimboe@redhat.com, knsathya@kernel.org,
        pbonzini@redhat.com, sdeep@vmware.com, seanjc@google.com,
        tony.luck@intel.com, vkuznets@redhat.com, wanpengli@tencent.com,
        thomas.lendacky@amd.com, brijesh.singh@amd.com, x86@kernel.org,
        linux-kernel@vger.kernel.org
References: <20220302142806.51844-1-kirill.shutemov@linux.intel.com>
 <20220302142806.51844-30-kirill.shutemov@linux.intel.com>
From:   Dave Hansen <dave.hansen@intel.com>
Subject: Re: [PATCHv5 29/30] ACPICA: Avoid cache flush inside virtual machines
In-Reply-To: <20220302142806.51844-30-kirill.shutemov@linux.intel.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 3/2/22 06:28, Kirill A. Shutemov wrote:
> WBINVD is not supported in TDX guest and triggers #VE. There's no robust
> way to emulate it. The kernel has to avoid it.

Not really.  It could just ignore any use of WBVIND.  That's why
hypervisors mostly do today.

> ACPI_FLUSH_CPU_CACHE() flushes caches usign WBINVD on entering sleep

					 ^ using
			
> states. It is required to prevent data loss.
> 
> While running inside virtual machine, the kernel can bypass cache
> flushing. Changing sleep state in a virtual machine doesn't affect the
> host system sleep state and cannot lead to data loss.

How's this?

Before entering sleep states, the ACPI code flushes caches to prevent
data loss using the WBINVD instruction.  This mechanism is required on
bare metal.

But, any use WBINVD inside of a guest is worthless.  Changing sleep
state in a virtual machine doesn't affect the host system sleep state
and cannot lead to data loss, so most hypervisors simply ignore it.
Despite this, the ACPI code calls WBINVD unconditionally anyway.  It's
useless, but also normally harmless.

In TDX guests, though, WBINVD stops being harmless; it triggers a
virtualization exception (#VE).  If the ACPI cache-flushing WBINVD were
left in place, TDX guests would need handling to recover from the exception.

Avoid using WBINVD whenever running under a hypervisor.  This both
removes the useless WBINVDs and saves TDX from implementing WBINVD handling.

> diff --git a/arch/x86/include/asm/acenv.h b/arch/x86/include/asm/acenv.h
> index 9aff97f0de7f..d937c55e717e 100644
> --- a/arch/x86/include/asm/acenv.h
> +++ b/arch/x86/include/asm/acenv.h
> @@ -13,7 +13,19 @@
>  
>  /* Asm macros */
>  
> -#define ACPI_FLUSH_CPU_CACHE()	wbinvd()
> +/*
> + * ACPI_FLUSH_CPU_CACHE() flushes caches on entering sleep states.
> + * It is required to prevent data loss.
> + *
> + * While running inside virtual machine, the kernel can bypass cache flushing.
> + * Changing sleep state in a virtual machine doesn't affect the host system
> + * sleep state and cannot lead to data loss.
> + */
> +#define ACPI_FLUSH_CPU_CACHE()					\
> +do {								\
> +	if (!cpu_feature_enabled(X86_FEATURE_HYPERVISOR))	\
> +		wbinvd();					\
> +} while (0)
>  
>  int __acpi_acquire_global_lock(unsigned int *lock);
>  int __acpi_release_global_lock(unsigned int *lock);