* [PATCHv3 0/2] x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default
@ 2025-01-13 13:14 Kirill A. Shutemov
2025-01-13 13:14 ` [PATCHv3 1/2] memremap: Pass down MEMREMAP_* flags to arch_memremap_wb() Kirill A. Shutemov
2025-01-13 13:14 ` [PATCHv3 2/2] x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default Kirill A. Shutemov
0 siblings, 2 replies; 14+ messages in thread
From: Kirill A. Shutemov @ 2025-01-13 13:14 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski
Cc: Albert Ou, Alexei Starovoitov, Andrea Parri, Arnd Bergmann,
Daniel Borkmann, Eric Chan, Jason Gunthorpe, Kai Huang,
Kefeng Wang, Kent Overstreet, Palmer Dabbelt, Paul Walmsley,
Russell King, Samuel Holland, Suren Baghdasaryan, Yuntao Wang,
linux-arm-kernel, linux-kernel, linux-riscv, Kirill A. Shutemov
From: "Kirill A. Shutemov" <kirill.shutemov@intel.com>
Make memremap(MEMREMAP_WB) produce encrypted/private mapping by default
unless MEMREMAP_DEC is specified.
It fixes crash on kexec in TDX guests if CONFIG_EISA is enabled.
v3:
- Update commit message;
- Add Cc: stable;
v2:
- Fix sparse warning;
Kirill A. Shutemov (2):
memremap: Pass down MEMREMAP_* flags to arch_memremap_wb()
x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default
arch/arm/include/asm/io.h | 2 +-
arch/arm/mm/ioremap.c | 2 +-
arch/arm/mm/nommu.c | 2 +-
arch/riscv/include/asm/io.h | 2 +-
arch/x86/include/asm/io.h | 3 +++
arch/x86/mm/ioremap.c | 8 ++++++++
kernel/iomem.c | 5 +++--
7 files changed, 18 insertions(+), 6 deletions(-)
--
2.45.2
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCHv3 1/2] memremap: Pass down MEMREMAP_* flags to arch_memremap_wb()
2025-01-13 13:14 [PATCHv3 0/2] x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default Kirill A. Shutemov
@ 2025-01-13 13:14 ` Kirill A. Shutemov
2025-01-13 18:42 ` [tip: x86/urgent] " tip-bot2 for Kirill A. Shutemov
2025-01-13 13:14 ` [PATCHv3 2/2] x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default Kirill A. Shutemov
1 sibling, 1 reply; 14+ messages in thread
From: Kirill A. Shutemov @ 2025-01-13 13:14 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski
Cc: Albert Ou, Alexei Starovoitov, Andrea Parri, Arnd Bergmann,
Daniel Borkmann, Eric Chan, Jason Gunthorpe, Kai Huang,
Kefeng Wang, Kent Overstreet, Palmer Dabbelt, Paul Walmsley,
Russell King, Samuel Holland, Suren Baghdasaryan, Yuntao Wang,
linux-arm-kernel, linux-kernel, linux-riscv, Kirill A. Shutemov,
stable
x86 version of arch_memremap_wb() needs the flags to decide if the mapping
has be encrypted or decrypted.
Pass down the flag to arch_memremap_wb(). All current implementations
ignore the argument.
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: stable@vger.kernel.org # 6.11+
---
arch/arm/include/asm/io.h | 2 +-
arch/arm/mm/ioremap.c | 2 +-
arch/arm/mm/nommu.c | 2 +-
arch/riscv/include/asm/io.h | 2 +-
kernel/iomem.c | 5 +++--
5 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index 1815748f5d2a..bae5edf348ef 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -381,7 +381,7 @@ void __iomem *ioremap_wc(resource_size_t res_cookie, size_t size);
void iounmap(volatile void __iomem *io_addr);
#define iounmap iounmap
-void *arch_memremap_wb(phys_addr_t phys_addr, size_t size);
+void *arch_memremap_wb(phys_addr_t phys_addr, size_t size, unsigned long flags);
#define arch_memremap_wb arch_memremap_wb
/*
diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index 794cfea9f9d4..9f7883e6db46 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -411,7 +411,7 @@ void __arm_iomem_set_ro(void __iomem *ptr, size_t size)
set_memory_ro((unsigned long)ptr, PAGE_ALIGN(size) / PAGE_SIZE);
}
-void *arch_memremap_wb(phys_addr_t phys_addr, size_t size)
+void *arch_memremap_wb(phys_addr_t phys_addr, size_t size, unsigned long flags)
{
return (__force void *)arch_ioremap_caller(phys_addr, size,
MT_MEMORY_RW,
diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
index c415f3859b20..279641f0780e 100644
--- a/arch/arm/mm/nommu.c
+++ b/arch/arm/mm/nommu.c
@@ -251,7 +251,7 @@ void __iomem *pci_remap_cfgspace(resource_size_t res_cookie, size_t size)
EXPORT_SYMBOL_GPL(pci_remap_cfgspace);
#endif
-void *arch_memremap_wb(phys_addr_t phys_addr, size_t size)
+void *arch_memremap_wb(phys_addr_t phys_addr, size_t size, unsigned long flags)
{
return (void *)phys_addr;
}
diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
index 1c5c641075d2..0257f4aa7ff4 100644
--- a/arch/riscv/include/asm/io.h
+++ b/arch/riscv/include/asm/io.h
@@ -136,7 +136,7 @@ __io_writes_outs(outs, u64, q, __io_pbr(), __io_paw())
#include <asm-generic/io.h>
#ifdef CONFIG_MMU
-#define arch_memremap_wb(addr, size) \
+#define arch_memremap_wb(addr, size, flags) \
((__force void *)ioremap_prot((addr), (size), _PAGE_KERNEL))
#endif
diff --git a/kernel/iomem.c b/kernel/iomem.c
index dc2120776e1c..75e61c1c6bc0 100644
--- a/kernel/iomem.c
+++ b/kernel/iomem.c
@@ -6,7 +6,8 @@
#include <linux/ioremap.h>
#ifndef arch_memremap_wb
-static void *arch_memremap_wb(resource_size_t offset, unsigned long size)
+static void *arch_memremap_wb(resource_size_t offset, unsigned long size,
+ unsigned long flags)
{
#ifdef ioremap_cache
return (__force void *)ioremap_cache(offset, size);
@@ -91,7 +92,7 @@ void *memremap(resource_size_t offset, size_t size, unsigned long flags)
if (is_ram == REGION_INTERSECTS)
addr = try_ram_remap(offset, size, flags);
if (!addr)
- addr = arch_memremap_wb(offset, size);
+ addr = arch_memremap_wb(offset, size, flags);
}
/*
--
2.45.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCHv3 2/2] x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default
2025-01-13 13:14 [PATCHv3 0/2] x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default Kirill A. Shutemov
2025-01-13 13:14 ` [PATCHv3 1/2] memremap: Pass down MEMREMAP_* flags to arch_memremap_wb() Kirill A. Shutemov
@ 2025-01-13 13:14 ` Kirill A. Shutemov
2025-01-13 18:41 ` [tip: x86/urgent] " tip-bot2 for Kirill A. Shutemov
2025-01-13 20:47 ` [PATCHv3 2/2] " Tom Lendacky
1 sibling, 2 replies; 14+ messages in thread
From: Kirill A. Shutemov @ 2025-01-13 13:14 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski
Cc: Albert Ou, Alexei Starovoitov, Andrea Parri, Arnd Bergmann,
Daniel Borkmann, Eric Chan, Jason Gunthorpe, Kai Huang,
Kefeng Wang, Kent Overstreet, Palmer Dabbelt, Paul Walmsley,
Russell King, Samuel Holland, Suren Baghdasaryan, Yuntao Wang,
linux-arm-kernel, linux-kernel, linux-riscv, Kirill A. Shutemov,
stable, Tom Lendacky, Ashish Kalra, Maciej W. Rozycki
Currently memremap(MEMREMAP_WB) can produce decrypted/shared mapping:
memremap(MEMREMAP_WB)
arch_memremap_wb()
ioremap_cache()
__ioremap_caller(.encrytped = false)
In such cases, the IORES_MAP_ENCRYPTED flag on the memory will determine
if the resulting mapping is encrypted or decrypted.
Creating a decrypted mapping without explicit request from the caller is
risky:
- It can inadvertently expose the guest's data and compromise the
guest.
- Accessing private memory via shared/decrypted mapping on TDX will
either trigger implicit conversion to shared or #VE (depending on
VMM implementation).
Implicit conversion is destructive: subsequent access to the same
memory via private mapping will trigger a hard-to-debug #VE crash.
The kernel already provides a way to request decrypted mapping
explicitly via the MEMREMAP_DEC flag.
Modify memremap(MEMREMAP_WB) to produce encrypted/private mapping by
default unless MEMREMAP_DEC is specified.
Fix the crash due to #VE on kexec in TDX guests if CONFIG_EISA is enabled.
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: stable@vger.kernel.org # 6.11+
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Ashish Kalra <ashish.kalra@amd.com>
Cc: "Maciej W. Rozycki" <macro@orcam.me.uk>
---
arch/x86/include/asm/io.h | 3 +++
arch/x86/mm/ioremap.c | 8 ++++++++
2 files changed, 11 insertions(+)
diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index ed580c7f9d0a..1a0dc2b2bf5b 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -175,6 +175,9 @@ extern void __iomem *ioremap_prot(resource_size_t offset, unsigned long size, un
extern void __iomem *ioremap_encrypted(resource_size_t phys_addr, unsigned long size);
#define ioremap_encrypted ioremap_encrypted
+void *arch_memremap_wb(phys_addr_t phys_addr, size_t size, unsigned long flags);
+#define arch_memremap_wb arch_memremap_wb
+
/**
* ioremap - map bus memory into CPU space
* @offset: bus address of the memory
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 8d29163568a7..3c36f3f5e688 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -503,6 +503,14 @@ void iounmap(volatile void __iomem *addr)
}
EXPORT_SYMBOL(iounmap);
+void *arch_memremap_wb(phys_addr_t phys_addr, size_t size, unsigned long flags)
+{
+ if (flags & MEMREMAP_DEC)
+ return (void __force *)ioremap_cache(phys_addr, size);
+
+ return (void __force *)ioremap_encrypted(phys_addr, size);
+}
+
/*
* Convert a physical pointer to a virtual kernel pointer for /dev/mem
* access
--
2.45.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [tip: x86/urgent] x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default
2025-01-13 13:14 ` [PATCHv3 2/2] x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default Kirill A. Shutemov
@ 2025-01-13 18:41 ` tip-bot2 for Kirill A. Shutemov
2025-01-13 20:47 ` [PATCHv3 2/2] " Tom Lendacky
1 sibling, 0 replies; 14+ messages in thread
From: tip-bot2 for Kirill A. Shutemov @ 2025-01-13 18:41 UTC (permalink / raw)
To: linux-tip-commits
Cc: Kirill A. Shutemov, Borislav Petkov (AMD), stable, #, 6.11+, x86,
linux-kernel
The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: 95093e066cfdd18271619248b569c26cfc8fa024
Gitweb: https://git.kernel.org/tip/95093e066cfdd18271619248b569c26cfc8fa024
Author: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
AuthorDate: Mon, 13 Jan 2025 15:14:59 +02:00
Committer: Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Mon, 13 Jan 2025 15:49:12 +01:00
x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default
Currently memremap(MEMREMAP_WB) can produce decrypted/shared mapping:
memremap(MEMREMAP_WB)
arch_memremap_wb()
ioremap_cache()
__ioremap_caller(.encrytped = false)
In such cases, the IORES_MAP_ENCRYPTED flag on the memory will determine
if the resulting mapping is encrypted or decrypted.
Creating a decrypted mapping without explicit request from the caller is
risky:
- It can inadvertently expose the guest's data and compromise the
guest.
- Accessing private memory via shared/decrypted mapping on TDX will
either trigger implicit conversion to shared or #VE (depending on
VMM implementation).
Implicit conversion is destructive: subsequent access to the same
memory via private mapping will trigger a hard-to-debug #VE crash.
The kernel already provides a way to request decrypted mapping
explicitly via the MEMREMAP_DEC flag.
Modify memremap(MEMREMAP_WB) to produce encrypted/private mapping by
default unless MEMREMAP_DEC is specified.
Fix a crash due to #VE on kexec in TDX guests if CONFIG_EISA is enabled.
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Cc: stable@vger.kernel.org # 6.11+
Link: https://lore.kernel.org/r/20250113131459.2008123-3-kirill.shutemov@linux.intel.com
---
arch/x86/include/asm/io.h | 3 +++
arch/x86/mm/ioremap.c | 8 ++++++++
2 files changed, 11 insertions(+)
diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index ed580c7..1a0dc2b 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -175,6 +175,9 @@ extern void __iomem *ioremap_prot(resource_size_t offset, unsigned long size, un
extern void __iomem *ioremap_encrypted(resource_size_t phys_addr, unsigned long size);
#define ioremap_encrypted ioremap_encrypted
+void *arch_memremap_wb(phys_addr_t phys_addr, size_t size, unsigned long flags);
+#define arch_memremap_wb arch_memremap_wb
+
/**
* ioremap - map bus memory into CPU space
* @offset: bus address of the memory
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 8d29163..3c36f3f 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -503,6 +503,14 @@ void iounmap(volatile void __iomem *addr)
}
EXPORT_SYMBOL(iounmap);
+void *arch_memremap_wb(phys_addr_t phys_addr, size_t size, unsigned long flags)
+{
+ if (flags & MEMREMAP_DEC)
+ return (void __force *)ioremap_cache(phys_addr, size);
+
+ return (void __force *)ioremap_encrypted(phys_addr, size);
+}
+
/*
* Convert a physical pointer to a virtual kernel pointer for /dev/mem
* access
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [tip: x86/urgent] memremap: Pass down MEMREMAP_* flags to arch_memremap_wb()
2025-01-13 13:14 ` [PATCHv3 1/2] memremap: Pass down MEMREMAP_* flags to arch_memremap_wb() Kirill A. Shutemov
@ 2025-01-13 18:42 ` tip-bot2 for Kirill A. Shutemov
0 siblings, 0 replies; 14+ messages in thread
From: tip-bot2 for Kirill A. Shutemov @ 2025-01-13 18:42 UTC (permalink / raw)
To: linux-tip-commits
Cc: Kirill A. Shutemov, Borislav Petkov (AMD), stable, #, 6.11+, x86,
linux-kernel
The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: 3663155bfb47855685ae5f0488117dcbe503eeac
Gitweb: https://git.kernel.org/tip/3663155bfb47855685ae5f0488117dcbe503eeac
Author: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
AuthorDate: Mon, 13 Jan 2025 15:14:58 +02:00
Committer: Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Mon, 13 Jan 2025 15:15:13 +01:00
memremap: Pass down MEMREMAP_* flags to arch_memremap_wb()
The x86 version of arch_memremap_wb() needs the flags to decide if the mapping
has be encrypted or decrypted.
Pass down the flag to arch_memremap_wb(). All current implementations
ignore the argument.
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Cc: stable@vger.kernel.org # 6.11+
Link: https://lore.kernel.org/r/20250113131459.2008123-2-kirill.shutemov@linux.intel.com
---
arch/arm/include/asm/io.h | 2 +-
arch/arm/mm/ioremap.c | 2 +-
arch/arm/mm/nommu.c | 2 +-
arch/riscv/include/asm/io.h | 2 +-
kernel/iomem.c | 5 +++--
5 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index 1815748..bae5edf 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -381,7 +381,7 @@ void __iomem *ioremap_wc(resource_size_t res_cookie, size_t size);
void iounmap(volatile void __iomem *io_addr);
#define iounmap iounmap
-void *arch_memremap_wb(phys_addr_t phys_addr, size_t size);
+void *arch_memremap_wb(phys_addr_t phys_addr, size_t size, unsigned long flags);
#define arch_memremap_wb arch_memremap_wb
/*
diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index 89f1c97..748698e 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -436,7 +436,7 @@ void __arm_iomem_set_ro(void __iomem *ptr, size_t size)
set_memory_ro((unsigned long)ptr, PAGE_ALIGN(size) / PAGE_SIZE);
}
-void *arch_memremap_wb(phys_addr_t phys_addr, size_t size)
+void *arch_memremap_wb(phys_addr_t phys_addr, size_t size, unsigned long flags)
{
return (__force void *)arch_ioremap_caller(phys_addr, size,
MT_MEMORY_RW,
diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
index c415f38..279641f 100644
--- a/arch/arm/mm/nommu.c
+++ b/arch/arm/mm/nommu.c
@@ -251,7 +251,7 @@ void __iomem *pci_remap_cfgspace(resource_size_t res_cookie, size_t size)
EXPORT_SYMBOL_GPL(pci_remap_cfgspace);
#endif
-void *arch_memremap_wb(phys_addr_t phys_addr, size_t size)
+void *arch_memremap_wb(phys_addr_t phys_addr, size_t size, unsigned long flags)
{
return (void *)phys_addr;
}
diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
index 1c5c641..0257f4a 100644
--- a/arch/riscv/include/asm/io.h
+++ b/arch/riscv/include/asm/io.h
@@ -136,7 +136,7 @@ __io_writes_outs(outs, u64, q, __io_pbr(), __io_paw())
#include <asm-generic/io.h>
#ifdef CONFIG_MMU
-#define arch_memremap_wb(addr, size) \
+#define arch_memremap_wb(addr, size, flags) \
((__force void *)ioremap_prot((addr), (size), _PAGE_KERNEL))
#endif
diff --git a/kernel/iomem.c b/kernel/iomem.c
index dc21207..75e61c1 100644
--- a/kernel/iomem.c
+++ b/kernel/iomem.c
@@ -6,7 +6,8 @@
#include <linux/ioremap.h>
#ifndef arch_memremap_wb
-static void *arch_memremap_wb(resource_size_t offset, unsigned long size)
+static void *arch_memremap_wb(resource_size_t offset, unsigned long size,
+ unsigned long flags)
{
#ifdef ioremap_cache
return (__force void *)ioremap_cache(offset, size);
@@ -91,7 +92,7 @@ void *memremap(resource_size_t offset, size_t size, unsigned long flags)
if (is_ram == REGION_INTERSECTS)
addr = try_ram_remap(offset, size, flags);
if (!addr)
- addr = arch_memremap_wb(offset, size);
+ addr = arch_memremap_wb(offset, size, flags);
}
/*
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCHv3 2/2] x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default
2025-01-13 13:14 ` [PATCHv3 2/2] x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default Kirill A. Shutemov
2025-01-13 18:41 ` [tip: x86/urgent] " tip-bot2 for Kirill A. Shutemov
@ 2025-01-13 20:47 ` Tom Lendacky
2025-01-13 20:55 ` Borislav Petkov
2025-01-14 7:27 ` Kirill A. Shutemov
1 sibling, 2 replies; 14+ messages in thread
From: Tom Lendacky @ 2025-01-13 20:47 UTC (permalink / raw)
To: Kirill A. Shutemov, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Andy Lutomirski
Cc: Albert Ou, Alexei Starovoitov, Andrea Parri, Arnd Bergmann,
Daniel Borkmann, Eric Chan, Jason Gunthorpe, Kai Huang,
Kefeng Wang, Kent Overstreet, Palmer Dabbelt, Paul Walmsley,
Russell King, Samuel Holland, Suren Baghdasaryan, Yuntao Wang,
linux-arm-kernel, linux-kernel, linux-riscv, stable, Ashish Kalra,
Maciej W. Rozycki
On 1/13/25 07:14, Kirill A. Shutemov wrote:
> Currently memremap(MEMREMAP_WB) can produce decrypted/shared mapping:
>
> memremap(MEMREMAP_WB)
> arch_memremap_wb()
> ioremap_cache()
> __ioremap_caller(.encrytped = false)
>
> In such cases, the IORES_MAP_ENCRYPTED flag on the memory will determine
> if the resulting mapping is encrypted or decrypted.
>
> Creating a decrypted mapping without explicit request from the caller is
> risky:
>
> - It can inadvertently expose the guest's data and compromise the
> guest.
>
> - Accessing private memory via shared/decrypted mapping on TDX will
> either trigger implicit conversion to shared or #VE (depending on
> VMM implementation).
>
> Implicit conversion is destructive: subsequent access to the same
> memory via private mapping will trigger a hard-to-debug #VE crash.
>
> The kernel already provides a way to request decrypted mapping
> explicitly via the MEMREMAP_DEC flag.
>
> Modify memremap(MEMREMAP_WB) to produce encrypted/private mapping by
> default unless MEMREMAP_DEC is specified.
>
> Fix the crash due to #VE on kexec in TDX guests if CONFIG_EISA is enabled.
This patch causes my bare-metal system to crash during boot when using
mem_encrypt=on:
[ 2.392934] efi: memattr: Entry type should be RuntimeServiceCode/Data
[ 2.393731] efi: memattr: ! 0x214c42f01f1162a-0xee70ac7bd1a9c629 [type=2028324321|attr=0x6590648fa4209879]
[ 2.394733] BUG: unable to handle page fault for address: ffffc900b4669017
[ 2.395729] #PF: supervisor read access in kernel mode
[ 2.395729] #PF: error_code(0x0000) - not-present page
[ 2.395729] PGD 8000100010067 P4D 8000100010067 PUD 0
[ 2.395729] Oops: Oops: 0000 [#1] PREEMPT SMP NOPTI
[ 2.395729] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 6.13.0-rc7-sos-testing #1
[ 2.395729] Hardware name: ...
[ 2.395729] RIP: 0010:efi_memattr_apply_permissions+0xa6/0x330
[ 2.395729] Code: 24 0f 48 8b 05 f3 30 a3 ff f6 c4 01 0f 85 66 02 00 00 31 db 4c 8d 6d 10 3b 5d 04 0f 83 4a 01 00 00 89 d8 0f af 45 08 4c 01 e8 <48> 8b 10 48 8b 70 08 4c 8b 40 18 48 89 54 24 10 48 8b 50 08 48 89
[ 2.395729] RSP: 0000:ffffffffb3803e18 EFLAGS: 00010296
[ 2.395729] RAX: ffffc900b4669017 RBX: 0000000000000001 RCX: 0000000000000000
[ 2.395729] RDX: 0000000000000000 RSI: ffffffffb3803cd8 RDI: 00000000ffffffff
[ 2.395729] RBP: ffffc900000b5018 R08: 00000000fffeffff R09: 0000000000000001
[ 2.395729] R10: 00000000fffeffff R11: ffff894048a80000 R12: ffffffffb434f1c0
[ 2.395729] R13: ffffc900000b5028 R14: ec5be84ccfb8b000 R15: ffffffffb3803e28
[ 2.395729] FS: 0000000000000000(0000) GS:ffff894049000000(0000) knlGS:0000000000000000
[ 2.395729] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 2.395729] CR2: ffffc900b4669017 CR3: 0008006f43832001 CR4: 0000000000770ef0
[ 2.395729] PKRU: 55555554
[ 2.395729] Call Trace:
[ 2.395729] <TASK>
[ 2.395729] ? __die+0x1f/0x60
[ 2.395729] ? page_fault_oops+0x80/0x150
[ 2.395729] ? exc_page_fault+0x15f/0x170
[ 2.395729] ? asm_exc_page_fault+0x22/0x30
[ 2.395729] ? __pfx_efi_update_mem_attr+0x10/0x10
[ 2.395729] ? efi_memattr_apply_permissions+0xa6/0x330
[ 2.395729] ? efi_memattr_apply_permissions+0x254/0x330
[ 2.395729] __efi_enter_virtual_mode+0x166/0x250
[ 2.395729] efi_enter_virtual_mode+0x2d/0x50
[ 2.395729] start_kernel+0x5d7/0x670
[ 2.395729] x86_64_start_reservations+0x14/0x30
[ 2.395729] x86_64_start_kernel+0x79/0x80
[ 2.395729] common_startup_64+0x13e/0x141
[ 2.395729] </TASK>
[ 2.395729] Modules linked in:
[ 2.395729] CR2: ffffc900b4669017
[ 2.395729] ---[ end trace 0000000000000000 ]---
[ 2.395729] RIP: 0010:efi_memattr_apply_permissions+0xa6/0x330
[ 2.395729] Code: 24 0f 48 8b 05 f3 30 a3 ff f6 c4 01 0f 85 66 02 00 00 31 db 4c 8d 6d 10 3b 5d 04 0f 83 4a 01 00 00 89 d8 0f af 45 08 4c 01 e8 <48> 8b 10 48 8b 70 08 4c 8b 40 18 48 89 54 24 10 48 8b 50 08 48 89
[ 2.395729] RSP: 0000:ffffffffb3803e18 EFLAGS: 00010296
[ 2.395729] RAX: ffffc900b4669017 RBX: 0000000000000001 RCX: 0000000000000000
[ 2.395729] RDX: 0000000000000000 RSI: ffffffffb3803cd8 RDI: 00000000ffffffff
[ 2.395729] RBP: ffffc900000b5018 R08: 00000000fffeffff R09: 0000000000000001
[ 2.395729] R10: 00000000fffeffff R11: ffff894048a80000 R12: ffffffffb434f1c0
[ 2.395729] R13: ffffc900000b5028 R14: ec5be84ccfb8b000 R15: ffffffffb3803e28
[ 2.395729] FS: 0000000000000000(0000) GS:ffff894049000000(0000) knlGS:0000000000000000
[ 2.395729] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 2.395729] CR2: ffffc900b4669017 CR3: 0008006f43832001 CR4: 0000000000770ef0
[ 2.395729] PKRU: 55555554
[ 2.395729] Kernel panic - not syncing: Fatal exception
[ 2.395729] ---[ end Kernel panic - not syncing: Fatal exception ]---
Thanks,
Tom
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: stable@vger.kernel.org # 6.11+
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Ashish Kalra <ashish.kalra@amd.com>
> Cc: "Maciej W. Rozycki" <macro@orcam.me.uk>
> ---
> arch/x86/include/asm/io.h | 3 +++
> arch/x86/mm/ioremap.c | 8 ++++++++
> 2 files changed, 11 insertions(+)
>
> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> index ed580c7f9d0a..1a0dc2b2bf5b 100644
> --- a/arch/x86/include/asm/io.h
> +++ b/arch/x86/include/asm/io.h
> @@ -175,6 +175,9 @@ extern void __iomem *ioremap_prot(resource_size_t offset, unsigned long size, un
> extern void __iomem *ioremap_encrypted(resource_size_t phys_addr, unsigned long size);
> #define ioremap_encrypted ioremap_encrypted
>
> +void *arch_memremap_wb(phys_addr_t phys_addr, size_t size, unsigned long flags);
> +#define arch_memremap_wb arch_memremap_wb
> +
> /**
> * ioremap - map bus memory into CPU space
> * @offset: bus address of the memory
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index 8d29163568a7..3c36f3f5e688 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -503,6 +503,14 @@ void iounmap(volatile void __iomem *addr)
> }
> EXPORT_SYMBOL(iounmap);
>
> +void *arch_memremap_wb(phys_addr_t phys_addr, size_t size, unsigned long flags)
> +{
> + if (flags & MEMREMAP_DEC)
> + return (void __force *)ioremap_cache(phys_addr, size);
> +
> + return (void __force *)ioremap_encrypted(phys_addr, size);
> +}
> +
> /*
> * Convert a physical pointer to a virtual kernel pointer for /dev/mem
> * access
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv3 2/2] x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default
2025-01-13 20:47 ` [PATCHv3 2/2] " Tom Lendacky
@ 2025-01-13 20:55 ` Borislav Petkov
2025-01-14 7:27 ` Kirill A. Shutemov
1 sibling, 0 replies; 14+ messages in thread
From: Borislav Petkov @ 2025-01-13 20:55 UTC (permalink / raw)
To: Tom Lendacky
Cc: Kirill A. Shutemov, Thomas Gleixner, Ingo Molnar, Dave Hansen,
x86, H. Peter Anvin, Andy Lutomirski, Albert Ou,
Alexei Starovoitov, Andrea Parri, Arnd Bergmann, Daniel Borkmann,
Eric Chan, Jason Gunthorpe, Kai Huang, Kefeng Wang,
Kent Overstreet, Palmer Dabbelt, Paul Walmsley, Russell King,
Samuel Holland, Suren Baghdasaryan, Yuntao Wang, linux-arm-kernel,
linux-kernel, linux-riscv, stable, Ashish Kalra,
Maciej W. Rozycki
On Mon, Jan 13, 2025 at 02:47:56PM -0600, Tom Lendacky wrote:
> This patch causes my bare-metal system to crash during boot when using
> mem_encrypt=on:
>
> [ 2.392934] efi: memattr: Entry type should be RuntimeServiceCode/Data
> [ 2.393731] efi: memattr: ! 0x214c42f01f1162a-0xee70ac7bd1a9c629 [type=2028324321|attr=0x6590648fa4209879]
> [ 2.394733] BUG: unable to handle page fault for address: ffffc900b4669017
A wild guess: looks like it tries to map EFI memory encrypted now...
Anyway, lemme zap. Those will have to go through the full motions of testing.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv3 2/2] x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default
2025-01-13 20:47 ` [PATCHv3 2/2] " Tom Lendacky
2025-01-13 20:55 ` Borislav Petkov
@ 2025-01-14 7:27 ` Kirill A. Shutemov
2025-01-14 14:33 ` Tom Lendacky
1 sibling, 1 reply; 14+ messages in thread
From: Kirill A. Shutemov @ 2025-01-14 7:27 UTC (permalink / raw)
To: Tom Lendacky
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski, Albert Ou, Alexei Starovoitov,
Andrea Parri, Arnd Bergmann, Daniel Borkmann, Eric Chan,
Jason Gunthorpe, Kai Huang, Kefeng Wang, Kent Overstreet,
Palmer Dabbelt, Paul Walmsley, Russell King, Samuel Holland,
Suren Baghdasaryan, Yuntao Wang, linux-arm-kernel, linux-kernel,
linux-riscv, stable, Ashish Kalra, Maciej W. Rozycki
On Mon, Jan 13, 2025 at 02:47:56PM -0600, Tom Lendacky wrote:
> On 1/13/25 07:14, Kirill A. Shutemov wrote:
> > Currently memremap(MEMREMAP_WB) can produce decrypted/shared mapping:
> >
> > memremap(MEMREMAP_WB)
> > arch_memremap_wb()
> > ioremap_cache()
> > __ioremap_caller(.encrytped = false)
> >
> > In such cases, the IORES_MAP_ENCRYPTED flag on the memory will determine
> > if the resulting mapping is encrypted or decrypted.
> >
> > Creating a decrypted mapping without explicit request from the caller is
> > risky:
> >
> > - It can inadvertently expose the guest's data and compromise the
> > guest.
> >
> > - Accessing private memory via shared/decrypted mapping on TDX will
> > either trigger implicit conversion to shared or #VE (depending on
> > VMM implementation).
> >
> > Implicit conversion is destructive: subsequent access to the same
> > memory via private mapping will trigger a hard-to-debug #VE crash.
> >
> > The kernel already provides a way to request decrypted mapping
> > explicitly via the MEMREMAP_DEC flag.
> >
> > Modify memremap(MEMREMAP_WB) to produce encrypted/private mapping by
> > default unless MEMREMAP_DEC is specified.
> >
> > Fix the crash due to #VE on kexec in TDX guests if CONFIG_EISA is enabled.
>
> This patch causes my bare-metal system to crash during boot when using
> mem_encrypt=on:
>
> [ 2.392934] efi: memattr: Entry type should be RuntimeServiceCode/Data
> [ 2.393731] efi: memattr: ! 0x214c42f01f1162a-0xee70ac7bd1a9c629 [type=2028324321|attr=0x6590648fa4209879]
Could you try if this helps?
diff --git a/drivers/firmware/efi/memattr.c b/drivers/firmware/efi/memattr.c
index c38b1a335590..b5051dcb7c1d 100644
--- a/drivers/firmware/efi/memattr.c
+++ b/drivers/firmware/efi/memattr.c
@@ -160,7 +160,7 @@ int __init efi_memattr_apply_permissions(struct mm_struct *mm,
if (WARN_ON(!efi_enabled(EFI_MEMMAP)))
return 0;
- tbl = memremap(efi_mem_attr_table, tbl_size, MEMREMAP_WB);
+ tbl = memremap(efi_mem_attr_table, tbl_size, MEMREMAP_WB | MEMREMAP_DEC);
if (!tbl) {
pr_err("Failed to map EFI Memory Attributes table @ 0x%lx\n",
efi_mem_attr_table);
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCHv3 2/2] x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default
2025-01-14 7:27 ` Kirill A. Shutemov
@ 2025-01-14 14:33 ` Tom Lendacky
2025-01-14 14:44 ` Kirill A. Shutemov
0 siblings, 1 reply; 14+ messages in thread
From: Tom Lendacky @ 2025-01-14 14:33 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski, Albert Ou, Alexei Starovoitov,
Andrea Parri, Arnd Bergmann, Daniel Borkmann, Eric Chan,
Jason Gunthorpe, Kai Huang, Kefeng Wang, Kent Overstreet,
Palmer Dabbelt, Paul Walmsley, Russell King, Samuel Holland,
Suren Baghdasaryan, Yuntao Wang, linux-arm-kernel, linux-kernel,
linux-riscv, stable, Ashish Kalra, Maciej W. Rozycki
On 1/14/25 01:27, Kirill A. Shutemov wrote:
> On Mon, Jan 13, 2025 at 02:47:56PM -0600, Tom Lendacky wrote:
>> On 1/13/25 07:14, Kirill A. Shutemov wrote:
>>> Currently memremap(MEMREMAP_WB) can produce decrypted/shared mapping:
>>>
>>> memremap(MEMREMAP_WB)
>>> arch_memremap_wb()
>>> ioremap_cache()
>>> __ioremap_caller(.encrytped = false)
>>>
>>> In such cases, the IORES_MAP_ENCRYPTED flag on the memory will determine
>>> if the resulting mapping is encrypted or decrypted.
>>>
>>> Creating a decrypted mapping without explicit request from the caller is
>>> risky:
>>>
>>> - It can inadvertently expose the guest's data and compromise the
>>> guest.
>>>
>>> - Accessing private memory via shared/decrypted mapping on TDX will
>>> either trigger implicit conversion to shared or #VE (depending on
>>> VMM implementation).
>>>
>>> Implicit conversion is destructive: subsequent access to the same
>>> memory via private mapping will trigger a hard-to-debug #VE crash.
>>>
>>> The kernel already provides a way to request decrypted mapping
>>> explicitly via the MEMREMAP_DEC flag.
>>>
>>> Modify memremap(MEMREMAP_WB) to produce encrypted/private mapping by
>>> default unless MEMREMAP_DEC is specified.
>>>
>>> Fix the crash due to #VE on kexec in TDX guests if CONFIG_EISA is enabled.
>>
>> This patch causes my bare-metal system to crash during boot when using
>> mem_encrypt=on:
>>
>> [ 2.392934] efi: memattr: Entry type should be RuntimeServiceCode/Data
>> [ 2.393731] efi: memattr: ! 0x214c42f01f1162a-0xee70ac7bd1a9c629 [type=2028324321|attr=0x6590648fa4209879]
>
> Could you try if this helps?
>
> diff --git a/drivers/firmware/efi/memattr.c b/drivers/firmware/efi/memattr.c
> index c38b1a335590..b5051dcb7c1d 100644
> --- a/drivers/firmware/efi/memattr.c
> +++ b/drivers/firmware/efi/memattr.c
> @@ -160,7 +160,7 @@ int __init efi_memattr_apply_permissions(struct mm_struct *mm,
> if (WARN_ON(!efi_enabled(EFI_MEMMAP)))
> return 0;
>
> - tbl = memremap(efi_mem_attr_table, tbl_size, MEMREMAP_WB);
> + tbl = memremap(efi_mem_attr_table, tbl_size, MEMREMAP_WB | MEMREMAP_DEC);
Well that would work for SME where EFI tables/data are not encrypted,
but will break for SEV where EFI tables/data are encrypted.
Thanks,
Tom
> if (!tbl) {
> pr_err("Failed to map EFI Memory Attributes table @ 0x%lx\n",
> efi_mem_attr_table);
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv3 2/2] x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default
2025-01-14 14:33 ` Tom Lendacky
@ 2025-01-14 14:44 ` Kirill A. Shutemov
2025-01-14 15:06 ` Tom Lendacky
0 siblings, 1 reply; 14+ messages in thread
From: Kirill A. Shutemov @ 2025-01-14 14:44 UTC (permalink / raw)
To: Tom Lendacky
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski, Albert Ou, Alexei Starovoitov,
Andrea Parri, Arnd Bergmann, Daniel Borkmann, Eric Chan,
Jason Gunthorpe, Kai Huang, Kefeng Wang, Kent Overstreet,
Palmer Dabbelt, Paul Walmsley, Russell King, Samuel Holland,
Suren Baghdasaryan, Yuntao Wang, linux-arm-kernel, linux-kernel,
linux-riscv, stable, Ashish Kalra, Maciej W. Rozycki
On Tue, Jan 14, 2025 at 08:33:39AM -0600, Tom Lendacky wrote:
> On 1/14/25 01:27, Kirill A. Shutemov wrote:
> > On Mon, Jan 13, 2025 at 02:47:56PM -0600, Tom Lendacky wrote:
> >> On 1/13/25 07:14, Kirill A. Shutemov wrote:
> >>> Currently memremap(MEMREMAP_WB) can produce decrypted/shared mapping:
> >>>
> >>> memremap(MEMREMAP_WB)
> >>> arch_memremap_wb()
> >>> ioremap_cache()
> >>> __ioremap_caller(.encrytped = false)
> >>>
> >>> In such cases, the IORES_MAP_ENCRYPTED flag on the memory will determine
> >>> if the resulting mapping is encrypted or decrypted.
> >>>
> >>> Creating a decrypted mapping without explicit request from the caller is
> >>> risky:
> >>>
> >>> - It can inadvertently expose the guest's data and compromise the
> >>> guest.
> >>>
> >>> - Accessing private memory via shared/decrypted mapping on TDX will
> >>> either trigger implicit conversion to shared or #VE (depending on
> >>> VMM implementation).
> >>>
> >>> Implicit conversion is destructive: subsequent access to the same
> >>> memory via private mapping will trigger a hard-to-debug #VE crash.
> >>>
> >>> The kernel already provides a way to request decrypted mapping
> >>> explicitly via the MEMREMAP_DEC flag.
> >>>
> >>> Modify memremap(MEMREMAP_WB) to produce encrypted/private mapping by
> >>> default unless MEMREMAP_DEC is specified.
> >>>
> >>> Fix the crash due to #VE on kexec in TDX guests if CONFIG_EISA is enabled.
> >>
> >> This patch causes my bare-metal system to crash during boot when using
> >> mem_encrypt=on:
> >>
> >> [ 2.392934] efi: memattr: Entry type should be RuntimeServiceCode/Data
> >> [ 2.393731] efi: memattr: ! 0x214c42f01f1162a-0xee70ac7bd1a9c629 [type=2028324321|attr=0x6590648fa4209879]
> >
> > Could you try if this helps?
> >
> > diff --git a/drivers/firmware/efi/memattr.c b/drivers/firmware/efi/memattr.c
> > index c38b1a335590..b5051dcb7c1d 100644
> > --- a/drivers/firmware/efi/memattr.c
> > +++ b/drivers/firmware/efi/memattr.c
> > @@ -160,7 +160,7 @@ int __init efi_memattr_apply_permissions(struct mm_struct *mm,
> > if (WARN_ON(!efi_enabled(EFI_MEMMAP)))
> > return 0;
> >
> > - tbl = memremap(efi_mem_attr_table, tbl_size, MEMREMAP_WB);
> > + tbl = memremap(efi_mem_attr_table, tbl_size, MEMREMAP_WB | MEMREMAP_DEC);
>
> Well that would work for SME where EFI tables/data are not encrypted,
> but will break for SEV where EFI tables/data are encrypted.
Hm. Why would it break for SEV? It brings the situation back to what it
was before the patch.
Note that that __ioremap_caller() would still check io_desc.flags before
mapping it as decrypted.
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv3 2/2] x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default
2025-01-14 14:44 ` Kirill A. Shutemov
@ 2025-01-14 15:06 ` Tom Lendacky
0 siblings, 0 replies; 14+ messages in thread
From: Tom Lendacky @ 2025-01-14 15:06 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski, Albert Ou, Alexei Starovoitov,
Andrea Parri, Arnd Bergmann, Daniel Borkmann, Eric Chan,
Jason Gunthorpe, Kai Huang, Kefeng Wang, Kent Overstreet,
Palmer Dabbelt, Paul Walmsley, Russell King, Samuel Holland,
Suren Baghdasaryan, Yuntao Wang, linux-arm-kernel, linux-kernel,
linux-riscv, stable, Ashish Kalra, Maciej W. Rozycki
On 1/14/25 08:44, Kirill A. Shutemov wrote:
> On Tue, Jan 14, 2025 at 08:33:39AM -0600, Tom Lendacky wrote:
>> On 1/14/25 01:27, Kirill A. Shutemov wrote:
>>> On Mon, Jan 13, 2025 at 02:47:56PM -0600, Tom Lendacky wrote:
>>>> On 1/13/25 07:14, Kirill A. Shutemov wrote:
>>>>> Currently memremap(MEMREMAP_WB) can produce decrypted/shared mapping:
>>>>>
>>>>> memremap(MEMREMAP_WB)
>>>>> arch_memremap_wb()
>>>>> ioremap_cache()
>>>>> __ioremap_caller(.encrytped = false)
>>>>>
>>>>> In such cases, the IORES_MAP_ENCRYPTED flag on the memory will determine
>>>>> if the resulting mapping is encrypted or decrypted.
>>>>>
>>>>> Creating a decrypted mapping without explicit request from the caller is
>>>>> risky:
>>>>>
>>>>> - It can inadvertently expose the guest's data and compromise the
>>>>> guest.
>>>>>
>>>>> - Accessing private memory via shared/decrypted mapping on TDX will
>>>>> either trigger implicit conversion to shared or #VE (depending on
>>>>> VMM implementation).
>>>>>
>>>>> Implicit conversion is destructive: subsequent access to the same
>>>>> memory via private mapping will trigger a hard-to-debug #VE crash.
>>>>>
>>>>> The kernel already provides a way to request decrypted mapping
>>>>> explicitly via the MEMREMAP_DEC flag.
>>>>>
>>>>> Modify memremap(MEMREMAP_WB) to produce encrypted/private mapping by
>>>>> default unless MEMREMAP_DEC is specified.
>>>>>
>>>>> Fix the crash due to #VE on kexec in TDX guests if CONFIG_EISA is enabled.
>>>>
>>>> This patch causes my bare-metal system to crash during boot when using
>>>> mem_encrypt=on:
>>>>
>>>> [ 2.392934] efi: memattr: Entry type should be RuntimeServiceCode/Data
>>>> [ 2.393731] efi: memattr: ! 0x214c42f01f1162a-0xee70ac7bd1a9c629 [type=2028324321|attr=0x6590648fa4209879]
>>>
>>> Could you try if this helps?
>>>
>>> diff --git a/drivers/firmware/efi/memattr.c b/drivers/firmware/efi/memattr.c
>>> index c38b1a335590..b5051dcb7c1d 100644
>>> --- a/drivers/firmware/efi/memattr.c
>>> +++ b/drivers/firmware/efi/memattr.c
>>> @@ -160,7 +160,7 @@ int __init efi_memattr_apply_permissions(struct mm_struct *mm,
>>> if (WARN_ON(!efi_enabled(EFI_MEMMAP)))
>>> return 0;
>>>
>>> - tbl = memremap(efi_mem_attr_table, tbl_size, MEMREMAP_WB);
>>> + tbl = memremap(efi_mem_attr_table, tbl_size, MEMREMAP_WB | MEMREMAP_DEC);
>>
>> Well that would work for SME where EFI tables/data are not encrypted,
>> but will break for SEV where EFI tables/data are encrypted.
>
> Hm. Why would it break for SEV? It brings the situation back to what it
> was before the patch.
Ah, true. I can try it and see how much further SME gets. Hopefully it
doesn't turn into a whack-a-mole thing.
Thanks,
Tom
>
> Note that that __ioremap_caller() would still check io_desc.flags before
> mapping it as decrypted.
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv3 2/2] x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default
@ 2025-01-14 16:54 Tom Lendacky
2025-01-15 10:23 ` Kirill A. Shutemov
0 siblings, 1 reply; 14+ messages in thread
From: Tom Lendacky @ 2025-01-14 16:54 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski, Albert Ou, Alexei Starovoitov,
Andrea Parri, Arnd Bergmann, Daniel Borkmann, Eric Chan,
Jason Gunthorpe, Kai Huang, Kefeng Wang, Kent Overstreet,
Palmer Dabbelt, Paul Walmsley, Russell King, Samuel Holland,
Suren Baghdasaryan, Yuntao Wang, linux-arm-kernel, linux-kernel,
linux-riscv, stable, Ashish Kalra, Maciej W. Rozycki
On 1/14/25 09:06, Tom Lendacky wrote:
> On 1/14/25 08:44, Kirill A. Shutemov wrote:
>> On Tue, Jan 14, 2025 at 08:33:39AM -0600, Tom Lendacky wrote:
>>> On 1/14/25 01:27, Kirill A. Shutemov wrote:
>>>> On Mon, Jan 13, 2025 at 02:47:56PM -0600, Tom Lendacky wrote:
>>>>> On 1/13/25 07:14, Kirill A. Shutemov wrote:
>>>>>> Currently memremap(MEMREMAP_WB) can produce decrypted/shared mapping:
>>>>>>
>>>>>> memremap(MEMREMAP_WB)
>>>>>> arch_memremap_wb()
>>>>>> ioremap_cache()
>>>>>> __ioremap_caller(.encrytped = false)
>>>>>>
>>>>>> In such cases, the IORES_MAP_ENCRYPTED flag on the memory will determine
>>>>>> if the resulting mapping is encrypted or decrypted.
>>>>>>
>>>>>> Creating a decrypted mapping without explicit request from the caller is
>>>>>> risky:
>>>>>>
>>>>>> - It can inadvertently expose the guest's data and compromise the
>>>>>> guest.
>>>>>>
>>>>>> - Accessing private memory via shared/decrypted mapping on TDX will
>>>>>> either trigger implicit conversion to shared or #VE (depending on
>>>>>> VMM implementation).
>>>>>>
>>>>>> Implicit conversion is destructive: subsequent access to the same
>>>>>> memory via private mapping will trigger a hard-to-debug #VE crash.
>>>>>>
>>>>>> The kernel already provides a way to request decrypted mapping
>>>>>> explicitly via the MEMREMAP_DEC flag.
>>>>>>
>>>>>> Modify memremap(MEMREMAP_WB) to produce encrypted/private mapping by
>>>>>> default unless MEMREMAP_DEC is specified.
>>>>>>
>>>>>> Fix the crash due to #VE on kexec in TDX guests if CONFIG_EISA is enabled.
>>>>>
>>>>> This patch causes my bare-metal system to crash during boot when using
>>>>> mem_encrypt=on:
>>>>>
>>>>> [ 2.392934] efi: memattr: Entry type should be RuntimeServiceCode/Data
>>>>> [ 2.393731] efi: memattr: ! 0x214c42f01f1162a-0xee70ac7bd1a9c629 [type=2028324321|attr=0x6590648fa4209879]
>>>>
>>>> Could you try if this helps?
>>>>
>>>> diff --git a/drivers/firmware/efi/memattr.c b/drivers/firmware/efi/memattr.c
>>>> index c38b1a335590..b5051dcb7c1d 100644
>>>> --- a/drivers/firmware/efi/memattr.c
>>>> +++ b/drivers/firmware/efi/memattr.c
>>>> @@ -160,7 +160,7 @@ int __init efi_memattr_apply_permissions(struct mm_struct *mm,
>>>> if (WARN_ON(!efi_enabled(EFI_MEMMAP)))
>>>> return 0;
>>>>
>>>> - tbl = memremap(efi_mem_attr_table, tbl_size, MEMREMAP_WB);
>>>> + tbl = memremap(efi_mem_attr_table, tbl_size, MEMREMAP_WB | MEMREMAP_DEC);
>>>
>>> Well that would work for SME where EFI tables/data are not encrypted,
>>> but will break for SEV where EFI tables/data are encrypted.
>>
>> Hm. Why would it break for SEV? It brings the situation back to what it
>> was before the patch.
>
> Ah, true. I can try it and see how much further SME gets. Hopefully it
> doesn't turn into a whack-a-mole thing.
Unfortunately, it is turning into a whack-a-mole thing.
But it looks the following works for SME:
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 3c36f3f5e688..ff3cd5fc8508 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -505,7 +505,7 @@ EXPORT_SYMBOL(iounmap);
void *arch_memremap_wb(phys_addr_t phys_addr, size_t size, unsigned long flags)
{
- if (flags & MEMREMAP_DEC)
+ if (flags & MEMREMAP_DEC || cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT))
return (void __force *)ioremap_cache(phys_addr, size);
return (void __force *)ioremap_encrypted(phys_addr, size);
I haven't had a chance to test the series on SEV, yet.
Thanks,
Tom
>
> Thanks,
> Tom
>
>>
>> Note that that __ioremap_caller() would still check io_desc.flags before
>> mapping it as decrypted.
>>
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCHv3 2/2] x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default
2025-01-14 16:54 Tom Lendacky
@ 2025-01-15 10:23 ` Kirill A. Shutemov
2025-01-15 10:34 ` Borislav Petkov
0 siblings, 1 reply; 14+ messages in thread
From: Kirill A. Shutemov @ 2025-01-15 10:23 UTC (permalink / raw)
To: Tom Lendacky
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski, Albert Ou, Alexei Starovoitov,
Andrea Parri, Arnd Bergmann, Daniel Borkmann, Eric Chan,
Jason Gunthorpe, Kai Huang, Kefeng Wang, Kent Overstreet,
Palmer Dabbelt, Paul Walmsley, Russell King, Samuel Holland,
Suren Baghdasaryan, Yuntao Wang, linux-arm-kernel, linux-kernel,
linux-riscv, stable, Ashish Kalra, Maciej W. Rozycki
On Tue, Jan 14, 2025 at 10:54:53AM -0600, Tom Lendacky wrote:
> On 1/14/25 09:06, Tom Lendacky wrote:
> > On 1/14/25 08:44, Kirill A. Shutemov wrote:
> >> On Tue, Jan 14, 2025 at 08:33:39AM -0600, Tom Lendacky wrote:
> >>> On 1/14/25 01:27, Kirill A. Shutemov wrote:
> >>>> On Mon, Jan 13, 2025 at 02:47:56PM -0600, Tom Lendacky wrote:
> >>>>> On 1/13/25 07:14, Kirill A. Shutemov wrote:
> >>>>>> Currently memremap(MEMREMAP_WB) can produce decrypted/shared mapping:
> >>>>>>
> >>>>>> memremap(MEMREMAP_WB)
> >>>>>> arch_memremap_wb()
> >>>>>> ioremap_cache()
> >>>>>> __ioremap_caller(.encrytped = false)
> >>>>>>
> >>>>>> In such cases, the IORES_MAP_ENCRYPTED flag on the memory will determine
> >>>>>> if the resulting mapping is encrypted or decrypted.
> >>>>>>
> >>>>>> Creating a decrypted mapping without explicit request from the caller is
> >>>>>> risky:
> >>>>>>
> >>>>>> - It can inadvertently expose the guest's data and compromise the
> >>>>>> guest.
> >>>>>>
> >>>>>> - Accessing private memory via shared/decrypted mapping on TDX will
> >>>>>> either trigger implicit conversion to shared or #VE (depending on
> >>>>>> VMM implementation).
> >>>>>>
> >>>>>> Implicit conversion is destructive: subsequent access to the same
> >>>>>> memory via private mapping will trigger a hard-to-debug #VE crash.
> >>>>>>
> >>>>>> The kernel already provides a way to request decrypted mapping
> >>>>>> explicitly via the MEMREMAP_DEC flag.
> >>>>>>
> >>>>>> Modify memremap(MEMREMAP_WB) to produce encrypted/private mapping by
> >>>>>> default unless MEMREMAP_DEC is specified.
> >>>>>>
> >>>>>> Fix the crash due to #VE on kexec in TDX guests if CONFIG_EISA is enabled.
> >>>>>
> >>>>> This patch causes my bare-metal system to crash during boot when using
> >>>>> mem_encrypt=on:
> >>>>>
> >>>>> [ 2.392934] efi: memattr: Entry type should be RuntimeServiceCode/Data
> >>>>> [ 2.393731] efi: memattr: ! 0x214c42f01f1162a-0xee70ac7bd1a9c629 [type=2028324321|attr=0x6590648fa4209879]
> >>>>
> >>>> Could you try if this helps?
> >>>>
> >>>> diff --git a/drivers/firmware/efi/memattr.c b/drivers/firmware/efi/memattr.c
> >>>> index c38b1a335590..b5051dcb7c1d 100644
> >>>> --- a/drivers/firmware/efi/memattr.c
> >>>> +++ b/drivers/firmware/efi/memattr.c
> >>>> @@ -160,7 +160,7 @@ int __init efi_memattr_apply_permissions(struct mm_struct *mm,
> >>>> if (WARN_ON(!efi_enabled(EFI_MEMMAP)))
> >>>> return 0;
> >>>>
> >>>> - tbl = memremap(efi_mem_attr_table, tbl_size, MEMREMAP_WB);
> >>>> + tbl = memremap(efi_mem_attr_table, tbl_size, MEMREMAP_WB | MEMREMAP_DEC);
> >>>
> >>> Well that would work for SME where EFI tables/data are not encrypted,
> >>> but will break for SEV where EFI tables/data are encrypted.
> >>
> >> Hm. Why would it break for SEV? It brings the situation back to what it
> >> was before the patch.
> >
> > Ah, true. I can try it and see how much further SME gets. Hopefully it
> > doesn't turn into a whack-a-mole thing.
>
> Unfortunately, it is turning into a whack-a-mole thing.
>
> But it looks the following works for SME:
>
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index 3c36f3f5e688..ff3cd5fc8508 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -505,7 +505,7 @@ EXPORT_SYMBOL(iounmap);
>
> void *arch_memremap_wb(phys_addr_t phys_addr, size_t size, unsigned long flags)
> {
> - if (flags & MEMREMAP_DEC)
> + if (flags & MEMREMAP_DEC || cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT))
> return (void __force *)ioremap_cache(phys_addr, size);
>
> return (void __force *)ioremap_encrypted(phys_addr, size);
>
>
> I haven't had a chance to test the series on SEV, yet.
Please do.
I am okay with the change above. Borislav, is it acceptable direction for
you?
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv3 2/2] x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default
2025-01-15 10:23 ` Kirill A. Shutemov
@ 2025-01-15 10:34 ` Borislav Petkov
0 siblings, 0 replies; 14+ messages in thread
From: Borislav Petkov @ 2025-01-15 10:34 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Tom Lendacky, Thomas Gleixner, Ingo Molnar, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski, Albert Ou, Alexei Starovoitov,
Andrea Parri, Arnd Bergmann, Daniel Borkmann, Eric Chan,
Jason Gunthorpe, Kai Huang, Kefeng Wang, Kent Overstreet,
Palmer Dabbelt, Paul Walmsley, Russell King, Samuel Holland,
Suren Baghdasaryan, Yuntao Wang, linux-arm-kernel, linux-kernel,
linux-riscv, stable, Ashish Kalra, Maciej W. Rozycki
On Wed, Jan 15, 2025 at 12:23:54PM +0200, Kirill A. Shutemov wrote:
> I am okay with the change above. Borislav, is it acceptable direction for
> you?
Yes, Tom and I have been talking offlist about making the *map code figure out
itself what type of encryption setting it should use, based on the platform.
It'll need a proper analysis, though, what all the possible usages are to see
whether that scheme would be adequeate.
Something to experiment with after the merge window.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2025-01-15 10:35 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-13 13:14 [PATCHv3 0/2] x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default Kirill A. Shutemov
2025-01-13 13:14 ` [PATCHv3 1/2] memremap: Pass down MEMREMAP_* flags to arch_memremap_wb() Kirill A. Shutemov
2025-01-13 18:42 ` [tip: x86/urgent] " tip-bot2 for Kirill A. Shutemov
2025-01-13 13:14 ` [PATCHv3 2/2] x86/mm: Make memremap(MEMREMAP_WB) map memory as encrypted by default Kirill A. Shutemov
2025-01-13 18:41 ` [tip: x86/urgent] " tip-bot2 for Kirill A. Shutemov
2025-01-13 20:47 ` [PATCHv3 2/2] " Tom Lendacky
2025-01-13 20:55 ` Borislav Petkov
2025-01-14 7:27 ` Kirill A. Shutemov
2025-01-14 14:33 ` Tom Lendacky
2025-01-14 14:44 ` Kirill A. Shutemov
2025-01-14 15:06 ` Tom Lendacky
-- strict thread matches above, loose matches on Subject: below --
2025-01-14 16:54 Tom Lendacky
2025-01-15 10:23 ` Kirill A. Shutemov
2025-01-15 10:34 ` Borislav Petkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox