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. >
