| Message ID | 20170418211941.10190.19751.stgit@tlendack-t1.amdoffice.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 18, 2017 at 04:19:42PM -0500, Tom Lendacky wrote: > Persistent memory is expected to persist across reboots. The encryption > key used by SME will change across reboots which will result in corrupted > persistent memory. Persistent memory is handed out by block devices > through memory remapping functions, so be sure not to map this memory as > encrypted. > > Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> > --- > arch/x86/mm/ioremap.c | 31 ++++++++++++++++++++++++++++++- > 1 file changed, 30 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c > index bce0604..55317ba 100644 > --- a/arch/x86/mm/ioremap.c > +++ b/arch/x86/mm/ioremap.c > @@ -425,17 +425,46 @@ void unxlate_dev_mem_ptr(phys_addr_t phys, void *addr) > * Examine the physical address to determine if it is an area of memory > * that should be mapped decrypted. If the memory is not part of the > * kernel usable area it was accessed and created decrypted, so these > - * areas should be mapped decrypted. > + * areas should be mapped decrypted. And since the encryption key can > + * change across reboots, persistent memory should also be mapped > + * decrypted. > */ > static bool memremap_should_map_decrypted(resource_size_t phys_addr, > unsigned long size) > { > + int is_pmem; > + > + /* > + * Check if the address is part of a persistent memory region. > + * This check covers areas added by E820, EFI and ACPI. > + */ > + is_pmem = region_intersects(phys_addr, size, IORESOURCE_MEM, > + IORES_DESC_PERSISTENT_MEMORY); > + if (is_pmem != REGION_DISJOINT) > + return true; > + > + /* > + * Check if the non-volatile attribute is set for an EFI > + * reserved area. > + */ > + if (efi_enabled(EFI_BOOT)) { > + switch (efi_mem_type(phys_addr)) { > + case EFI_RESERVED_TYPE: > + if (efi_mem_attributes(phys_addr) & EFI_MEMORY_NV) > + return true; > + break; > + default: > + break; > + } > + } > + > /* Check if the address is outside kernel usable area */ > switch (e820__get_entry_type(phys_addr, phys_addr + size - 1)) { > case E820_TYPE_RESERVED: > case E820_TYPE_ACPI: > case E820_TYPE_NVS: > case E820_TYPE_UNUSABLE: > + case E820_TYPE_PRAM: Can't you simply add: case E820_TYPE_PMEM: here too and thus get rid of the region_intersects() thing above? Because, for example, e820_type_to_iores_desc() maps E820_TYPE_PMEM to IORES_DESC_PERSISTENT_MEMORY so those should be equivalent...
On 5/16/2017 9:04 AM, Borislav Petkov wrote: > On Tue, Apr 18, 2017 at 04:19:42PM -0500, Tom Lendacky wrote: >> Persistent memory is expected to persist across reboots. The encryption >> key used by SME will change across reboots which will result in corrupted >> persistent memory. Persistent memory is handed out by block devices >> through memory remapping functions, so be sure not to map this memory as >> encrypted. >> >> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> >> --- >> arch/x86/mm/ioremap.c | 31 ++++++++++++++++++++++++++++++- >> 1 file changed, 30 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c >> index bce0604..55317ba 100644 >> --- a/arch/x86/mm/ioremap.c >> +++ b/arch/x86/mm/ioremap.c >> @@ -425,17 +425,46 @@ void unxlate_dev_mem_ptr(phys_addr_t phys, void *addr) >> * Examine the physical address to determine if it is an area of memory >> * that should be mapped decrypted. If the memory is not part of the >> * kernel usable area it was accessed and created decrypted, so these >> - * areas should be mapped decrypted. >> + * areas should be mapped decrypted. And since the encryption key can >> + * change across reboots, persistent memory should also be mapped >> + * decrypted. >> */ >> static bool memremap_should_map_decrypted(resource_size_t phys_addr, >> unsigned long size) >> { >> + int is_pmem; >> + >> + /* >> + * Check if the address is part of a persistent memory region. >> + * This check covers areas added by E820, EFI and ACPI. >> + */ >> + is_pmem = region_intersects(phys_addr, size, IORESOURCE_MEM, >> + IORES_DESC_PERSISTENT_MEMORY); >> + if (is_pmem != REGION_DISJOINT) >> + return true; >> + >> + /* >> + * Check if the non-volatile attribute is set for an EFI >> + * reserved area. >> + */ >> + if (efi_enabled(EFI_BOOT)) { >> + switch (efi_mem_type(phys_addr)) { >> + case EFI_RESERVED_TYPE: >> + if (efi_mem_attributes(phys_addr) & EFI_MEMORY_NV) >> + return true; >> + break; >> + default: >> + break; >> + } >> + } >> + >> /* Check if the address is outside kernel usable area */ >> switch (e820__get_entry_type(phys_addr, phys_addr + size - 1)) { >> case E820_TYPE_RESERVED: >> case E820_TYPE_ACPI: >> case E820_TYPE_NVS: >> case E820_TYPE_UNUSABLE: >> + case E820_TYPE_PRAM: > > Can't you simply add: > > case E820_TYPE_PMEM: > > here too and thus get rid of the region_intersects() thing above? > > Because, for example, e820_type_to_iores_desc() maps E820_TYPE_PMEM to > IORES_DESC_PERSISTENT_MEMORY so those should be equivalent... I'll have to double-check this, but I believe that when persistent memory is identified through the NFIT table it adds it as a resource but doesn't add it as an e820 entry so I can't rely on the type being returned as E820_TYPE_PMEM by e820__get_entry_type(). Thanks, Tom >
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c index bce0604..55317ba 100644 --- a/arch/x86/mm/ioremap.c +++ b/arch/x86/mm/ioremap.c @@ -425,17 +425,46 @@ void unxlate_dev_mem_ptr(phys_addr_t phys, void *addr) * Examine the physical address to determine if it is an area of memory * that should be mapped decrypted. If the memory is not part of the * kernel usable area it was accessed and created decrypted, so these - * areas should be mapped decrypted. + * areas should be mapped decrypted. And since the encryption key can + * change across reboots, persistent memory should also be mapped + * decrypted. */ static bool memremap_should_map_decrypted(resource_size_t phys_addr, unsigned long size) { + int is_pmem; + + /* + * Check if the address is part of a persistent memory region. + * This check covers areas added by E820, EFI and ACPI. + */ + is_pmem = region_intersects(phys_addr, size, IORESOURCE_MEM, + IORES_DESC_PERSISTENT_MEMORY); + if (is_pmem != REGION_DISJOINT) + return true; + + /* + * Check if the non-volatile attribute is set for an EFI + * reserved area. + */ + if (efi_enabled(EFI_BOOT)) { + switch (efi_mem_type(phys_addr)) { + case EFI_RESERVED_TYPE: + if (efi_mem_attributes(phys_addr) & EFI_MEMORY_NV) + return true; + break; + default: + break; + } + } + /* Check if the address is outside kernel usable area */ switch (e820__get_entry_type(phys_addr, phys_addr + size - 1)) { case E820_TYPE_RESERVED: case E820_TYPE_ACPI: case E820_TYPE_NVS: case E820_TYPE_UNUSABLE: + case E820_TYPE_PRAM: return true; default: break;
Persistent memory is expected to persist across reboots. The encryption key used by SME will change across reboots which will result in corrupted persistent memory. Persistent memory is handed out by block devices through memory remapping functions, so be sure not to map this memory as encrypted. Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> --- arch/x86/mm/ioremap.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)