On Fri, Dec 10, 2021 at 09:43:04AM -0600, Brijesh Singh wrote:

The early_set_memory_{encrypt,decrypt}() are used for changing the

					^
					ed()

page from decrypted (shared) to encrypted (private) and vice versa.
When SEV-SNP is active, the page state transition needs to go through
additional steps.

If the page is transitioned from shared to private, then perform the
following after the encryption attribute is set in the page table:

1. Issue the page state change VMGEXIT to add the page as a private
   in the RMP table.
2. Validate the page after its successfully added in the RMP table.

To maintain the security guarantees, if the page is transitioned from
private to shared, then perform the following before clearing the
encryption attribute from the page table.

1. Invalidate the page.
2. Issue the page state change VMGEXIT to make the page shared in the
   RMP table.

The early_set_memory_{encrypt,decrypt} can be called before the GHCB

ditto.

is setup, use the SNP page state MSR protocol VMGEXIT defined in the GHCB
specification to request the page state change in the RMP table.

While at it, add a helper snp_prep_memory() that can be used outside
the sev specific files to change the page state for a specified memory

"outside of the sev specific"? What is that trying to say?

/me goes and looks at the whole patchset...

Right, so that is used only in probe_roms(). So that should say:

"Add a helper ... which will be used in probe_roms(), in a later patch."

range.

Signed-off-by: Brijesh Singh <redacted>
---
 arch/x86/include/asm/sev.h |  10 ++++
 arch/x86/kernel/sev.c      | 102 +++++++++++++++++++++++++++++++++++++
 arch/x86/mm/mem_encrypt.c  |  51 +++++++++++++++++--

Right, for the next revision, that file is called mem_encrypt_amd.c now.

...

Yeah, looking at this again, I don't really like this multiplexing.
Let's do this instead, diff ontop:

---

diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
index c14fd8254198..e3f7a84449bb 100644
--- a/arch/x86/mm/mem_encrypt_amd.c
+++ b/arch/x86/mm/mem_encrypt_amd.c

@@ -49,24 +49,18 @@ EXPORT_SYMBOL(sme_me_mask);
 static char sme_early_buffer[PAGE_SIZE] __initdata __aligned(PAGE_SIZE);
 
 /*
- * When SNP is active, change the page state from private to shared before
- * copying the data from the source to destination and restore after the copy.
- * This is required because the source address is mapped as decrypted by the
- * caller of the routine.
+ * SNP-specific routine which needs to additionally change the page state from
+ * private to shared before copying the data from the source to destination and
+ * restore after the copy.
  */
 static inline void __init snp_memcpy(void *dst, void *src, size_t sz,
 				     unsigned long paddr, bool decrypt)
 {
 	unsigned long npages = PAGE_ALIGN(sz) >> PAGE_SHIFT;
 
-	if (!cc_platform_has(CC_ATTR_SEV_SNP) || !decrypt) {
-		memcpy(dst, src, sz);
-		return;
-	}
-
 	/*
-	 * With SNP, the paddr needs to be accessed decrypted, mark the page
-	 * shared in the RMP table before copying it.
+	 * @paddr needs to be accessed decrypted, mark the page shared in the
+	 * RMP table before copying it.
 	 */
 	early_snp_set_memory_shared((unsigned long)__va(paddr), paddr, npages);
 

@@ -124,8 +118,13 @@ static void __init __sme_early_enc_dec(resource_size_t paddr,
 		 * Use a temporary buffer, of cache-line multiple size, to
 		 * avoid data corruption as documented in the APM.
 		 */
-		snp_memcpy(sme_early_buffer, src, len, paddr, enc);
-		snp_memcpy(dst, sme_early_buffer, len, paddr, !enc);
+		if (cc_platform_has(CC_ATTR_SEV_SNP)) {
+			snp_memcpy(sme_early_buffer, src, len, paddr, enc);
+			snp_memcpy(dst, sme_early_buffer, len, paddr, !enc);
+		} else {
+			memcpy(sme_early_buffer, src, len);
+			memcpy(dst, sme_early_buffer, len);
+		}
 
 		early_memunmap(dst, len);
 		early_memunmap(src, len);

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette