Re: [Qemu-devel] [kvm-unit-tests PATCH v3 10/10] arm/arm64: gic: don't just use zero

[Andrew Jones]

On Fri, Sep 02, 2016 at 11:43:33AM +0200, Auger Eric wrote:
> Hi Drew,
> 
> On 15/07/2016 15:00, Andrew Jones wrote:
> > Allow user to select who sends ipis and with which irq,
> > rather than just always sending irq=0 from cpu0.
> > 
> > Signed-off-by: Andrew Jones 
> > 
> > ---
> > v2: actually check that the irq received was the irq sent,
> > and (for gicv2) that the sender is the expected one.
> > ---
> >  arm/gic.c | 80 
> > ++-
> >  1 file changed, 64 insertions(+), 16 deletions(-)
> > 
> > diff --git a/arm/gic.c b/arm/gic.c
> > index fc7ef241de3e2..d3ab97d4ae470 100644
> > --- a/arm/gic.c
> > +++ b/arm/gic.c
> > @@ -11,6 +11,7 @@
> >   * This work is licensed under the terms of the GNU LGPL, version 2.
> >   */
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -33,6 +34,8 @@ static struct gic *gic;
> >  static int gic_version;
> >  static int acked[NR_CPUS], spurious[NR_CPUS];
> >  static cpumask_t ready;
> > +static int sender;
> > +static u32 irq;
> >  
> >  static void nr_cpu_check(int nr)
> >  {
> > @@ -85,7 +88,16 @@ static void check_acked(cpumask_t *mask)
> >  
> >  static u32 gicv2_read_iar(void)
> >  {
> > -   return readl(gicv2_cpu_base() + GIC_CPU_INTACK);
> > +   u32 iar = readl(gicv2_cpu_base() + GIC_CPU_INTACK);
> > +   int src = (iar >> 10) & 7;
> > +
> > +   if (src != sender) {
> > +   report("cpu%d received IPI from unexpected source cpu%d "
> > +  "(expected cpu%d)",
> > +  false, smp_processor_id(), src, sender);
> > +   }
> > +
> > +   return iar & 0x3ff;
> you can use GICC_IAR_INT_ID_MASK instead

OK

> >  }
> >  
> >  static void gicv2_write_eoi(u32 irq)
> > @@ -99,9 +111,15 @@ static void ipi_handler(struct pt_regs *regs __unused)
> >  
> > if (iar != GICC_INT_SPURIOUS) {
> > gic->write_eoi(iar);
> > -   smp_rmb(); /* pairs with wmb in ipi_test functions */
> > -   ++acked[smp_processor_id()];
> > -   smp_wmb(); /* pairs with rmb in check_acked */
> > +   if (iar == irq) {
> > +   smp_rmb(); /* pairs with wmb in ipi_test functions */
> > +   ++acked[smp_processor_id()];
> > +   smp_wmb(); /* pairs with rmb in check_acked */
> > +   } else {
> > +   report("cpu%d received unexpected irq %u "
> > +  "(expected %u)",
> > +  false, smp_processor_id(), iar, irq);
> > +   }
> > } else {
> > ++spurious[smp_processor_id()];
> > smp_wmb();
> > @@ -110,19 +128,19 @@ static void ipi_handler(struct pt_regs *regs __unused)
> >  
> >  static void gicv2_ipi_send_self(void)
> >  {
> > -   writel(2 << 24, gicv2_dist_base() + GIC_DIST_SOFTINT);
> > +   writel(2 << 24 | irq, gicv2_dist_base() + GIC_DIST_SOFTINT);
> >  }
> >  
> >  static void gicv2_ipi_send_tlist(cpumask_t *mask)
> >  {
> > u8 tlist = (u8)cpumask_bits(mask)[0];
> >  
> > -   writel(tlist << 16, gicv2_dist_base() + GIC_DIST_SOFTINT);
> > +   writel(tlist << 16 | irq, gicv2_dist_base() + GIC_DIST_SOFTINT);
> >  }
> >  
> >  static void gicv2_ipi_send_broadcast(void)
> >  {
> > -   writel(1 << 24, gicv2_dist_base() + GIC_DIST_SOFTINT);
> > +   writel(1 << 24 | irq, gicv2_dist_base() + GIC_DIST_SOFTINT);
> >  }
> >  
> >  #define ICC_SGI1R_AFFINITY_1_SHIFT 16
> > @@ -165,7 +183,7 @@ static void gicv3_ipi_send_tlist(cpumask_t *mask)
> >  
> > sgi1r = (MPIDR_TO_SGI_AFFINITY(cluster_id, 3)   |
> >  MPIDR_TO_SGI_AFFINITY(cluster_id, 2)   |
> > -/* irq << 24   | */
> > +irq << 24  |
> >  MPIDR_TO_SGI_AFFINITY(cluster_id, 1)   |
> >  tlist);
> >  
> > @@ -187,7 +205,7 @@ static void gicv3_ipi_send_self(void)
> >  
> >  static void gicv3_ipi_send_broadcast(void)
> >  {
> > -   gicv3_write_sgi1r(1ULL << 40);
> > +   gicv3_write_sgi1r(1ULL << 40 | irq << 24);
> > isb();
> >  }
> >  
> > @@ -199,7 +217,7 @@ static void ipi_test_self(void)
> > memset(acked, 0, sizeof(acked));
> > smp_wmb();
> > cpumask_clear();
> > -   cpumask_set_cpu(0, );
> > +   cpumask_set_cpu(smp_processor_id(), );
> > gic->ipi.send_self();
> > check_acked();
> > report_prefix_pop();
> > @@ -214,7 +232,7 @@ static void ipi_test_smp(void)
> > memset(acked, 0, sizeof(acked));
> > smp_wmb();
> > cpumask_copy(, _present_mask);
> > -   for (i = 0; i < nr_cpus; i += 2)
> > +   for (i = smp_processor_id() & 1; i < nr_cpus; i += 2)
> > cpumask_clear_cpu(i, );
> > gic->ipi.send_tlist();
> > check_acked();
> > @@ -224,7 +242,7 @@ static void ipi_test_smp(void)
> > memset(acked, 0, sizeof(acked));
> > smp_wmb();
> > cpumask_copy(, _present_mask);
> > -   cpumask_clear_cpu(0, );
> > +   

Re: [Qemu-devel] [kvm-unit-tests PATCH v3 08/10] arm/arm64: gicv2: add an IPI test

[Andrew Jones]

On Thu, Sep 01, 2016 at 06:42:50PM +0200, Auger Eric wrote:
> Hi Drew,
> 
> On 15/07/2016 15:00, Andrew Jones wrote:
> > Signed-off-by: Andrew Jones 
> > ---
> > v2: add more details in the output if a test fails,
> > report spurious interrupts if we get them
> > ---
> >  arm/Makefile.common |   6 +-
> >  arm/gic.c   | 194 
> > 
> >  arm/unittests.cfg   |   7 ++
> >  3 files changed, 204 insertions(+), 3 deletions(-)
> >  create mode 100644 arm/gic.c
> > 
> > diff --git a/arm/Makefile.common b/arm/Makefile.common
> > index 41239c37e0920..bc38183ab86e0 100644
> > --- a/arm/Makefile.common
> > +++ b/arm/Makefile.common
> > @@ -9,9 +9,9 @@ ifeq ($(LOADADDR),)
> > LOADADDR = 0x4000
> >  endif
> >  
> > -tests-common = \
> > -   $(TEST_DIR)/selftest.flat \
> > -   $(TEST_DIR)/spinlock-test.flat
> > +tests-common  = $(TEST_DIR)/selftest.flat
> > +tests-common += $(TEST_DIR)/spinlock-test.flat
> > +tests-common += $(TEST_DIR)/gic.flat
> >  
> >  all: test_cases
> >  
> > diff --git a/arm/gic.c b/arm/gic.c
> > new file mode 100644
> > index 0..cf7ec1c90413c
> > --- /dev/null
> > +++ b/arm/gic.c
> > @@ -0,0 +1,194 @@
> > +/*
> > + * GIC tests
> > + *
> > + * GICv2
> > + *   . test sending/receiving IPIs
> > + *
> > + * Copyright (C) 2016, Red Hat Inc, Andrew Jones 
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +static int gic_version;
> > +static int acked[NR_CPUS], spurious[NR_CPUS];
> > +static cpumask_t ready;
> > +
> > +static void nr_cpu_check(int nr)
> > +{
> > +   if (nr_cpus < nr)
> > +   report_abort("At least %d cpus required", nr);
> > +}
> > +
> > +static void wait_on_ready(void)
> > +{
> > +   cpumask_set_cpu(smp_processor_id(), );
> > +   while (!cpumask_full())
> > +   cpu_relax();
> > +}
> > +
> > +static void check_acked(cpumask_t *mask)
> > +{
> > +   int missing = 0, extra = 0, unexpected = 0;
> > +   int nr_pass, cpu, i;
> > +
> > +   /* Wait up to 5s for all interrupts to be delivered */
> > +   for (i = 0; i < 50; ++i) {
> > +   mdelay(100);
> > +   nr_pass = 0;
> > +   for_each_present_cpu(cpu) {
> Couldn't we use for_each_cpu(cpu, mask)?

If we did, then we wouldn't be able to detect delivery of interrupts to
the wrong cpus. Note, we don't run the second loop if everything looks
good here. That one is just to better report a detected problem(s).

> > +   smp_rmb();
> > +   nr_pass += cpumask_test_cpu(cpu, mask) ?
> > +   acked[cpu] == 1 : acked[cpu] == 0;
> > +   }
> > +   if (nr_pass == nr_cpus) {
> > +   report("Completed in %d ms", true, ++i * 100);
> > +   return;
> > +   }
> > +   }
> > +
> > +   for_each_present_cpu(cpu) {
> > +   if (cpumask_test_cpu(cpu, mask)) {
> here we can't since we count unexpected
> 
> > +   if (!acked[cpu])
> > +   ++missing;
> > +   else if (acked[cpu] > 1)
> > +   ++extra;
> > +   } else {
> > +   if (acked[cpu])
> > +   ++unexpected;
> > +   }
> > +   }
> > +
> > +   report("Timed-out (5s). ACKS: missing=%d extra=%d unexpected=%d",
> > +  false, missing, extra, unexpected);
> > +}
> > +
> > +static void ipi_handler(struct pt_regs *regs __unused)
> > +{
> > +   u32 iar = readl(gicv2_cpu_base() + GIC_CPU_INTACK);
> > +
> > +   if (iar != GICC_INT_SPURIOUS) {
> > +   writel(iar, gicv2_cpu_base() + GIC_CPU_EOI);
> if EOIMode is set may need to DIR as well.

OK, I'll look into that

> > +   smp_rmb(); /* pairs with wmb in ipi_test functions */
> > +   ++acked[smp_processor_id()];
> > +   smp_wmb(); /* pairs with rmb in check_acked */
> > +   } else {
> > +   ++spurious[smp_processor_id()];
> > +   smp_wmb();
> > +   }
> > +}
> > +
> > +static void ipi_test_self(void)
> > +{
> > +   cpumask_t mask;
> > +
> > +   report_prefix_push("self");
> > +   memset(acked, 0, sizeof(acked));
> > +   smp_wmb();
> > +   cpumask_clear();
> > +   cpumask_set_cpu(0, );
> > +   writel(2 << 24, gicv2_dist_base() + GIC_DIST_SOFTINT);
> > +   check_acked();
> > +   report_prefix_pop();
> > +}
> > +
> > +static void ipi_test_smp(void)
> > +{
> > +   cpumask_t mask;
> > +   unsigned long tlist;
> > +
> > +   report_prefix_push("target-list");
> > +   memset(acked, 0, sizeof(acked));
> > +   smp_wmb();
> > +   tlist = cpumask_bits(_present_mask)[0] & 0xaa;
> > +   cpumask_bits()[0] = tlist;
> > +   writel((u8)tlist << 16, gicv2_dist_base() + GIC_DIST_SOFTINT);
> > +   check_acked();
> > +   report_prefix_pop();
> > +
> > +   report_prefix_push("broadcast");
> > 

Re: [Qemu-devel] [kvm-unit-tests PATCH v3 09/10] arm/arm64: gicv3: add an IPI test

[Andrew Jones]

On Thu, Sep 01, 2016 at 06:42:44PM +0200, Auger Eric wrote:
> Hi Drew,
> 
> On 15/07/2016 15:00, Andrew Jones wrote:
> > Signed-off-by: Andrew Jones 
> > 
> > ---
> > v2: use IRM for gicv3 broadcast
> > ---
> >  arm/gic.c | 157 
> > ++
> >  arm/unittests.cfg |   6 +++
> >  2 files changed, 154 insertions(+), 9 deletions(-)
> > 
> > diff --git a/arm/gic.c b/arm/gic.c
> > index cf7ec1c90413c..fc7ef241de3e2 100644
> > --- a/arm/gic.c
> > +++ b/arm/gic.c
> > @@ -3,6 +3,8 @@
> >   *
> >   * GICv2
> >   *   . test sending/receiving IPIs
> > + * GICv3
> > + *   . test sending/receiving IPIs
> >   *
> >   * Copyright (C) 2016, Red Hat Inc, Andrew Jones 
> >   *
> > @@ -16,6 +18,18 @@
> >  #include 
> >  #include 
> >  
> > +struct gic {
> > +   struct {
> > +   void (*enable)(void);
> > +   void (*send_self)(void);
> > +   void (*send_tlist)(cpumask_t *);
> > +   void (*send_broadcast)(void);
> > +   } ipi;
> > +   u32 (*read_iar)(void);
> > +   void (*write_eoi)(u32);
> > +};
> > +
> > +static struct gic *gic;
> >  static int gic_version;
> >  static int acked[NR_CPUS], spurious[NR_CPUS];
> >  static cpumask_t ready;
> > @@ -69,12 +83,22 @@ static void check_acked(cpumask_t *mask)
> >false, missing, extra, unexpected);
> >  }
> >  
> > +static u32 gicv2_read_iar(void)
> > +{
> > +   return readl(gicv2_cpu_base() + GIC_CPU_INTACK);
> > +}
> > +
> > +static void gicv2_write_eoi(u32 irq)
> > +{
> > +   writel(irq, gicv2_cpu_base() + GIC_CPU_EOI);
> > +}
> > +
> >  static void ipi_handler(struct pt_regs *regs __unused)
> >  {
> > -   u32 iar = readl(gicv2_cpu_base() + GIC_CPU_INTACK);
> > +   u32 iar = gic->read_iar();
> >  
> > if (iar != GICC_INT_SPURIOUS) {
> > -   writel(iar, gicv2_cpu_base() + GIC_CPU_EOI);
> > +   gic->write_eoi(iar);
> > smp_rmb(); /* pairs with wmb in ipi_test functions */
> > ++acked[smp_processor_id()];
> > smp_wmb(); /* pairs with rmb in check_acked */
> > @@ -84,6 +108,89 @@ static void ipi_handler(struct pt_regs *regs __unused)
> > }
> >  }
> >  
> > +static void gicv2_ipi_send_self(void)
> > +{
> > +   writel(2 << 24, gicv2_dist_base() + GIC_DIST_SOFTINT);
> > +}
> > +
> > +static void gicv2_ipi_send_tlist(cpumask_t *mask)
> > +{
> > +   u8 tlist = (u8)cpumask_bits(mask)[0];
> > +
> > +   writel(tlist << 16, gicv2_dist_base() + GIC_DIST_SOFTINT);
> > +}
> > +
> > +static void gicv2_ipi_send_broadcast(void)
> > +{
> > +   writel(1 << 24, gicv2_dist_base() + GIC_DIST_SOFTINT);
> > +}
> > +
> > +#define ICC_SGI1R_AFFINITY_1_SHIFT 16
> > +#define ICC_SGI1R_AFFINITY_2_SHIFT 32
> > +#define ICC_SGI1R_AFFINITY_3_SHIFT 48
> > +#define MPIDR_TO_SGI_AFFINITY(cluster_id, level) \
> > +   (MPIDR_AFFINITY_LEVEL(cluster_id, level) << ICC_SGI1R_AFFINITY_## level 
> > ## _SHIFT)
> > +
> > +static void gicv3_ipi_send_tlist(cpumask_t *mask)
> > +{
> This function really is indigestible to me. Can't we reuse the linux
> kernel gic_compute_target_list routine. Since the code looks similar why
> not trying to be as close as possible to the kernel?

It is the same, but simplified :-) In Linux, gic_compute_target_list is
not the only function you need for the implementation. You also need to
look at the one caller, which is gic_raise_softirq. I'll add a couple of
comments to this version, which simply inlines the kernel's version.

Thanks,
drew 

> 
> Thanks
> 
> Eric
> > +   u16 tlist;
> > +   int cpu;
> > +
> > +   for_each_cpu(cpu, mask) {
> > +   u64 mpidr = cpus[cpu], sgi1r;
> > +   u64 cluster_id = mpidr & ~0xffUL;
> > +
> > +   tlist = 0;
> > +
> > +   while (cpu < nr_cpus) {
> > +   if ((mpidr & 0xff) >= 16) {
> > +   printf("cpu%d MPIDR:aff0 is %d (>= 16)!\n",
> > +   cpu, (int)(mpidr & 0xff));
> > +   break;
> > +   }
> > +
> > +   tlist |= 1 << (mpidr & 0xf);
> > +
> > +   cpu = cpumask_next(cpu, mask);
> > +   if (cpu >= nr_cpus)
> > +   break;
> > +
> > +   mpidr = cpus[cpu];
> > +
> > +   if (cluster_id != (mpidr & ~0xffUL)) {
> > +   --cpu;
> > +   break;
> > +   }
> > +   }
> > +
> > +   sgi1r = (MPIDR_TO_SGI_AFFINITY(cluster_id, 3)   |
> > +MPIDR_TO_SGI_AFFINITY(cluster_id, 2)   |
> > +/* irq << 24   | */
> > +MPIDR_TO_SGI_AFFINITY(cluster_id, 1)   |
> > +tlist);
> > +
> > +   gicv3_write_sgi1r(sgi1r);
> > +   }
> > +
> > +   /* Force the above writes to ICC_SGI1R_EL1 to be executed */
> > +   isb();
> > +}
> > +
> > +static void gicv3_ipi_send_self(void)
> > 

