Intel TDX doesn't allow VMM to directly access guest private memory.
Any memory that is required for communication with the VMM must be
shared explicitly. The same rule applies for any DMA to and from the
TDX guest. All DMA pages have to be marked as shared pages. A generic way
to achieve this without any changes to device drivers is to use the
SWIOTLB framework.
Force SWIOTLB on TD guest and make SWIOTLB buffer shared by generalizing
mem_encrypt_init() to cover TDX.
Co-developed-by: Kuppuswamy Sathyanarayanan <[email protected]>
Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
Reviewed-by: Andi Kleen <[email protected]>
Reviewed-by: Tony Luck <[email protected]>
Signed-off-by: Kirill A. Shutemov <[email protected]>
---
arch/x86/Kconfig | 2 +-
arch/x86/coco/core.c | 1 +
arch/x86/coco/tdx.c | 3 +++
arch/x86/include/asm/mem_encrypt.h | 6 +++---
arch/x86/mm/mem_encrypt.c | 9 ++++++++-
5 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 98efb35ed7b1..1312cefb927d 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -885,7 +885,7 @@ config INTEL_TDX_GUEST
depends on X86_64 && CPU_SUP_INTEL
depends on X86_X2APIC
select ARCH_HAS_CC_PLATFORM
- select DYNAMIC_PHYSICAL_MASK
+ select X86_MEM_ENCRYPT
select X86_MCE
help
Support running as a guest under Intel TDX. Without this support,
diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c
index 9778cf4c6901..b10326f91d4f 100644
--- a/arch/x86/coco/core.c
+++ b/arch/x86/coco/core.c
@@ -22,6 +22,7 @@ static bool intel_cc_platform_has(enum cc_attr attr)
case CC_ATTR_GUEST_UNROLL_STRING_IO:
case CC_ATTR_HOTPLUG_DISABLED:
case CC_ATTR_GUEST_MEM_ENCRYPT:
+ case CC_ATTR_MEM_ENCRYPT:
return true;
default:
return false;
diff --git a/arch/x86/coco/tdx.c b/arch/x86/coco/tdx.c
index 2168ee25a52c..429a1ba42667 100644
--- a/arch/x86/coco/tdx.c
+++ b/arch/x86/coco/tdx.c
@@ -5,6 +5,7 @@
#define pr_fmt(fmt) "tdx: " fmt
#include <linux/cpufeature.h>
+#include <linux/swiotlb.h>
#include <asm/coco.h>
#include <asm/tdx.h>
#include <asm/vmx.h>
@@ -627,5 +628,7 @@ void __init tdx_early_init(void)
x86_platform.guest.enc_tlb_flush_required = tdx_tlb_flush_required;
x86_platform.guest.enc_status_change_finish = tdx_enc_status_changed;
+ swiotlb_force = SWIOTLB_FORCE;
+
pr_info("Guest detected\n");
}
diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
index e2c6f433ed10..88ceaf3648b3 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -49,9 +49,6 @@ void __init early_set_mem_enc_dec_hypercall(unsigned long vaddr, int npages,
void __init mem_encrypt_free_decrypted_mem(void);
-/* Architecture __weak replacement functions */
-void __init mem_encrypt_init(void);
-
void __init sev_es_init_vc_handling(void);
#define __bss_decrypted __section(".bss..decrypted")
@@ -89,6 +86,9 @@ static inline void mem_encrypt_free_decrypted_mem(void) { }
#endif /* CONFIG_AMD_MEM_ENCRYPT */
+/* Architecture __weak replacement functions */
+void __init mem_encrypt_init(void);
+
/*
* The __sme_pa() and __sme_pa_nodebug() macros are meant for use when
* writing to or comparing values from the cr3 register. Having the
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index 50d209939c66..10ee40b5204b 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -42,7 +42,14 @@ bool force_dma_unencrypted(struct device *dev)
static void print_mem_encrypt_feature_info(void)
{
- pr_info("AMD Memory Encryption Features active:");
+ pr_info("Memory Encryption Features active:");
+
+ if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST)) {
+ pr_cont(" Intel TDX\n");
+ return;
+ }
+
+ pr_cont("AMD ");
/* Secure Memory Encryption */
if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)) {
--
2.34.1
On Thu, Mar 10, 2022 at 08:29:01AM -0600, Tom Lendacky wrote:
> > void __init mem_encrypt_init(void)
> > {
> > if (!cc_platform_has(CC_ATTR_MEM_ENCRYPT))
>
> If you make this cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT), then it should
> work for both, I would think. If you use CC_ATTR_MEM_ENCRYPT, you'll force
> bare-metal SME to always use bounce buffers when doing I/O. But SME can do
> I/O to encrypted memory if the device supports 64-bit DMA or if the IOMMU is
> being used, so we don't want to force SWIOTLB in this case.
http://git.infradead.org/users/hch/misc.git/commitdiff/18b0547fe0467cb48e64ee403f50f2587fe04e3a
On 3/2/22 06:28, Kirill A. Shutemov wrote:
> --- a/arch/x86/coco/tdx.c
> +++ b/arch/x86/coco/tdx.c
> @@ -5,6 +5,7 @@
> #define pr_fmt(fmt) "tdx: " fmt
>
> #include <linux/cpufeature.h>
> +#include <linux/swiotlb.h>
> #include <asm/coco.h>
> #include <asm/tdx.h>
> #include <asm/vmx.h>
> @@ -627,5 +628,7 @@ void __init tdx_early_init(void)
> x86_platform.guest.enc_tlb_flush_required = tdx_tlb_flush_required;
> x86_platform.guest.enc_status_change_finish = tdx_enc_status_changed;
>
> + swiotlb_force = SWIOTLB_FORCE;
> +
> pr_info("Guest detected\n");
> }
AMD currently does:
if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
swiotlb_force = SWIOTLB_FORCE;
which somewhat begs the question of why we can't do the
swiotlb_force = SWIOTLB_FORCE;
thing in:
void __init mem_encrypt_init(void)
{
if (!cc_platform_has(CC_ATTR_MEM_ENCRYPT))
return;
/// Here
I recall there being a reason for this. But I don't see any mention in
the changelog.
On 3/9/22 14:07, Dave Hansen wrote:
> On 3/2/22 06:28, Kirill A. Shutemov wrote:
>> --- a/arch/x86/coco/tdx.c
>> +++ b/arch/x86/coco/tdx.c
>> @@ -5,6 +5,7 @@
>> #define pr_fmt(fmt) "tdx: " fmt
>>
>> #include <linux/cpufeature.h>
>> +#include <linux/swiotlb.h>
>> #include <asm/coco.h>
>> #include <asm/tdx.h>
>> #include <asm/vmx.h>
>> @@ -627,5 +628,7 @@ void __init tdx_early_init(void)
>> x86_platform.guest.enc_tlb_flush_required = tdx_tlb_flush_required;
>> x86_platform.guest.enc_status_change_finish = tdx_enc_status_changed;
>>
>> + swiotlb_force = SWIOTLB_FORCE;
>> +
>> pr_info("Guest detected\n");
>> }
>
> AMD currently does:
>
> if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
> swiotlb_force = SWIOTLB_FORCE;
>
> which somewhat begs the question of why we can't do the
>
> swiotlb_force = SWIOTLB_FORCE;
>
> thing in:
>
> void __init mem_encrypt_init(void)
> {
> if (!cc_platform_has(CC_ATTR_MEM_ENCRYPT))
If you make this cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT), then it
should work for both, I would think. If you use CC_ATTR_MEM_ENCRYPT,
you'll force bare-metal SME to always use bounce buffers when doing I/O.
But SME can do I/O to encrypted memory if the device supports 64-bit DMA
or if the IOMMU is being used, so we don't want to force SWIOTLB in this case.
Thanks,
Tom
> return;
>
> /// Here
>
> I recall there being a reason for this. But I don't see any mention in
> the changelog.