[PATCH V7 0/8] Support more than 8 vcpus on arm64 with GICv3

[PATCH V7 0/8] Support more than 8 vcpus on arm64 with GICv3

[Chen Baozi]

From: Chen Baozi <baozich@gmail.com>

Currently the number of vcpus on arm64 with GICv3 is limited up to 8 due
to the fixed size of redistributor mmio region. Increasing the size
makes the number expand to 16 because of AFF0 restriction on GICv3.
To create a guest up to 128 vCPUs, which is the maxium number that GIC-500
can support, this patchset uses the AFF1 information to create a mapping
relation between vCPUID and vMPIDR and deals with the related issues.

These patches are written based upon Julien's "GICv2 on GICv3" series
and the IROUTER emulation cleanup patch.

Changes from V6:
* Use the new 'struct sgi_target' instead of cpumask_t in vgic_to_sgi.
* Make the domain_max_vcpus to return MAX_VIRT_CPUS and avoid to split
  arch_domain_init.

Changes form V5:
* Rework gicv3_sgir_to_cpumask in #5
* Rework #8 to split arch_domain_create into two parts:
  - arch_domain_preinit to initialise vgic_ops before evtchn_init is
    called
  - the rest of logic remains in arch_domain_create
* Use a field value in struct vgic_ops instead of the function point
  for max_vcpus.
* Minor changes according to previous reviews.

Changes from V4:
* Split the patch 4/8 of V3 into two part:
  - Use cpumask_t type for vcpu_mask in vgic_to_sgi.
  - Use AFF1 when translating ICC_SGI1R_EL1 to cpumask.
* Use a more efficient algorithm when calculate cpumask.
* Add a patch to call arch_domain_create before evtchn_init, because
  evtchn_init needs vgic info which is initialised during
  acrh_domain_create.
* Get the max vcpu info from vgic_ops.
* Minor changes according to previous reviews.

Changes from V3:
* Drop the wrong patch that altering domain_max_vcpus to a macro.
* Change the domain_max_vcpus to return value accodring to the version
  of the vGIC in used.

Changes from V2:
* Reorder the patch which increases MAX_VIRT_CPUS to the last to make
  this series bisectable.
* Drop the dynamic re-distributor region allocation patch in tools.
* Use cpumask_t type instead of unsigned long in vgic_to_sgi and do the
  translation from GICD_SGIR to vcpu_mask in both vGICv2 and vGICv3.
* Make domain_max_vcpus be alias of max_vcpus in struct domain

Changes from V1:
* Use the way that expanding the GICR address space to support up to 128
  redistributor in guest memory layout rather than use the dynamic
  allocation.
* Add support to include AFF1 information in vMPIDR/logical CPUID.

Chen Baozi (10):
  xen/arm: gic-v3: Increase the size of GICR in address space for guest
  xen/arm: Add functions of mapping between vCPUID and virtual affinity
  xen/arm: Use the new functions for vCPUID/vaffinity transformation
  xen/arm: Use cpumask_t type for vcpu_mask in vgic_to_sgi
  xen/arm64: gicv3: Use AFF1 when translating ICC_SGI1R_EL1 to cpumask
  tools/libxl: Set 'reg' of cpu node equal to MPIDR affinity for domU
  xen/arm: Set 'reg' of cpu node for dom0 to match MPIDR's affinity
  xen: Add arch_domain_preinit to initialise vGIC before evtchn_init
  xen/arm: make domain_max_vcpus return value from vgic_ops
  xen/arm64: increase MAX_VIRT_CPUS to 128 on arm64


Chen Baozi (8):
  xen/arm: gic-v3: Increase the size of GICR in address space for guest
  xen/arm: Add functions of mapping between vCPUID and virtual affinity
  xen/arm: Use the new functions for vCPUID/vaffinity transformation
  xen/arm: Use struct sgi_target instead of vcpu_mask in vgic_to_sgi
  tools/libxl: Set 'reg' of cpu node equal to MPIDR affinity for domU
  xen/arm: Set 'reg' of cpu node for dom0 to match MPIDR's affinity
  xen/arm: make domain_max_vcpus return value from vgic_ops
  xen/arm64: increase MAX_VIRT_CPUS to 128 on arm64

 tools/libxl/libxl_arm.c           | 14 ++++++++++--
 xen/arch/arm/domain.c             | 15 ++++++++-----
 xen/arch/arm/domain_build.c       | 14 +++++++++---
 xen/arch/arm/vgic-v2.c            |  8 ++++---
 xen/arch/arm/vgic-v3.c            | 20 ++++++++++++-----
 xen/arch/arm/vgic.c               | 46 +++++++++++++++++++--------------------
 xen/arch/arm/vpsci.c              |  5 +----
 xen/include/asm-arm/config.h      |  4 ++++
 xen/include/asm-arm/domain.h      | 39 +++++++++++++++++++++++++++++++--
 xen/include/asm-arm/gic.h         |  1 +
 xen/include/asm-arm/gic_v3_defs.h |  4 ++++
 xen/include/asm-arm/vgic.h        | 11 +++++++++-
 xen/include/public/arch-arm.h     |  4 ++--
 13 files changed, 134 insertions(+), 51 deletions(-)

-- 
2.1.4

[PATCH v7 1/8] xen/arm: gic-v3: Increase the size of GICR in address space for guest

[Chen Baozi]

From: Chen Baozi <baozich@gmail.com>

Currently it only supports up to 8 vCPUs. Increase the region to hold
up to 128 vCPUs, which is the maximum number that GIC-500 supports.

