| Message ID | 20201125155113.192079-5-alexandru.elisei@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | GIC fixes and improvements | expand |
Hi Alexandru, On 11/25/20 4:51 PM, Alexandru Elisei wrote: > The GICv3 driver executes a DSB barrier before sending an IPI, which > ensures that memory accesses have completed. This removes the need to > enforce ordering with respect to stats_reset() in the IPI handler. > > For GICv2, we still need the DMB to ensure ordering between the read of the > GICC_IAR MMIO register and the read from the acked array. It also matches > what the Linux GICv2 driver does in gic_handle_irq(). > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > --- > arm/gic.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/arm/gic.c b/arm/gic.c > index 4e947e8516a2..7befda2a8673 100644 > --- a/arm/gic.c > +++ b/arm/gic.c > @@ -60,7 +60,6 @@ static void stats_reset(void) > bad_sender[i] = -1; > bad_irq[i] = -1; > } > - smp_wmb(); Here we are (pair removed). Still the one in check_acked still exists. > } > > static void check_acked(const char *testname, cpumask_t *mask) > @@ -150,7 +149,13 @@ static void ipi_handler(struct pt_regs *regs __unused) > > if (irqnr != GICC_INT_SPURIOUS) { > gic_write_eoir(irqstat); > - smp_rmb(); /* pairs with wmb in stats_reset */ > + /* > + * Make sure data written before the IPI was triggered is > + * observed after the IAR is read. Pairs with the smp_wmb > + * when sending the IPI. > + */ > + if (gic_version() == 2) > + smp_rmb(); > ++acked[smp_processor_id()]; > check_ipi_sender(irqstat); > check_irqnr(irqnr); > Thanks Eric
Hi Eric, On 12/1/20 4:48 PM, Auger Eric wrote: > Hi Alexandru, > > On 11/25/20 4:51 PM, Alexandru Elisei wrote: >> The GICv3 driver executes a DSB barrier before sending an IPI, which >> ensures that memory accesses have completed. This removes the need to >> enforce ordering with respect to stats_reset() in the IPI handler. >> >> For GICv2, we still need the DMB to ensure ordering between the read of the >> GICC_IAR MMIO register and the read from the acked array. It also matches >> what the Linux GICv2 driver does in gic_handle_irq(). >> >> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> >> --- >> arm/gic.c | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/arm/gic.c b/arm/gic.c >> index 4e947e8516a2..7befda2a8673 100644 >> --- a/arm/gic.c >> +++ b/arm/gic.c >> @@ -60,7 +60,6 @@ static void stats_reset(void) >> bad_sender[i] = -1; >> bad_irq[i] = -1; >> } >> - smp_wmb(); > Here we are (pair removed). Still the one in check_acked still exists. The smp_rmb() from check_acked() is there to implement the message passing pattern wrt the writes from the ipi_handler() function, not the writes from stats_reset(). See the next patch where I try to explain how the barriers should work. Thanks, Alex >> } >> >> static void check_acked(const char *testname, cpumask_t *mask) >> @@ -150,7 +149,13 @@ static void ipi_handler(struct pt_regs *regs __unused) >> >> if (irqnr != GICC_INT_SPURIOUS) { >> gic_write_eoir(irqstat); >> - smp_rmb(); /* pairs with wmb in stats_reset */ >> + /* >> + * Make sure data written before the IPI was triggered is >> + * observed after the IAR is read. Pairs with the smp_wmb >> + * when sending the IPI. >> + */ >> + if (gic_version() == 2) >> + smp_rmb(); >> ++acked[smp_processor_id()]; >> check_ipi_sender(irqstat); >> check_irqnr(irqnr); >> > Thanks > > Eric >
Hi, On 11/25/20 4:51 PM, Alexandru Elisei wrote: > The GICv3 driver executes a DSB barrier before sending an IPI, which > ensures that memory accesses have completed. This removes the need to > enforce ordering with respect to stats_reset() in the IPI handler. > > For GICv2, we still need the DMB to ensure ordering between the read of the > GICC_IAR MMIO register and the read from the acked array. It also matches > what the Linux GICv2 driver does in gic_handle_irq(). > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric > --- > arm/gic.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/arm/gic.c b/arm/gic.c > index 4e947e8516a2..7befda2a8673 100644 > --- a/arm/gic.c > +++ b/arm/gic.c > @@ -60,7 +60,6 @@ static void stats_reset(void) > bad_sender[i] = -1; > bad_irq[i] = -1; > } > - smp_wmb(); > } > > static void check_acked(const char *testname, cpumask_t *mask) > @@ -150,7 +149,13 @@ static void ipi_handler(struct pt_regs *regs __unused) > > if (irqnr != GICC_INT_SPURIOUS) { > gic_write_eoir(irqstat); > - smp_rmb(); /* pairs with wmb in stats_reset */ > + /* > + * Make sure data written before the IPI was triggered is > + * observed after the IAR is read. Pairs with the smp_wmb > + * when sending the IPI. > + */ > + if (gic_version() == 2) > + smp_rmb(); > ++acked[smp_processor_id()]; > check_ipi_sender(irqstat); > check_irqnr(irqnr); >
diff --git a/arm/gic.c b/arm/gic.c index 4e947e8516a2..7befda2a8673 100644 --- a/arm/gic.c +++ b/arm/gic.c @@ -60,7 +60,6 @@ static void stats_reset(void) bad_sender[i] = -1; bad_irq[i] = -1; } - smp_wmb(); } static void check_acked(const char *testname, cpumask_t *mask) @@ -150,7 +149,13 @@ static void ipi_handler(struct pt_regs *regs __unused) if (irqnr != GICC_INT_SPURIOUS) { gic_write_eoir(irqstat); - smp_rmb(); /* pairs with wmb in stats_reset */ + /* + * Make sure data written before the IPI was triggered is + * observed after the IAR is read. Pairs with the smp_wmb + * when sending the IPI. + */ + if (gic_version() == 2) + smp_rmb(); ++acked[smp_processor_id()]; check_ipi_sender(irqstat); check_irqnr(irqnr);
The GICv3 driver executes a DSB barrier before sending an IPI, which ensures that memory accesses have completed. This removes the need to enforce ordering with respect to stats_reset() in the IPI handler. For GICv2, we still need the DMB to ensure ordering between the read of the GICC_IAR MMIO register and the read from the acked array. It also matches what the Linux GICv2 driver does in gic_handle_irq(). Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> --- arm/gic.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)