On 17/12/2020 14:13, Alexandru Elisei wrote:
> The gicv{2,3}-active test sends an IPI from the boot CPU to itself, then
> checks that the interrupt has been received as expected. There is no need
> to use inter-processor memory synchronization primitives on code that runs
> on the same CPU, so remove the unneeded memory barriers.
>
> Reviewed-by: Eric Auger <eric.auger@xxxxxxxxxx>
> Signed-off-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>
I am not fully convinced this is a wise move. Indeed the existing
barriers look wrong, and *currently* we just use
ipi_clear_active_handler() for a single CPU test only, but the handler
function itself is not restricted to that use case, if I am not mistaken.
But I think for now this fix is fine, and the comment should point out
the limitation well enough, so:
Reviewed-by: Andre Przywara <andre.przywara@xxxxxxx>
Cheers,
Andre
> ---
> arm/gic.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/arm/gic.c b/arm/gic.c
> index ca61dba2986c..1c9f4a01b6e4 100644
> --- a/arm/gic.c
> +++ b/arm/gic.c
> @@ -364,6 +364,7 @@ static struct gic gicv3 = {
> },
> };
>
> +/* Runs on the same CPU as the sender, no need for memory synchronization */
> static void ipi_clear_active_handler(struct pt_regs *regs __unused)
> {
> u32 irqstat = gic_read_iar();
> @@ -380,13 +381,10 @@ static void ipi_clear_active_handler(struct pt_regs *regs __unused)
>
> writel(val, base + GICD_ICACTIVER);
>
> - smp_rmb(); /* pairs with wmb in stats_reset */
> ++acked[smp_processor_id()];
> check_irqnr(irqnr);
> - smp_wmb(); /* pairs with rmb in check_acked */
> } else {
> ++spurious[smp_processor_id()];
> - smp_wmb();
> }
> }
>
>