Signed-off-by: Chen Baozi <baozich@gmail.com>
Reviewed-by: Julien Grall <julien.grall@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/include/public/arch-arm.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index c029e0f..ec0c261 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -388,8 +388,8 @@ struct xen_arch_domainconfig {
 #define GUEST_GICV3_RDIST_STRIDE   0x20000ULL
 #define GUEST_GICV3_RDIST_REGIONS  1
 
-#define GUEST_GICV3_GICR0_BASE     0x03020000ULL    /* vCPU0 - vCPU7 */
-#define GUEST_GICV3_GICR0_SIZE     0x00100000ULL
+#define GUEST_GICV3_GICR0_BASE     0x03020000ULL    /* vCPU0 - vCPU127 */
+#define GUEST_GICV3_GICR0_SIZE     0x01000000ULL
 
 /*
  * 16MB == 4096 pages reserved for guest to use as a region to map its
-- 
2.1.4

[PATCH v7 2/8] xen/arm: Add functions of mapping between vCPUID and virtual affinity

[Chen Baozi]

From: Chen Baozi <baozich@gmail.com>

GICv3 restricts that the maximum number of CPUs in affinity 0 (one
cluster) is 16. (See the note of 'Bits[15:0]' in '5.7.29 ICC_SGI0R_EL1
ICC_SGI1R_EL1 and ICC_ASGI1R_EL1, GICv3 Architecture Specification')
That is to say the upper 4 bits of affinity 0 is unused. Current
implementation considers that AFF0 is equal to vCPUID, which makes all
vCPUs in one cluster, limiting its number to 16. If we would like to
support more than 16 number of vCPU in one guest, we need to make use
of AFF1. Considering the unused upper 4 bits, we need to create a pair
of functions mapping the vCPUID and virtual affinity.

Signed-off-by: Chen Baozi <baozich@gmail.com>
Reviewed-by: Julien Grall <julien.grall@citrix.com>
---
 xen/include/asm-arm/domain.h | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 75b17af..35b9a6d 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -266,6 +266,44 @@ static inline unsigned int domain_max_vcpus(const struct domain *d)
     return MAX_VIRT_CPUS;
 }
 
+/*
+ * Due to the restriction of GICv3, the number of vCPUs in AFF0 is
+ * limited to 16, thus only the first 4 bits of AFF0 are legal. We will
+ * use the first 2 affinity levels here, expanding the number of vCPU up
+ * to 4096 (16*256), which is more than the PEs that GIC-500 supports.
+ *
+ * Since we don't save information of vCPU's topology (affinity) in
+ * vMPIDR at the moment, we map the vcpuid to the vMPIDR linearly.
+ */
+static inline unsigned int vaffinity_to_vcpuid(register_t vaff)
+{
+    unsigned int vcpuid;
+
+    vaff &= MPIDR_HWID_MASK;
+
+    vcpuid = MPIDR_AFFINITY_LEVEL(vaff, 0);
+    vcpuid |= MPIDR_AFFINITY_LEVEL(vaff, 1) << 4;
+
+    return vcpuid;
+}
+
+static inline register_t vcpuid_to_vaffinity(unsigned int vcpuid)
+{
+    register_t vaff;
+
+    /*
+     * Right now only AFF0 and AFF1 are supported in virtual affinity.
+     * Since only the first 4 bits in AFF0 are used in GICv3, the
+     * available bits are 12 (4+8).
+     */
+    BUILD_BUG_ON(!(MAX_VIRT_CPUS < ((1 << 12))));
+
+    vaff = (vcpuid & 0x0f) << MPIDR_LEVEL_SHIFT(0);
+    vaff |= ((vcpuid >> 4) & MPIDR_LEVEL_MASK) << MPIDR_LEVEL_SHIFT(1);
+
+    return vaff;
+}
+
 #endif /* __ASM_DOMAIN_H__ */
 
 /*
-- 
2.1.4

[PATCH v7 3/8] xen/arm: Use the new functions for vCPUID/vaffinity transformation

[Chen Baozi]

From: Chen Baozi <baozich@gmail.com>

There are 3 places to change:

* Initialise vMPIDR value in vcpu_initialise()
* Find the vCPU from vMPIDR affinity information when accessing GICD
  registers in vGIC
* Find the vCPU from vMPIDR affinity information when booting with vPSCI
  in vGIC
  - Also make the code for PSCI 0.1 use MPIDR-like value as the cpuid.

Signed-off-by: Chen Baozi <baozich@gmail.com>
Reviewed-by: Julien Grall <julien.grall@citrix.com>
---
 xen/arch/arm/domain.c  | 6 +-----
 xen/arch/arm/vgic-v3.c | 2 +-
 xen/arch/arm/vpsci.c   | 5 +----
 3 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 2bde26e..0cf147c 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -501,11 +501,7 @@ int vcpu_initialise(struct vcpu *v)
 
     v->arch.sctlr = SCTLR_GUEST_INIT;
 
-    /*
-     * By default exposes an SMP system with AFF0 set to the VCPU ID
-     * TODO: Handle multi-threading processor and cluster
-     */
-    v->arch.vmpidr = MPIDR_SMP | (v->vcpu_id << MPIDR_AFF0_SHIFT);
+    v->arch.vmpidr = MPIDR_SMP | vcpuid_to_vaffinity(v->vcpu_id);
 
     v->arch.actlr = READ_SYSREG32(ACTLR_EL1);
 
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 540f85f..ef9a71a 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -61,7 +61,7 @@ static struct vcpu *vgic_v3_irouter_to_vcpu(struct domain *d, uint64_t irouter)
     if ( irouter & GICD_IROUTER_SPI_MODE_ANY )
         return d->vcpu[0];
 
-    vcpu_id = irouter & MPIDR_AFF0_MASK;
+    vcpu_id = vaffinity_to_vcpuid(irouter);
     if ( vcpu_id >= d->max_vcpus )
         return NULL;
 
diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
index 5d899be..aebe1e2 100644
--- a/xen/arch/arm/vpsci.c
+++ b/xen/arch/arm/vpsci.c
@@ -32,10 +32,7 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
     int is_thumb = entry_point & 1;
     register_t vcpuid;
 
-    if( ver == XEN_PSCI_V_0_2 )
-        vcpuid = (target_cpu & MPIDR_HWID_MASK);
-    else
-        vcpuid = target_cpu;
+    vcpuid = vaffinity_to_vcpuid(target_cpu);
 
     if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
         return PSCI_INVALID_PARAMETERS;
-- 
2.1.4

[PATCH v7 4/8] xen/arm: Use struct sgi_target instead of vcpu_mask in vgic_to_sgi

[Chen Baozi]

From: Chen Baozi <baozich@gmail.com>

The old unsigned long type of vcpu_mask can only express 64 cpus at the
most, which might not be enough for the guest which used vGICv3. We
introduce a new struct sgi_target for the target cpu list of SGI, which
holds the affinity path information. For GICv2 that has no affinity
level, we can just set the corresponding fields to be 0.

Signed-off-by: Chen Baozi <baozich@gmail.com>
---
 xen/arch/arm/vgic-v2.c            |  7 +++---
 xen/arch/arm/vgic-v3.c            | 10 +++++----
 xen/arch/arm/vgic.c               | 46 +++++++++++++++++++--------------------
 xen/include/asm-arm/gic.h         |  1 +
 xen/include/asm-arm/gic_v3_defs.h |  4 ++++
 xen/include/asm-arm/vgic.h        |  9 +++++++-
 6 files changed, 46 insertions(+), 31 deletions(-)

diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index 3be1a51..5949cf1 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -201,16 +201,17 @@ static int vgic_v2_to_sgi(struct vcpu *v, register_t sgir)
     int virq;
     int irqmode;
     enum gic_sgi_mode sgi_mode;
-    unsigned long vcpu_mask = 0;
+    struct sgi_target target;
 
+    memset(&target, 0, sizeof(struct sgi_target));
     irqmode = (sgir & GICD_SGI_TARGET_LIST_MASK) >> GICD_SGI_TARGET_LIST_SHIFT;
     virq = (sgir & GICD_SGI_INTID_MASK);
-    vcpu_mask = (sgir & GICD_SGI_TARGET_MASK) >> GICD_SGI_TARGET_SHIFT;
 
     /* Map GIC sgi value to enum value */
     switch ( irqmode )
     {
     case GICD_SGI_TARGET_LIST_VAL:
+        target.list = (sgir & GICD_SGI_TARGET_MASK) >> GICD_SGI_TARGET_SHIFT;
         sgi_mode = SGI_TARGET_LIST;
         break;
     case GICD_SGI_TARGET_OTHERS_VAL:
@@ -226,7 +227,7 @@ static int vgic_v2_to_sgi(struct vcpu *v, register_t sgir)
         return 0;
     }
 
-    return vgic_to_sgi(v, sgir, sgi_mode, virq, vcpu_mask);
+    return vgic_to_sgi(v, sgir, sgi_mode, virq, &target);
 }
 
 static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index ef9a71a..93610d0 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -977,17 +977,19 @@ static int vgic_v3_to_sgi(struct vcpu *v, register_t sgir)
     int virq;
     int irqmode;
     enum gic_sgi_mode sgi_mode;
