diff mbox series

[v8,10/11] irqchip/sifive-plic: Initialize the plic handler when cpu comes online

Message ID 20200212014822.28684-11-atish.patra@wdc.com (mailing list archive)
State New, archived
Headers show
Series Add support for SBI v0.2 and CPU hotplug | expand

Commit Message

Atish Patra Feb. 12, 2020, 1:48 a.m. UTC
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 <atish.patra@wdc.com>
---
 drivers/irqchip/irq-sifive-plic.c | 34 ++++++++++++++++++++++++-------
 include/linux/cpuhotplug.h        |  1 +
 2 files changed, 28 insertions(+), 7 deletions(-)

Comments

Anup Patel Feb. 12, 2020, 5:10 a.m. UTC | #1
On Wed, Feb 12, 2020 at 7:21 AM Atish Patra <atish.patra@wdc.com> 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 <atish.patra@wdc.com>
> ---
>  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
Thomas Gleixner Feb. 13, 2020, 11:01 a.m. UTC | #2
Atish Patra <atish.patra@wdc.com> 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
Atish Patra Feb. 13, 2020, 7:01 p.m. UTC | #3
On Thu, Feb 13, 2020 at 3:02 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Atish Patra <atish.patra@wdc.com> 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
>
diff mbox series

Patch

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,