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

2019-12-11 Thread Jan Beulich
On 11.12.2019 11:58, Hongyan Xia wrote:
> map_pages_to_xen and modify_xen_mappings are performing almost exactly
> the same operations when shattering an l2 PTE, the only difference
> being whether we want to flush.
> 
> Signed-off-by: Hongyan Xia 

As before comments on patch 1 apply here as well.

Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v3 2/2] x86/mm: factor out the code for shattering an l2 PTE

2019-12-11 Thread Hongyan Xia
map_pages_to_xen and modify_xen_mappings are performing almost exactly
the same operations when shattering an l2 PTE, the only difference
being whether we want to flush.

Signed-off-by: Hongyan Xia 

---
Changes in v3:
- style and indentation changes.
- return -ENOMEM instead of -1.

Changes in v2:
- improve asm.
- re-read pl2e from memory when taking the lock.
- move the allocation of l1t inside the shatter function.
---
 xen/arch/x86/mm.c | 96 ---
 1 file changed, 49 insertions(+), 47 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 97f11b6016..e5ba6b52fb 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5151,6 +5151,52 @@ 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 l2 entry and populate l1. If virt is passed in, also do flush. */
+static int shatter_l2e(l2_pgentry_t *pl2e, unsigned long virt, bool locking)
+{
+unsigned int i;
+l2_pgentry_t ol2e = *pl2e;
+l1_pgentry_t l1e = l1e_from_paddr(l2e_get_paddr(ol2e),
+  lNf_to_l1f(l2e_get_flags(ol2e)));
+l1_pgentry_t *l1t = alloc_xen_pagetable();
+
+if ( !l1t )
+return -ENOMEM;
+
+for ( i = 0; i < L1_PAGETABLE_ENTRIES; i++ )
+{
+l1e_write(l1t + i, l1e);
+l1e = l1e_from_intpte(l1e_get_intpte(l1e) + PAGE_SIZE);
+}
+
+if ( locking )
+spin_lock(_pgdir_lock);
+if ( (l2e_get_flags(*pl2e) & _PAGE_PRESENT) &&
+ (l2e_get_flags(*pl2e) & _PAGE_PSE) )
+{
+l2e_write_atomic(pl2e,
+l2e_from_paddr((paddr_t)virt_to_maddr(l1t), __PAGE_HYPERVISOR));
+l1t = NULL;
+}
+if ( locking )
+spin_unlock(_pgdir_lock);
+
+if ( virt )
+{
+unsigned int flush_flags =
+FLUSH_TLB | FLUSH_ORDER(PAGETABLE_ORDER);
+
+if ( l2e_get_flags(ol2e) & _PAGE_GLOBAL )
+flush_flags |= FLUSH_TLB_GLOBAL;
+flush_area(virt, flush_flags);
+}
+
+if ( l1t )
+free_xen_pagetable(l1t);
+
+return 0;
+}
+
 /* Shatter an l3 entry and populate l2. If virt is passed in, also do flush. */
 static int shatter_l3e(l3_pgentry_t *pl3e, unsigned long virt, bool locking)
 {
@@ -5364,9 +5410,6 @@ int map_pages_to_xen(
 }
 else if ( l2e_get_flags(*pl2e) & _PAGE_PSE )
 {
-unsigned int flush_flags =
-FLUSH_TLB | FLUSH_ORDER(PAGETABLE_ORDER);
-
 /* Skip this PTE if there is no change. */
 if ( (((l2e_get_pfn(*pl2e) & ~(L1_PAGETABLE_ENTRIES - 1)) +
l1_table_offset(virt)) == mfn_x(mfn)) &&
@@ -5385,32 +5428,9 @@ int map_pages_to_xen(
 goto check_l3;
 }
 
-pl1e = alloc_xen_pagetable();
-if ( pl1e == NULL )
+/* Pass virt to indicate we need to flush. */
+if ( shatter_l2e(pl2e, virt, locking) )
 return -ENOMEM;
-
-for ( i = 0; i < L1_PAGETABLE_ENTRIES; i++ )
-l1e_write([i],
-  l1e_from_pfn(l2e_get_pfn(*pl2e) + i,
-   lNf_to_l1f(l2e_get_flags(*pl2e;
-
-if ( l2e_get_flags(*pl2e) & _PAGE_GLOBAL )
-flush_flags |= FLUSH_TLB_GLOBAL;
-
-if ( locking )
-spin_lock(_pgdir_lock);
-if ( (l2e_get_flags(*pl2e) & _PAGE_PRESENT) &&
- (l2e_get_flags(*pl2e) & _PAGE_PSE) )
-{
-l2e_write_atomic(pl2e, l2e_from_mfn(virt_to_mfn(pl1e),
-__PAGE_HYPERVISOR));
-pl1e = NULL;
-}
-if ( locking )
-spin_unlock(_pgdir_lock);
-flush_area(virt, flush_flags);
-if ( pl1e )
-free_xen_pagetable(pl1e);
 }
 
 pl1e  = l2e_to_l1e(*pl2e) + l1_table_offset(virt);
@@ -5633,26 +5653,8 @@ int modify_xen_mappings(unsigned long s, unsigned long 
e, unsigned int nf)
 else
 {
 /* PSE: shatter the superpage and try again. */
-pl1e = alloc_xen_pagetable();
-if ( !pl1e )
+if ( shatter_l2e(pl2e, 0, locking) )
 return -ENOMEM;
-for ( i = 0; i < L1_PAGETABLE_ENTRIES; i++ )
-l1e_write([i],
-  l1e_from_pfn(l2e_get_pfn(*pl2e) + i,
-   l2e_get_flags(*pl2e) & ~_PAGE_PSE));
-if ( locking )
-spin_lock(_pgdir_lock);
-if ( (l2e_get_flags(*pl2e) & _PAGE_PRESENT) &&
-