[PATCH v2 2/2] irqchip/sifive-plic: Disable S-mode IRQs if running in M-mode

[Anup Patel]

On Tue, Mar 1, 2022 at 10:43 PM Niklas Cassel <Niklas.Cassel at wdc.com> wrote:
>
> From: Niklas Cassel <niklas.cassel at wdc.com>
>
> When detecting a context for a privilege mode different from the current
> running privilege mode, we simply skip to the next context register.
>
> This means that we never clear the S-mode enable bits when running in
> M-mode.
>
> On canaan k210, a bunch of S-mode interrupts are enabled by the bootrom.
> These S-mode specific interrupts should never trigger, since we never set
> the mie.SEIE bit in the parent interrupt controller (riscv-intc).
>
> However, we will be able to see the mip.SEIE bit set as pending.
>
> This isn't a good default when CONFIG_RISCV_M_MODE is set, since in that
> case we will never enter a lower privilege mode (e.g. S-mode).
>
> Let's clear the S-mode enable bits when running the kernel in M-mode, such
> that we won't have a interrupt pending bit set, which we will never clear.
>
> Signed-off-by: Niklas Cassel <niklas.cassel at wdc.com>

Looks good to me.

Reviewed-by: Anup Patel <anup at brainfault.org>

Regards,
Anup

> ---
>  drivers/irqchip/irq-sifive-plic.c | 26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> index fc9da94eb816..e6193d66c0ae 100644
> --- a/drivers/irqchip/irq-sifive-plic.c
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -81,17 +81,23 @@ static int plic_parent_irq __ro_after_init;
>  static bool plic_cpuhp_setup_done __ro_after_init;
>  static DEFINE_PER_CPU(struct plic_handler, plic_handlers);
>
> -static inline void plic_toggle(struct plic_handler *handler,
> -                               int hwirq, int enable)
> +static inline void __plic_toggle(void __iomem *enable_base,
> +                                int hwirq, int enable)
>  {
> -       u32 __iomem *reg = handler->enable_base + (hwirq / 32) * sizeof(u32);
> +       u32 __iomem *reg = enable_base + (hwirq / 32) * sizeof(u32);
>         u32 hwirq_mask = 1 << (hwirq % 32);
>
> -       raw_spin_lock(&handler->enable_lock);
>         if (enable)
>                 writel(readl(reg) | hwirq_mask, reg);
>         else
>                 writel(readl(reg) & ~hwirq_mask, reg);
> +}
> +
> +static inline void plic_toggle(struct plic_handler *handler,
> +                              int hwirq, int enable)
> +{
> +       raw_spin_lock(&handler->enable_lock);
> +       __plic_toggle(handler->enable_base, hwirq, enable);
>         raw_spin_unlock(&handler->enable_lock);
>  }
>
> @@ -324,8 +330,18 @@ static int __init plic_init(struct device_node *node,
>                  * Skip contexts other than external interrupts for our
>                  * privilege level.
>                  */
> -               if (parent.args[0] != RV_IRQ_EXT)
> +               if (parent.args[0] != RV_IRQ_EXT) {
> +                       /* Disable S-mode enable bits if running in M-mode. */
> +                       if (IS_ENABLED(CONFIG_RISCV_M_MODE)) {
> +                               void __iomem *enable_base = priv->regs +
> +                                       CONTEXT_ENABLE_BASE +
> +                                       i * CONTEXT_ENABLE_SIZE;
> +
> +                               for (hwirq = 1; hwirq <= nr_irqs; hwirq++)
> +                                       __plic_toggle(enable_base, hwirq, 0);
> +                       }
>                         continue;
> +               }
>
>                 hartid = riscv_of_parent_hartid(parent.np);
>                 if (hartid < 0) {
> --
> 2.35.1
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv