On 13/10/2022 10:40, Henry Wang wrote:
Hi Andrew,
-----Original Message-----
From: Andrew Cooper <andrew.coop...@citrix.com>
Subject: Re: [PATCH] xen/arm: p2m: Populate pages for GICv2 mapping in
arch_domain_create()
On 13/10/2022 09:38, Henry Wang wrote:
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
and keep a buffer, populate 16 pages as the default value to the P2M
pages pool in arch_domain_create() at the domain creation stage to
satisfy the GICv2 requirement.
Fixes: cbea5a1149ca ("xen/arm: Allocate and free P2M pages from the P2M
pool")
Suggested-by: Julien Grall <jgr...@amazon.com>
Signed-off-by: Henry Wang <henry.w...@arm.com>
---
This should also be backported to 4.13, 4.14, 4.15 and 4.16.
---
xen/arch/arm/domain.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 2c84e6dbbb..e40e2bcba1 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -740,6 +740,20 @@ int arch_domain_create(struct domain *d,
BUG();
}
+ spin_lock(&d->arch.paging.lock);
+ /*
+ * 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.
+ */
+ if ( (rc = p2m_set_allocation(d, 16, NULL)) != 0 )
+ {
+ p2m_set_allocation(d, 0, NULL);
+ spin_unlock(&d->arch.paging.lock);
+ goto fail;
+ }
+ spin_unlock(&d->arch.paging.lock);
Generally, this would be better written as
spin_lock();
if ( rc = p2m_set_allocation(16) )
p2m_set_allocation(0)
spin_unlock();
if ( rc )
goto fail;
to reduce the number of spin_unlock() calls and make the error paths
more clear. However...
I think in Arm's arch_domain_create(), all the error handling are the
same style using:
if ( (rc = <function>) !=0 )
goto fail;
and we need to keep them the same?
We don't have too. I agree with Andrew's point. How about the following:
spin_lock()
rc = ...
spin_unlock()
if ( rc )
goto fail;
Cheers,
--
Julien Grall
