2024-01-29 14:45:24

by Lucas Stach

Subject: [PATCH] genirq: use relaxed access by default for irq_reg_{readl,writel}

irqchip access does not require any memory ordering between other
memory transactions and the IRQ controller peripheral access.
As all architectures now implement the relaxed MMIO accessors we
can switch the irq_reg_{readl,writel} helpers to use them, in
order to avoid potentially costly barriers in the IRQ handling
hotpath.

Signed-off-by: Lucas Stach <[email protected]>
---
include/linux/irq.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 90081afa10ce..fa1597db7887 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -1218,7 +1218,7 @@ static inline void irq_reg_writel(struct irq_chip_generic *gc,
if (gc->reg_writel)
gc->reg_writel(val, gc->reg_base + reg_offset);
else
- writel(val, gc->reg_base + reg_offset);
+ writel_relaxed(val, gc->reg_base + reg_offset);
}

static inline u32 irq_reg_readl(struct irq_chip_generic *gc,
@@ -1227,7 +1227,7 @@ static inline u32 irq_reg_readl(struct irq_chip_generic *gc,
if (gc->reg_readl)
return gc->reg_readl(gc->reg_base + reg_offset);
else
- return readl(gc->reg_base + reg_offset);
+ return readl_relaxed(gc->reg_base + reg_offset);
}

struct irq_matrix;
--
2.39.2

2024-01-30 12:00:09

by Marc Zyngier

Subject: Re: [PATCH] genirq: use relaxed access by default for irq_reg_{readl,writel}

On Mon, 29 Jan 2024 14:45:02 +0000,
Lucas Stach <[email protected]> wrote:
>
> irqchip access does not require any memory ordering between other
> memory transactions and the IRQ controller peripheral access.
> As all architectures now implement the relaxed MMIO accessors we
> can switch the irq_reg_{readl,writel} helpers to use them, in
> order to avoid potentially costly barriers in the IRQ handling
> hotpath.
>
> Signed-off-by: Lucas Stach <[email protected]>
> ---
> include/linux/irq.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 90081afa10ce..fa1597db7887 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -1218,7 +1218,7 @@ static inline void irq_reg_writel(struct irq_chip_generic *gc,
> if (gc->reg_writel)
> gc->reg_writel(val, gc->reg_base + reg_offset);
> else
> - writel(val, gc->reg_base + reg_offset);
> + writel_relaxed(val, gc->reg_base + reg_offset);
> }
>
> static inline u32 irq_reg_readl(struct irq_chip_generic *gc,
> @@ -1227,7 +1227,7 @@ static inline u32 irq_reg_readl(struct irq_chip_generic *gc,
> if (gc->reg_readl)
> return gc->reg_readl(gc->reg_base + reg_offset);
> else
> - return readl(gc->reg_base + reg_offset);
> + return readl_relaxed(gc->reg_base + reg_offset);
> }
>

If this relaxation is introduced, it really should be documented and
require a buy-in, because unsuspecting drivers may implicitly depend
on the stronger ordering.

I'm a strong advocate of the relaxed ordering, but changing this
wholesale is potentially dangerous.

M.

--
Without deviation from the norm, progress is not possible.