check_acked() has several peculiarities: is the only function among the check_* functions which calls report() directly, it does two things (waits for interrupts and checks for misfired interrupts) and it also mixes printf, report_info and report calls. check_acked() also reports a pass and returns as soon all the target CPUs have received interrupts, However, a CPU not having received an interrupt *now* does not guarantee not receiving an erroneous interrupt if we wait long enough. Rework the function by splitting it into two separate functions, each with a single responsibility: wait_for_interrupts(), which waits for the expected interrupts to fire, and check_acked() which checks that interrupts have been received as expected. wait_for_interrupts() also waits an extra 100 milliseconds after the expected interrupts have been received in an effort to make sure we don't miss misfiring interrupts. Splitting check_acked() into two functions will also allow us to customize the behavior of each function in the future more easily without using an unnecessarily long list of arguments for check_acked(). CC: Andre Przywara <andre.przywara@xxxxxxx> Signed-off-by: Alexandru Elisei <alexandru.elisei@xxxxxxx> --- arm/gic.c | 74 ++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 26 deletions(-) diff --git a/arm/gic.c b/arm/gic.c index af4d4645c0ae..2c96cf49ce8c 100644 --- a/arm/gic.c +++ b/arm/gic.c @@ -61,41 +61,43 @@ static void stats_reset(void) } } -static void check_acked(const char *testname, cpumask_t *mask) +static void wait_for_interrupts(cpumask_t *mask) { - int missing = 0, extra = 0, unexpected = 0; int nr_pass, cpu, i; - bool bad = false; /* Wait up to 5s for all interrupts to be delivered */ - for (i = 0; i < 50; ++i) { + for (i = 0; i < 50; i++) { mdelay(100); nr_pass = 0; for_each_present_cpu(cpu) { + /* + * A CPU having received more than one interrupts will + * show up in check_acked(), and no matter how long we + * wait it cannot un-receive it. Consider at least one + * interrupt as a pass. + */ nr_pass += cpumask_test_cpu(cpu, mask) ? - acked[cpu] == 1 : acked[cpu] == 0; - smp_rmb(); /* pairs with smp_wmb in ipi_handler */ - - if (bad_sender[cpu] != -1) { - printf("cpu%d received IPI from wrong sender %d\n", - cpu, bad_sender[cpu]); - bad = true; - } - - if (bad_irq[cpu] != -1) { - printf("cpu%d received wrong irq %d\n", - cpu, bad_irq[cpu]); - bad = true; - } + acked[cpu] >= 1 : acked[cpu] == 0; } + if (nr_pass == nr_cpus) { - report(!bad, "%s", testname); if (i) - report_info("took more than %d ms", i * 100); + report_info("interrupts took more than %d ms", i * 100); + /* Wait for unexpected interrupts to fire */ + mdelay(100); return; } } + report_info("interrupts timed-out (5s)"); +} + +static bool check_acked(cpumask_t *mask) +{ + int missing = 0, extra = 0, unexpected = 0; + bool pass = true; + int cpu; + for_each_present_cpu(cpu) { if (cpumask_test_cpu(cpu, mask)) { if (!acked[cpu]) @@ -106,11 +108,28 @@ static void check_acked(const char *testname, cpumask_t *mask) if (acked[cpu]) ++unexpected; } + smp_rmb(); /* pairs with smp_wmb in ipi_handler */ + + if (bad_sender[cpu] != -1) { + report_info("cpu%d received IPI from wrong sender %d", + cpu, bad_sender[cpu]); + pass = false; + } + + if (bad_irq[cpu] != -1) { + report_info("cpu%d received wrong irq %d", + cpu, bad_irq[cpu]); + pass = false; + } + } + + if (missing || extra || unexpected) { + report_info("ACKS: missing=%d extra=%d unexpected=%d", + missing, extra, unexpected); + pass = false; } - report(false, "%s", testname); - report_info("Timed-out (5s). ACKS: missing=%d extra=%d unexpected=%d", - missing, extra, unexpected); + return pass; } static void check_spurious(void) @@ -299,7 +318,8 @@ static void ipi_test_self(void) cpumask_clear(&mask); cpumask_set_cpu(smp_processor_id(), &mask); gic->ipi.send_self(); - check_acked("IPI: self", &mask); + wait_for_interrupts(&mask); + report(check_acked(&mask), "Interrupts received"); report_prefix_pop(); } @@ -314,7 +334,8 @@ static void ipi_test_smp(void) for (i = smp_processor_id() & 1; i < nr_cpus; i += 2) cpumask_clear_cpu(i, &mask); gic_ipi_send_mask(IPI_IRQ, &mask); - check_acked("IPI: directed", &mask); + wait_for_interrupts(&mask); + report(check_acked(&mask), "Interrupts received"); report_prefix_pop(); report_prefix_push("broadcast"); @@ -322,7 +343,8 @@ static void ipi_test_smp(void) cpumask_copy(&mask, &cpu_present_mask); cpumask_clear_cpu(smp_processor_id(), &mask); gic->ipi.send_broadcast(); - check_acked("IPI: broadcast", &mask); + wait_for_interrupts(&mask); + report(check_acked(&mask), "Interrupts received"); report_prefix_pop(); } -- 2.30.0