Message ID | 1478798481-25030-8-git-send-email-drjones@redhat.com |
---|---|
State | Superseded |
Headers | show |
Hi, more a comment loosely related to this patch ... > diff --git a/arm/unittests.cfg b/arm/unittests.cfg > index 3f6fa45c587e..68bf5cd6008f 100644 > --- a/arm/unittests.cfg > +++ b/arm/unittests.cfg > @@ -54,3 +54,10 @@ file = selftest.flat > smp = $MAX_SMP > extra_params = -append 'smp' > groups = selftest > + > +# Test GIC emulation > +[gicv2-ipi] > +file = gic.flat > +smp = $((($MAX_SMP < 8)?$MAX_SMP:8)) So here we always go with the maximum number of VCPUs in the guest. However (as you also noted in your cover-letter) running with a different number of CPUs might be interesting, for instance with less than 8 CPUs on a GICv2 (the ITARGETSR register must be masked) or in general with an odd number (both literally and in the broader sense). I have a test case with passes with 8 VCPUs but fails with less. Is there any good way to run some tests multiple times with different numbers of VCPUS? Shall we add some "set" functionality to the smp parameter, so that we can specify a list of desired test points? Cheers, Andre.
On Fri, Nov 11, 2016 at 11:13:46AM +0000, Andre Przywara wrote: > Hi, > > more a comment loosely related to this patch ... > > > diff --git a/arm/unittests.cfg b/arm/unittests.cfg > > index 3f6fa45c587e..68bf5cd6008f 100644 > > --- a/arm/unittests.cfg > > +++ b/arm/unittests.cfg > > @@ -54,3 +54,10 @@ file = selftest.flat > > smp = $MAX_SMP > > extra_params = -append 'smp' > > groups = selftest > > + > > +# Test GIC emulation > > +[gicv2-ipi] > > +file = gic.flat > > +smp = $((($MAX_SMP < 8)?$MAX_SMP:8)) > > So here we always go with the maximum number of VCPUs in the guest. > However (as you also noted in your cover-letter) running with a > different number of CPUs might be interesting, for instance with less > than 8 CPUs on a GICv2 (the ITARGETSR register must be masked) or in > general with an odd number (both literally and in the broader sense). I > have a test case with passes with 8 VCPUs but fails with less. > > Is there any good way to run some tests multiple times with different > numbers of VCPUS? > Shall we add some "set" functionality to the smp parameter, so that we > can specify a list of desired test points? > We can just add multiple entries, e.g. [gicv2-ipi] file = gic.flat smp = $((($MAX_SMP < 8)?$MAX_SMP:8)) [gicv2-ipi-3] file = gic.flat smp = $((($MAX_SMP > 3)?3:$MAX_SMP)) or whatever. But we need to always consider MAX_SMP, since some machines may less than 8. Thanks, drew
On Fri, Nov 11, 2016 at 02:13:31PM +0100, Andrew Jones wrote: > On Fri, Nov 11, 2016 at 11:13:46AM +0000, Andre Przywara wrote: > > Hi, > > > > more a comment loosely related to this patch ... > > > > > diff --git a/arm/unittests.cfg b/arm/unittests.cfg > > > index 3f6fa45c587e..68bf5cd6008f 100644 > > > --- a/arm/unittests.cfg > > > +++ b/arm/unittests.cfg > > > @@ -54,3 +54,10 @@ file = selftest.flat > > > smp = $MAX_SMP > > > extra_params = -append 'smp' > > > groups = selftest > > > + > > > +# Test GIC emulation > > > +[gicv2-ipi] > > > +file = gic.flat > > > +smp = $((($MAX_SMP < 8)?$MAX_SMP:8)) > > > > So here we always go with the maximum number of VCPUs in the guest. > > However (as you also noted in your cover-letter) running with a > > different number of CPUs might be interesting, for instance with less > > than 8 CPUs on a GICv2 (the ITARGETSR register must be masked) or in > > general with an odd number (both literally and in the broader sense). I > > have a test case with passes with 8 VCPUs but fails with less. > > > > Is there any good way to run some tests multiple times with different > > numbers of VCPUS? > > Shall we add some "set" functionality to the smp parameter, so that we > > can specify a list of desired test points? > > > > We can just add multiple entries, e.g. > > [gicv2-ipi] > file = gic.flat > smp = $((($MAX_SMP < 8)?$MAX_SMP:8)) > [gicv2-ipi-3] > file = gic.flat > smp = $((($MAX_SMP > 3)?3:$MAX_SMP)) > > or whatever. But we need to always consider MAX_SMP, since some > machines may less than 8. > Hmm, thinking about this some more, the unit test needs to know how many processors the test wants, in order to ensure it's testing correctly. We should provide the number to both -smp and -append, like we do for selftest-setup. So, we can have one test that doesn't care, just uses MAX_SMP or 8, like this patch introduces, but then for each test that does care we need, e.g. smp = 3 extra_params = '... smp=3 ...' Then the unit test will start exactly 2 secondaries (or abort if they're not available) Anyway, I don't think this is something we should extend the framework for, but rather address it with unittests.cfg and unit test input validation. Thanks, drew
diff --git a/arm/Makefile.common b/arm/Makefile.common index 41239c37e092..bc38183ab86e 100644 --- a/arm/Makefile.common +++ b/arm/Makefile.common @@ -9,9 +9,9 @@ ifeq ($(LOADADDR),) LOADADDR = 0x40000000 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 000000000000..06092aec7c08 --- /dev/null +++ b/arm/gic.c @@ -0,0 +1,195 @@ +/* + * GIC tests + * + * GICv2 + * + test sending/receiving IPIs + * + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com> + * + * This work is licensed under the terms of the GNU LGPL, version 2. + */ +#include <libcflat.h> +#include <asm/setup.h> +#include <asm/processor.h> +#include <asm/gic.h> +#include <asm/smp.h> +#include <asm/barrier.h> +#include <asm/io.h> + +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(), &ready); + while (!cpumask_full(&ready)) + 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) { + 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)) { + 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 irqstat = readl(gicv2_cpu_base() + GICC_IAR); + u32 irqnr = irqstat & GICC_IAR_INT_ID_MASK; + + if (irqnr != GICC_INT_SPURIOUS) { + writel(irqstat, gicv2_cpu_base() + GICC_EOIR); + 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(&mask); + cpumask_set_cpu(0, &mask); + writel(2 << 24, gicv2_dist_base() + GICD_SGIR); + check_acked(&mask); + 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(&cpu_present_mask)[0] & 0xaa; + cpumask_bits(&mask)[0] = tlist; + writel((u8)tlist << 16, gicv2_dist_base() + GICD_SGIR); + check_acked(&mask); + report_prefix_pop(); + + report_prefix_push("broadcast"); + memset(acked, 0, sizeof(acked)); + smp_wmb(); + cpumask_copy(&mask, &cpu_present_mask); + cpumask_clear_cpu(0, &mask); + writel(1 << 24, gicv2_dist_base() + GICD_SGIR); + check_acked(&mask); + report_prefix_pop(); +} + +static void ipi_enable(void) +{ + gicv2_enable_defaults(); +#ifdef __arm__ + install_exception_handler(EXCPTN_IRQ, ipi_handler); +#else + install_irq_handler(EL1H_IRQ, ipi_handler); +#endif + local_irq_enable(); +} + +static void ipi_recv(void) +{ + ipi_enable(); + cpumask_set_cpu(smp_processor_id(), &ready); + while (1) + wfi(); +} + +int main(int argc, char **argv) +{ + char pfx[8]; + int cpu; + + gic_version = gic_init(); + if (!gic_version) + report_abort("No gic present!"); + + snprintf(pfx, 8, "gicv%d", gic_version); + report_prefix_push(pfx); + + if (argc < 2) { + + report_prefix_push("ipi"); + ipi_enable(); + ipi_test_self(); + report_prefix_pop(); + + } else if (!strcmp(argv[1], "ipi")) { + + report_prefix_push(argv[1]); + nr_cpu_check(2); + + for_each_present_cpu(cpu) { + if (cpu == 0) + continue; + smp_boot_secondary(cpu, ipi_recv); + } + ipi_enable(); + wait_on_ready(); + ipi_test_self(); + ipi_test_smp(); + + smp_rmb(); + for_each_present_cpu(cpu) { + if (spurious[cpu]) { + printf("ipi: WARN: cpu%d got %d spurious " + "interrupts\n", + spurious[cpu], smp_processor_id()); + } + } + + report_prefix_pop(); + + } else { + report_abort("Unknown subtest '%s'", argv[1]); + } + + return report_summary(); +} diff --git a/arm/unittests.cfg b/arm/unittests.cfg index 3f6fa45c587e..68bf5cd6008f 100644 --- a/arm/unittests.cfg +++ b/arm/unittests.cfg @@ -54,3 +54,10 @@ file = selftest.flat smp = $MAX_SMP extra_params = -append 'smp' groups = selftest + +# Test GIC emulation +[gicv2-ipi] +file = gic.flat +smp = $((($MAX_SMP < 8)?$MAX_SMP:8)) +extra_params = -machine gic-version=2 -append 'ipi' +groups = gic diff --git a/lib/arm/asm/gic-v2.h b/lib/arm/asm/gic-v2.h index c2d5fecd4886..8b3f7ed6790c 100644 --- a/lib/arm/asm/gic-v2.h +++ b/lib/arm/asm/gic-v2.h @@ -13,7 +13,9 @@ #endif #define GICD_ENABLE 0x1 + #define GICC_ENABLE 0x1 +#define GICC_IAR_INT_ID_MASK 0x3ff #ifndef __ASSEMBLY__ diff --git a/lib/arm/asm/gic.h b/lib/arm/asm/gic.h index d44e47bcf404..a16645708c35 100644 --- a/lib/arm/asm/gic.h +++ b/lib/arm/asm/gic.h @@ -12,6 +12,7 @@ #define GICD_TYPER 0x0004 #define GICD_ISENABLER 0x0100 #define GICD_IPRIORITYR 0x0400 +#define GICD_SGIR 0x0f00 #define GICD_TYPER_IRQS(typer) ((((typer) & 0x1f) + 1) * 32) #define GICD_INT_EN_SET_SGI 0x0000ffff @@ -19,8 +20,11 @@ #define GICC_CTLR 0x0000 #define GICC_PMR 0x0004 +#define GICC_IAR 0x000c +#define GICC_EOIR 0x0010 #define GICC_INT_PRI_THRESHOLD 0xf0 +#define GICC_INT_SPURIOUS 0x3ff #ifndef __ASSEMBLY__