-    unsigned long vcpu_mask = 0;
+    struct sgi_target target;
 
+    memset(&target, 0, sizeof(struct sgi_target));
     irqmode = (sgir >> ICH_SGI_IRQMODE_SHIFT) & ICH_SGI_IRQMODE_MASK;
     virq = (sgir >> ICH_SGI_IRQ_SHIFT ) & ICH_SGI_IRQ_MASK;
-    /* SGI's are injected at Rdist level 0. ignoring affinity 1, 2, 3 */
-    vcpu_mask = sgir & ICH_SGI_TARGETLIST_MASK;
 
     /* Map GIC sgi value to enum value */
     switch ( irqmode )
     {
     case ICH_SGI_TARGET_LIST:
+        /* We assume that only AFF1 is used in ICC_SGI1R_EL1. */
+        target.aff1 = (sgir >> ICH_SGI_AFFINITY_LEVEL(1)) & ICH_SGI_AFFx_MASK;
+        target.list = sgir & ICH_SGI_TARGETLIST_MASK;
         sgi_mode = SGI_TARGET_LIST;
         break;
     case ICH_SGI_TARGET_OTHERS:
@@ -998,7 +1000,7 @@ static int vgic_v3_to_sgi(struct vcpu *v, register_t sgir)
         return 0;
     }
 
-    return vgic_to_sgi(v, sgir, sgi_mode, virq, vcpu_mask);
+    return vgic_to_sgi(v, sgir, sgi_mode, virq, &target);
 }
 
 static int vgic_v3_emulate_sysreg(struct cpu_user_regs *regs, union hsr hsr)
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 7b387b7..0173c8d 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -318,15 +318,14 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
     }
 }
 
