[RFC 0/4] Support more than 8 vcpus on arm64 with GICv3

[RFC 0/4] 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. In this patch series, I
postpone setting the size of GICR0 to the point when max_vcpus of a domU is
determined to support more than 8 redistributors.

However, I am not quite sure that decoupling the rdist base and size
setting of domU to different functions is appropriate, though I am now
able to create both a dom0 and a domU of 8+ vcpus with these patches.
So any comments/suggestions are welcomed.

These patches are written based upon Julien's "GICv2 on GICv3" series.

Chen Baozi (4):
  xen/arm64: Map the redistributor region by max_vcpus of domU
    danamically
  xen/arm64: increase MAX_VIRT_CPUS to 128 on arm64
  tools/libxl: Make DT node of GICv3 according to max_vcpus
  xen/arm: Remove unnecessary GUEST_GICV3_GICR0_SIZE macro.

 tools/libxl/libxl_arm.c       |  6 +++---
 xen/arch/arm/vgic-v3.c        | 27 +++++++++++++++++++--------
 xen/common/domctl.c           | 10 ++++++++++
 xen/include/asm-arm/config.h  |  4 ++++
 xen/include/asm-arm/vgic.h    |  3 +++
 xen/include/public/arch-arm.h |  3 +--
 6 files changed, 40 insertions(+), 13 deletions(-)

-- 
2.1.4

[RFC 1/4] xen/arm64: Map the redistributor region by max_vcpus of domU danamically

[Chen Baozi]

From: Chen Baozi <baozich@gmail.com>

The number of redistributor is determined by the number of CPU
interface. So we postpone redistributor mmio size initialization to
the point when the max_vcpus is set.

Signed-off-by: Chen Baozi <baozich@gmail.com>
---
 xen/arch/arm/vgic-v3.c     | 24 +++++++++++++++++++-----
 xen/common/domctl.c        | 10 ++++++++++
 xen/include/asm-arm/vgic.h |  3 +++
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 45d54a2..a0c1de9 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -1143,6 +1143,18 @@ static int vgic_v3_vcpu_init(struct vcpu *v)
     return 0;
 }
 
+void vgic_v3_rdist_map(struct domain *d)
+{
+    /* The first redistributor should contain enough space for all CPUs */
+    d->arch.vgic.rdist_regions[0].size = d->arch.vgic.rdist_stride * (d->max_vcpus);
+
+    register_mmio_handler(d, &vgic_rdistr_mmio_handler,
+        d->arch.vgic.rdist_regions[0].base,
+        d->arch.vgic.rdist_regions[0].size);
+
+    return;
+}
+
 static int vgic_v3_domain_init(struct domain *d)
 {
     int i, idx;
@@ -1194,7 +1206,6 @@ static int vgic_v3_domain_init(struct domain *d)
         /* The first redistributor should contain enough space for all CPUs */
         BUILD_BUG_ON((GUEST_GICV3_GICR0_SIZE / GUEST_GICV3_RDIST_STRIDE) < MAX_VIRT_CPUS);
         d->arch.vgic.rdist_regions[0].base = GUEST_GICV3_GICR0_BASE;
-        d->arch.vgic.rdist_regions[0].size = GUEST_GICV3_GICR0_SIZE;
         d->arch.vgic.rdist_regions[0].first_cpu = 0;
     }
 
@@ -1214,10 +1225,13 @@ static int vgic_v3_domain_init(struct domain *d)
      * redistributors. The handler will take care to choose which
      * redistributor is targeted.
      */
-    for ( i = 0; i < d->arch.vgic.nr_regions; i++ )
-        register_mmio_handler(d, &vgic_rdistr_mmio_handler,
-            d->arch.vgic.rdist_regions[i].base,
-            d->arch.vgic.rdist_regions[i].size);
+    if ( is_hardware_domain(d) )
+    {
+        for ( i = 0; i < d->arch.vgic.nr_regions; i++ )
+            register_mmio_handler(d, &vgic_rdistr_mmio_handler,
+                d->arch.vgic.rdist_regions[i].base,
+                d->arch.vgic.rdist_regions[i].size);
+    }
 
     d->arch.vgic.ctlr = VGICD_CTLR_DEFAULT;
 
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index e571e76..43b9f79 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -33,6 +33,11 @@
 #include <public/domctl.h>
 #include <xsm/xsm.h>
 
