[xen stable-4.16] xen/arm: p2m: Populate pages for GICv2 mapping in p2m_init()

[patchbot@xxxxxxx]

commit e5a5bdeba6a0c3eacd2ba39c1ee36b3c54e77dca
Author:     Henry Wang <Henry.Wang@xxxxxxx>
AuthorDate: Tue Oct 25 09:21:12 2022 +0000
Commit:     Julien Grall <jgrall@xxxxxxxxxx>
CommitDate: Tue Oct 25 20:54:26 2022 +0100

    xen/arm: p2m: Populate pages for GICv2 mapping in p2m_init()
    
    Hardware using GICv2 needs to create a P2M mapping of 8KB GICv2 area
    when the domain is created. Considering the worst case of page tables
    which requires 6 P2M pages as the two pages will be consecutive but not
    necessarily in the same L3 page table and keep a buffer, populate 16
    pages as the default value to the P2M pages pool in p2m_init() at the
    domain creation stage to satisfy the GICv2 requirement. For GICv3, the
    above-mentioned P2M mapping is not necessary, but since the allocated
    16 pages here would not be lost, hence populate these pages
    unconditionally.
    
    With the default 16 P2M pages populated, there would be a case that
    failures would happen in the domain creation with P2M pages already in
    use. To properly free the P2M for this case, firstly support the
    optionally preemption of p2m_teardown(), then call p2m_teardown() and
    p2m_set_allocation(d, 0, NULL) non-preemptively in p2m_final_teardown().
    As non-preemptive p2m_teardown() should only return 0, use a
    BUG_ON to confirm that.
    
    Since p2m_final_teardown() is called either after
    domain_relinquish_resources() where relinquish_p2m_mapping() has been
    called, or from failure path of domain_create()/arch_domain_create()
    where mappings that require p2m_put_l3_page() should never be created,
    relinquish_p2m_mapping() is not added in p2m_final_teardown(), add
    in-code comments to refer this.
    
    Fixes: cbea5a1149ca ("xen/arm: Allocate and free P2M pages from the P2M 
pool")
    Suggested-by: Julien Grall <jgrall@xxxxxxxxxx>
    Signed-off-by: Henry Wang <Henry.Wang@xxxxxxx>
    Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>
    Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
    (cherry picked from commit: c7cff1188802646eaa38e918e5738da0e84949be)
---
 xen/arch/arm/domain.c     |  2 +-
 xen/arch/arm/p2m.c        | 34 ++++++++++++++++++++++++++++++++--
 xen/include/asm-arm/p2m.h | 14 ++++++++++----
 3 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index a818f33a1a..c7feaa323a 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -1059,7 +1059,7 @@ int domain_relinquish_resources(struct domain *d)
             return ret;
 
     PROGRESS(p2m):
-        ret = p2m_teardown(d);
+        ret = p2m_teardown(d, true);
         if ( ret )
             return ret;
 
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 4f7d923ad9..6f87e17c1d 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1661,7 +1661,7 @@ static void p2m_free_vmid(struct domain *d)
     spin_unlock(&vmid_alloc_lock);
 }
 
-int p2m_teardown(struct domain *d)
+int p2m_teardown(struct domain *d, bool allow_preemption)
 {
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
     unsigned long count = 0;
@@ -1669,6 +1669,9 @@ int p2m_teardown(struct domain *d)
     unsigned int i;
     int rc = 0;
 
+    if ( page_list_empty(&p2m->pages) )
+        return 0;
+
     p2m_write_lock(p2m);
 
     /*
@@ -1692,7 +1695,7 @@ int p2m_teardown(struct domain *d)
         p2m_free_page(p2m->domain, pg);
         count++;
         /* Arbitrarily preempt every 512 iterations */
-        if ( !(count % 512) && hypercall_preempt_check() )
+        if ( allow_preemption && !(count % 512) && hypercall_preempt_check() )
         {
             rc = -ERESTART;
             break;
@@ -1712,7 +1715,20 @@ void p2m_final_teardown(struct domain *d)
     if ( !p2m->domain )
         return;
 
+    /*
+     * No need to call relinquish_p2m_mapping() here because
+     * p2m_final_teardown() is called either after 
domain_relinquish_resources()
+     * where relinquish_p2m_mapping() has been called, or from failure path of
+     * domain_create()/arch_domain_create() where mappings that require
+     * p2m_put_l3_page() should never be created. For the latter case, also see
+     * comment on top of the p2m_set_entry() for more info.
+     */
+
+    BUG_ON(p2m_teardown(d, false));
     ASSERT(page_list_empty(&p2m->pages));
+
+    while ( p2m_teardown_allocation(d) == -ERESTART )
+        continue; /* No preemption support here */
     ASSERT(page_list_empty(&d->arch.paging.p2m_freelist));
 
     if ( p2m->root )
@@ -1779,6 +1795,20 @@ int p2m_init(struct domain *d)
     if ( rc )
         return rc;
 
+    /*
+     * Hardware using GICv2 needs to create a P2M mapping of 8KB GICv2 area
+     * when the domain is created. Considering the worst case for page
+     * tables and keep a buffer, populate 16 pages to the P2M pages pool here.
+     * For GICv3, the above-mentioned P2M mapping is not necessary, but since
+     * the allocated 16 pages here would not be lost, hence populate these
+     * pages unconditionally.
+     */
+    spin_lock(&d->arch.paging.lock);
+    rc = p2m_set_allocation(d, 16, NULL);
+    spin_unlock(&d->arch.paging.lock);
+    if ( rc )
+        return rc;
+
     return 0;
 }
 
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index c9598740bd..b2725206e8 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -194,14 +194,18 @@ int p2m_init(struct domain *d);
 
 /*
  * The P2M resources are freed in two parts:
- *  - p2m_teardown() will be called when relinquish the resources. It
- *    will free large resources (e.g. intermediate page-tables) that
- *    requires preemption.
+ *  - p2m_teardown() will be called preemptively when relinquish the
+ *    resources, in which case it will free large resources (e.g. intermediate
+ *    page-tables) that requires preemption.
  *  - p2m_final_teardown() will be called when domain struct is been
  *    freed. This *cannot* be preempted and therefore one small
  *    resources should be freed here.
+ *  Note that p2m_final_teardown() will also call p2m_teardown(), to properly
+ *  free the P2M when failures happen in the domain creation with P2M pages
+ *  already in use. In this case p2m_teardown() is called non-preemptively and
+ *  p2m_teardown() will always return 0.
  */
-int p2m_teardown(struct domain *d);
+int p2m_teardown(struct domain *d, bool allow_preemption);
 void p2m_final_teardown(struct domain *d);
 
 /*
@@ -266,6 +270,8 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
 /*
  * Direct set a p2m entry: only for use by the P2M code.
  * The P2M write lock should be taken.
+ * TODO: Add a check in __p2m_set_entry() to avoid creating a mapping in
+ * arch_domain_create() that requires p2m_put_l3_page() to be called.
  */
 int p2m_set_entry(struct p2m_domain *p2m,
                   gfn_t sgfn,
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.16