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