-/* TODO: unsigned long is used to fit vcpu_mask.*/
 int vgic_to_sgi(struct vcpu *v, register_t sgir, enum gic_sgi_mode irqmode, int virq,
-                unsigned long vcpu_mask)
+                struct sgi_target *target)
 {
     struct domain *d = v->domain;
     int vcpuid;
     int i;
-
-    ASSERT(d->max_vcpus < 8*sizeof(vcpu_mask));
+    unsigned int base;
+    unsigned long int bitmap;
 
     ASSERT( virq < 16 );
 
@@ -334,29 +333,40 @@ int vgic_to_sgi(struct vcpu *v, register_t sgir, enum gic_sgi_mode irqmode, int
     {
     case SGI_TARGET_LIST:
         perfc_incr(vgic_sgi_list);
+        base = (target->aff1 << 4) | (target->aff2 << 12) |
+               (target->aff3 << 20);
+        bitmap = target->list;
+        for_each_set_bit( i, &bitmap, sizeof(target->list)*8 )
+        {
+            vcpuid = base + i;
+            if ( d->vcpu[vcpuid] != NULL && !is_vcpu_online(d->vcpu[vcpuid]) )
+            {
+                gprintk(XENLOG_WARNING, "VGIC: write r=%"PRIregister" \
+                        target->list=%hx, wrong CPUTargetList \n",
+                        sgir, target->list);
+                continue;
+            }
+            vgic_vcpu_inject_irq(d->vcpu[vcpuid], virq);
+        }
         break;
     case SGI_TARGET_OTHERS:
-        /*
-         * We expect vcpu_mask to be 0 for SGI_TARGET_OTHERS and
-         * SGI_TARGET_SELF mode. So Force vcpu_mask to 0
-         */
         perfc_incr(vgic_sgi_others);
-        vcpu_mask = 0;
         for ( i = 0; i < d->max_vcpus; i++ )
         {
             if ( i != current->vcpu_id && d->vcpu[i] != NULL &&
                  is_vcpu_online(d->vcpu[i]) )
-                set_bit(i, &vcpu_mask);
+                vgic_vcpu_inject_irq(d->vcpu[i], virq);
         }
         break;
     case SGI_TARGET_SELF:
         /*
          * We expect vcpu_mask to be 0 for SGI_TARGET_OTHERS and
-         * SGI_TARGET_SELF mode. So Force vcpu_mask to 0
+         * SGI_TARGET_SELF mode. Since we have already clear the
+         * cpumask on the caller of this function, we assume it
+         * has been set to 0 here.
          */
         perfc_incr(vgic_sgi_self);
-        vcpu_mask = 0;
-        set_bit(current->vcpu_id, &vcpu_mask);
+        vgic_vcpu_inject_irq(d->vcpu[current->vcpu_id], virq);
         break;
     default:
         gprintk(XENLOG_WARNING,
@@ -365,16 +375,6 @@ int vgic_to_sgi(struct vcpu *v, register_t sgir, enum gic_sgi_mode irqmode, int
         return 0;
     }
 
-    for_each_set_bit( vcpuid, &vcpu_mask, d->max_vcpus )
-    {
-        if ( d->vcpu[vcpuid] != NULL && !is_vcpu_online(d->vcpu[vcpuid]) )
-        {
-            gprintk(XENLOG_WARNING, "VGIC: write r=%"PRIregister" \
-                    vcpu_mask=%lx, wrong CPUTargetList\n", sgir, vcpu_mask);
-            continue;
-        }
-        vgic_vcpu_inject_irq(d->vcpu[vcpuid], virq);
-    }
     return 1;
 }
 
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 463fffb..c6ef4bf 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -64,6 +64,7 @@
 #define GICD_SGI_TARGET_SELF_VAL     (2)
 #define GICD_SGI_TARGET_SHIFT        (16)
 #define GICD_SGI_TARGET_MASK         (0xFFUL<<GICD_SGI_TARGET_SHIFT)
+#define GICD_SGI_TARGET_BITS         (8)
 #define GICD_SGI_GROUP1              (1UL<<15)
 #define GICD_SGI_INTID_MASK          (0xFUL)
 
diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h
index 556f114..333aa56 100644
--- a/xen/include/asm-arm/gic_v3_defs.h
+++ b/xen/include/asm-arm/gic_v3_defs.h
@@ -152,6 +152,10 @@
 #define ICH_SGI_IRQ_SHIFT            24
 #define ICH_SGI_IRQ_MASK             0xf
 #define ICH_SGI_TARGETLIST_MASK      0xffff
+#define ICH_SGI_TARGET_BITS          16
+#define ICH_SGI_AFFx_MASK            0xff
+#define ICH_SGI_AFFINITY_LEVEL(x)    (16 * (x))
+
 #endif /* __ASM_ARM_GIC_V3_DEFS_H__ */
 
 /*
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index 6dcdf9f..cc68c27 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -98,6 +98,13 @@ struct vgic_irq_rank {
     };
 };
 
+struct sgi_target {
+    uint8_t aff3;
+    uint8_t aff2;
+    uint8_t aff1;
+    uint16_t list;
+};
+
 struct vgic_ops {
     /* Initialize vGIC */
     int (*vcpu_init)(struct vcpu *v);
@@ -201,7 +208,7 @@ DEFINE_VGIC_OPS(3)
 extern int vcpu_vgic_free(struct vcpu *v);
 extern int vgic_to_sgi(struct vcpu *v, register_t sgir,
                        enum gic_sgi_mode irqmode, int virq,
-                       unsigned long vcpu_mask);
+                       struct sgi_target *target);
 extern void vgic_migrate_irq(struct vcpu *old, struct vcpu *new, unsigned int irq);
 
 /* Reserve a specific guest vIRQ */
-- 
2.1.4

[PATCH v7 5/8] tools/libxl: Set 'reg' of cpu node equal to MPIDR affinity for domU

[Chen Baozi]

From: Chen Baozi <baozich@gmail.com>

According to ARM CPUs bindings, the reg field should match the MPIDR's
affinity bits. We will use AFF0 and AFF1 when constructing the reg value
of the guest at the moment, for it is enough for the current max vcpu
number.

Signed-off-by: Chen Baozi <baozich@gmail.com>
Reviewed-by: Julien Grall <julien.grall@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl_arm.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index c5088c4..5f3c434 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -272,6 +272,7 @@ static int make_cpus_node(libxl__gc *gc, void *fdt, int nr_cpus,
                           const struct arch_info *ainfo)
 {
     int res, i;
+    uint64_t mpidr_aff;
 
     res = fdt_begin_node(fdt, "cpus");
     if (res) return res;
@@ -283,7 +284,16 @@ static int make_cpus_node(libxl__gc *gc, void *fdt, int nr_cpus,
     if (res) return res;
 
     for (i = 0; i < nr_cpus; i++) {
-        const char *name = GCSPRINTF("cpu@%d", i);
+        const char *name;
+
+        /*
+         * According to ARM CPUs bindings, the reg field should match
+         * the MPIDR's affinity bits. We will use AFF0 and AFF1 when
+         * constructing the reg value of the guest at the moment, for it
+         * is enough for the current max vcpu number.
+         */
+        mpidr_aff = (i & 0x0f) | (((i >> 4) & 0xff) << 8);
+        name = GCSPRINTF("cpu@%"PRIx64, mpidr_aff);
 
         res = fdt_begin_node(fdt, name);
         if (res) return res;
@@ -297,7 +307,7 @@ static int make_cpus_node(libxl__gc *gc, void *fdt, int nr_cpus,
         res = fdt_property_string(fdt, "enable-method", "psci");
         if (res) return res;
 
-        res = fdt_property_regs(gc, fdt, 1, 0, 1, (uint64_t)i);
+        res = fdt_property_regs(gc, fdt, 1, 0, 1, mpidr_aff);
         if (res) return res;
 
         res = fdt_end_node(fdt);
-- 
2.1.4

[PATCH v7 6/8] xen/arm: Set 'reg' of cpu node for dom0 to match MPIDR's affinity

[Chen Baozi]

From: Chen Baozi <baozich@gmail.com>

According to ARM CPUs bindings, the reg field should match the MPIDR's
affinity bits. We will use AFF0 and AFF1 when constructing the reg value
of the guest at the moment, for it is enough for the current max vcpu
number.

Signed-off-by: Chen Baozi <baozich@gmail.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/domain_build.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index a156de9..12b46ca 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -712,6 +712,7 @@ static int make_cpus_node(const struct domain *d, void *fdt,
     char buf[15];
     u32 clock_frequency;
     bool_t clock_valid;
+    uint64_t mpidr_aff;
 
     DPRINT("Create cpus node\n");
 
@@ -761,9 +762,16 @@ static int make_cpus_node(const struct domain *d, void *fdt,
 
     for ( cpu = 0; cpu < d->max_vcpus; cpu++ )
     {
-        DPRINT("Create cpu@%u node\n", cpu);
+        /*
+         * According to ARM CPUs bindings, the reg field should match
+         * the MPIDR's affinity bits. We will use AFF0 and AFF1 when
+         * constructing the reg value of the guest at the moment, for it
+         * is enough for the current max vcpu number.
+         */
+        mpidr_aff = vcpuid_to_vaffinity(cpu);
+        DPRINT("Create cpu@%"PRIx64" (logical CPUID: %d) node\n", mpidr_aff, cpu);
 
-        snprintf(buf, sizeof(buf), "cpu@%u", cpu);
+        snprintf(buf, sizeof(buf), "cpu@%lx", mpidr_aff);
         res = fdt_begin_node(fdt, buf);
         if ( res )
             return res;
@@ -776,7 +784,7 @@ static int make_cpus_node(const struct domain *d, void *fdt,
         if ( res )
             return res;
 
-        res = fdt_property_cell(fdt, "reg", cpu);
+        res = fdt_property_cell(fdt, "reg", mpidr_aff);
         if ( res )
             return res;
 
-- 
2.1.4

[PATCH v7 7/8] xen/arm: make domain_max_vcpus return value from vgic_ops

[Chen Baozi]

From: Chen Baozi <baozich@gmail.com>

When a guest uses vGICv2, the maximum number of vCPU it can support
should not be as many as MAX_VIRT_CPUS, which will be more than 8
when GICv3 is used on arm64. 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@gmail.com>
---
 xen/arch/arm/domain.c        | 9 +++++++++
 xen/arch/arm/vgic-v2.c       | 1 +
 xen/arch/arm/vgic-v3.c       | 7 +++++++
 xen/include/asm-arm/domain.h | 5 +----
 xen/include/asm-arm/vgic.h   | 2 ++
 5 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 0cf147c..df71a60 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -890,6 +890,15 @@ void vcpu_block_unless_event_pending(struct vcpu *v)
         vcpu_unblock(current);
 }
 
+unsigned int domain_max_vcpus(const struct domain *d)
+{
+    if ( !d->arch.vgic.handler )
+        return MAX_VIRT_CPUS;
+    else
+        return min_t(unsigned int, MAX_VIRT_CPUS,
+                     d->arch.vgic.handler->max_vcpus);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index 5949cf1..bbeb740 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -585,6 +585,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,
+    .max_vcpus = 8,
 };
 
 /*
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 93610d0..417729b 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -32,6 +32,12 @@
 #include <asm/gic.h>
 #include <asm/vgic.h>
 
+/*
+ * We will use both AFF1 and AFF0 in (v)MPIDR. Thus, the max number of CPU
+ * that can be supported is up to 4096(256*16) in theory.
+ */
+#define GICV3_MAX_CPUS  4096
+
 /* GICD_PIDRn register values for ARM implementations */
 #define GICV3_GICD_PIDR0  0x92
 #define GICV3_GICD_PIDR1  0xb4
@@ -1204,6 +1210,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,
+    .max_vcpus = GICV3_MAX_CPUS,
 };
 
 /*
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 35b9a6d..23598dd 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/vgic.h b/xen/include/asm-arm/vgic.h
index cc68c27..419e767 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -117,6 +117,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);
+    /* Maximum number of vCPU supported */
+    const unsigned int max_vcpus;
 };
 
 /* Number of ranks of interrupt registers for a domain */
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH v7 8/8] xen/arm64: increase MAX_VIRT_CPUS to 128 on arm64

[Chen Baozi]

From: Chen Baozi <baozich@gmail.com>

After we have increased the size of GICR in address space for guest
and made use of both AFF0 and AFF1 in (v)MPIDR, we are now able to
support up to 4096 vCPUs in theory. However, it will cost 512M
address space for GICR region, which is unnecessary big at the
moment. Considering the max CPU number that GIC-500 can support and
the old value of MAX_VIRT_CPUS before commit aa25a61, we increase
its value to 128.

Signed-off-by: Chen Baozi <baozich@gmail.com>
---
Since the domain_max_vcpus has been changed to depends on vgic_ops,
we could have done more work in order to drop the definition of
MAX_VIRT_CPUS. However, because it is still used for some conditional
compilation in common code, I think that would be better done in a
separate cleanup patch series.

 xen/arch/arm/vgic-v3.c       | 1 -
 xen/include/asm-arm/config.h | 4 ++++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 417729b..9012818 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -895,7 +895,6 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
         rank = vgic_rank_offset(v, 64, gicd_reg - GICD_IROUTER,
                                 DABT_DOUBLE_WORD);
         if ( rank == NULL ) goto write_ignore;
-        BUG_ON(v->domain->max_vcpus > 8);
         new_irouter = *r;
         vgic_lock_rank(v, rank, flags);
 
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index 3b23e05..817c216 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -47,7 +47,11 @@
 #define NR_CPUS 128
 #endif
 
+#ifdef CONFIG_ARM_64
+#define MAX_VIRT_CPUS 128
+#else
 #define MAX_VIRT_CPUS 8
+#endif
 
 #define asmlinkage /* Nothing needed */
 
-- 
2.1.4

Re: [PATCH v7 4/8] xen/arm: Use struct sgi_target instead of vcpu_mask in vgic_to_sgi

[Chen Baozi]

On Thu, Jun 11, 2015 at 09:05:06PM +0800, Chen Baozi wrote:
> From: Chen Baozi <baozich@gmail.com>
> 
> The old unsigned long type of vcpu_mask can only express 64 cpus at the
> most, which might not be enough for the guest which used vGICv3. We
> introduce a new struct sgi_target for the target cpu list of SGI, which
> holds the affinity path information. For GICv2 that has no affinity
> level, we can just set the corresponding fields to be 0.
> 
> Signed-off-by: Chen Baozi <baozich@gmail.com>
> ---
>  xen/arch/arm/vgic-v2.c            |  7 +++---
>  xen/arch/arm/vgic-v3.c            | 10 +++++----
>  xen/arch/arm/vgic.c               | 46 +++++++++++++++++++--------------------
>  xen/include/asm-arm/gic.h         |  1 +
>  xen/include/asm-arm/gic_v3_defs.h |  4 ++++
>  xen/include/asm-arm/vgic.h        |  9 +++++++-
>  6 files changed, 46 insertions(+), 31 deletions(-)
> 
> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
> index 3be1a51..5949cf1 100644
> --- a/xen/arch/arm/vgic-v2.c
> +++ b/xen/arch/arm/vgic-v2.c
> @@ -201,16 +201,17 @@ static int vgic_v2_to_sgi(struct vcpu *v, register_t sgir)
>      int virq;
>      int irqmode;
>      enum gic_sgi_mode sgi_mode;
> -    unsigned long vcpu_mask = 0;
> +    struct sgi_target target;
>  
> +    memset(&target, 0, sizeof(struct sgi_target));
>      irqmode = (sgir & GICD_SGI_TARGET_LIST_MASK) >> GICD_SGI_TARGET_LIST_SHIFT;
>      virq = (sgir & GICD_SGI_INTID_MASK);
> -    vcpu_mask = (sgir & GICD_SGI_TARGET_MASK) >> GICD_SGI_TARGET_SHIFT;
>  
>      /* Map GIC sgi value to enum value */
>      switch ( irqmode )
>      {
>      case GICD_SGI_TARGET_LIST_VAL:
> +        target.list = (sgir & GICD_SGI_TARGET_MASK) >> GICD_SGI_TARGET_SHIFT;
>          sgi_mode = SGI_TARGET_LIST;
>          break;
>      case GICD_SGI_TARGET_OTHERS_VAL:
> @@ -226,7 +227,7 @@ static int vgic_v2_to_sgi(struct vcpu *v, register_t sgir)
>          return 0;
>      }
>  
> -    return vgic_to_sgi(v, sgir, sgi_mode, virq, vcpu_mask);
> +    return vgic_to_sgi(v, sgir, sgi_mode, virq, &target);
>  }
>  
>  static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index ef9a71a..93610d0 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -977,17 +977,19 @@ static int vgic_v3_to_sgi(struct vcpu *v, register_t sgir)
>      int virq;
>      int irqmode;
>      enum gic_sgi_mode sgi_mode;
> -    unsigned long vcpu_mask = 0;
> +    struct sgi_target target;
>  
> +    memset(&target, 0, sizeof(struct sgi_target));
>      irqmode = (sgir >> ICH_SGI_IRQMODE_SHIFT) & ICH_SGI_IRQMODE_MASK;
>      virq = (sgir >> ICH_SGI_IRQ_SHIFT ) & ICH_SGI_IRQ_MASK;
> -    /* SGI's are injected at Rdist level 0. ignoring affinity 1, 2, 3 */
> -    vcpu_mask = sgir & ICH_SGI_TARGETLIST_MASK;
>  
>      /* Map GIC sgi value to enum value */
>      switch ( irqmode )
>      {
>      case ICH_SGI_TARGET_LIST:
> +        /* We assume that only AFF1 is used in ICC_SGI1R_EL1. */
> +        target.aff1 = (sgir >> ICH_SGI_AFFINITY_LEVEL(1)) & ICH_SGI_AFFx_MASK;
> +        target.list = sgir & ICH_SGI_TARGETLIST_MASK;
>          sgi_mode = SGI_TARGET_LIST;
>          break;
>      case ICH_SGI_TARGET_OTHERS:
> @@ -998,7 +1000,7 @@ static int vgic_v3_to_sgi(struct vcpu *v, register_t sgir)
>          return 0;
>      }
>  
> -    return vgic_to_sgi(v, sgir, sgi_mode, virq, vcpu_mask);
> +    return vgic_to_sgi(v, sgir, sgi_mode, virq, &target);
>  }
>  
>  static int vgic_v3_emulate_sysreg(struct cpu_user_regs *regs, union hsr hsr)
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 7b387b7..0173c8d 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -318,15 +318,14 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
>      }
>  }
>  
> -/* TODO: unsigned long is used to fit vcpu_mask.*/
>  int vgic_to_sgi(struct vcpu *v, register_t sgir, enum gic_sgi_mode irqmode, int virq,
> -                unsigned long vcpu_mask)
> +                struct sgi_target *target)
>  {
>      struct domain *d = v->domain;
>      int vcpuid;
>      int i;
> -
> -    ASSERT(d->max_vcpus < 8*sizeof(vcpu_mask));
> +    unsigned int base;
> +    unsigned long int bitmap;
>  
>      ASSERT( virq < 16 );
>  
> @@ -334,29 +333,40 @@ int vgic_to_sgi(struct vcpu *v, register_t sgir, enum gic_sgi_mode irqmode, int
>      {
>      case SGI_TARGET_LIST:
>          perfc_incr(vgic_sgi_list);
> +        base = (target->aff1 << 4) | (target->aff2 << 12) |
> +               (target->aff3 << 20);
> +        bitmap = target->list;
> +        for_each_set_bit( i, &bitmap, sizeof(target->list)*8 )
> +        {
> +            vcpuid = base + i;
> +            if ( d->vcpu[vcpuid] != NULL && !is_vcpu_online(d->vcpu[vcpuid]) )
> +            {
> +                gprintk(XENLOG_WARNING, "VGIC: write r=%"PRIregister" \
> +                        target->list=%hx, wrong CPUTargetList \n",
> +                        sgir, target->list);
> +                continue;
> +            }
> +            vgic_vcpu_inject_irq(d->vcpu[vcpuid], virq);
> +        }
>          break;
>      case SGI_TARGET_OTHERS:
> -        /*
> -         * We expect vcpu_mask to be 0 for SGI_TARGET_OTHERS and
> -         * SGI_TARGET_SELF mode. So Force vcpu_mask to 0
> -         */
>          perfc_incr(vgic_sgi_others);
> -        vcpu_mask = 0;
>          for ( i = 0; i < d->max_vcpus; i++ )
>          {
>              if ( i != current->vcpu_id && d->vcpu[i] != NULL &&
>                   is_vcpu_online(d->vcpu[i]) )
> -                set_bit(i, &vcpu_mask);
> +                vgic_vcpu_inject_irq(d->vcpu[i], virq);
>          }
>          break;
>      case SGI_TARGET_SELF:
>          /*
>           * We expect vcpu_mask to be 0 for SGI_TARGET_OTHERS and
> -         * SGI_TARGET_SELF mode. So Force vcpu_mask to 0
> +         * SGI_TARGET_SELF mode. Since we have already clear the
> +         * cpumask on the caller of this function, we assume it
> +         * has been set to 0 here.
>           */

Sorry, the comment should be removed in this version...

>          perfc_incr(vgic_sgi_self);
> -        vcpu_mask = 0;
> -        set_bit(current->vcpu_id, &vcpu_mask);
> +        vgic_vcpu_inject_irq(d->vcpu[current->vcpu_id], virq);
>          break;
>      default:
>          gprintk(XENLOG_WARNING,
> @@ -365,16 +375,6 @@ int vgic_to_sgi(struct vcpu *v, register_t sgir, enum gic_sgi_mode irqmode, int
>          return 0;
>      }
>  
> -    for_each_set_bit( vcpuid, &vcpu_mask, d->max_vcpus )
> -    {
> -        if ( d->vcpu[vcpuid] != NULL && !is_vcpu_online(d->vcpu[vcpuid]) )
> -        {
> -            gprintk(XENLOG_WARNING, "VGIC: write r=%"PRIregister" \
> -                    vcpu_mask=%lx, wrong CPUTargetList\n", sgir, vcpu_mask);
> -            continue;
> -        }
> -        vgic_vcpu_inject_irq(d->vcpu[vcpuid], virq);
> -    }
>      return 1;
>  }
>  
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 463fffb..c6ef4bf 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -64,6 +64,7 @@
>  #define GICD_SGI_TARGET_SELF_VAL     (2)
>  #define GICD_SGI_TARGET_SHIFT        (16)
>  #define GICD_SGI_TARGET_MASK         (0xFFUL<<GICD_SGI_TARGET_SHIFT)
> +#define GICD_SGI_TARGET_BITS         (8)
>  #define GICD_SGI_GROUP1              (1UL<<15)
>  #define GICD_SGI_INTID_MASK          (0xFUL)
>  
> diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h
> index 556f114..333aa56 100644
> --- a/xen/include/asm-arm/gic_v3_defs.h
> +++ b/xen/include/asm-arm/gic_v3_defs.h
> @@ -152,6 +152,10 @@
>  #define ICH_SGI_IRQ_SHIFT            24
>  #define ICH_SGI_IRQ_MASK             0xf
>  #define ICH_SGI_TARGETLIST_MASK      0xffff
> +#define ICH_SGI_TARGET_BITS          16
> +#define ICH_SGI_AFFx_MASK            0xff
> +#define ICH_SGI_AFFINITY_LEVEL(x)    (16 * (x))
> +
>  #endif /* __ASM_ARM_GIC_V3_DEFS_H__ */
>  
>  /*
> diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
> index 6dcdf9f..cc68c27 100644
> --- a/xen/include/asm-arm/vgic.h
> +++ b/xen/include/asm-arm/vgic.h
> @@ -98,6 +98,13 @@ struct vgic_irq_rank {
>      };
>  };
>  
> +struct sgi_target {
> +    uint8_t aff3;
> +    uint8_t aff2;
> +    uint8_t aff1;
> +    uint16_t list;
> +};
> +
>  struct vgic_ops {
>      /* Initialize vGIC */
>      int (*vcpu_init)(struct vcpu *v);
> @@ -201,7 +208,7 @@ DEFINE_VGIC_OPS(3)
>  extern int vcpu_vgic_free(struct vcpu *v);
>  extern int vgic_to_sgi(struct vcpu *v, register_t sgir,
>                         enum gic_sgi_mode irqmode, int virq,
> -                       unsigned long vcpu_mask);
> +                       struct sgi_target *target);
>  extern void vgic_migrate_irq(struct vcpu *old, struct vcpu *new, unsigned int irq);
>  
>  /* Reserve a specific guest vIRQ */
> -- 
> 2.1.4
> 

Re: [PATCH v7 4/8] xen/arm: Use struct sgi_target instead of vcpu_mask in vgic_to_sgi

[Julien Grall]

Hi Chen,

On 11/06/2015 09:05, Chen Baozi wrote:
> From: Chen Baozi <baozich@gmail.com>
>
> The old unsigned long type of vcpu_mask can only express 64 cpus at the
> most, which might not be enough for the guest which used vGICv3. We
> introduce a new struct sgi_target for the target cpu list of SGI, which
> holds the affinity path information. For GICv2 that has no affinity
> level, we can just set the corresponding fields to be 0.

The whole point of this patch is to add support for AFF1 in GICv3... not 
replacing vcpu_mask by sgi_target. The latter is just how you decided to 
implement AFF1.

Please ensure that your commit message/tittle reflect it.

[..]

>   static int vgic_v3_emulate_sysreg(struct cpu_user_regs *regs, union hsr hsr)
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 7b387b7..0173c8d 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -318,15 +318,14 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
>       }
>   }
>
> -/* TODO: unsigned long is used to fit vcpu_mask.*/
>   int vgic_to_sgi(struct vcpu *v, register_t sgir, enum gic_sgi_mode irqmode, int virq,
> -                unsigned long vcpu_mask)
> +                struct sgi_target *target)

