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.
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.
Signed-off-by: Kirill A. Shutemov <[email protected]>
---
arch/x86/include/asm/acenv.h | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
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);
--
2.34.1
On 3/15/22 19:08, Kirill A. Shutemov wrote:
> 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.
>
> 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.
Looks good. Did you have more acks on this earlier that got removed? I
thought I remembered more acks on earlier versions.
Reviewed-by: Dave Hansen <[email protected]>
On Wed, Mar 16 2022 at 05:08, Kirill A. Shutemov wrote:
> 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.
>
> 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.
>
> Signed-off-by: Kirill A. Shutemov <[email protected]>
Reviewed-by: Thomas Gleixner <[email protected]>
On Wed, Mar 16, 2022 at 3:13 PM Dave Hansen <[email protected]> wrote:
>
> On 3/15/22 19:08, Kirill A. Shutemov wrote:
> > 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.
> >
> > 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.
>
> Looks good. Did you have more acks on this earlier that got removed? I
> thought I remembered more acks on earlier versions.
>
> Reviewed-by: Dave Hansen <[email protected]>
Yeah, my previous reviewed-by still stands:
Reviewed-by: Dan Williams <[email protected]>
On Wed, Mar 16, 2022 at 03:13:18PM -0700, Dave Hansen wrote:
> On 3/15/22 19:08, Kirill A. Shutemov wrote:
> > 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.
> >
> > 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.
>
> Looks good. Did you have more acks on this earlier that got removed? I
> thought I remembered more acks on earlier versions.
I missed Dan's Reviewed-by, but it was the only one that got to my inbox.
We had few (actually few too many) different approaches to address WBINVD
and some of them got acks. Dan was the only one who acked this version,
before current submission.
--
Kirill A. Shutemov