diff mbox series

[v5,1/2] arm/p2m: Rework p2m_init()

Message ID 20221018142346.52272-2-Henry.Wang@arm.com (mailing list archive)
State New, archived
Headers show
Series arm/p2m: XSA-409 fix | expand

Commit Message

Henry Wang Oct. 18, 2022, 2:23 p.m. UTC 
From: Andrew Cooper <andrew.cooper3@citrix.com>

p2m_init() is mostly trivial initialisation, but has two fallible operations
which are on either side of the backpointer trigger for teardown to take
actions.

p2m_free_vmid() is idempotent with a failed p2m_alloc_vmid(), so rearrange
p2m_init() to perform all trivial setup, then set the backpointer, then
perform all fallible setup.

This will simplify a future bugfix which needs to add a third fallible
operation.

No practical change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
v5 changes:
- Correct typo in commit message, add reviewed-by from Arm maintainers.
---
 xen/arch/arm/p2m.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

Comments

Henry Wang Oct. 18, 2022, 2:48 p.m. UTC | #1 
> -----Original Message-----
> Subject: [PATCH v5 1/2] arm/p2m: Rework p2m_init()
> 
> From: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> p2m_init() is mostly trivial initialisation, but has two fallible operations
> which are on either side of the backpointer trigger for teardown to take
> actions.
> 
> p2m_free_vmid() is idempotent with a failed p2m_alloc_vmid(), so rearrange
> p2m_init() to perform all trivial setup, then set the backpointer, then
> perform all fallible setup.
> 
> This will simplify a future bugfix which needs to add a third fallible
> operation.
> 
> No practical change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Julien Grall <jgrall@amazon.com>
> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Release-acked-by: Henry Wang <Henry.Wang@arm.com>

Kind regards,
Henry
diff mbox series

Patch

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index f17500ddf3..6826f63150 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1754,7 +1754,7 @@  void p2m_final_teardown(struct domain *d)
 int p2m_init(struct domain *d)
 {
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
-    int rc = 0;
+    int rc;
     unsigned int cpu;
 
     rwlock_init(&p2m->lock);
@@ -1763,11 +1763,6 @@  int p2m_init(struct domain *d)
     INIT_PAGE_LIST_HEAD(&d->arch.paging.p2m_freelist);
 
     p2m->vmid = INVALID_VMID;
-
-    rc = p2m_alloc_vmid(d);
-    if ( rc != 0 )
-        return rc;
-
     p2m->max_mapped_gfn = _gfn(0);
     p2m->lowest_mapped_gfn = _gfn(ULONG_MAX);
 
@@ -1783,8 +1778,6 @@  int p2m_init(struct domain *d)
     p2m->clean_pte = is_iommu_enabled(d) &&
         !iommu_has_feature(d, IOMMU_FEAT_COHERENT_WALK);
 
-    rc = p2m_alloc_table(d);
-
     /*
      * Make sure that the type chosen to is able to store the an vCPU ID
      * between 0 and the maximum of virtual CPUS supported as long as
@@ -1797,13 +1790,20 @@  int p2m_init(struct domain *d)
        p2m->last_vcpu_ran[cpu] = INVALID_VCPU_ID;
 
     /*
-     * Besides getting a domain when we only have the p2m in hand,
-     * the back pointer to domain is also used in p2m_teardown()
-     * as an end-of-initialization indicator.
+     * "Trivial" initialisation is now complete.  Set the backpointer so
+     * p2m_teardown() and friends know to do something.
      */
     p2m->domain = d;
 
-    return rc;
+    rc = p2m_alloc_vmid(d);
+    if ( rc )
+        return rc;
+
+    rc = p2m_alloc_table(d);
+    if ( rc )
+        return rc;
+
+    return 0;
 }
 
 /*