linux-kernel - Re: [PATCH V2 2/2] irqchip/sifive-plic: Add thead,c9xx-plic support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAAhSdy32wkwH5k3iwdUNDsXjUNX8icQwcz_h2E6UixH7ZmD5KQ@mail.gmail.com>
Date:   Tue, 12 Oct 2021 22:10:26 +0530
From:   Anup Patel <anup@...infault.org>
To:     Guo Ren <guoren@...nel.org>
Cc:     Atish Patra <atish.patra@....com>, Marc Zyngier <maz@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Palmer Dabbelt <palmer@...belt.com>,
        "linux-kernel@...r.kernel.org List" <linux-kernel@...r.kernel.org>,
        linux-riscv <linux-riscv@...ts.infradead.org>,
        Guo Ren <guoren@...ux.alibaba.com>
Subject: Re: [PATCH V2 2/2] irqchip/sifive-plic: Add thead,c9xx-plic support

On Tue, Oct 12, 2021 at 9:04 PM <guoren@...nel.org> wrote:
>
> From: Guo Ren <guoren@...ux.alibaba.com>
>
> thead,c9xx-plic would mask IRQ with readl(claim), so it needn't
> mask/unmask which needed in RISC-V PLIC.
>
> When in IRQS_ONESHOT & IRQCHIP_EOI_THREADED path, unnecessary mask
> operation would cause a blocking irq bug in thead,c9xx-plic. Because
> when IRQ is disabled in c9xx, writel(hwirq, claim) would be invalid.
>
> Signed-off-by: Guo Ren <guoren@...ux.alibaba.com>
> Cc: Anup Patel <anup@...infault.org>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Marc Zyngier <maz@...nel.org>
> Cc: Palmer Dabbelt <palmer@...belt.com>
> Cc: Atish Patra <atish.patra@....com>
>
> ---
>
> Changes since V2:
>  - Add a separate compatible string "thead,c9xx-plic"
>  - set irq_mask/unmask of "plic_chip" to NULL and point
>    irq_enable/disable of "plic_chip" to plic_irq_mask/unmask
>  - Add a detailed comment block in plic_init() about the
>    differences in Claim/Completion process of RISC-V PLIC and C9xx
>    PLIC.
> ---
>  drivers/irqchip/irq-sifive-plic.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> index cf74cfa82045..3756b1c147c3 100644
> --- a/drivers/irqchip/irq-sifive-plic.c
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -79,6 +79,7 @@ struct plic_handler {
>  };
>  static int plic_parent_irq __ro_after_init;
>  static bool plic_cpuhp_setup_done __ro_after_init;
> +static bool disable_mask_unmask __ro_after_init;
>  static DEFINE_PER_CPU(struct plic_handler, plic_handlers);
>
>  static inline void plic_toggle(struct plic_handler *handler,
> @@ -181,6 +182,13 @@ static int plic_irqdomain_map(struct irq_domain *d, unsigned int irq,
>  {
>         struct plic_priv *priv = d->host_data;
>
> +       if (disable_mask_unmask) {
> +               plic_chip.irq_mask      = NULL;
> +               plic_chip.irq_unmask    = NULL;
> +               plic_chip.irq_enable    = plic_irq_unmask;
> +               plic_chip.irq_disable   = plic_irq_mask;
> +       }
> +
>         irq_domain_set_info(d, irq, hwirq, &plic_chip, d->host_data,
>                             handle_fasteoi_irq, NULL, NULL);
>         irq_set_noprobe(irq);
> @@ -390,5 +398,14 @@ static int __init plic_init(struct device_node *node,
>         return error;
>  }
>
> +static int __init thead_c9xx_plic_init(struct device_node *node,
> +               struct device_node *parent)
> +{
> +       disable_mask_unmask = true;

The plic_irqdomain_map() is called for each irq so "plic_chip"
will be updated multiple times.

You can drop the disable_mask_unmask variable and instead
directly update "plic_chip" here.

> +
> +       return plic_init(node, parent);
> +}
> +
>  IRQCHIP_DECLARE(sifive_plic, "sifive,plic-1.0.0", plic_init);
>  IRQCHIP_DECLARE(riscv_plic0, "riscv,plic0", plic_init); /* for legacy systems */
> +IRQCHIP_DECLARE(thead_c9xx_plic, "thead,c9xx-plic", thead_c9xx_plic_init);
> --
> 2.25.1
>

Regards,
Anup