Hi Alexandru,
On 1/29/21 5:36 PM, Alexandru Elisei wrote:
> GICv2 generates IPIs with a MMIO write to the GICD_SGIR register. A common
> pattern for IPI usage is for the IPI receiver to read data written to
> memory by the sender. The armv7 and armv8 architectures implement a
> weakly-ordered memory model, which means that barriers are required to make
> sure that the expected values are observed.
>
> Because the receiver CPU must observe the write to memory that generated
> the IPI when reading the GICC_IAR MMIO register, we only need to ensure
> ordering of memory accesses, and not completion. The same pattern can be
> observed in the Linux GICv2 irqchip driver (more details in commit
> 8adbf57fc429 ("irqchip: gic: use dmb ishst instead of dsb when raising a
> softirq")).
>
> However, it turns out that no changes are needed to the way GICv2 sends
> IPIs because of the implicit barriers in the MMIO writel and readl
> functions. Writel executes a wmb() (DST ST) before the MMIO write, and
> readl executes a rmb() (DST LD) after the MMIO read. According to ARM DDI
> 0406C.d and ARM DDI 0487F.b, the DSB instruction:
>
> "[..] acts as a stronger barrier than a DMB and all ordering that is
> created by a DMB with specific options is also generated by a DSB with the
> same options."
>
> which means that the correct memory ordering is enforced.
>
> It's not immediately obvious that the proper barriers are in place, so add
> a comment explaining that correct memory synchronization is implemented.
>
> Signed-off-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>
Reviewed-by: Eric Auger <eric.auger@xxxxxxxxxx>
Thanks
Eric
> ---
> lib/arm/gic-v2.c | 6 ++++++
> arm/gic.c | 7 +++++++
> 2 files changed, 13 insertions(+)
>
> diff --git a/lib/arm/gic-v2.c b/lib/arm/gic-v2.c
> index dc6a97c600ec..786d6a4e4c6e 100644
> --- a/lib/arm/gic-v2.c
> +++ b/lib/arm/gic-v2.c
> @@ -45,6 +45,11 @@ void gicv2_ipi_send_single(int irq, int cpu)
> {
> assert(cpu < 8);
> assert(irq < 16);
> + /*
> + * The wmb() in writel and rmb() in readl() from gicv2_read_iar() are
> + * sufficient for ensuring that stores that happen in program order
> + * before the IPI will be visible after the interrupt is acknowledged.
> + */
> writel(1 << (cpu + 16) | irq, gicv2_dist_base() + GICD_SGIR);
> }
>
> @@ -53,5 +58,6 @@ void gicv2_ipi_send_mask(int irq, const cpumask_t *dest)
> u8 tlist = (u8)cpumask_bits(dest)[0];
>
> assert(irq < 16);
> + /* No barriers needed, same situation as gicv2_ipi_send_single() */
> writel(tlist << 16 | irq, gicv2_dist_base() + GICD_SGIR);
> }
> diff --git a/arm/gic.c b/arm/gic.c
> index fee48f9b4ccb..e2e053aeb823 100644
> --- a/arm/gic.c
> +++ b/arm/gic.c
> @@ -260,11 +260,18 @@ static void check_lpi_hits(int *expected, const char *msg)
>
> static void gicv2_ipi_send_self(void)
> {
> + /*
> + * The wmb() in writel and rmb() when acknowledging the interrupt are
> + * sufficient for ensuring that writes that happen in program order
> + * before the interrupt are observed in the interrupt handler after
> + * acknowledging the interrupt.
> + */
> writel(2 << 24 | IPI_IRQ, gicv2_dist_base() + GICD_SGIR);
> }
>
> static void gicv2_ipi_send_broadcast(void)
> {
> + /* No barriers are needed, same situation as gicv2_ipi_send_self() */
> writel(1 << 24 | IPI_IRQ, gicv2_dist_base() + GICD_SGIR);
> }
>
>
