[2/4] x86/virt/tdx: Advertise the CC_ATTR_HOST_MEM_INCOHERENT for TDX host
Commit Message
From: Kai Huang <kai.huang@intel.com>
On the TDX capable platform, during kexec() the old kernel needs to
flush dirty cachelines of all TDX private memory otherwise they may
silently corrupt the new kernel's memory.
Advertise the new introduced CC_ATTR_HOST_MEM_INCOHERENT attribute for
TDX host platform so the cache will be flushed during kexec().
Note theoretically cache flush is only needed when TDX module is
initialized, but the module initialization is done at runtime so just
advertise the CC attribute when the platform has TDX enabled.
Signed-off-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Reviewed-by: Chao Gao <chao.gao@intel.com>
---
arch/x86/Kconfig | 1 +
arch/x86/coco/core.c | 21 ++++++++++++++++++++-
arch/x86/virt/vmx/tdx/tdx.c | 3 +++
include/linux/cc_platform.h | 3 ++-
4 files changed, 26 insertions(+), 2 deletions(-)
Comments
On 1/31/24 03:31, Huang, Kai wrote:
> From: Kai Huang <kai.huang@intel.com>
>
> On the TDX capable platform, during kexec() the old kernel needs to
> flush dirty cachelines of all TDX private memory otherwise they may
> silently corrupt the new kernel's memory.
>
> Advertise the new introduced CC_ATTR_HOST_MEM_INCOHERENT attribute for
> TDX host platform so the cache will be flushed during kexec().
So, you're setting a new bit, CC_ATTR_HOST_MEM_INCOHERENT. The way I
like to deal with these is to go back and look at the definition of
CC_ATTR_HOST_MEM_INCOHERENT and see whether the changelog here convinces
me that CC_ATTR_HOST_MEM_INCOHERENT is being set appropriately. Bonus
points if this changelog uses the same nomenclature as the comment
describing CC_ATTR_HOST_MEM_INCOHERENT.
How well does this match the comment above CC_ATTR_HOST_MEM_INCOHERENT?
> Note theoretically cache flush is only needed when TDX module is
> initialized, but the module initialization is done at runtime so just
> advertise the CC attribute when the platform has TDX enabled.
I find this really hard to parse. Here's a rewrite, as usual:
A TDX-host-capable system might not actually have any incoherent
memory. This can occur if a TDX module was never initialized or
if the caches have been flushed since the last time TDX was
used. Ignore that case. Eliminate the need for any locking and
assume that any TDX-host-capable system might have incoherent
memory by always setting CC_ATTR_HOST_MEM_INCOHERENT.
On 1/02/2024 1:11 am, Dave Hansen wrote:
> On 1/31/24 03:31, Huang, Kai wrote:
>> From: Kai Huang <kai.huang@intel.com>
>>
>> On the TDX capable platform, during kexec() the old kernel needs to
>> flush dirty cachelines of all TDX private memory otherwise they may
>> silently corrupt the new kernel's memory.
>>
>> Advertise the new introduced CC_ATTR_HOST_MEM_INCOHERENT attribute for
>> TDX host platform so the cache will be flushed during kexec().
>
> So, you're setting a new bit, CC_ATTR_HOST_MEM_INCOHERENT. The way I
> like to deal with these is to go back and look at the definition of
> CC_ATTR_HOST_MEM_INCOHERENT and see whether the changelog here convinces
> me that CC_ATTR_HOST_MEM_INCOHERENT is being set appropriately. Bonus
> points if this changelog uses the same nomenclature as the comment
> describing CC_ATTR_HOST_MEM_INCOHERENT.
>
> How well does this match the comment above CC_ATTR_HOST_MEM_INCOHERENT?
I will try to improve the changelog: explain what does
CC_ATTR_HOST_MEM_INCOHERENT mean, and why it is suitable for TDX host.
Please let me know if you have more comments. Thanks.
>
>> Note theoretically cache flush is only needed when TDX module is
>> initialized, but the module initialization is done at runtime so just
>> advertise the CC attribute when the platform has TDX enabled.
>
> I find this really hard to parse. Here's a rewrite, as usual:
>
> A TDX-host-capable system might not actually have any incoherent
> memory. This can occur if a TDX module was never initialized or
> if the caches have been flushed since the last time TDX was
> used. Ignore that case. Eliminate the need for any locking and
> assume that any TDX-host-capable system might have incoherent
> memory by always setting CC_ATTR_HOST_MEM_INCOHERENT.
I appreciate, as usual. :-)
@@ -1975,6 +1975,7 @@ config INTEL_TDX_HOST
depends on CONTIG_ALLOC
depends on !KEXEC_CORE
depends on X86_MCE
+ select ARCH_HAS_CC_PLATFORM
help
Intel Trust Domain Extensions (TDX) protects guest VMs from malicious
host and certain physical attacks. This option enables necessary TDX
@@ -12,11 +12,12 @@
#include <asm/coco.h>
#include <asm/processor.h>
+#include <asm/cpufeature.h>
enum cc_vendor cc_vendor __ro_after_init = CC_VENDOR_NONE;
static u64 cc_mask __ro_after_init;
-static bool noinstr intel_cc_platform_has(enum cc_attr attr)
+static bool noinstr intel_cc_platform_guest_has(enum cc_attr attr)
{
switch (attr) {
case CC_ATTR_GUEST_UNROLL_STRING_IO:
@@ -29,6 +30,24 @@ static bool noinstr intel_cc_platform_has(enum cc_attr attr)
}
}
+static bool noinstr intel_cc_platform_host_has(enum cc_attr attr)
+{
+ switch (attr) {
+ case CC_ATTR_HOST_MEM_INCOHERENT:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static bool noinstr intel_cc_platform_has(enum cc_attr attr)
+{
+ if (boot_cpu_has(X86_FEATURE_TDX_HOST_PLATFORM))
+ return intel_cc_platform_host_has(attr);
+
+ return intel_cc_platform_guest_has(attr);
+}
+
/*
* Handle the SEV-SNP vTOM case where sme_me_mask is zero, and
* the other levels of SME/SEV functionality, including C-bit
@@ -37,6 +37,7 @@
#include <asm/intel-family.h>
#include <asm/processor.h>
#include <asm/mce.h>
+#include <asm/coco.h>
#include "tdx.h"
static u32 tdx_global_keyid __ro_after_init;
@@ -1488,5 +1489,7 @@ void __init tdx_init(void)
setup_force_cpu_cap(X86_FEATURE_TDX_HOST_PLATFORM);
+ cc_vendor = CC_VENDOR_INTEL;
+
check_tdx_erratum();
}
@@ -53,7 +53,8 @@ enum cc_attr {
* Use this in places where the cache coherency of the memory matters
* but the encryption status does not.
*
- * Includes all systems that set CC_ATTR_HOST_MEM_ENCRYPT.
+ * Includes all systems that set CC_ATTR_HOST_MEM_ENCRYPT, but
+ * additionally adds TDX hosts.
*/
CC_ATTR_HOST_MEM_INCOHERENT,