[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH V5 09/10] xen/arm: make domain_max_vcpus return value from vgic_ops



Hi Chen,

On 30/05/2015 12:07, Chen Baozi wrote:
From: Chen Baozi <baozich@xxxxxxxxx>

When a guest uses vGICv2, the maximum number of vCPU it can support
should not be as many as MAX_VIRT_CPUS, which is 128 at the moment.
So the domain_max_vcpus should return the value according to the vGIC
the domain uses.

We didn't keep it as the old static inline form because it will break
compilation when access the member of struct domain:

In file included from xen/include/xen/domain.h:6:0,
                  from xen/include/xen/sched.h:10,
                  from arm64/asm-offsets.c:10:
xen/include/asm/domain.h: In function âdomain_max_vcpusâ:
xen/include/asm/domain.h:266:10: error: dereferencing pointer to incomplete type
      if (d->arch.vgic.version == GIC_V2)
           ^

Signed-off-by: Chen Baozi <baozich@xxxxxxxxx>
---
  xen/arch/arm/domain.c        | 5 +++++
  xen/arch/arm/domain_build.c  | 2 +-
  xen/arch/arm/vgic-v2.c       | 6 ++++++
  xen/arch/arm/vgic-v3.c       | 6 ++++++
  xen/include/asm-arm/domain.h | 5 +----
  xen/include/asm-arm/gic.h    | 3 +++
  xen/include/asm-arm/vgic.h   | 2 ++
  7 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 0cf147c..c638ff5 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -890,6 +890,11 @@ void vcpu_block_unless_event_pending(struct vcpu *v)
          vcpu_unblock(current);
  }

+unsigned int domain_max_vcpus(const struct domain *d)
+{
+    return d->arch.vgic.handler->get_max_vcpus();

As long as we keep MAX_VIRT_VCPUS in the common vGIC drivers, we need to make sure that we don't return a superior value because technically, with your changes, the vGICv3 could support more than 128 vCPUs...

I would do: return min(MAX_VIRT_CPUS, d->arch...)

+}
+
  /*
   * Local variables:
   * mode: C
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 5591d82..88cfcee 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -769,7 +769,7 @@ static int make_cpus_node(const struct domain *d, void *fdt,
           * is enough for the current max vcpu number.
           */
          mpidr_aff = vcpuid_to_vaffinity(cpu);
-        DPRINT("Create cpu@%x node\n", mpidr_aff);
+        DPRINT("Create cpu@%x (logical CPUID: %d) node\n", mpidr_aff, cpu);

This change belongs to patch #7...


          snprintf(buf, sizeof(buf), "cpu@%x", mpidr_aff);
          res = fdt_begin_node(fdt, buf);
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index 17a3c9f..206d289 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -508,6 +508,11 @@ static struct vcpu *vgic_v2_get_target_vcpu(struct vcpu 
*v, unsigned int irq)
      return v_target;
  }

+static int vgic_v2_get_max_vcpus(void)
+{
+    return GICV2_MAX_CPUS;
+}
+
  static int vgic_v2_get_irq_priority(struct vcpu *v, unsigned int irq)
  {
      int priority;
@@ -594,6 +599,7 @@ const struct vgic_ops vgic_v2_ops = {
      .domain_init = vgic_v2_domain_init,
      .get_irq_priority = vgic_v2_get_irq_priority,
      .get_target_vcpu = vgic_v2_get_target_vcpu,
+    .get_max_vcpus = vgic_v2_get_max_vcpus,
  };

  /*
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 21d8d3f..e79b010 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -1048,6 +1048,11 @@ static int vgic_v3_emulate_sysreg(struct cpu_user_regs 
*regs, union hsr hsr)
      }
  }

+static int vgic_v3_get_max_vcpus(void)
+{
+    return GICV3_MAX_CPUS;
+};
+
  static const struct mmio_handler_ops vgic_rdistr_mmio_handler = {
      .read_handler  = vgic_v3_rdistr_mmio_read,
      .write_handler = vgic_v3_rdistr_mmio_write,
@@ -1220,6 +1225,7 @@ const struct vgic_ops vgic_v3_ops = {
      .get_irq_priority = vgic_v3_get_irq_priority,
      .get_target_vcpu  = vgic_v3_get_target_vcpu,
      .emulate_sysreg  = vgic_v3_emulate_sysreg,
+    .get_max_vcpus = vgic_v3_get_max_vcpus,
  };

  /*
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index b7b5cd2..b525bec 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -261,10 +261,7 @@ struct arch_vcpu
  void vcpu_show_execution_state(struct vcpu *);
  void vcpu_show_registers(const struct vcpu *);

-static inline unsigned int domain_max_vcpus(const struct domain *d)
-{
-    return MAX_VIRT_CPUS;
-}
+unsigned int domain_max_vcpus(const struct domain *);

  /*
   * Due to the restriction of GICv3, the number of vCPUs in AFF0 is
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index c6ef4bf..55f99cd 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -18,6 +18,9 @@
  #ifndef __ASM_ARM_GIC_H__
  #define __ASM_ARM_GIC_H__

+#define GICV2_MAX_CPUS  8
+#define GICV3_MAX_CPUS  128

This doesn't need to be exported and can go in each vGIC driver.

Furthermore, technically, the vGICv3 driver is able to support more than 128 vCPUs...

+
  #define NR_GIC_LOCAL_IRQS  NR_LOCAL_IRQS
  #define NR_GIC_SGI         16
  #define MAX_RDIST_COUNT    4
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index 2f413e1..1157b04 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -110,6 +110,8 @@ struct vgic_ops {
      struct vcpu *(*get_target_vcpu)(struct vcpu *v, unsigned int irq);
      /* vGIC sysreg emulation */
      int (*emulate_sysreg)(struct cpu_user_regs *regs, union hsr hsr);
+    /* Get the maximum number of vCPU supported */
+    int (*get_max_vcpus)(void);

Why did you create a function? The value never change so a field value would have been better...

  };

  /* Number of ranks of interrupt registers for a domain */


--
Julien Grall

_______________________________________________
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®.