Thread (57 messages) 57 messages, 7 authors, 2023-01-09

RE: [PATCH 3/6] x86/tdx: Support vmalloc() for tdx_enc_status_changed()

From: Dexuan Cui <decui@microsoft.com>
Date: 2022-11-23 04:01:48
Also in: linux-arch, lkml

From: Dave Hansen <redacted>
Sent: Monday, November 21, 2022 1:01 PM
[...]
On 11/21/22 11:51, Dexuan Cui wrote:
quoted
-static bool tdx_enc_status_changed(unsigned long vaddr, int numpages,
bool enc)
quoted
+static bool tdx_enc_status_changed_for_contiguous_pages(unsigned long
vaddr,
quoted
+							int numpages, bool enc)
That naming is unfortunate.

First, it's getting way too long.

Second, you don't need two of these functions because it's contiguous or
not.  It's because tdx_enc_status_changed() only works on the direct map.
Will try to make one function with better naming.
quoted
+static bool tdx_enc_status_changed_for_vmalloc(unsigned long vaddr,
+					       int numpages, bool enc)
+{
+	void *start_va = (void *)vaddr;
+	void *end_va = start_va + numpages * PAGE_SIZE;
+	phys_addr_t pa;
+
+	if (offset_in_page(vaddr) != 0)
+		return false;
+
+	while (start_va < end_va) {
+		pa = slow_virt_to_phys(start_va);
+		if (!enc)
+			pa |= cc_mkdec(0);
+
+		if (!tdx_map_gpa(pa, pa + PAGE_SIZE, enc))
+			return false;
+
+		/*
+		 * private->shared conversion requires only MapGPA call.
+		 *
+		 * For shared->private conversion, accept the page using
+		 * TDX_ACCEPT_PAGE TDX module call.
+		 */
+		if (enc && !try_accept_one(&pa, PAGE_SIZE, PG_LEVEL_4K))
+			return false;
Don't we support large vmalloc() mappings these days?
I just noticed Nicholas Piggin's huge vmalloc mapping patches that were
merged in April 2021. I'll take a look and see what I can use here.
quoted
+		start_va += PAGE_SIZE;
+	}
+
+	return true;
+}
I really don't like the copy-and-paste fork here.

I'd almost just rather have this *one* "vmalloc" copy that does
slow_virt_to_phys() on direct map addresses than have two copies.
It looks like typically set_memory_{de/en}crypted() are not invoked
frequently when Linux is running, e.g. the swiotlb bounce buffers are
only initialzed once with set_memory_decrypt(). A driver runs in a guest
on a hypervisor typically also only initializes the buffers (which need to
be shared with the hypervisor) with set_memory_decrypt() once when
the driver initializes the device. So, it looks like slow_virt_to_phys() may be
acceptable for configuous memory pages as well.
Can you please look into making *one* function that works on either kind
of mapping?
Ok. Looking into this using slow_virt_to_phys() for direct map
addresses as well.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help