Re: [RFC Part1 PATCH v3 06/17] x86/mm: Use encrypted access of boot related data with SEV

[Tom Lendacky]

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 

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 
Signed-off-by: Brijesh Singh 
---
  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);
+   }

Re: [RFC Part1 PATCH v3 06/17] x86/mm: Use encrypted access of boot related data with SEV

[Borislav Petkov]

On Mon, Jul 24, 2017 at 02:07:46PM -0500, Brijesh Singh wrote:
> From: Tom Lendacky 
> 
> 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 
> Signed-off-by: Brijesh Singh 
> ---
>  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);
> + }

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 

[RFC Part1 PATCH v3 06/17] x86/mm: Use encrypted access of boot related data with SEV

[Brijesh Singh]

From: Tom Lendacky 

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 
Signed-off-by: Brijesh Singh 
---
 arch/x86/mm/ioremap.c | 44 
 1 file changed, 32 insertions(+), 12 deletions(-)

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;
 }
-- 
2.9.4