+#ifdef CONFIG_ARM_64
+#include <asm/gic.h>
+#include <asm/vgic.h>
+#endif
+
 static DEFINE_SPINLOCK(domctl_lock);
 DEFINE_SPINLOCK(vcpu_alloc_lock);
 
@@ -680,6 +685,11 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
             d->max_vcpus = max;
         }
 
+#ifdef CONFIG_ARM_64
+        if (!is_hardware_domain(d) && d->arch.vgic.version == GIC_V3)
+            vgic_v3_rdist_map(d);
+#endif
+
         for ( i = 0; i < max; i++ )
         {
             if ( d->vcpu[i] != NULL )
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index 6dcdf9f..4d7edc6 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -191,6 +191,9 @@ extern struct vgic_irq_rank *vgic_rank_irq(struct vcpu *v, unsigned int irq);
 extern int vgic_emulate(struct cpu_user_regs *regs, union hsr hsr);
 extern void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n);
 extern void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n);
+#ifdef CONFIG_ARM_64
+extern void vgic_v3_rdist_map(struct domain *d);
+#endif
 
 #define DEFINE_VGIC_OPS(version)                        \
     extern const struct vgic_ops vgic_v##version##_ops;
-- 
2.1.4

Re: [RFC 1/4] xen/arm64: Map the redistributor region by max_vcpus of domU danamically

[Julien Grall]

Hi Chen,

On 14/05/15 15:14, Chen Baozi wrote:
> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
> index e571e76..43b9f79 100644
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -33,6 +33,11 @@
>  #include <public/domctl.h>
>  #include <xsm/xsm.h>
>  
> +#ifdef CONFIG_ARM_64
> +#include <asm/gic.h>
> +#include <asm/vgic.h>
> +#endif
> +
>  static DEFINE_SPINLOCK(domctl_lock);
>  DEFINE_SPINLOCK(vcpu_alloc_lock);
>  
> @@ -680,6 +685,11 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>              d->max_vcpus = max;
>          }
>  
> +#ifdef CONFIG_ARM_64
> +        if (!is_hardware_domain(d) && d->arch.vgic.version == GIC_V3)
> +            vgic_v3_rdist_map(d);
> +#endif

That is very hackish. It's common with other architecture and we are
trying to be vgic agnostic in general. A vGIC callback would be more
suitable.

Although, as said on the cover letter. It's not necessary to allocate
dynamically. We can expand the current region to support up to 16/128 vCPUs.

Regards,

-- 
Julien Grall

[RFC 2/4] xen/arm64: increase MAX_VIRT_CPUS to 128 on arm64

[Chen Baozi]

From: Chen Baozi <baozich@gmail.com>

GIC-500 supports up to 128 cores in a single SoC. Since the
redistributor register map is no longer set by fixed size, which limits
the number of vcpu, we increase MAX_VIRT_CPUS to 128 and remove the
corresponding restriction.

Signed-off-by: Chen Baozi <baozich@gmail.com>
---
 xen/arch/arm/vgic-v3.c       | 3 ---
 xen/include/asm-arm/config.h | 4 ++++
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index a0c1de9..83e7344 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -906,7 +906,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);
 
@@ -1203,8 +1202,6 @@ static int vgic_v3_domain_init(struct domain *d)
         d->arch.vgic.nr_regions = GUEST_GICV3_RDIST_REGIONS;
         d->arch.vgic.rdist_stride = GUEST_GICV3_RDIST_STRIDE;
 
-        /* The first redistributor should contain enough space for all CPUs */
-        BUILD_BUG_ON((GUEST_GICV3_GICR0_SIZE / GUEST_GICV3_RDIST_STRIDE) < MAX_VIRT_CPUS);
         d->arch.vgic.rdist_regions[0].base = GUEST_GICV3_GICR0_BASE;
         d->arch.vgic.rdist_regions[0].first_cpu = 0;
     }
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: [RFC 2/4] xen/arm64: increase MAX_VIRT_CPUS to 128 on arm64

[Julien Grall]

Hi Chen,

On 14/05/15 15:14, Chen Baozi wrote:
> From: Chen Baozi <baozich@gmail.com>
> 
> GIC-500 supports up to 128 cores in a single SoC. Since the
> redistributor register map is no longer set by fixed size, which limits
> the number of vcpu, we increase MAX_VIRT_CPUS to 128 and remove the
> corresponding restriction.
> 
> Signed-off-by: Chen Baozi <baozich@gmail.com>
> ---
>  xen/arch/arm/vgic-v3.c       | 3 ---
>  xen/include/asm-arm/config.h | 4 ++++
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index a0c1de9..83e7344 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -906,7 +906,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);

This was here to catch bump of MAX_VIRT without adapting the vGIC code.

The current vGICv3 is not supporting more than 16 vCPUs because it only
cares about AFF0.

>          new_irouter = *r;
>          vgic_lock_rank(v, rank, flags);
>  
> @@ -1203,8 +1202,6 @@ static int vgic_v3_domain_init(struct domain *d)
>          d->arch.vgic.nr_regions = GUEST_GICV3_RDIST_REGIONS;
>          d->arch.vgic.rdist_stride = GUEST_GICV3_RDIST_STRIDE;
>  
> -        /* The first redistributor should contain enough space for all CPUs */
> -        BUILD_BUG_ON((GUEST_GICV3_GICR0_SIZE / GUEST_GICV3_RDIST_STRIDE) < MAX_VIRT_CPUS);

I'd like to keep this BUILD_BUG_ON. It ensures that we reserved enough
space in the guest layout for the redistributors.

Regards,

-- 
Julien Grall

[RFC 3/4] tools/libxl: Make DT node of GICv3 according to max_vcpus

[Chen Baozi]

From: Chen Baozi <baozich@gmail.com>

Since the size of GICR is determined by the number of CPU
cores, add 'nr_cpus' parameter when creating its DT node
and set gicr0_size dynamically.

Signed-off-by: Chen Baozi <baozich@gmail.com>
---
 tools/libxl/libxl_arm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index c5088c4..75d2aed 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -407,13 +407,13 @@ static int make_gicv2_node(libxl__gc *gc, void *fdt,
     return 0;
 }
 
-static int make_gicv3_node(libxl__gc *gc, void *fdt)
+static int make_gicv3_node(libxl__gc *gc, void *fdt, const int nr_cpus)
 {
     int res;
     const uint64_t gicd_base = GUEST_GICV3_GICD_BASE;
     const uint64_t gicd_size = GUEST_GICV3_GICD_SIZE;
     const uint64_t gicr0_base = GUEST_GICV3_GICR0_BASE;
-    const uint64_t gicr0_size = GUEST_GICV3_GICR0_SIZE;
+    const uint64_t gicr0_size = GUEST_GICV3_RDIST_STRIDE * nr_cpus;
     const char *name = GCSPRINTF("interrupt-controller@%"PRIx64, gicd_base);
 
     res = fdt_begin_node(fdt, name);
@@ -640,7 +640,7 @@ next_resize:
                                  GUEST_GICC_BASE, GUEST_GICC_SIZE) );
             break;
         case XEN_DOMCTL_CONFIG_GIC_V3:
-            FDT( make_gicv3_node(gc, fdt) );
+            FDT( make_gicv3_node(gc, fdt, info->max_vcpus) );
             break;
         default:
             LOG(ERROR, "Unknown GIC version %s",
-- 
2.1.4

[RFC 4/4] xen/arm: Remove unnecessary GUEST_GICV3_GICR0_SIZE macro.

[Chen Baozi]

From: Chen Baozi <baozich@gmail.com>

Signed-off-by: Chen Baozi <baozich@gmail.com>
---
 xen/include/public/arch-arm.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index c029e0f..cbcda74 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -388,8 +388,7 @@ 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
 
 /*
  * 16MB == 4096 pages reserved for guest to use as a region to map its
-- 
2.1.4

Re: [RFC 4/4] xen/arm: Remove unnecessary GUEST_GICV3_GICR0_SIZE macro.

[Julien Grall]

Hi Chen,

On 14/05/15 15:14, Chen Baozi wrote:
> From: Chen Baozi <baozich@gmail.com>
> 
> Signed-off-by: Chen Baozi <baozich@gmail.com>
> ---
>  xen/include/public/arch-arm.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index c029e0f..cbcda74 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -388,8 +388,7 @@ struct xen_arch_domainconfig {
>  #define GUEST_GICV3_RDIST_STRIDE   0x20000ULL
>  #define GUEST_GICV3_RDIST_REGIONS  1
>  
> -#define GUEST_GICV3_GICR0_BASE     0x03020000ULL    /* vCPU0 - vCPU7 */

The /* vCPU0 - vCPU7 */ was useful. Please update to whatever you will
be use.

> -#define GUEST_GICV3_GICR0_SIZE     0x00100000ULL
> +#define GUEST_GICV3_GICR0_BASE     0x03020000ULL

Please don't drop it. Even if you dropped all the usage, it's necessary
for documentation purpose.

If someone wants to change the guest layout, he will know that the
re-dist region will be this size.

I would also like to keep the BUILD_BUG_ON in vgic-v3 in order to check
if there is enough space reserved in the guest layout for the re-dist
(see your patch #2).

Regards,


-- 
Julien Grall

Re: [RFC 0/4] Support more than 8 vcpus on arm64 with GICv3

[Julien Grall]

Hi Chen,

On 14/05/15 15:14, Chen Baozi wrote:
> 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. In this patch series, I
> postpone setting the size of GICR0 to the point when max_vcpus of a domU is
> determined to support more than 8 redistributors.

I don't think postponing is necessary. We have plenty of space in the
RAM guest layout to reserve a region for 128 redistributors.

If the guest is trying to access to wrong re-distributor, it will
receive a data abort. It's already the case today when the guest is
using less than 8 vCPUs.

Although you would need to reshuffle a bit the layout. With your
solution, if the guest is using 128 vCPUs it will overlap the
grant-table region, magic page (xenstore, xenconsole,...) and the
beginning of the RAM. whoops ;).

> However, I am not quite sure that decoupling the rdist base and size
> setting of domU to different functions is appropriate, though I am now
> able to create both a dom0 and a domU of 8+ vcpus with these patches.
> So any comments/suggestions are welcomed.

I'm afraid to say that your suggestion is only enough to support up to
16 vCPUs per guest.

The vGIC is only using the affinity 0 of the MPIDR (AFF1, AFF2 and AFF3
are ignored).

Affinity 0 represents the CPU in a cluster and AFF{1,2,3} the cluster
ID. Each cluster can support up to 16 CPUs.

You will also need to change the way to domain is creating the MIPDR,
currently it considers that AFF0 == vcpu_id (see vcpu_initialise).

Lastly, you need more care for the GICv2 case. We don't want a user to
create a guest with more than 8 vCPUs.

Even though supporting more than 16 vCPUs would nice, it would require
more work. I would be fine if you decide to only bump to 16 vCPUs for now.

Regards,

-- 
Julien Grall

Re: [RFC 0/4] Support more than 8 vcpus on arm64 with GICv3

[Julien Grall]

On 14/05/15 18:48, Julien Grall wrote:
> Although you would need to reshuffle a bit the layout. With your
> solution, if the guest is using 128 vCPUs it will overlap the
> grant-table region, magic page (xenstore, xenconsole,...) and the
> beginning of the RAM. whoops ;).

Hmmm... forget this paragraph, I miscalculated the final value :/.

There is enough space for accommodating 128 vCPUs.

Regards,

-- 
Julien Grall