Re: [Qemu-devel] [kvm-unit-tests PATCH v3 07/10] arm/arm64: add initial gicv3 support

[Andrew Jones]

On Thu, Sep 01, 2016 at 12:19:52PM +0200, Auger Eric wrote:
> Hi Drew,
> 
> On 15/07/2016 15:00, Andrew Jones wrote:
> > Signed-off-by: Andrew Jones 
> > 
> > ---
> > v2: configure irqs as NS GRP1
> > ---
> >  lib/arm/asm/arch_gicv3.h   | 184 ++
> >  lib/arm/asm/gic-v3.h   | 321 
> > +
> >  lib/arm/asm/gic.h  |   1 +
> >  lib/arm/gic.c  |  73 +++
> >  lib/arm64/asm/arch_gicv3.h | 169 
> >  lib/arm64/asm/gic-v3.h |   1 +
> >  lib/arm64/asm/sysreg.h |  44 +++
> >  7 files changed, 793 insertions(+)
> >  create mode 100644 lib/arm/asm/arch_gicv3.h
> >  create mode 100644 lib/arm/asm/gic-v3.h
> >  create mode 100644 lib/arm64/asm/arch_gicv3.h
> >  create mode 100644 lib/arm64/asm/gic-v3.h
> >  create mode 100644 lib/arm64/asm/sysreg.h
> > 
> > diff --git a/lib/arm/asm/arch_gicv3.h b/lib/arm/asm/arch_gicv3.h
> > new file mode 100644
> > index 0..d529a7eb62807
> > --- /dev/null
> > +++ b/lib/arm/asm/arch_gicv3.h
> > @@ -0,0 +1,184 @@
> > +/*
> > + * All ripped off from arch/arm/include/asm/arch_gicv3.h
> > + *
> > + * Copyright (C) 2016, Red Hat Inc, Andrew Jones 
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#ifndef _ASMARM_ARCH_GICV3_H_
> > +#define _ASMARM_ARCH_GICV3_H_
> > +
> > +#ifndef __ASSEMBLY__
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define __stringify xstr
> > +
> > +
> > +#define __ACCESS_CP15(CRn, Op1, CRm, Op2)  p15, Op1, %0, CRn, CRm, Op2
> > +#define __ACCESS_CP15_64(Op1, CRm) p15, Op1, %Q0, %R0, CRm
> > +
> > +#define ICC_EOIR1  __ACCESS_CP15(c12, 0, c12, 1)
> > +#define ICC_DIR__ACCESS_CP15(c12, 0, c11, 1)
> > +#define ICC_IAR1   __ACCESS_CP15(c12, 0, c12, 0)
> > +#define ICC_SGI1R  __ACCESS_CP15_64(0, c12)
> > +#define ICC_PMR__ACCESS_CP15(c4, 0, c6, 0)
> > +#define ICC_CTLR   __ACCESS_CP15(c12, 0, c12, 4)
> > +#define ICC_SRE__ACCESS_CP15(c12, 0, c12, 5)
> > +#define ICC_IGRPEN1__ACCESS_CP15(c12, 0, c12, 7)
> > +
> > +#define ICC_HSRE   __ACCESS_CP15(c12, 4, c9, 5)
> > +
> > +#define ICH_VSEIR  __ACCESS_CP15(c12, 4, c9, 4)
> > +#define ICH_HCR__ACCESS_CP15(c12, 4, c11, 0)
> > +#define ICH_VTR__ACCESS_CP15(c12, 4, c11, 1)
> > +#define ICH_MISR   __ACCESS_CP15(c12, 4, c11, 2)
> > +#define ICH_EISR   __ACCESS_CP15(c12, 4, c11, 3)
> > +#define ICH_ELSR   __ACCESS_CP15(c12, 4, c11, 5)
> > +#define ICH_VMCR   __ACCESS_CP15(c12, 4, c11, 7)
> > +
> > +#define __LR0(x)   __ACCESS_CP15(c12, 4, c12, x)
> > +#define __LR8(x)   __ACCESS_CP15(c12, 4, c13, x)
> > +
> > +#define ICH_LR0__LR0(0)
> > +#define ICH_LR1__LR0(1)
> > +#define ICH_LR2__LR0(2)
> > +#define ICH_LR3__LR0(3)
> > +#define ICH_LR4__LR0(4)
> > +#define ICH_LR5__LR0(5)
> > +#define ICH_LR6__LR0(6)
> > +#define ICH_LR7__LR0(7)
> > +#define ICH_LR8__LR8(0)
> > +#define ICH_LR9__LR8(1)
> > +#define ICH_LR10   __LR8(2)
> > +#define ICH_LR11   __LR8(3)
> > +#define ICH_LR12   __LR8(4)
> > +#define ICH_LR13   __LR8(5)
> > +#define ICH_LR14   __LR8(6)
> > +#define ICH_LR15   __LR8(7)
> > +
> > +/* LR top half */
> > +#define __LRC0(x)  __ACCESS_CP15(c12, 4, c14, x)
> > +#define __LRC8(x)  __ACCESS_CP15(c12, 4, c15, x)
> > +
> > +#define ICH_LRC0   __LRC0(0)
> > +#define ICH_LRC1   __LRC0(1)
> > +#define ICH_LRC2   __LRC0(2)
> > +#define ICH_LRC3   __LRC0(3)
> > +#define ICH_LRC4   __LRC0(4)
> > +#define ICH_LRC5   __LRC0(5)
> > +#define ICH_LRC6   __LRC0(6)
> > +#define ICH_LRC7   __LRC0(7)
> > +#define ICH_LRC8   __LRC8(0)
> > +#define ICH_LRC9   __LRC8(1)
> > +#define ICH_LRC10  __LRC8(2)
> > +#define ICH_LRC11  __LRC8(3)
> > +#define ICH_LRC12  __LRC8(4)
> > +#define ICH_LRC13  __LRC8(5)
> > +#define ICH_LRC14  __LRC8(6)
> > +#define ICH_LRC15  __LRC8(7)
> > +
> > +#define __AP0Rx(x) __ACCESS_CP15(c12, 4, c8, x)
> > +#define ICH_AP0R0  __AP0Rx(0)
> 

