| Message ID | 1468587641-7300-11-git-send-email-drjones@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 <drjones@redhat.com> > > > > --- > > 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 <libcflat.h> > > +#include <util.h> > > #include <asm/setup.h> > > #include <asm/processor.h> > > #include <asm/gic.h> > > @@ -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(&mask); > > - cpumask_set_cpu(0, &mask); > > + cpumask_set_cpu(smp_processor_id(), &mask); > > gic->ipi.send_self(); > > check_acked(&mask); > > report_prefix_pop(); > > @@ -214,7 +232,7 @@ static void ipi_test_smp(void) > > memset(acked, 0, sizeof(acked)); > > smp_wmb(); > > cpumask_copy(&mask, &cpu_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, &mask); > > gic->ipi.send_tlist(&mask); > > check_acked(&mask); > > @@ -224,7 +242,7 @@ static void ipi_test_smp(void) > > memset(acked, 0, sizeof(acked)); > > smp_wmb(); > > cpumask_copy(&mask, &cpu_present_mask); > > - cpumask_clear_cpu(0, &mask); > > + cpumask_clear_cpu(smp_processor_id(), &mask); > > gic->ipi.send_broadcast(); > > check_acked(&mask); > > report_prefix_pop(); > > @@ -241,6 +259,15 @@ static void ipi_enable(void) > > local_irq_enable(); > > } > > > > +static void ipi_send(void) > > +{ > > + ipi_enable(); > > + wait_on_ready(); > > + ipi_test_self(); > > + ipi_test_smp(); > > + exit(report_summary()); > > +} > > + > > static void ipi_recv(void) > > { > > ipi_enable(); > > @@ -300,19 +327,40 @@ int main(int argc, char **argv) > > report_prefix_pop(); > > > > } else if (!strcmp(argv[1], "ipi")) { > > + int off, i = 1; > > + long val; > > > > report_prefix_push(argv[1]); > > + > > + while (--argc != 1) { > > + off = parse_keyval(argv[++i], &val); > > + if (off == -1) > > + continue; > > + argv[i][off] = '\0'; > > + if (strcmp(argv[i], "sender") == 0) > > + sender = val; > > + else if (strcmp(argv[i], "irq") == 0) > > + irq = val; > > + } > > + > > nr_cpu_check(2); > > ipi_enable(); > > > > for_each_present_cpu(cpu) { > > if (cpu == 0) > > continue; > > - smp_boot_secondary(cpu, ipi_recv); > > + if (cpu == sender) > > + smp_boot_secondary(cpu, ipi_send); > > + else > > + smp_boot_secondary(cpu, ipi_recv); > > + } > > + if (sender == 0) { > > + wait_on_ready(); > > + ipi_test_self(); > > + ipi_test_smp(); > > + } else { > > + ipi_recv(); > > } > > - wait_on_ready(); > > - ipi_test_self(); > > - ipi_test_smp(); > > > > smp_rmb(); > > for_each_present_cpu(cpu) { > > > > I ran the tests on both Cavium & Seattle HW without noticing issues. > > I guess the file organization & infra will need to change while adding > some new tests, removing some globals ... but that's a good start to Can you explain what you have in mind for removing some globals, changing things? I can change it now :-) As for adding new tests, though, while I think we can extend arm/gic.c with more tests, anytime a unit test differs too much from a framework of a current unit test (group of tests) it should just create a new unit test file in arm/ and then do whatever it likes. The common code lib/* is where changes should be made if the framework doesn't work - but I should get that as close to right as possible before merging the series. > show how to write new tests. I am a bit concerned by the LOCs and > redundancies with the kernel with possible out-of-sync macros but well I > think there is no other way. Me neither. We've debated using kernel headers straight from a kernel source directory in the past, but opted against it. I don't think we'll need to synch too much though. If somebody writes a test that needs new defines, then that's when it can be synched. > > I will spend some time writing some tests within the next weeks. Awesome. Of course that was some weeks ago that you wrote that :-) Thanks for the review of the series. I'll send a v4 soon. drew
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 <libcflat.h> +#include <util.h> #include <asm/setup.h> #include <asm/processor.h> #include <asm/gic.h> @@ -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; } 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(&mask); - cpumask_set_cpu(0, &mask); + cpumask_set_cpu(smp_processor_id(), &mask); gic->ipi.send_self(); check_acked(&mask); report_prefix_pop(); @@ -214,7 +232,7 @@ static void ipi_test_smp(void) memset(acked, 0, sizeof(acked)); smp_wmb(); cpumask_copy(&mask, &cpu_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, &mask); gic->ipi.send_tlist(&mask); check_acked(&mask); @@ -224,7 +242,7 @@ static void ipi_test_smp(void) memset(acked, 0, sizeof(acked)); smp_wmb(); cpumask_copy(&mask, &cpu_present_mask); - cpumask_clear_cpu(0, &mask); + cpumask_clear_cpu(smp_processor_id(), &mask); gic->ipi.send_broadcast(); check_acked(&mask); report_prefix_pop(); @@ -241,6 +259,15 @@ static void ipi_enable(void) local_irq_enable(); } +static void ipi_send(void) +{ + ipi_enable(); + wait_on_ready(); + ipi_test_self(); + ipi_test_smp(); + exit(report_summary()); +} + static void ipi_recv(void) { ipi_enable(); @@ -300,19 +327,40 @@ int main(int argc, char **argv) report_prefix_pop(); } else if (!strcmp(argv[1], "ipi")) { + int off, i = 1; + long val; report_prefix_push(argv[1]); + + while (--argc != 1) { + off = parse_keyval(argv[++i], &val); + if (off == -1) + continue; + argv[i][off] = '\0'; + if (strcmp(argv[i], "sender") == 0) + sender = val; + else if (strcmp(argv[i], "irq") == 0) + irq = val; + } + nr_cpu_check(2); ipi_enable(); for_each_present_cpu(cpu) { if (cpu == 0) continue; - smp_boot_secondary(cpu, ipi_recv); + if (cpu == sender) + smp_boot_secondary(cpu, ipi_send); + else + smp_boot_secondary(cpu, ipi_recv); + } + if (sender == 0) { + wait_on_ready(); + ipi_test_self(); + ipi_test_smp(); + } else { + ipi_recv(); } - wait_on_ready(); - ipi_test_self(); - ipi_test_smp(); smp_rmb(); for_each_present_cpu(cpu) {
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 <drjones@redhat.com> --- 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(-) -- 2.7.4