| Message ID | 20170724190757.11278-7-brijesh.singh@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jul 24, 2017 at 02:07:46PM -0500, Brijesh Singh wrote: > From: Tom Lendacky <thomas.lendacky@amd.com> > > When Secure Encrypted Virtualization (SEV) is active, boot data (such as > EFI related data, setup data) is encrypted and needs to be accessed as > such when mapped. Update the architecture override in early_memremap to > keep the encryption attribute when mapping this data. > > Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > --- > arch/x86/mm/ioremap.c | 44 ++++++++++++++++++++++++++++++++------------ > 1 file changed, 32 insertions(+), 12 deletions(-) ... > @@ -590,10 +598,15 @@ bool arch_memremap_can_ram_remap(resource_size_t phys_addr, unsigned long size, > if (flags & MEMREMAP_DEC) > return false; > > - if (memremap_is_setup_data(phys_addr, size) || > - memremap_is_efi_data(phys_addr, size) || > - memremap_should_map_decrypted(phys_addr, size)) > - return false; > + if (sme_active()) { > + if (memremap_is_setup_data(phys_addr, size) || > + memremap_is_efi_data(phys_addr, size) || > + memremap_should_map_decrypted(phys_addr, size)) > + return false; > + } else if (sev_active()) { > + if (memremap_should_map_decrypted(phys_addr, size)) > + return false; > + } > > return true; > } I guess this function's hind part can be simplified to: if (sme_active()) { if (memremap_is_setup_data(phys_addr, size) || memremap_is_efi_data(phys_addr, size)) return false; } return ! memremap_should_map_decrypted(phys_addr, size); } > @@ -608,15 +621,22 @@ pgprot_t __init early_memremap_pgprot_adjust(resource_size_t phys_addr, > unsigned long size, > pgprot_t prot) And this one in a similar manner... > { > - if (!sme_active()) > + if (!sme_active() && !sev_active()) > return prot; ... and you don't need that check... > - if (early_memremap_is_setup_data(phys_addr, size) || > - memremap_is_efi_data(phys_addr, size) || > - memremap_should_map_decrypted(phys_addr, size)) > - prot = pgprot_decrypted(prot); > - else > - prot = pgprot_encrypted(prot); > + if (sme_active()) { ... if you're going to do it here too. > + if (early_memremap_is_setup_data(phys_addr, size) || > + memremap_is_efi_data(phys_addr, size) || > + memremap_should_map_decrypted(phys_addr, size)) > + prot = pgprot_decrypted(prot); > + else > + prot = pgprot_encrypted(prot); > + } else if (sev_active()) { And here. > + if (memremap_should_map_decrypted(phys_addr, size)) > + prot = pgprot_decrypted(prot); > + else > + prot = pgprot_encrypted(prot); > + }
On 7/27/2017 8:31 AM, Borislav Petkov wrote: > On Mon, Jul 24, 2017 at 02:07:46PM -0500, Brijesh Singh wrote: >> From: Tom Lendacky <thomas.lendacky@amd.com> >> >> When Secure Encrypted Virtualization (SEV) is active, boot data (such as >> EFI related data, setup data) is encrypted and needs to be accessed as >> such when mapped. Update the architecture override in early_memremap to >> keep the encryption attribute when mapping this data. >> >> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> >> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> >> --- >> arch/x86/mm/ioremap.c | 44 ++++++++++++++++++++++++++++++++------------ >> 1 file changed, 32 insertions(+), 12 deletions(-) > > ... > >> @@ -590,10 +598,15 @@ bool arch_memremap_can_ram_remap(resource_size_t phys_addr, unsigned long size, >> if (flags & MEMREMAP_DEC) >> return false; >> >> - if (memremap_is_setup_data(phys_addr, size) || >> - memremap_is_efi_data(phys_addr, size) || >> - memremap_should_map_decrypted(phys_addr, size)) >> - return false; >> + if (sme_active()) { >> + if (memremap_is_setup_data(phys_addr, size) || >> + memremap_is_efi_data(phys_addr, size) || >> + memremap_should_map_decrypted(phys_addr, size)) >> + return false; >> + } else if (sev_active()) { >> + if (memremap_should_map_decrypted(phys_addr, size)) >> + return false; >> + } >> >> return true; >> } > > I guess this function's hind part can be simplified to: > > if (sme_active()) { > if (memremap_is_setup_data(phys_addr, size) || > memremap_is_efi_data(phys_addr, size)) > return false; > } > > return ! memremap_should_map_decrypted(phys_addr, size); > } > Ok, definitely cleaner. >> @@ -608,15 +621,22 @@ pgprot_t __init early_memremap_pgprot_adjust(resource_size_t phys_addr, >> unsigned long size, >> pgprot_t prot) > > And this one in a similar manner... > >> { >> - if (!sme_active()) >> + if (!sme_active() && !sev_active()) >> return prot; > > ... and you don't need that check... > >> - if (early_memremap_is_setup_data(phys_addr, size) || >> - memremap_is_efi_data(phys_addr, size) || >> - memremap_should_map_decrypted(phys_addr, size)) >> - prot = pgprot_decrypted(prot); >> - else >> - prot = pgprot_encrypted(prot); >> + if (sme_active()) { > > ... if you're going to do it here too. > >> + if (early_memremap_is_setup_data(phys_addr, size) || >> + memremap_is_efi_data(phys_addr, size) || >> + memremap_should_map_decrypted(phys_addr, size)) >> + prot = pgprot_decrypted(prot); >> + else >> + prot = pgprot_encrypted(prot); >> + } else if (sev_active()) { > > And here. Will do. Thanks, Tom > >> + if (memremap_should_map_decrypted(phys_addr, size)) >> + prot = pgprot_decrypted(prot); >> + else >> + prot = pgprot_encrypted(prot); >> + } >
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c index 34f0e18..c0be7cf 100644 --- a/arch/x86/mm/ioremap.c +++ b/arch/x86/mm/ioremap.c @@ -422,6 +422,9 @@ void unxlate_dev_mem_ptr(phys_addr_t phys, void *addr) * areas should be mapped decrypted. And since the encryption key can * change across reboots, persistent memory should also be mapped * decrypted. + * + * If SEV is active, that implies that BIOS/UEFI also ran encrypted so + * only persistent memory should be mapped decrypted. */ static bool memremap_should_map_decrypted(resource_size_t phys_addr, unsigned long size) @@ -458,6 +461,11 @@ static bool memremap_should_map_decrypted(resource_size_t phys_addr, case E820_TYPE_ACPI: case E820_TYPE_NVS: case E820_TYPE_UNUSABLE: + /* For SEV, these areas are encrypted */ + if (sev_active()) + break; + /* Fallthrough */ + case E820_TYPE_PRAM: return true; default: @@ -581,7 +589,7 @@ static bool __init early_memremap_is_setup_data(resource_size_t phys_addr, bool arch_memremap_can_ram_remap(resource_size_t phys_addr, unsigned long size, unsigned long flags) { - if (!sme_active()) + if (!sme_active() && !sev_active()) return true; if (flags & MEMREMAP_ENC) @@ -590,10 +598,15 @@ bool arch_memremap_can_ram_remap(resource_size_t phys_addr, unsigned long size, if (flags & MEMREMAP_DEC) return false; - if (memremap_is_setup_data(phys_addr, size) || - memremap_is_efi_data(phys_addr, size) || - memremap_should_map_decrypted(phys_addr, size)) - return false; + if (sme_active()) { + if (memremap_is_setup_data(phys_addr, size) || + memremap_is_efi_data(phys_addr, size) || + memremap_should_map_decrypted(phys_addr, size)) + return false; + } else if (sev_active()) { + if (memremap_should_map_decrypted(phys_addr, size)) + return false; + } return true; } @@ -608,15 +621,22 @@ pgprot_t __init early_memremap_pgprot_adjust(resource_size_t phys_addr, unsigned long size, pgprot_t prot) { - if (!sme_active()) + if (!sme_active() && !sev_active()) return prot; - if (early_memremap_is_setup_data(phys_addr, size) || - memremap_is_efi_data(phys_addr, size) || - memremap_should_map_decrypted(phys_addr, size)) - prot = pgprot_decrypted(prot); - else - prot = pgprot_encrypted(prot); + if (sme_active()) { + if (early_memremap_is_setup_data(phys_addr, size) || + memremap_is_efi_data(phys_addr, size) || + memremap_should_map_decrypted(phys_addr, size)) + prot = pgprot_decrypted(prot); + else + prot = pgprot_encrypted(prot); + } else if (sev_active()) { + if (memremap_should_map_decrypted(phys_addr, size)) + prot = pgprot_decrypted(prot); + else + prot = pgprot_encrypted(prot); + } return prot; }