Re: [Qemu-devel] [kvm-unit-tests PATCH v3 06/10] arm/arm64: add initial gicv2 support

[Andrew Jones]

On Thu, Sep 01, 2016 at 12:20:06PM +0200, Auger Eric wrote:
> 
> 
> On 15/07/2016 15:00, Andrew Jones wrote:
> > Add some gicv2 support. This just adds init and enable
> > functions, allowing unit tests to start messing with it.
> > 
> > Signed-off-by: Andrew Jones 
> > ---
> >  arm/Makefile.common|  1 +
> >  lib/arm/asm/gic-v2.h   | 74 
> > ++
> >  lib/arm/asm/gic.h  | 20 ++
> >  lib/arm/gic.c  | 69 ++
> >  lib/arm64/asm/gic-v2.h |  1 +
> >  lib/arm64/asm/gic.h|  1 +
> >  6 files changed, 166 insertions(+)
> >  create mode 100644 lib/arm/asm/gic-v2.h
> >  create mode 100644 lib/arm/asm/gic.h
> >  create mode 100644 lib/arm/gic.c
> >  create mode 100644 lib/arm64/asm/gic-v2.h
> >  create mode 100644 lib/arm64/asm/gic.h
> > 
> > diff --git a/arm/Makefile.common b/arm/Makefile.common
> > index ccb554d9251a4..41239c37e0920 100644
> > --- a/arm/Makefile.common
> > +++ b/arm/Makefile.common
> > @@ -42,6 +42,7 @@ cflatobjs += lib/arm/mmu.o
> >  cflatobjs += lib/arm/bitops.o
> >  cflatobjs += lib/arm/psci.o
> >  cflatobjs += lib/arm/smp.o
> > +cflatobjs += lib/arm/gic.o
> >  
> >  libeabi = lib/arm/libeabi.a
> >  eabiobjs = lib/arm/eabi_compat.o
> > diff --git a/lib/arm/asm/gic-v2.h b/lib/arm/asm/gic-v2.h
> > new file mode 100644
> > index 0..973c2bf3cc796
> > --- /dev/null
> > +++ b/lib/arm/asm/gic-v2.h
> > @@ -0,0 +1,74 @@
> > +/*
> > + * All GIC* defines are lifted from include/linux/irqchip/arm-gic.h
> > + *
> > + * Copyright (C) 2016, Red Hat Inc, Andrew Jones 
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#ifndef _ASMARM_GIC_V2_H_
> > +#define _ASMARM_GIC_V2_H_
> > +
> > +#define GIC_CPU_CTRL   0x00
> > +#define GIC_CPU_PRIMASK0x04
> > +#define GIC_CPU_BINPOINT   0x08
> > +#define GIC_CPU_INTACK 0x0c
> > +#define GIC_CPU_EOI0x10
> > +#define GIC_CPU_RUNNINGPRI 0x14
> > +#define GIC_CPU_HIGHPRI0x18
> > +#define GIC_CPU_ALIAS_BINPOINT 0x1c
> > +#define GIC_CPU_ACTIVEPRIO 0xd0
> > +#define GIC_CPU_IDENT  0xfc
> > +#define GIC_CPU_DEACTIVATE 0x1000
> > +
> > +#define GICC_ENABLE0x1
> > +#define GICC_INT_PRI_THRESHOLD 0xf0
> > +
> > +#define GIC_CPU_CTRL_EOImodeNS (1 << 9)
> > +
> > +#define GICC_IAR_INT_ID_MASK   0x3ff
> > +#define GICC_INT_SPURIOUS  1023
> > +#define GICC_DIS_BYPASS_MASK   0x1e0
> > +
> > +#define GIC_DIST_CTRL  0x000
> > +#define GIC_DIST_CTR   0x004
> you could add #define GIC_DIST_IIDR   0x008
> which can be found in arm-gic.h

I'll resync with the latest kernel headers.

> > +#define GIC_DIST_IGROUP0x080
> > +#define GIC_DIST_ENABLE_SET0x100
> > +#define GIC_DIST_ENABLE_CLEAR  0x180
> > +#define GIC_DIST_PENDING_SET   0x200
> > +#define GIC_DIST_PENDING_CLEAR 0x280
> > +#define GIC_DIST_ACTIVE_SET0x300
> > +#define GIC_DIST_ACTIVE_CLEAR  0x380
> > +#define GIC_DIST_PRI   0x400
> > +#define GIC_DIST_TARGET0x800
> > +#define GIC_DIST_CONFIG0xc00
> > +#define GIC_DIST_SOFTINT   0xf00
> > +#define GIC_DIST_SGI_PENDING_CLEAR 0xf10
> > +#define GIC_DIST_SGI_PENDING_SET   0xf20
> > +
> > +#define GICD_ENABLE0x1
> > +#define GICD_DISABLE   0x0
> > +#define GICD_INT_ACTLOW_LVLTRIG0x0
> > +#define GICD_INT_EN_CLR_X320x
> > +#define GICD_INT_EN_SET_SGI0x
> > +#define GICD_INT_EN_CLR_PPI0x
> > +#define GICD_INT_DEF_PRI   0xa0
> > +#define GICD_INT_DEF_PRI_X4((GICD_INT_DEF_PRI << 24) |\
> > +   (GICD_INT_DEF_PRI << 16) |\
> > +   (GICD_INT_DEF_PRI << 8) |\
> > +   GICD_INT_DEF_PRI)
> > +#ifndef __ASSEMBLY__
> > +
> > +struct gicv2_data {
> > +   void *dist_base;
> > +   void *cpu_base;
> > +};
> > +extern struct gicv2_data gicv2_data;
> > +
> > +#define gicv2_dist_base()  (gicv2_data.dist_base)
> > +#define gicv2_cpu_base()   (gicv2_data.cpu_base)
> > +
> > +extern int gicv2_init(void);
> > +extern void gicv2_enable_defaults(void);
> > +
> > +#endif /* !__ASSEMBLY__ */
> > +#endif /* _ASMARM_GIC_V2_H_ */
> > diff --git a/lib/arm/asm/gic.h b/lib/arm/asm/gic.h
> > new file mode 100644
> > index 0..b1237d1c5ef22
> > --- /dev/null
> > +++ b/lib/arm/asm/gic.h
> > @@ -0,0 +1,20 @@
> > +/*
> > + * Copyright (C) 2016, Red Hat Inc, Andrew Jones 

Re: [Qemu-devel] [kvm-unit-tests PATCH v3 03/10] arm/arm64: smp: support more than 8 cpus

[Andrew Jones]

On Tue, Aug 30, 2016 at 04:28:52PM +0200, Auger Eric wrote:
> Hi Drew,
> 
> Proper commit message?
> ... also selects the vgic model corresponding to the host

Sure

> > Reviewed-by: Alex Bennée 
> > Signed-off-by: Andrew Jones 
> > ---
> >  arm/run   | 19 ---
> >  arm/selftest.c|  5 -
> >  lib/arm/asm/processor.h   |  9 +++--
> >  lib/arm/asm/setup.h   |  4 ++--
> >  lib/arm/setup.c   | 12 +++-
> >  lib/arm64/asm/processor.h |  9 +++--
> >  6 files changed, 43 insertions(+), 15 deletions(-)
> > 
> > diff --git a/arm/run b/arm/run
> > index a2f35ef6a7e63..2d0698619606e 100755
> > --- a/arm/run
> > +++ b/arm/run
> > @@ -31,13 +31,6 @@ if [ -z "$ACCEL" ]; then
> > fi
> >  fi
> >  
> > -if [ "$HOST" = "aarch64" ] && [ "$ACCEL" = "kvm" ]; then
> > -   processor="host"
> > -   if [ "$ARCH" = "arm" ]; then
> > -   processor+=",aarch64=off"
> > -   fi
> > -fi
> > -
> >  qemu="${QEMU:-qemu-system-$ARCH_NAME}"
> >  qpath=$(which $qemu 2>/dev/null)
> >  
> > @@ -53,6 +46,18 @@ fi
> >  
> >  M='-machine virt'
> >  
> > +if [ "$ACCEL" = "kvm" ]; then
> > +   if $qemu $M,\? 2>&1 | grep gic-version > /dev/null; then
> > +   M+=',gic-version=host'
> > +   fi
> > +   if [ "$HOST" = "aarch64" ]; then
> > +   processor="host"
> > +   if [ "$ARCH" = "arm" ]; then
> > +   processor+=",aarch64=off"
> > +   fi
> > +   fi
> > +fi
> > +
> >  if ! $qemu $M -device '?' 2>&1 | grep virtconsole > /dev/null; then
> > echo "$qpath doesn't support virtio-console for chr-testdev. Exiting."
> > exit 2
> > diff --git a/arm/selftest.c b/arm/selftest.c
> > index 196164f5313de..2f117f795d2dc 100644
> > --- a/arm/selftest.c
> > +++ b/arm/selftest.c
> > @@ -312,9 +312,10 @@ static bool psci_check(void)
> >  static cpumask_t smp_reported;
> >  static void cpu_report(void)
> >  {
> > +   unsigned long mpidr = get_mpidr();
> > int cpu = smp_processor_id();
> >  
> > -   report("CPU%d online", true, cpu);
> > +   report("CPU(%3d) mpidr=%lx", mpidr_to_cpu(mpidr) == cpu, cpu, mpidr);
> > cpumask_set_cpu(cpu, _reported);
> > halt();
> >  }
> > @@ -343,6 +344,7 @@ int main(int argc, char **argv)
> >  
> > } else if (strcmp(argv[1], "smp") == 0) {
> >  
> > +   unsigned long mpidr = get_mpidr();
> > int cpu;
> >  
> > report("PSCI version", psci_check());
> > @@ -353,6 +355,7 @@ int main(int argc, char **argv)
> > smp_boot_secondary(cpu, cpu_report);
> > }
> >  
> > +   report("CPU(%3d) mpidr=%lx", mpidr_to_cpu(mpidr) == 0, 0, 
> > mpidr);
> > cpumask_set_cpu(0, _reported);
> > while (!cpumask_full(_reported))
> > cpu_relax();
> > diff --git a/lib/arm/asm/processor.h b/lib/arm/asm/processor.h
> > index f25e7eee3666c..d2048f5f5f7e6 100644
> > --- a/lib/arm/asm/processor.h
> > +++ b/lib/arm/asm/processor.h
> > @@ -40,8 +40,13 @@ static inline unsigned int get_mpidr(void)
> > return mpidr;
> >  }
> >  
> > -/* Only support Aff0 for now, up to 4 cpus */
> > -#define mpidr_to_cpu(mpidr) ((int)((mpidr) & 0xff))
> > +#define MPIDR_HWID_BITMASK 0xff
> > +extern int mpidr_to_cpu(unsigned long mpidr);
> > +
> > +#define MPIDR_LEVEL_SHIFT(level) \
> > +   (((1 << level) >> 1) << 3)
> can't we have level << 3?
> > +#define MPIDR_AFFINITY_LEVEL(mpidr, level) \
> > +   ((mpidr >> MPIDR_LEVEL_SHIFT(level)) & 0xff)
> >  
> >  extern void start_usr(void (*func)(void *arg), void *arg, unsigned long 
> > sp_usr);
> >  extern bool is_user(void);
> > diff --git a/lib/arm/asm/setup.h b/lib/arm/asm/setup.h
> > index cb8fdbd38dd5d..c501c6ddd8657 100644
> > --- a/lib/arm/asm/setup.h
> > +++ b/lib/arm/asm/setup.h
> > @@ -10,8 +10,8 @@
> >  #include 
> >  #include 
> >  
> > -#define NR_CPUS8
> > -extern u32 cpus[NR_CPUS];
> > +#define NR_CPUS255
> 256?

