[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH V2 05/10] xen/arm: vgic: Use dynamic memory allocation for vgic_rdist_region
On 06/27/2016 07:38 AM, Julien Grall wrote:
Hi Shanker,
On 26/06/16 18:48, Shanker Donthineni wrote:
The number of Re-distributor regions allowed for dom0 is hardcoded
s/Re-distributor/Redistributor/
to a compile time macro MAX_RDIST_COUNT which is 4. On some systems,
s/a compile time macro/a define/
s/On some/Some/
especially latest server chips might have more than 4 redistributors.
I would add a comma after 'chips'.
NIT: s/redistributors/Redistributors/
Either we have to increase MAX_RDIST_COUNT to a bigger number or
allocate memory based on number of redistributors that are found in
s/based on number/based on the number/
MADT table. In the worst case scenario, the macro MAX_RDIST_COUNT
should be equal to CONFIG_NR_CPUS in order to support per CPU
Redistributors.
Increasing MAX_RDIST_COUNT has side effect, it blows 'struct domain'
s/has side/has a/
size and hits BUILD_BUG_ON() in domain build code path.
struct domain *alloc_domain_struct(void)
{
struct domain *d;
BUILD_BUG_ON(sizeof(*d) > PAGE_SIZE);
d = alloc_xenheap_pages(0, 0);
if ( d == NULL )
return NULL;
...
This patch uses the second approach to fix the BUILD_BUG().
Signed-off-by: Shanker Donthineni <shankerd@xxxxxxxxxxxxxx>
---
Changes since v1:
Keep 'struct vgic_rdist_region' definition inside 'struct
arch_domain'.
xen/arch/arm/vgic-v2.c | 6 ++++++
xen/arch/arm/vgic-v3.c | 22 +++++++++++++++++++---
xen/arch/arm/vgic.c | 1 +
xen/include/asm-arm/domain.h | 2 +-
xen/include/asm-arm/vgic.h | 2 ++
5 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index 9adb4a9..f5778e6 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -699,9 +699,15 @@ static int vgic_v2_domain_init(struct domain *d)
return 0;
}
+static void vgic_v2_domain_free(struct domain *d)
+{
+ /* Nothing to be cleanup for this driver */
+}
+
static const struct vgic_ops vgic_v2_ops = {
.vcpu_init = vgic_v2_vcpu_init,
.domain_init = vgic_v2_domain_init,
+ .domain_free = vgic_v2_domain_free,
.max_vcpus = 8,
};
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index b37a7c0..e877e9e 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -1393,7 +1393,19 @@ static int vgic_v3_vcpu_init(struct vcpu *v)
static int vgic_v3_domain_init(struct domain *d)
{
- int i;
+ struct vgic_rdist_region *rdist_regions;
+ int rdist_count, i;
+
+ /* Allocate memory for Re-distributor regions */
+ rdist_count = is_hardware_domain(d) ? vgic_v3_hw.nr_rdist_regions :
+ GUEST_GICV3_RDIST_REGIONS;
I would directly introduce the inline helper in this patch, rather
than in patch #10.
Okay, I'll add a helper function as part of this patch.
+
+ rdist_regions = xzalloc_array(struct vgic_rdist_region,
rdist_count);
+ if ( !rdist_regions )
+ return -ENOMEM;
+
+ d->arch.vgic.nr_regions = rdist_count;
+ d->arch.vgic.rdist_regions = rdist_regions;
/*
* Domain 0 gets the hardware address.
[...]
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index a2fccc0..fbb763a 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -128,6 +128,8 @@ struct vgic_ops {
int (*vcpu_init)(struct vcpu *v);
/* Domain specific initialization of vGIC */
int (*domain_init)(struct domain *d);
+ /* Release resources that are allocated by domain_init */
s/are/were/
+ void (*domain_free)(struct domain *d);
/* vGIC sysreg emulation */
int (*emulate_sysreg)(struct cpu_user_regs *regs, union hsr hsr);
/* Maximum number of vCPU supported */
Regards,
--
Shanker Donthineni
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
