[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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.