[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 1/2] x86/mm: factor out the code for shattering an l3 PTE

Hi Hongyan,

On 06/12/2019 15:53, Hongyan Xia wrote:
map_pages_to_xen and modify_xen_mappings are performing almost exactly
the same operations when shattering an l3 PTE, the only difference
being whether we want to flush.

Signed-off-by: Hongyan Xia <hongyxia@xxxxxxxxxx>
  xen/arch/x86/mm.c | 85 ++++++++++++++++++++++-------------------------
  1 file changed, 40 insertions(+), 45 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 7d4dd80a85..42aaaa1083 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5151,6 +5151,43 @@ l1_pgentry_t *virt_to_xen_l1e(unsigned long v)
                           flush_area_local((const void *)v, f) : \
                           flush_area_all((const void *)v, f))
+/* Shatter an l3 entry and populate l2. If virt is passed in, also do flush. */
+static void shatter_l3e(l3_pgentry_t *pl3e, l2_pgentry_t *l2t,
+        unsigned long virt, bool locking)
+    unsigned int i;
+    l3_pgentry_t ol3e = *pl3e;
+    for ( i = 0; i < L2_PAGETABLE_ENTRIES; i++ )
+        l2e_write(l2t + i,
+                  l2e_from_pfn(l3e_get_pfn(ol3e) +
+                               (i << PAGETABLE_ORDER),
+                               l3e_get_flags(ol3e)));

I understand this is just a factor out of the current code, but the code feels wrong (at least in theory) and wasteful.

You would allocate the L2 table, prepare it and then free it if the L3 entry has _PAGE_PRESENT or _PAGE_PSE cleared.

Also, in theory, there is nothing preventing the l3 flags to be modified behind your back. So you could end up to generate the l2 entries with the old l3 flags.

In nutshell, it feels to me that the shattering/allocation should happen with the lock taken. This would also allow you to not allocate the l2 table when you are removing the page.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.