The kernel defines KVM_MAX_VCPUS as VGIC_V3_MAX_CPUS, which is
currently defined as 255. I was just being consistent.

> > +extern u64 cpus[NR_CPUS];
> maybe worth commenting the semantic of cpus[i]?

sure

> >  extern int nr_cpus;
> what about MAX_CPUS instead of NR_CPUS?

kernel uses NR_CPUS, I want to be consistent.

> >  
> >  #define NR_MEM_REGIONS 8
> > diff --git a/lib/arm/setup.c b/lib/arm/setup.c
> > index 7e7b39f11dde1..b6e2d5815e723 100644
> > --- a/lib/arm/setup.c
> > +++ b/lib/arm/setup.c
> > @@ -24,12 +24,22 @@ extern unsigned long stacktop;
> >  extern void io_init(void);
> >  extern void setup_args_progname(const char *args);
> >  
> > -u32 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) };
> > +u64 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) };
> >  int nr_cpus;
> >  
> >  struct mem_region mem_regions[NR_MEM_REGIONS];
> >  phys_addr_t __phys_offset, __phys_end;
> >  
> > +int mpidr_to_cpu(unsigned long mpidr)
> > +{
> > +   int i;
> > +
> > + 

Re: [PATCH] arm64: KVM: Take S1 walks into account when determining S2 write faults

[Marc Zyngier]

On 17/10/16 11:20, Will Deacon wrote:
> On Thu, Sep 29, 2016 at 09:14:32PM +0200, Christoffer Dall wrote:
>> On Thu, Sep 29, 2016 at 12:37:01PM +0100, Will Deacon wrote:
>>> The WnR bit in the HSR/ESR_EL2 indicates whether a data abort was
>>> generated by a read or a write instruction. For stage 2 data aborts
>>> generated by a stage 1 translation table walk (i.e. the actual page
>>> table access faults at EL2), the WnR bit therefore reports whether the
>>> instruction generating the walk was a load or a store, *not* whether the
>>> page table walker was reading or writing the entry.
>>>
>>> For page tables marked as read-only at stage 2 (e.g. due to KSM merging
>>> them with the tables from another guest), this could result in livelock,
>>> where a page table walk generated by a load instruction attempts to
>>> set the access flag in the stage 1 descriptor, but fails to trigger
>>> CoW in the host since only a read fault is reported.
>>>
>>> This patch modifies the arm64 kvm_vcpu_dabt_iswrite function to
>>> take into account stage 2 faults in stage 1 walks. Since DBM cannot be
>>> disabled at EL2 for CPUs that implement it, we assume that these faults
>>> are always causes by writes, avoiding the livelock situation at the
>>> expense of occasional, spurious CoWs.
>>>
>>> We could, in theory, do a bit better by checking the guest TCR
>>> configuration and inspecting the page table to see why the PTE faulted.
>>> However, I doubt this is measurable in practice, and the threat of
>>> livelock is real.
>>>
>>> Cc: Marc Zyngier 
>>> Cc: Christoffer Dall 
>>> Cc: Julien Grall 
>>> Signed-off-by: Will Deacon 
>>
>> Reviewed-by: Christoffer Dall 
>>
>> Applied,
> 
> This doesn't seem to be in 4.9-rc1. Could you please dig it up?

Looks like this patch has been lingering in -queue. I'll push it on
master as a fix for -rc2.

Thanks for the heads up.

M.
-- 
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm