* [PATCH v2 0/2] sifive-plic minor improvements
@ 2022-03-01 17:13 Niklas Cassel
2022-03-01 17:13 ` [PATCH v2 1/2] irqchip/sifive-plic: Improve naming scheme for per context offsets Niklas Cassel
2022-03-01 17:13 ` [PATCH v2 2/2] irqchip/sifive-plic: Disable S-mode IRQs if running in M-mode Niklas Cassel
0 siblings, 2 replies; 6+ messages in thread
From: Niklas Cassel @ 2022-03-01 17:13 UTC (permalink / raw)
To: Thomas Gleixner, Marc Zyngier, Palmer Dabbelt, Paul Walmsley, Albert Ou
Cc: damien.lemoal, Niklas Cassel, linux-riscv
From: Niklas Cassel <niklas.cassel@wdc.com>
Hello there,
This small series has two small improvements for the sifive-plic
interrupt-controller driver.
Thankful for reviews.
Changes since v1:
-Addressed review comments by Anup.
Kind regards,
Niklas
Niklas Cassel (2):
irqchip/sifive-plic: Improve naming scheme for per context offsets
irqchip/sifive-plic: Disable S-mode IRQs if running in M-mode
drivers/irqchip/irq-sifive-plic.c | 40 +++++++++++++++++++++----------
1 file changed, 28 insertions(+), 12 deletions(-)
--
2.35.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] irqchip/sifive-plic: Improve naming scheme for per context offsets
2022-03-01 17:13 [PATCH v2 0/2] sifive-plic minor improvements Niklas Cassel
@ 2022-03-01 17:13 ` Niklas Cassel
2022-03-01 17:28 ` Anup Patel
2022-03-01 17:13 ` [PATCH v2 2/2] irqchip/sifive-plic: Disable S-mode IRQs if running in M-mode Niklas Cassel
1 sibling, 1 reply; 6+ messages in thread
From: Niklas Cassel @ 2022-03-01 17:13 UTC (permalink / raw)
To: Thomas Gleixner, Marc Zyngier, Palmer Dabbelt, Paul Walmsley
Cc: damien.lemoal, Niklas Cassel, linux-riscv
From: Niklas Cassel <niklas.cassel@wdc.com>
The PLIC supports a fixed number of contexts (15872).
Each context has fixed register offsets in PLIC.
The number of contexts that we need to initialize depends on the privilege
modes supported by each hart. Therefore, this mapping between PLIC context
registers to hart privilege modes is platform specific, and is currently
supplied via device tree.
For example, canaan,k210 has the following mapping:
Context0: hart0 M-mode
Context1: hart0 S-mode
Context2: hart1 M-mode
Context3: hart1 S-mode
While sifive,fu540 has the following mapping:
Context0: hart0 M-mode
Context1: hart1 M-mode
Context2: hart1 S-mode
Because the number of contexts per hart is not fixed, the names
ENABLE_PER_HART and CONTEXT_PER_HART for the register offsets are quite
confusing and might mislead the reader to think that these are fixed
register offsets per hart.
Rename the offsets to more clearly highlight that these are per PLIC
context and not per hart.
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
drivers/irqchip/irq-sifive-plic.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
index 09cc98266d30..fc9da94eb816 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -44,8 +44,8 @@
* Each hart context has a vector of interrupt enable bits associated with it.
* There's one bit for each interrupt source.
*/
-#define ENABLE_BASE 0x2000
-#define ENABLE_PER_HART 0x80
+#define CONTEXT_ENABLE_BASE 0x2000
+#define CONTEXT_ENABLE_SIZE 0x80
/*
* Each hart context has a set of control registers associated with it. Right
@@ -53,7 +53,7 @@
* take an interrupt, and a register to claim interrupts.
*/
#define CONTEXT_BASE 0x200000
-#define CONTEXT_PER_HART 0x1000
+#define CONTEXT_SIZE 0x1000
#define CONTEXT_THRESHOLD 0x00
#define CONTEXT_CLAIM 0x04
@@ -361,11 +361,11 @@ static int __init plic_init(struct device_node *node,
cpumask_set_cpu(cpu, &priv->lmask);
handler->present = true;
- handler->hart_base =
- priv->regs + CONTEXT_BASE + i * CONTEXT_PER_HART;
+ handler->hart_base = priv->regs + CONTEXT_BASE +
+ i * CONTEXT_SIZE;
raw_spin_lock_init(&handler->enable_lock);
- handler->enable_base =
- priv->regs + ENABLE_BASE + i * ENABLE_PER_HART;
+ handler->enable_base = priv->regs + CONTEXT_ENABLE_BASE +
+ i * CONTEXT_ENABLE_SIZE;
handler->priv = priv;
done:
for (hwirq = 1; hwirq <= nr_irqs; hwirq++)
--
2.35.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] irqchip/sifive-plic: Disable S-mode IRQs if running in M-mode
2022-03-01 17:13 [PATCH v2 0/2] sifive-plic minor improvements Niklas Cassel
2022-03-01 17:13 ` [PATCH v2 1/2] irqchip/sifive-plic: Improve naming scheme for per context offsets Niklas Cassel
@ 2022-03-01 17:13 ` Niklas Cassel
2022-03-01 17:30 ` Anup Patel
2022-03-02 13:01 ` Marc Zyngier
1 sibling, 2 replies; 6+ messages in thread
From: Niklas Cassel @ 2022-03-01 17:13 UTC (permalink / raw)
To: Thomas Gleixner, Marc Zyngier, Palmer Dabbelt, Paul Walmsley, Albert Ou
Cc: damien.lemoal, Niklas Cassel, linux-riscv
From: Niklas Cassel <niklas.cassel@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@wdc.com>
---
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@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] irqchip/sifive-plic: Improve naming scheme for per context offsets
2022-03-01 17:13 ` [PATCH v2 1/2] irqchip/sifive-plic: Improve naming scheme for per context offsets Niklas Cassel
@ 2022-03-01 17:28 ` Anup Patel
0 siblings, 0 replies; 6+ messages in thread
From: Anup Patel @ 2022-03-01 17:28 UTC (permalink / raw)
To: Niklas Cassel
Cc: Thomas Gleixner, Marc Zyngier, Palmer Dabbelt, Paul Walmsley,
damien.lemoal, linux-riscv
On Tue, Mar 1, 2022 at 10:43 PM Niklas Cassel <Niklas.Cassel@wdc.com> wrote:
>
> From: Niklas Cassel <niklas.cassel@wdc.com>
>
> The PLIC supports a fixed number of contexts (15872).
> Each context has fixed register offsets in PLIC.
>
> The number of contexts that we need to initialize depends on the privilege
> modes supported by each hart. Therefore, this mapping between PLIC context
> registers to hart privilege modes is platform specific, and is currently
> supplied via device tree.
>
> For example, canaan,k210 has the following mapping:
> Context0: hart0 M-mode
> Context1: hart0 S-mode
> Context2: hart1 M-mode
> Context3: hart1 S-mode
>
> While sifive,fu540 has the following mapping:
> Context0: hart0 M-mode
> Context1: hart1 M-mode
> Context2: hart1 S-mode
>
> Because the number of contexts per hart is not fixed, the names
> ENABLE_PER_HART and CONTEXT_PER_HART for the register offsets are quite
> confusing and might mislead the reader to think that these are fixed
> register offsets per hart.
>
> Rename the offsets to more clearly highlight that these are per PLIC
> context and not per hart.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> drivers/irqchip/irq-sifive-plic.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> index 09cc98266d30..fc9da94eb816 100644
> --- a/drivers/irqchip/irq-sifive-plic.c
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -44,8 +44,8 @@
> * Each hart context has a vector of interrupt enable bits associated with it.
> * There's one bit for each interrupt source.
> */
> -#define ENABLE_BASE 0x2000
> -#define ENABLE_PER_HART 0x80
> +#define CONTEXT_ENABLE_BASE 0x2000
> +#define CONTEXT_ENABLE_SIZE 0x80
>
> /*
> * Each hart context has a set of control registers associated with it. Right
> @@ -53,7 +53,7 @@
> * take an interrupt, and a register to claim interrupts.
> */
> #define CONTEXT_BASE 0x200000
> -#define CONTEXT_PER_HART 0x1000
> +#define CONTEXT_SIZE 0x1000
> #define CONTEXT_THRESHOLD 0x00
> #define CONTEXT_CLAIM 0x04
>
> @@ -361,11 +361,11 @@ static int __init plic_init(struct device_node *node,
>
> cpumask_set_cpu(cpu, &priv->lmask);
> handler->present = true;
> - handler->hart_base =
> - priv->regs + CONTEXT_BASE + i * CONTEXT_PER_HART;
> + handler->hart_base = priv->regs + CONTEXT_BASE +
> + i * CONTEXT_SIZE;
> raw_spin_lock_init(&handler->enable_lock);
> - handler->enable_base =
> - priv->regs + ENABLE_BASE + i * ENABLE_PER_HART;
> + handler->enable_base = priv->regs + CONTEXT_ENABLE_BASE +
> + i * CONTEXT_ENABLE_SIZE;
> handler->priv = priv;
> done:
> for (hwirq = 1; hwirq <= nr_irqs; hwirq++)
> --
> 2.35.1
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] irqchip/sifive-plic: Disable S-mode IRQs if running in M-mode
2022-03-01 17:13 ` [PATCH v2 2/2] irqchip/sifive-plic: Disable S-mode IRQs if running in M-mode Niklas Cassel
@ 2022-03-01 17:30 ` Anup Patel
2022-03-02 13:01 ` Marc Zyngier
1 sibling, 0 replies; 6+ messages in thread
From: Anup Patel @ 2022-03-01 17:30 UTC (permalink / raw)
To: Niklas Cassel
Cc: Thomas Gleixner, Marc Zyngier, Palmer Dabbelt, Paul Walmsley,
Albert Ou, damien.lemoal, linux-riscv
On Tue, Mar 1, 2022 at 10:43 PM Niklas Cassel <Niklas.Cassel@wdc.com> wrote:
>
> From: Niklas Cassel <niklas.cassel@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@wdc.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] irqchip/sifive-plic: Disable S-mode IRQs if running in M-mode
2022-03-01 17:13 ` [PATCH v2 2/2] irqchip/sifive-plic: Disable S-mode IRQs if running in M-mode Niklas Cassel
2022-03-01 17:30 ` Anup Patel
@ 2022-03-02 13:01 ` Marc Zyngier
1 sibling, 0 replies; 6+ messages in thread
From: Marc Zyngier @ 2022-03-02 13:01 UTC (permalink / raw)
To: Niklas Cassel
Cc: Thomas Gleixner, Palmer Dabbelt, Paul Walmsley, Albert Ou,
damien.lemoal, linux-riscv
On 2022-03-01 17:13, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@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@wdc.com>
> ---
> 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)
While you're at it, please drop the inline attributes. They really
serve no purpose, as that's the job of the compiler.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-03-02 13:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-01 17:13 [PATCH v2 0/2] sifive-plic minor improvements Niklas Cassel
2022-03-01 17:13 ` [PATCH v2 1/2] irqchip/sifive-plic: Improve naming scheme for per context offsets Niklas Cassel
2022-03-01 17:28 ` Anup Patel
2022-03-01 17:13 ` [PATCH v2 2/2] irqchip/sifive-plic: Disable S-mode IRQs if running in M-mode Niklas Cassel
2022-03-01 17:30 ` Anup Patel
2022-03-02 13:01 ` Marc Zyngier
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.