| Message ID | 20181127100317.12809-2-anup@brainfault.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | IRQ affinity support in PLIC driver | expand |
On 11/27/18 2:03 AM, Anup Patel wrote: > This patch does following optimizations: > 1. Pre-compute hart base for each context handler > 2. Pre-compute enable base for each context handler > 3. Have enable lock for each context handler instead > of global plic_toggle_lock > > Signed-off-by: Anup Patel <anup@brainfault.org> > --- > drivers/irqchip/irq-sifive-plic.c | 41 +++++++++++++------------------ > 1 file changed, 17 insertions(+), 24 deletions(-) > > diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c > index 357e9daf94ae..56fce648a901 100644 > --- a/drivers/irqchip/irq-sifive-plic.c > +++ b/drivers/irqchip/irq-sifive-plic.c > @@ -60,36 +60,24 @@ static void __iomem *plic_regs; > struct plic_handler { > bool present; > int ctxid; > + void __iomem *hart_base; > + raw_spinlock_t enable_lock; > + void __iomem *enable_base; It should be u32. Otherwise, plic_toggle calculates incorrect address and it does not boot on Unlheased. > }; > static DEFINE_PER_CPU(struct plic_handler, plic_handlers); > > -static inline void __iomem *plic_hart_offset(int ctxid) > +static inline void plic_toggle(struct plic_handler *handler, > + int hwirq, int enable) > { > - return plic_regs + CONTEXT_BASE + ctxid * CONTEXT_PER_HART; > -} > - > -static inline u32 __iomem *plic_enable_base(int ctxid) > -{ > - return plic_regs + ENABLE_BASE + ctxid * ENABLE_PER_HART; > -} > - > -/* > - * Protect mask operations on the registers given that we can't assume that > - * atomic memory operations work on them. > - */ Should we keep the comment for enable_lock ? > -static DEFINE_RAW_SPINLOCK(plic_toggle_lock); > - > -static inline void plic_toggle(int ctxid, int hwirq, int enable) > -{ > - u32 __iomem *reg = plic_enable_base(ctxid) + (hwirq / 32); > + u32 __iomem *reg = handler->enable_base + (hwirq / 32); > u32 hwirq_mask = 1 << (hwirq % 32); > > - raw_spin_lock(&plic_toggle_lock); > + raw_spin_lock(&handler->enable_lock); > if (enable) > writel(readl(reg) | hwirq_mask, reg); > else > writel(readl(reg) & ~hwirq_mask, reg); > - raw_spin_unlock(&plic_toggle_lock); > + raw_spin_unlock(&handler->enable_lock); > } > > static inline void plic_irq_toggle(struct irq_data *d, int enable) > @@ -101,7 +89,7 @@ static inline void plic_irq_toggle(struct irq_data *d, int enable) > struct plic_handler *handler = per_cpu_ptr(&plic_handlers, cpu); > > if (handler->present) > - plic_toggle(handler->ctxid, d->hwirq, enable); > + plic_toggle(handler, d->hwirq, enable); > } > } > > @@ -150,7 +138,7 @@ static struct irq_domain *plic_irqdomain; > static void plic_handle_irq(struct pt_regs *regs) > { > struct plic_handler *handler = this_cpu_ptr(&plic_handlers); > - void __iomem *claim = plic_hart_offset(handler->ctxid) + CONTEXT_CLAIM; > + void __iomem *claim = handler->hart_base + CONTEXT_CLAIM; > irq_hw_number_t hwirq; > > WARN_ON_ONCE(!handler->present); > @@ -240,11 +228,16 @@ static int __init plic_init(struct device_node *node, > handler = per_cpu_ptr(&plic_handlers, cpu); > handler->present = true; > handler->ctxid = i; > + handler->hart_base = > + plic_regs + CONTEXT_BASE + i * CONTEXT_PER_HART; > + raw_spin_lock_init(&handler->enable_lock); > + handler->enable_base = > + plic_regs + ENABLE_BASE + i * ENABLE_PER_HART; > > /* priority must be > threshold to trigger an interrupt */ > - writel(0, plic_hart_offset(i) + CONTEXT_THRESHOLD); > + writel(0, handler->hart_base + CONTEXT_THRESHOLD); > for (hwirq = 1; hwirq <= nr_irqs; hwirq++) > - plic_toggle(i, hwirq, 0); > + plic_toggle(handler, hwirq, 0); > nr_mapped++; > } > >
On Fri, Nov 30, 2018 at 6:05 AM Atish Patra <atish.patra@wdc.com> wrote: > > On 11/27/18 2:03 AM, Anup Patel wrote: > > This patch does following optimizations: > > 1. Pre-compute hart base for each context handler > > 2. Pre-compute enable base for each context handler > > 3. Have enable lock for each context handler instead > > of global plic_toggle_lock > > > > Signed-off-by: Anup Patel <anup@brainfault.org> > > --- > > drivers/irqchip/irq-sifive-plic.c | 41 +++++++++++++------------------ > > 1 file changed, 17 insertions(+), 24 deletions(-) > > > > diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c > > index 357e9daf94ae..56fce648a901 100644 > > --- a/drivers/irqchip/irq-sifive-plic.c > > +++ b/drivers/irqchip/irq-sifive-plic.c > > @@ -60,36 +60,24 @@ static void __iomem *plic_regs; > > struct plic_handler { > > bool present; > > int ctxid; > > + void __iomem *hart_base; > > + raw_spinlock_t enable_lock; > > + void __iomem *enable_base; > > It should be u32. Otherwise, plic_toggle calculates incorrect address > and it does not boot on Unlheased. Good catch. I did not see this issue on QEMU because we have very IRQs over there. > > > }; > > static DEFINE_PER_CPU(struct plic_handler, plic_handlers); > > > > -static inline void __iomem *plic_hart_offset(int ctxid) > > +static inline void plic_toggle(struct plic_handler *handler, > > + int hwirq, int enable) > > { > > - return plic_regs + CONTEXT_BASE + ctxid * CONTEXT_PER_HART; > > -} > > - > > -static inline u32 __iomem *plic_enable_base(int ctxid) > > -{ > > - return plic_regs + ENABLE_BASE + ctxid * ENABLE_PER_HART; > > -} > > - > > -/* > > - * Protect mask operations on the registers given that we can't assume that > > - * atomic memory operations work on them. > > - */ > > Should we keep the comment for enable_lock ? Sure, I will retain the comment for enable_lock. > > > -static DEFINE_RAW_SPINLOCK(plic_toggle_lock); > > - > > -static inline void plic_toggle(int ctxid, int hwirq, int enable) > > -{ > > - u32 __iomem *reg = plic_enable_base(ctxid) + (hwirq / 32); > > + u32 __iomem *reg = handler->enable_base + (hwirq / 32); > > u32 hwirq_mask = 1 << (hwirq % 32); > > > > - raw_spin_lock(&plic_toggle_lock); > > + raw_spin_lock(&handler->enable_lock); > > if (enable) > > writel(readl(reg) | hwirq_mask, reg); > > else > > writel(readl(reg) & ~hwirq_mask, reg); > > - raw_spin_unlock(&plic_toggle_lock); > > + raw_spin_unlock(&handler->enable_lock); > > } > > > > static inline void plic_irq_toggle(struct irq_data *d, int enable) > > @@ -101,7 +89,7 @@ static inline void plic_irq_toggle(struct irq_data *d, int enable) > > struct plic_handler *handler = per_cpu_ptr(&plic_handlers, cpu); > > > > if (handler->present) > > - plic_toggle(handler->ctxid, d->hwirq, enable); > > + plic_toggle(handler, d->hwirq, enable); > > } > > } > > > > @@ -150,7 +138,7 @@ static struct irq_domain *plic_irqdomain; > > static void plic_handle_irq(struct pt_regs *regs) > > { > > struct plic_handler *handler = this_cpu_ptr(&plic_handlers); > > - void __iomem *claim = plic_hart_offset(handler->ctxid) + CONTEXT_CLAIM; > > + void __iomem *claim = handler->hart_base + CONTEXT_CLAIM; > > irq_hw_number_t hwirq; > > > > WARN_ON_ONCE(!handler->present); > > @@ -240,11 +228,16 @@ static int __init plic_init(struct device_node *node, > > handler = per_cpu_ptr(&plic_handlers, cpu); > > handler->present = true; > > handler->ctxid = i; > > + handler->hart_base = > > + plic_regs + CONTEXT_BASE + i * CONTEXT_PER_HART; > > + raw_spin_lock_init(&handler->enable_lock); > > + handler->enable_base = > > + plic_regs + ENABLE_BASE + i * ENABLE_PER_HART; > > > > /* priority must be > threshold to trigger an interrupt */ > > - writel(0, plic_hart_offset(i) + CONTEXT_THRESHOLD); > > + writel(0, handler->hart_base + CONTEXT_THRESHOLD); > > for (hwirq = 1; hwirq <= nr_irqs; hwirq++) > > - plic_toggle(i, hwirq, 0); > > + plic_toggle(handler, hwirq, 0); > > nr_mapped++; > > } > > > > > -- Anup
diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c index 357e9daf94ae..56fce648a901 100644 --- a/drivers/irqchip/irq-sifive-plic.c +++ b/drivers/irqchip/irq-sifive-plic.c @@ -60,36 +60,24 @@ static void __iomem *plic_regs; struct plic_handler { bool present; int ctxid; + void __iomem *hart_base; + raw_spinlock_t enable_lock; + void __iomem *enable_base; }; static DEFINE_PER_CPU(struct plic_handler, plic_handlers); -static inline void __iomem *plic_hart_offset(int ctxid) +static inline void plic_toggle(struct plic_handler *handler, + int hwirq, int enable) { - return plic_regs + CONTEXT_BASE + ctxid * CONTEXT_PER_HART; -} - -static inline u32 __iomem *plic_enable_base(int ctxid) -{ - return plic_regs + ENABLE_BASE + ctxid * ENABLE_PER_HART; -} - -/* - * Protect mask operations on the registers given that we can't assume that - * atomic memory operations work on them. - */ -static DEFINE_RAW_SPINLOCK(plic_toggle_lock); - -static inline void plic_toggle(int ctxid, int hwirq, int enable) -{ - u32 __iomem *reg = plic_enable_base(ctxid) + (hwirq / 32); + u32 __iomem *reg = handler->enable_base + (hwirq / 32); u32 hwirq_mask = 1 << (hwirq % 32); - raw_spin_lock(&plic_toggle_lock); + raw_spin_lock(&handler->enable_lock); if (enable) writel(readl(reg) | hwirq_mask, reg); else writel(readl(reg) & ~hwirq_mask, reg); - raw_spin_unlock(&plic_toggle_lock); + raw_spin_unlock(&handler->enable_lock); } static inline void plic_irq_toggle(struct irq_data *d, int enable) @@ -101,7 +89,7 @@ static inline void plic_irq_toggle(struct irq_data *d, int enable) struct plic_handler *handler = per_cpu_ptr(&plic_handlers, cpu); if (handler->present) - plic_toggle(handler->ctxid, d->hwirq, enable); + plic_toggle(handler, d->hwirq, enable); } } @@ -150,7 +138,7 @@ static struct irq_domain *plic_irqdomain; static void plic_handle_irq(struct pt_regs *regs) { struct plic_handler *handler = this_cpu_ptr(&plic_handlers); - void __iomem *claim = plic_hart_offset(handler->ctxid) + CONTEXT_CLAIM; + void __iomem *claim = handler->hart_base + CONTEXT_CLAIM; irq_hw_number_t hwirq; WARN_ON_ONCE(!handler->present); @@ -240,11 +228,16 @@ static int __init plic_init(struct device_node *node, handler = per_cpu_ptr(&plic_handlers, cpu); handler->present = true; handler->ctxid = i; + handler->hart_base = + plic_regs + CONTEXT_BASE + i * CONTEXT_PER_HART; + raw_spin_lock_init(&handler->enable_lock); + handler->enable_base = + plic_regs + ENABLE_BASE + i * ENABLE_PER_HART; /* priority must be > threshold to trigger an interrupt */ - writel(0, plic_hart_offset(i) + CONTEXT_THRESHOLD); + writel(0, handler->hart_base + CONTEXT_THRESHOLD); for (hwirq = 1; hwirq <= nr_irqs; hwirq++) - plic_toggle(i, hwirq, 0); + plic_toggle(handler, hwirq, 0); nr_mapped++; }
This patch does following optimizations: 1. Pre-compute hart base for each context handler 2. Pre-compute enable base for each context handler 3. Have enable lock for each context handler instead of global plic_toggle_lock Signed-off-by: Anup Patel <anup@brainfault.org> --- drivers/irqchip/irq-sifive-plic.c | 41 +++++++++++++------------------ 1 file changed, 17 insertions(+), 24 deletions(-)