const struct sgi_target...

[..]

> @@ -334,29 +333,40 @@ int vgic_to_sgi(struct vcpu *v, register_t sgir, enum gic_sgi_mode irqmode, int
>       {
>       case SGI_TARGET_LIST:
>           perfc_incr(vgic_sgi_list);
> +        base = (target->aff1 << 4) | (target->aff2 << 12) |
> +               (target->aff3 << 20);
> +        bitmap = target->list;
> +        for_each_set_bit( i, &bitmap, sizeof(target->list)*8 )

Coding style.

sizeof(target->list) * 8

[..]

> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 463fffb..c6ef4bf 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -64,6 +64,7 @@
>   #define GICD_SGI_TARGET_SELF_VAL     (2)
>   #define GICD_SGI_TARGET_SHIFT        (16)
>   #define GICD_SGI_TARGET_MASK         (0xFFUL<<GICD_SGI_TARGET_SHIFT)
> +#define GICD_SGI_TARGET_BITS         (8)
>   #define GICD_SGI_GROUP1              (1UL<<15)
>   #define GICD_SGI_INTID_MASK          (0xFUL)
>
> diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h
> index 556f114..333aa56 100644
> --- a/xen/include/asm-arm/gic_v3_defs.h
> +++ b/xen/include/asm-arm/gic_v3_defs.h
> @@ -152,6 +152,10 @@
>   #define ICH_SGI_IRQ_SHIFT            24
>   #define ICH_SGI_IRQ_MASK             0xf
>   #define ICH_SGI_TARGETLIST_MASK      0xffff
> +#define ICH_SGI_TARGET_BITS          16
> +#define ICH_SGI_AFFx_MASK            0xff
> +#define ICH_SGI_AFFINITY_LEVEL(x)    (16 * (x))
> +
>   #endif /* __ASM_ARM_GIC_V3_DEFS_H__ */
>
>   /*
> diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
> index 6dcdf9f..cc68c27 100644
> --- a/xen/include/asm-arm/vgic.h
> +++ b/xen/include/asm-arm/vgic.h
> @@ -98,6 +98,13 @@ struct vgic_irq_rank {
>       };
>   };
>
> +struct sgi_target {
> +    uint8_t aff3;
> +    uint8_t aff2;

I don't see why you are not using aff2 and aff3 at all the the vGICv* 
drivers. Please drop them.

> +    uint8_t aff1;
> +    uint16_t list;
> +};
> +

Regards,

-- 
Julien Grall

Re: [PATCH v7 3/8] xen/arm: Use the new functions for vCPUID/vaffinity transformation

[Julien Grall]

Hi Chen,

On 11/06/2015 09:05, Chen Baozi wrote:
> From: Chen Baozi <baozich@gmail.com>
>
> There are 3 places to change:
>
> * Initialise vMPIDR value in vcpu_initialise()
> * Find the vCPU from vMPIDR affinity information when accessing GICD
>    registers in vGIC
> * Find the vCPU from vMPIDR affinity information when booting with vPSCI
>    in vGIC
>    - Also make the code for PSCI 0.1 use MPIDR-like value as the cpuid.

After the discussion on the previous version of this patch [1], I was 
expected at least some update in the commit message. Or if it wasn't 
clear, carry on the discussion after (---) until we agreed on what to do.

Regards,

[1] http://lists.xen.org/archives/html/xen-devel/2015-06/msg00807.html

-- 
Julien Grall

Re: [PATCH v7 6/8] xen/arm: Set 'reg' of cpu node for dom0 to match MPIDR's affinity

[Julien Grall]

Hi Chen,

On 11/06/2015 09:05, Chen Baozi wrote:
> From: Chen Baozi <baozich@gmail.com>
>
> According to ARM CPUs bindings, the reg field should match the MPIDR's
> affinity bits. We will use AFF0 and AFF1 when constructing the reg value
> of the guest at the moment, for it is enough for the current max vcpu
> number.
>
> Signed-off-by: Chen Baozi <baozich@gmail.com>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Julien Grall <julien.grall@citrix.com>

Regards,

-- 
Julien Grall

Re: [PATCH v7 7/8] xen/arm: make domain_max_vcpus return value from vgic_ops

[Julien Grall]

Hi Chen,

On 11/06/2015 09:05, Chen Baozi wrote:
> From: Chen Baozi <baozich@gmail.com>
>
> When a guest uses vGICv2, the maximum number of vCPU it can support
> should not be as many as MAX_VIRT_CPUS, which will be more than 8
> when GICv3 is used on arm64.

This sentence is not clear to me. What about:

"Each vGIC driver support a different maximum numbers of vCPUs. They are 
relying on the domain creation code to never create domain with vCPUs 
than their limitation.

The vGICv2 is limited to 8 vCPUs because of the specifications. In 
another side, the vGICv3 has been modified to support 2 level of 
affinity (AFF1 and AFF0) which theoretically allow a guest to use more 
than 4096."

 > So the domain_max_vcpus should return
> the value according to the vGIC the domain uses.

Which is not entirely true with your the new changes in this version. 
Please update the commit message.

> 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@gmail.com>
> ---
>   xen/arch/arm/domain.c        | 9 +++++++++
>   xen/arch/arm/vgic-v2.c       | 1 +
>   xen/arch/arm/vgic-v3.c       | 7 +++++++
>   xen/include/asm-arm/domain.h | 5 +----
>   xen/include/asm-arm/vgic.h   | 2 ++
>   5 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 0cf147c..df71a60 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -890,6 +890,15 @@ void vcpu_block_unless_event_pending(struct vcpu *v)
>           vcpu_unblock(current);
>   }
>
> +unsigned int domain_max_vcpus(const struct domain *d)
> +{
> +    if ( !d->arch.vgic.handler )
> +        return MAX_VIRT_CPUS;

You should mention somewhere (commit message and maybe code) why this is 
necessary. Otherwise someone may want to remove it later by mistake.

> +    else
> +        return min_t(unsigned int, MAX_VIRT_CPUS,
> +                     d->arch.vgic.handler->max_vcpus);
> +}
> +
>   /*
>    * Local variables:
>    * mode: C
> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
> index 5949cf1..bbeb740 100644
> --- a/xen/arch/arm/vgic-v2.c
> +++ b/xen/arch/arm/vgic-v2.c
> @@ -585,6 +585,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,
> +    .max_vcpus = 8,
>   };
>
>   /*
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index 93610d0..417729b 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -32,6 +32,12 @@
>   #include <asm/gic.h>
>   #include <asm/vgic.h>
>
> +/*
> + * We will use both AFF1 and AFF0 in (v)MPIDR. Thus, the max number of CPU
> + * that can be supported is up to 4096(256*16) in theory.
> + */
> +#define GICV3_MAX_CPUS  4096
> +

As you did for GICv2, please remove the define and opencode the value 
below. You can move the comment on top of the field assignation.

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel