[RFC,v2,05/32] x86: Use encrypted access of BOOT related data with SEV

Commit Message

Brijesh Singh March 2, 2017, 3:12 p.m. UTC

From: Tom Lendacky <[e-mail]>

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 <[e-mail]>
---
 arch/x86/mm/ioremap.c |   36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

Comments

Borislav Petkov March 7, 2017, 11:09 a.m. UTC | #1

On Thu, Mar 02, 2017 at 10:12:59AM -0500, Brijesh Singh wrote:
> From: Tom Lendacky <[e-mail]>
> 
> 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.

This could also explain why persistent memory needs to be accessed
decrypted with SEV.

In general, what the difference in that aspect is in respect to SME. And
I'd write that in the comment over the function. And not say "E820 areas
are checked in making this determination." because that is visible but
say *why* we need to check those ranges and determine access depending
on their type.

Tom Lendacky March 16, 2017, 7:03 p.m. UTC | #2

On 3/7/2017 5:09 AM, Borislav Petkov wrote:
> On Thu, Mar 02, 2017 at 10:12:59AM -0500, Brijesh Singh wrote:
>> From: Tom Lendacky <[e-mail]>
>>
>> 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.
>
> This could also explain why persistent memory needs to be accessed
> decrypted with SEV.

I'll add some comments about why persistent memory needs to be accessed
decrypted (because the encryption key changes across reboots) for both
SME and SEV.

>
> In general, what the difference in that aspect is in respect to SME. And
> I'd write that in the comment over the function. And not say "E820 areas
> are checked in making this determination." because that is visible but
> say *why* we need to check those ranges and determine access depending
> on their type.

Will do.

Thanks,
Tom

>

Patch

diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index c6cb921..c400ab5 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -462,12 +462,31 @@  static bool memremap_is_setup_data(resource_size_t phys_addr,
 }
 
 /*
- * This function determines if an address should be mapped encrypted.
- * Boot setup data, EFI data and E820 areas are checked in making this
- * determination.
+ * This function determines if an address should be mapped encrypted when
+ * SEV is active.  E820 areas are checked in making this determination.
  */
-static bool memremap_should_map_encrypted(resource_size_t phys_addr,
-					  unsigned long size)
+static bool memremap_sev_should_map_encrypted(resource_size_t phys_addr,
+					      unsigned long size)
+{
+	/* Check if the address is in persistent memory */
+	switch (e820__get_entry_type(phys_addr, phys_addr + size - 1)) {
+	case E820_TYPE_PMEM:
+	case E820_TYPE_PRAM:
+		return false;
+	default:
+		break;
+	}
+
+	return true;
+}
+
+/*
+ * This function determines if an address should be mapped encrypted when
+ * SME is active.  Boot setup data, EFI data and E820 areas are checked in
+ * making this determination.
+ */
+static bool memremap_sme_should_map_encrypted(resource_size_t phys_addr,
+					      unsigned long size)
 {
 	/*
 	 * SME is not active, return true:
@@ -508,6 +527,13 @@  static bool memremap_should_map_encrypted(resource_size_t phys_addr,
 	return true;
 }
 
+static bool memremap_should_map_encrypted(resource_size_t phys_addr,
+					  unsigned long size)
+{
+	return sev_active() ? memremap_sev_should_map_encrypted(phys_addr, size)
+			    : memremap_sme_should_map_encrypted(phys_addr, size);
+}
+
 /*
  * Architecure function to determine if RAM remap is allowed.
  */