Messages in this thread | | | Subject | Re: [RFC Part1 PATCH v3 06/17] x86/mm: Use encrypted access of boot related data with SEV | From | Tom Lendacky <> | Date | Thu, 17 Aug 2017 13:05:38 -0500 |
| |
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); >> + } >
| |