Stable Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 1/3] irqchip/sifive-plic: Set default irq affinity in plic_irqdomain_map()
       [not found] <20200518091441.94843-1-anup.patel@wdc.com>
@ 2020-05-18  9:14 ` Anup Patel
  2020-05-21 22:06   ` Palmer Dabbelt
  2020-05-18  9:14 ` [PATCH v2 2/3] irqchip/sifive-plic: Setup cpuhp once after boot CPU handler is present Anup Patel
  1 sibling, 1 reply; 6+ messages in thread
From: Anup Patel @ 2020-05-18  9:14 UTC (permalink / raw)
  To: Palmer Dabbelt, Paul Walmsley, Thomas Gleixner, Jason Cooper,
	Marc Zyngier
  Cc: Atish Patra, Alistair Francis, Anup Patel, linux-riscv,
	linux-kernel, Anup Patel, stable

For multiple PLIC instances, each PLIC can only target a subset of
CPUs which is represented by "lmask" in the "struct plic_priv".

Currently, the default irq affinity for each PLIC interrupt is all
online CPUs which is illegal value for default irq affinity when we
have multiple PLIC instances. To fix this, we now set "lmask" as the
default irq affinity in for each interrupt in plic_irqdomain_map().

Fixes: f1ad1133b18f ("irqchip/sifive-plic: Add support for multiple PLICs")
Cc: stable@vger.kernel.org
Signed-off-by: Anup Patel <anup.patel@wdc.com>
---
 drivers/irqchip/irq-sifive-plic.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
index 822e074c0600..9f7f8ce88c00 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -176,9 +176,12 @@ static struct irq_chip plic_chip = {
 static int plic_irqdomain_map(struct irq_domain *d, unsigned int irq,
 			      irq_hw_number_t hwirq)
 {
+	struct plic_priv *priv = d->host_data;
+
 	irq_domain_set_info(d, irq, hwirq, &plic_chip, d->host_data,
 			    handle_fasteoi_irq, NULL, NULL);
 	irq_set_noprobe(irq);
+	irq_set_affinity(irq, &priv->lmask);
 	return 0;
 }
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 2/3] irqchip/sifive-plic: Setup cpuhp once after boot CPU handler is present
       [not found] <20200518091441.94843-1-anup.patel@wdc.com>
  2020-05-18  9:14 ` [PATCH v2 1/3] irqchip/sifive-plic: Set default irq affinity in plic_irqdomain_map() Anup Patel
@ 2020-05-18  9:14 ` Anup Patel
  2020-05-21 22:06   ` Palmer Dabbelt
  1 sibling, 1 reply; 6+ messages in thread
From: Anup Patel @ 2020-05-18  9:14 UTC (permalink / raw)
  To: Palmer Dabbelt, Paul Walmsley, Thomas Gleixner, Jason Cooper,
	Marc Zyngier
  Cc: Atish Patra, Alistair Francis, Anup Patel, linux-riscv,
	linux-kernel, Anup Patel, stable

For multiple PLIC instances, the plic_init() is called once for each
PLIC instance. Due to this we have two issues:
1. cpuhp_setup_state() is called multiple times
2. plic_starting_cpu() can crash for boot CPU if cpuhp_setup_state()
   is called before boot CPU PLIC handler is available.

This patch fixes both above issues.

Fixes: f1ad1133b18f ("irqchip/sifive-plic: Add support for multiple PLICs")
Cc: stable@vger.kernel.org
Signed-off-by: Anup Patel <anup.patel@wdc.com>
---
 drivers/irqchip/irq-sifive-plic.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
index 9f7f8ce88c00..6c54abf5cc5e 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -76,6 +76,7 @@ struct plic_handler {
 	void __iomem		*enable_base;
 	struct plic_priv	*priv;
 };
+static bool plic_cpuhp_setup_done;
 static DEFINE_PER_CPU(struct plic_handler, plic_handlers);
 
 static inline void plic_toggle(struct plic_handler *handler,
@@ -285,6 +286,7 @@ static int __init plic_init(struct device_node *node,
 	int error = 0, nr_contexts, nr_handlers = 0, i;
 	u32 nr_irqs;
 	struct plic_priv *priv;
+	struct plic_handler *handler;
 
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -313,7 +315,6 @@ 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;
 
@@ -367,9 +368,18 @@ static int __init plic_init(struct device_node *node,
 		nr_handlers++;
 	}
 
-	cpuhp_setup_state(CPUHP_AP_IRQ_SIFIVE_PLIC_STARTING,
+	/*
+	 * We can have multiple PLIC instances so setup cpuhp state only
+	 * when context handler for current/boot CPU is present.
+	 */
+	handler = this_cpu_ptr(&plic_handlers);
+	if (handler->present && !plic_cpuhp_setup_done) {
+		cpuhp_setup_state(CPUHP_AP_IRQ_SIFIVE_PLIC_STARTING,
 				  "irqchip/sifive/plic:starting",
 				  plic_starting_cpu, plic_dying_cpu);
+		plic_cpuhp_setup_done = true;
+	}
+
 	pr_info("mapped %d interrupts with %d handlers for %d contexts.\n",
 		nr_irqs, nr_handlers, nr_contexts);
 	set_handle_irq(plic_handle_irq);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/3] irqchip/sifive-plic: Set default irq affinity in plic_irqdomain_map()
  2020-05-18  9:14 ` [PATCH v2 1/3] irqchip/sifive-plic: Set default irq affinity in plic_irqdomain_map() Anup Patel
@ 2020-05-21 22:06   ` Palmer Dabbelt
  2020-05-22  6:44     ` Anup Patel
  0 siblings, 1 reply; 6+ messages in thread
From: Palmer Dabbelt @ 2020-05-21 22:06 UTC (permalink / raw)
  To: Anup Patel
  Cc: Paul Walmsley, tglx, jason, Marc Zyngier, Atish Patra,
	Alistair Francis, anup, linux-riscv, linux-kernel, Anup Patel,
	stable

On Mon, 18 May 2020 02:14:39 PDT (-0700), Anup Patel wrote:
> For multiple PLIC instances, each PLIC can only target a subset of
> CPUs which is represented by "lmask" in the "struct plic_priv".
>
> Currently, the default irq affinity for each PLIC interrupt is all
> online CPUs which is illegal value for default irq affinity when we
> have multiple PLIC instances. To fix this, we now set "lmask" as the
> default irq affinity in for each interrupt in plic_irqdomain_map().
>
> Fixes: f1ad1133b18f ("irqchip/sifive-plic: Add support for multiple PLICs")
> Cc: stable@vger.kernel.org
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> ---
>  drivers/irqchip/irq-sifive-plic.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> index 822e074c0600..9f7f8ce88c00 100644
> --- a/drivers/irqchip/irq-sifive-plic.c
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -176,9 +176,12 @@ static struct irq_chip plic_chip = {
>  static int plic_irqdomain_map(struct irq_domain *d, unsigned int irq,
>  			      irq_hw_number_t hwirq)
>  {
> +	struct plic_priv *priv = d->host_data;
> +
>  	irq_domain_set_info(d, irq, hwirq, &plic_chip, d->host_data,
>  			    handle_fasteoi_irq, NULL, NULL);

If you're going to re-spin this, d->host_data could be priv here.

>  	irq_set_noprobe(irq);
> +	irq_set_affinity(irq, &priv->lmask);
>  	return 0;
>  }

Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>
Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/3] irqchip/sifive-plic: Setup cpuhp once after boot CPU handler is present
  2020-05-18  9:14 ` [PATCH v2 2/3] irqchip/sifive-plic: Setup cpuhp once after boot CPU handler is present Anup Patel
@ 2020-05-21 22:06   ` Palmer Dabbelt
  2020-05-22  6:46     ` Anup Patel
  0 siblings, 1 reply; 6+ messages in thread
From: Palmer Dabbelt @ 2020-05-21 22:06 UTC (permalink / raw)
  To: Anup Patel
  Cc: Paul Walmsley, tglx, jason, Marc Zyngier, Atish Patra,
	Alistair Francis, anup, linux-riscv, linux-kernel, Anup Patel,
	stable

On Mon, 18 May 2020 02:14:40 PDT (-0700), Anup Patel wrote:
> For multiple PLIC instances, the plic_init() is called once for each
> PLIC instance. Due to this we have two issues:
> 1. cpuhp_setup_state() is called multiple times
> 2. plic_starting_cpu() can crash for boot CPU if cpuhp_setup_state()
>    is called before boot CPU PLIC handler is available.
>
> This patch fixes both above issues.
>
> Fixes: f1ad1133b18f ("irqchip/sifive-plic: Add support for multiple PLICs")
> Cc: stable@vger.kernel.org
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> ---
>  drivers/irqchip/irq-sifive-plic.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> index 9f7f8ce88c00..6c54abf5cc5e 100644
> --- a/drivers/irqchip/irq-sifive-plic.c
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -76,6 +76,7 @@ struct plic_handler {
>  	void __iomem		*enable_base;
>  	struct plic_priv	*priv;
>  };
> +static bool plic_cpuhp_setup_done;
>  static DEFINE_PER_CPU(struct plic_handler, plic_handlers);
>
>  static inline void plic_toggle(struct plic_handler *handler,
> @@ -285,6 +286,7 @@ static int __init plic_init(struct device_node *node,
>  	int error = 0, nr_contexts, nr_handlers = 0, i;
>  	u32 nr_irqs;
>  	struct plic_priv *priv;
> +	struct plic_handler *handler;
>
>  	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
>  	if (!priv)
> @@ -313,7 +315,6 @@ 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;
>
> @@ -367,9 +368,18 @@ static int __init plic_init(struct device_node *node,
>  		nr_handlers++;
>  	}
>
> -	cpuhp_setup_state(CPUHP_AP_IRQ_SIFIVE_PLIC_STARTING,
> +	/*
> +	 * We can have multiple PLIC instances so setup cpuhp state only
> +	 * when context handler for current/boot CPU is present.
> +	 */
> +	handler = this_cpu_ptr(&plic_handlers);
> +	if (handler->present && !plic_cpuhp_setup_done) {
> +		cpuhp_setup_state(CPUHP_AP_IRQ_SIFIVE_PLIC_STARTING,
>  				  "irqchip/sifive/plic:starting",
>  				  plic_starting_cpu, plic_dying_cpu);
> +		plic_cpuhp_setup_done = true;

So presumably something else is preventing multiple plic_init() calls from
executing at the same time?  Assuming that's the case

Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>
Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>

> +	}
> +
>  	pr_info("mapped %d interrupts with %d handlers for %d contexts.\n",
>  		nr_irqs, nr_handlers, nr_contexts);
>  	set_handle_irq(plic_handle_irq);

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/3] irqchip/sifive-plic: Set default irq affinity in plic_irqdomain_map()
  2020-05-21 22:06   ` Palmer Dabbelt
@ 2020-05-22  6:44     ` Anup Patel
  0 siblings, 0 replies; 6+ messages in thread
From: Anup Patel @ 2020-05-22  6:44 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Anup Patel, Paul Walmsley, Thomas Gleixner, Jason Cooper,
	Marc Zyngier, Atish Patra, Alistair Francis, linux-riscv,
	linux-kernel@vger.kernel.org List, stable

On Fri, May 22, 2020 at 3:36 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Mon, 18 May 2020 02:14:39 PDT (-0700), Anup Patel wrote:
> > For multiple PLIC instances, each PLIC can only target a subset of
> > CPUs which is represented by "lmask" in the "struct plic_priv".
> >
> > Currently, the default irq affinity for each PLIC interrupt is all
> > online CPUs which is illegal value for default irq affinity when we
> > have multiple PLIC instances. To fix this, we now set "lmask" as the
> > default irq affinity in for each interrupt in plic_irqdomain_map().
> >
> > Fixes: f1ad1133b18f ("irqchip/sifive-plic: Add support for multiple PLICs")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > ---
> >  drivers/irqchip/irq-sifive-plic.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> > index 822e074c0600..9f7f8ce88c00 100644
> > --- a/drivers/irqchip/irq-sifive-plic.c
> > +++ b/drivers/irqchip/irq-sifive-plic.c
> > @@ -176,9 +176,12 @@ static struct irq_chip plic_chip = {
> >  static int plic_irqdomain_map(struct irq_domain *d, unsigned int irq,
> >                             irq_hw_number_t hwirq)
> >  {
> > +     struct plic_priv *priv = d->host_data;
> > +
> >       irq_domain_set_info(d, irq, hwirq, &plic_chip, d->host_data,
> >                           handle_fasteoi_irq, NULL, NULL);
>
> If you're going to re-spin this, d->host_data could be priv here.

The controller's private data is named "host_data" for "struct irq_domain"
in Linux irq subsystem hence the usage.

>
> >       irq_set_noprobe(irq);
> > +     irq_set_affinity(irq, &priv->lmask);
> >       return 0;
> >  }
>
> Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>
> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>

Thanks,
Anup

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/3] irqchip/sifive-plic: Setup cpuhp once after boot CPU handler is present
  2020-05-21 22:06   ` Palmer Dabbelt
@ 2020-05-22  6:46     ` Anup Patel
  0 siblings, 0 replies; 6+ messages in thread
From: Anup Patel @ 2020-05-22  6:46 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Anup Patel, Paul Walmsley, Thomas Gleixner, Jason Cooper,
	Marc Zyngier, Atish Patra, Alistair Francis, linux-riscv,
	linux-kernel@vger.kernel.org List, stable

On Fri, May 22, 2020 at 3:36 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Mon, 18 May 2020 02:14:40 PDT (-0700), Anup Patel wrote:
> > For multiple PLIC instances, the plic_init() is called once for each
> > PLIC instance. Due to this we have two issues:
> > 1. cpuhp_setup_state() is called multiple times
> > 2. plic_starting_cpu() can crash for boot CPU if cpuhp_setup_state()
> >    is called before boot CPU PLIC handler is available.
> >
> > This patch fixes both above issues.
> >
> > Fixes: f1ad1133b18f ("irqchip/sifive-plic: Add support for multiple PLICs")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > ---
> >  drivers/irqchip/irq-sifive-plic.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> > index 9f7f8ce88c00..6c54abf5cc5e 100644
> > --- a/drivers/irqchip/irq-sifive-plic.c
> > +++ b/drivers/irqchip/irq-sifive-plic.c
> > @@ -76,6 +76,7 @@ struct plic_handler {
> >       void __iomem            *enable_base;
> >       struct plic_priv        *priv;
> >  };
> > +static bool plic_cpuhp_setup_done;
> >  static DEFINE_PER_CPU(struct plic_handler, plic_handlers);
> >
> >  static inline void plic_toggle(struct plic_handler *handler,
> > @@ -285,6 +286,7 @@ static int __init plic_init(struct device_node *node,
> >       int error = 0, nr_contexts, nr_handlers = 0, i;
> >       u32 nr_irqs;
> >       struct plic_priv *priv;
> > +     struct plic_handler *handler;
> >
> >       priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> >       if (!priv)
> > @@ -313,7 +315,6 @@ 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;
> >
> > @@ -367,9 +368,18 @@ static int __init plic_init(struct device_node *node,
> >               nr_handlers++;
> >       }
> >
> > -     cpuhp_setup_state(CPUHP_AP_IRQ_SIFIVE_PLIC_STARTING,
> > +     /*
> > +      * We can have multiple PLIC instances so setup cpuhp state only
> > +      * when context handler for current/boot CPU is present.
> > +      */
> > +     handler = this_cpu_ptr(&plic_handlers);
> > +     if (handler->present && !plic_cpuhp_setup_done) {
> > +             cpuhp_setup_state(CPUHP_AP_IRQ_SIFIVE_PLIC_STARTING,
> >                                 "irqchip/sifive/plic:starting",
> >                                 plic_starting_cpu, plic_dying_cpu);
> > +             plic_cpuhp_setup_done = true;
>
> So presumably something else is preventing multiple plic_init() calls from
> executing at the same time?  Assuming that's the case

AFAIK, interrupt controller and timer probing happens sequentially on
boot CPU before all secondary CPUs are brought-up.

>
> Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>
> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
>
> > +     }
> > +
> >       pr_info("mapped %d interrupts with %d handlers for %d contexts.\n",
> >               nr_irqs, nr_handlers, nr_contexts);
> >       set_handle_irq(plic_handle_irq);

Thanks,
Anup

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200518091441.94843-1-anup.patel@wdc.com>
2020-05-18  9:14 ` [PATCH v2 1/3] irqchip/sifive-plic: Set default irq affinity in plic_irqdomain_map() Anup Patel
2020-05-21 22:06   ` Palmer Dabbelt
2020-05-22  6:44     ` Anup Patel
2020-05-18  9:14 ` [PATCH v2 2/3] irqchip/sifive-plic: Setup cpuhp once after boot CPU handler is present Anup Patel
2020-05-21 22:06   ` Palmer Dabbelt
2020-05-22  6:46     ` Anup Patel

Stable Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://lore.kernel.org/stable \
		stable@vger.kernel.org
	public-inbox-index stable

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git