diff mbox series

[RESEND,v9,2/2] x86/tdx: Support vmalloc() for tdx_enc_status_changed()

Message ID 20230811021246.821-3-decui@microsoft.com
State New
Headers
Series Support TDX guests on Hyper-V (the x86/tdx part) |

Commit Message

Dexuan Cui Aug. 11, 2023, 2:12 a.m. UTC 
  When a TDX guest runs on Hyper-V, the hv_netvsc driver's netvsc_init_buf()
allocates buffers using vzalloc(), and needs to share the buffers with the
host OS by calling set_memory_decrypted(), which is not working for
vmalloc() yet. Add the support by handling the pages one by one.

Co-developed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
 arch/x86/coco/tdx/tdx.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

Changes in v2:
  Changed tdx_enc_status_changed() in place.

Changes in v3:
  No change since v2.

Changes in v4:
  Added Kirill's Co-developed-by since Kirill helped to improve the
    code by adding tdx_enc_status_changed_phys().

  Thanks Kirill for the clarification on load_unaligned_zeropad()!

Changes in v5:
  Added Kirill's Signed-off-by.
  Added Michael's Reviewed-by.

Changes in v6: None.

Changes in v7: None.
  Note: there was a race between set_memory_encrypted() and
  load_unaligned_zeropad(), which has been fixed by the 3 patches of
  Kirill in the x86/tdx branch of the tip tree.

Changes in v8:
  Rebased to tip.git's master branch.

Changes in v9:
  Added Kuppuswamy Sathyanarayanan's Reviewed-by.

Comments

Dave Hansen Aug. 11, 2023, 2:09 p.m. UTC | #1 
On 8/10/23 19:12, Dexuan Cui wrote:
> +	if (!is_vmalloc_addr((void *)start))
> +		return tdx_enc_status_changed_phys(__pa(start), __pa(end), enc);
> +
> +	while (start < end) {
> +		phys_addr_t start_pa = slow_virt_to_phys((void *)start);
> +		phys_addr_t end_pa = start_pa + PAGE_SIZE;
> +
> +		if (!tdx_enc_status_changed_phys(start_pa, end_pa, enc))
> +			return false;
> +
> +		start += PAGE_SIZE;
> +	}

This creates two different paths for vmalloc() and the direct map.
There are two different ways to do va=>pa conversion, for instance.
Here's a single loop that works for both cases:

	unsigned long step = end - start;
	unsigned long addr;

	/* Step through page-by-page for vmalloc() mappings: */
	if (is_vmalloc_addr((void *)vaddr))
		step = PAGE_SIZE;

	for (addr = start; addr < end; addr += step) {
		phys_addr_t start_pa = slow_virt_to_phys(addr);
		phys_addr_t end_pa   = start_pa + step;

		if (!tdx_enc_status_changed_phys(start_pa, end_pa, enc))
			return false;
	}

Note that this also doesn't abuse 'start' by making it a loop variable.
It also, uh, uses a for() loop.

The only downside is that it costs a page table walk for direct map
virt=>phys conversion.  I can live with that.
Dexuan Cui Aug. 11, 2023, 7:08 p.m. UTC | #2 
> From: Dave Hansen <dave.hansen@intel.com>
> Sent: Friday, August 11, 2023 7:10 AM
> [...]
> This creates two different paths for vmalloc() and the direct map.
> There are two different ways to do va=>pa conversion, for instance.
> Here's a single loop that works for both cases:
> 
> 	unsigned long step = end - start;
> 	unsigned long addr;
> 
> 	/* Step through page-by-page for vmalloc() mappings: */
> 	if (is_vmalloc_addr((void *)vaddr))
> 		step = PAGE_SIZE;
> 
> 	for (addr = start; addr < end; addr += step) {
> 		phys_addr_t start_pa = slow_virt_to_phys(addr);
> 		phys_addr_t end_pa   = start_pa + step;
> 
> 		if (!tdx_enc_status_changed_phys(start_pa, end_pa, enc))
> 			return false;
> 	}
> 
> Note that this also doesn't abuse 'start' by making it a loop variable.
> It also, uh, uses a for() loop.
> 
> The only downside is that it costs a page table walk for direct map
> virt=>phys conversion.  I can live with that.

Your version is concise and more readable.  
Thanks for improving the patch! I'll use this in v10 shortly.
diff mbox series

Patch

diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 746075d20cd2..c1a2423a8159 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -7,6 +7,7 @@ 
 #include <linux/cpufeature.h>
 #include <linux/export.h>
 #include <linux/io.h>
+#include <linux/mm.h>
 #include <asm/coco.h>
 #include <asm/tdx.h>
 #include <asm/vmx.h>
@@ -753,6 +754,19 @@  static bool tdx_map_gpa(phys_addr_t start, phys_addr_t end, bool enc)
 	return false;
 }
 
+static bool tdx_enc_status_changed_phys(phys_addr_t start, phys_addr_t end,
+					bool enc)
+{
+	if (!tdx_map_gpa(start, end, enc))
+		return false;
+
+	/* shared->private conversion requires memory to be accepted before use */
+	if (enc)
+		return tdx_accept_memory(start, end);
+
+	return true;
+}
+
 /*
  * Inform the VMM of the guest's intent for this physical page: shared with
  * the VMM or private to the guest.  The VMM is expected to change its mapping
@@ -760,15 +774,24 @@  static bool tdx_map_gpa(phys_addr_t start, phys_addr_t end, bool enc)
  */
 static bool tdx_enc_status_changed(unsigned long vaddr, int numpages, bool enc)
 {
-	phys_addr_t start = __pa(vaddr);
-	phys_addr_t end   = __pa(vaddr + numpages * PAGE_SIZE);
+	unsigned long start = vaddr;
+	unsigned long end = start + numpages * PAGE_SIZE;
 
-	if (!tdx_map_gpa(start, end, enc))
+	if (offset_in_page(start) != 0)
 		return false;
 
-	/* shared->private conversion requires memory to be accepted before use */
-	if (enc)
-		return tdx_accept_memory(start, end);
+	if (!is_vmalloc_addr((void *)start))
+		return tdx_enc_status_changed_phys(__pa(start), __pa(end), enc);
+
+	while (start < end) {
+		phys_addr_t start_pa = slow_virt_to_phys((void *)start);
+		phys_addr_t end_pa = start_pa + PAGE_SIZE;
+
+		if (!tdx_enc_status_changed_phys(start_pa, end_pa, enc))
+			return false;
+
+		start += PAGE_SIZE;
+	}
 
 	return true;
 }