From: Anup Patel <anup@brainfault.org>
To: Guo Ren <guoren@kernel.org>
Cc: Guo Ren <guoren@linux.alibaba.com>,
Jason Cooper <jason@lakedaemon.net>,
wesley@sifive.com, Marc Zyngier <maz@kernel.org>,
Palmer Dabbelt <palmerdabbelt@google.com>,
"linux-kernel@vger.kernel.org List"
<linux-kernel@vger.kernel.org>,
linux-riscv <linux-riscv@lists.infradead.org>,
Atish Patra <atish.patra@wdc.com>,
Yash Shah <yash.shah@sifive.com>, Zong Li <zong.li@sifive.com>,
Paul Walmsley <paul.walmsley@sifive.com>,
Greentime Hu <greentime.hu@sifive.com>,
Thomas Gleixner <tglx@linutronix.de>,
Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH 3/3] irqchip/irq-sifive-plic: Fixup set_affinity enable irq unexpected
Date: Fri, 23 Oct 2020 17:03:26 +0530 [thread overview]
Message-ID: <CAAhSdy3aOmUSY7x=BVVKCb5JD39H+_zOhBCuwgUxgtJLKqTrDw@mail.gmail.com> (raw)
In-Reply-To: <1603448245-79429-3-git-send-email-guoren@kernel.org>
On Fri, Oct 23, 2020 at 3:48 PM <guoren@kernel.org> wrote:
>
> From: Guo Ren <guoren@linux.alibaba.com>
>
> For PLIC, we only have enable registers to control per hart's irq
> affinity and irq_set_affinity would call plic_irq_toggle to enable
> the IRQ's routing. So we can't enable irq in irq_domain_map before
> request_irq, it'll let uninitialized devices' irq exception.
>
> The solution is to check the irq has been enabled, just like what
> irq-gic-v3 has done in gic_set_affinity.
>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> ---
> drivers/irqchip/irq-sifive-plic.c | 45 ++++++++++++++++++++++++++++++++++++---
> 1 file changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> index 0003322..1a63859 100644
> --- a/drivers/irqchip/irq-sifive-plic.c
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -130,6 +130,36 @@ static void plic_irq_mask(struct irq_data *d)
> }
>
> #ifdef CONFIG_SMP
> +static inline bool plic_toggle_is_enabled(struct plic_handler *handler,
> + int hwirq)
> +{
> + u32 __iomem *reg = handler->enable_base + (hwirq / 32) * sizeof(u32);
> + u32 hwirq_mask = 1 << (hwirq % 32);
> +
> + if (readl(reg) & hwirq_mask)
> + return true;
> + else
> + return false;
> +}
> +
> +static inline bool plic_irq_is_enabled(const struct cpumask *mask,
> + struct irq_data *d)
> +{
> + int cpu;
> +
> + for_each_cpu(cpu, mask) {
> + struct plic_handler *handler = per_cpu_ptr(&plic_handlers, cpu);
> +
> + if (!handler->present)
> + continue;
> +
> + if (plic_toggle_is_enabled(handler, d->hwirq))
> + return true;
> + }
> +
> + return false;
> +}
> +
> static int plic_set_affinity(struct irq_data *d,
> const struct cpumask *mask_val, bool force)
> {
> @@ -141,8 +171,10 @@ static int plic_set_affinity(struct irq_data *d,
>
> irq_data_update_effective_affinity(d, &amask);
>
> - plic_irq_toggle(&priv->lmask, d, 0);
> - plic_irq_toggle(&amask, d, 1);
> + if (plic_irq_is_enabled(&priv->lmask, d)) {
> + plic_irq_toggle(&priv->lmask, d, 0);
> + plic_irq_toggle(&amask, d, 1);
> + }
>
> return IRQ_SET_MASK_OK_DONE;
> }
> @@ -168,12 +200,19 @@ static struct irq_chip plic_chip = {
> static int plic_irqdomain_map(struct irq_domain *d, unsigned int irq,
> irq_hw_number_t hwirq)
> {
> + unsigned int cpu;
> struct plic_priv *priv = d->host_data;
>
> irq_domain_set_info(d, irq, hwirq, &plic_chip, d->host_data,
> handle_fasteoi_irq, NULL, NULL);
> irq_set_noprobe(irq);
> - irq_set_affinity(irq, &priv->lmask);
> +
> + cpu = cpumask_any_and(&priv->lmask, cpu_online_mask);
> + if (WARN_ON_ONCE(cpu >= nr_cpu_ids))
> + return -EINVAL;
> +
> + irq_set_affinity(irq, cpumask_of(cpu));
> +
> return 0;
> }
>
> --
> 2.7.4
>
Greentime (SiFive) has already proposed a patch to fix this.
Refer, https://lkml.org/lkml/2020/10/20/188
Only the plic_irqdomain_map() change which sets the correct
default affinity looks good to me.
Regards,
Anup
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2020-10-23 12:23 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-23 10:17 [PATCH 1/3] irqchip/irq-sifive-plic: Fixup wrong size of xxx_PER_HART and reg base guoren
2020-10-23 10:17 ` [PATCH 2/3] irqchip/irq-sifive-plic: Fixup couldn't broadcast to multi CPUs guoren
2020-10-23 11:27 ` Anup Patel
2020-10-24 3:38 ` Guo Ren
2020-10-23 10:17 ` [PATCH 3/3] irqchip/irq-sifive-plic: Fixup set_affinity enable irq unexpected guoren
2020-10-23 11:33 ` Anup Patel [this message]
2020-10-23 11:42 ` [PATCH 1/3] irqchip/irq-sifive-plic: Fixup wrong size of xxx_PER_HART and reg base Anup Patel
2020-10-24 3:09 ` Guo Ren
2020-10-25 9:17 ` Anup Patel
2020-10-26 15:01 ` Guo Ren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAAhSdy3aOmUSY7x=BVVKCb5JD39H+_zOhBCuwgUxgtJLKqTrDw@mail.gmail.com' \
--to=anup@brainfault.org \
--cc=atish.patra@wdc.com \
--cc=greentime.hu@sifive.com \
--cc=guoren@kernel.org \
--cc=guoren@linux.alibaba.com \
--cc=hch@lst.de \
--cc=jason@lakedaemon.net \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=maz@kernel.org \
--cc=palmerdabbelt@google.com \
--cc=paul.walmsley@sifive.com \
--cc=tglx@linutronix.de \
--cc=wesley@sifive.com \
--cc=yash.shah@sifive.com \
--cc=zong.li@sifive.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).