Currently, plic threshold and priority are only initialized once in the
beginning. However, threshold can be set to disabled if cpu is marked
offline with cpu hotplug feature. This will not allow to change the
irq affinity to a cpu that just came online.
Add plic specific cpu hotplug callback and initialize the per cpu handler
when cpu comes online.
Signed-off-by: Atish Patra <[email protected]>
---
drivers/irqchip/irq-sifive-plic.c | 34 ++++++++++++++++++++++++-------
include/linux/cpuhotplug.h | 1 +
2 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
index 0aca5807a119..9b564b19f4bf 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -4,6 +4,7 @@
* Copyright (C) 2018 Christoph Hellwig
*/
#define pr_fmt(fmt) "plic: " fmt
+#include <linux/cpu.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/irq.h>
@@ -55,6 +56,8 @@
#define CONTEXT_THRESHOLD 0x00
#define CONTEXT_CLAIM 0x04
+#define PLIC_DISABLE_THRESHOLD 0xffffffff
+
static void __iomem *plic_regs;
struct plic_handler {
@@ -208,6 +211,26 @@ static int plic_find_hart_id(struct device_node *node)
return -1;
}
+static void plic_handler_init(struct plic_handler *handler, u32 threshold)
+{
+ irq_hw_number_t hwirq;
+
+ /* priority must be > threshold to trigger an interrupt */
+ writel(threshold, handler->hart_base + CONTEXT_THRESHOLD);
+ for (hwirq = 1; hwirq < plic_irqdomain->hwirq_max; hwirq++)
+ plic_toggle(handler, hwirq, 0);
+}
+
+static int plic_starting_cpu(unsigned int cpu)
+{
+ u32 threshold = 0;
+ struct plic_handler *handler = per_cpu_ptr(&plic_handlers, cpu);
+
+ plic_handler_init(handler, threshold);
+
+ return 0;
+}
+
static int __init plic_init(struct device_node *node,
struct device_node *parent)
{
@@ -243,9 +266,7 @@ static int __init plic_init(struct device_node *node,
for (i = 0; i < nr_contexts; i++) {
struct of_phandle_args parent;
struct plic_handler *handler;
- irq_hw_number_t hwirq;
int cpu, hartid;
- u32 threshold = 0;
if (of_irq_parse_one(node, i, &parent)) {
pr_err("failed to parse parent for context %d.\n", i);
@@ -279,7 +300,7 @@ static int __init plic_init(struct device_node *node,
handler = per_cpu_ptr(&plic_handlers, cpu);
if (handler->present) {
pr_warn("handler already present for context %d.\n", i);
- threshold = 0xffffffff;
+ plic_handler_init(handler, PLIC_DISABLE_THRESHOLD);
goto done;
}
@@ -291,13 +312,12 @@ static int __init plic_init(struct device_node *node,
plic_regs + ENABLE_BASE + i * ENABLE_PER_HART;
done:
- /* priority must be > threshold to trigger an interrupt */
- writel(threshold, handler->hart_base + CONTEXT_THRESHOLD);
- for (hwirq = 1; hwirq <= nr_irqs; hwirq++)
- plic_toggle(handler, hwirq, 0);
nr_handlers++;
}
+ cpuhp_setup_state(CPUHP_AP_IRQ_SIFIVE_PLIC_STARTING,
+ "irqchip/sifive/plic:starting",
+ plic_starting_cpu, NULL);
pr_info("mapped %d interrupts with %d handlers for %d contexts.\n",
nr_irqs, nr_handlers, nr_contexts);
set_handle_irq(plic_handle_irq);
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index e51ee772b9f5..5360e03db08c 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -100,6 +100,7 @@ enum cpuhp_state {
CPUHP_AP_IRQ_ARMADA_XP_STARTING,
CPUHP_AP_IRQ_BCM2836_STARTING,
CPUHP_AP_IRQ_MIPS_GIC_STARTING,
+ CPUHP_AP_IRQ_SIFIVE_PLIC_STARTING,
CPUHP_AP_ARM_MVEBU_COHERENCY,
CPUHP_AP_MICROCODE_LOADER,
CPUHP_AP_PERF_X86_AMD_UNCORE_STARTING,
--
2.24.0
On Wed, Feb 12, 2020 at 7:21 AM Atish Patra <[email protected]> wrote:
>
> Currently, plic threshold and priority are only initialized once in the
> beginning. However, threshold can be set to disabled if cpu is marked
> offline with cpu hotplug feature. This will not allow to change the
> irq affinity to a cpu that just came online.
>
> Add plic specific cpu hotplug callback and initialize the per cpu handler
> when cpu comes online.
>
> Signed-off-by: Atish Patra <[email protected]>
> ---
> drivers/irqchip/irq-sifive-plic.c | 34 ++++++++++++++++++++++++-------
> include/linux/cpuhotplug.h | 1 +
> 2 files changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> index 0aca5807a119..9b564b19f4bf 100644
> --- a/drivers/irqchip/irq-sifive-plic.c
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -4,6 +4,7 @@
> * Copyright (C) 2018 Christoph Hellwig
> */
> #define pr_fmt(fmt) "plic: " fmt
> +#include <linux/cpu.h>
> #include <linux/interrupt.h>
> #include <linux/io.h>
> #include <linux/irq.h>
> @@ -55,6 +56,8 @@
> #define CONTEXT_THRESHOLD 0x00
> #define CONTEXT_CLAIM 0x04
>
> +#define PLIC_DISABLE_THRESHOLD 0xffffffff
> +
> static void __iomem *plic_regs;
>
> struct plic_handler {
> @@ -208,6 +211,26 @@ static int plic_find_hart_id(struct device_node *node)
> return -1;
> }
>
> +static void plic_handler_init(struct plic_handler *handler, u32 threshold)
> +{
> + irq_hw_number_t hwirq;
> +
> + /* priority must be > threshold to trigger an interrupt */
> + writel(threshold, handler->hart_base + CONTEXT_THRESHOLD);
> + for (hwirq = 1; hwirq < plic_irqdomain->hwirq_max; hwirq++)
> + plic_toggle(handler, hwirq, 0);
Ensuring that all IRQs are disabled is only required at boot-time. For run-time,
CPU hotplug, I am sure Linux irq subsystem will migrate-and-disable IRQs
routed to given CPU when the CPU does down.
Further, we should also ensure that all IRQs are disabled for PLIC contexts
not used by S-mode Linux kernel.
Based on the above rationale, the loop to disable all IRQs should still be
part of plic_init().
> +}
> +
> +static int plic_starting_cpu(unsigned int cpu)
> +{
> + u32 threshold = 0;
> + struct plic_handler *handler = per_cpu_ptr(&plic_handlers, cpu);
> +
> + plic_handler_init(handler, threshold);
> +
> + return 0;
> +}
> +
Addition to PLIC context threshold programming, the plic_starting_cpu()
should also set IE_EIE bit in CSR_IE instead of doing it in trap_init()
function of arch/riscv/kernel.trap.c
You can also define plic_stoping_cpu() which does the reverse of what
plic_starting_cpu() is doing.
> static int __init plic_init(struct device_node *node,
> struct device_node *parent)
> {
> @@ -243,9 +266,7 @@ static int __init plic_init(struct device_node *node,
> for (i = 0; i < nr_contexts; i++) {
> struct of_phandle_args parent;
> struct plic_handler *handler;
> - irq_hw_number_t hwirq;
> int cpu, hartid;
> - u32 threshold = 0;
>
> if (of_irq_parse_one(node, i, &parent)) {
> pr_err("failed to parse parent for context %d.\n", i);
> @@ -279,7 +300,7 @@ static int __init plic_init(struct device_node *node,
> handler = per_cpu_ptr(&plic_handlers, cpu);
> if (handler->present) {
> pr_warn("handler already present for context %d.\n", i);
> - threshold = 0xffffffff;
> + plic_handler_init(handler, PLIC_DISABLE_THRESHOLD);
> goto done;
> }
>
> @@ -291,13 +312,12 @@ static int __init plic_init(struct device_node *node,
> plic_regs + ENABLE_BASE + i * ENABLE_PER_HART;
>
> done:
> - /* priority must be > threshold to trigger an interrupt */
> - writel(threshold, handler->hart_base + CONTEXT_THRESHOLD);
> - for (hwirq = 1; hwirq <= nr_irqs; hwirq++)
> - plic_toggle(handler, hwirq, 0);
> nr_handlers++;
> }
>
> + cpuhp_setup_state(CPUHP_AP_IRQ_SIFIVE_PLIC_STARTING,
> + "irqchip/sifive/plic:starting",
> + plic_starting_cpu, NULL);
> pr_info("mapped %d interrupts with %d handlers for %d contexts.\n",
> nr_irqs, nr_handlers, nr_contexts);
> set_handle_irq(plic_handle_irq);
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index e51ee772b9f5..5360e03db08c 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -100,6 +100,7 @@ enum cpuhp_state {
> CPUHP_AP_IRQ_ARMADA_XP_STARTING,
> CPUHP_AP_IRQ_BCM2836_STARTING,
> CPUHP_AP_IRQ_MIPS_GIC_STARTING,
> + CPUHP_AP_IRQ_SIFIVE_PLIC_STARTING,
> CPUHP_AP_ARM_MVEBU_COHERENCY,
> CPUHP_AP_MICROCODE_LOADER,
> CPUHP_AP_PERF_X86_AMD_UNCORE_STARTING,
> --
> 2.24.0
>
Regards,
Anup
Atish Patra <[email protected]> writes:
> +static void plic_handler_init(struct plic_handler *handler, u32 threshold)
> +{
> + irq_hw_number_t hwirq;
> +
> + /* priority must be > threshold to trigger an interrupt */
> + writel(threshold, handler->hart_base + CONTEXT_THRESHOLD);
> + for (hwirq = 1; hwirq < plic_irqdomain->hwirq_max; hwirq++)
> + plic_toggle(handler, hwirq, 0);
> +}
> +
> +static int plic_starting_cpu(unsigned int cpu)
> +{
> + u32 threshold = 0;
Pointless variable. Also you use PLIC_DISABLE_THRESHOLD down below, so
please add a proper define for enable as well.
> + struct plic_handler *handler = per_cpu_ptr(&plic_handlers, cpu);
this_cpu_ptr*&...)
The callback is guaranteed to run on the plugged in CPU.
> - threshold = 0xffffffff;
> + plic_handler_init(handler, PLIC_DISABLE_THRESHOLD);
Thanks,
tglx
On Thu, Feb 13, 2020 at 3:02 AM Thomas Gleixner <[email protected]> wrote:
>
> Atish Patra <[email protected]> writes:
>
> > +static void plic_handler_init(struct plic_handler *handler, u32 threshold)
> > +{
> > + irq_hw_number_t hwirq;
> > +
> > + /* priority must be > threshold to trigger an interrupt */
> > + writel(threshold, handler->hart_base + CONTEXT_THRESHOLD);
> > + for (hwirq = 1; hwirq < plic_irqdomain->hwirq_max; hwirq++)
> > + plic_toggle(handler, hwirq, 0);
> > +}
>
> > +
> > +static int plic_starting_cpu(unsigned int cpu)
> > +{
> > + u32 threshold = 0;
>
> Pointless variable. Also you use PLIC_DISABLE_THRESHOLD down below, so
> please add a proper define for enable as well.
>
Sure. Will do that.
> > + struct plic_handler *handler = per_cpu_ptr(&plic_handlers, cpu);
>
> this_cpu_ptr*&...)
>
> The callback is guaranteed to run on the plugged in CPU.
>
Ah yes. I will change it to this_cpu_ptr. Thanks.
> > - threshold = 0xffffffff;
> > + plic_handler_init(handler, PLIC_DISABLE_THRESHOLD);
>
> Thanks,
>
> tglx
>
--
Regards,
Atish