All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Lucas Zampieri <lzampier@redhat.com>, linux-kernel@vger.kernel.org
Cc: Lucas Zampieri <lzampier@redhat.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Samuel Holland <samuel.holland@sifive.com>,
	stable@vger.kernel.org, linux-riscv@lists.infradead.org,
	Jia Wang <wangjia@ultrarisc.com>
Subject: Re: [PATCH] irqchip/sifive-plic: avoid interrupt ID 0 handling during suspend/resume
Date: Sun, 21 Sep 2025 10:27:59 +0200	[thread overview]
Message-ID: <87ikhc5hww.ffs@tglx> (raw)
In-Reply-To: <20250915162847.103445-1-lzampier@redhat.com>

On Mon, Sep 15 2025 at 17:28, Lucas Zampieri wrote:

> To: linux-kernel@vger.kernel.org
> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> Cc: Samuel Holland <samuel.holland@sifive.com>
> Cc: stable@vger.kernel.org
> Cc: linux-riscv@lists.infradead.org
> Cc: Thomas Gleixner <tglx@linutronix.de>

How is this Cc list relevant in explaining the changes here?

> According to the PLIC specification[1], global interrupt sources are
> assigned small unsigned integer identifiers beginning at the value 1.
> An interrupt ID of 0 is reserved to mean "no interrupt".
>
> The current plic_irq_resume() and plic_irq_suspend() functions incorrectly
> starts the loop from index 0, which could access the reserved interrupt ID
> 0 register space.
> This fix changes the loop to start from index 1, skipping the reserved
> interrupt ID 0 as per the PLIC specification.

s/This fix changes/Change/

And please separate this from the explanation above.

https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#patch-submission-notes

> This prevents potential undefined behavior when accessing the reserved
> register space during suspend/resume cycles.
>
> Fixes: e80f0b6a2cf3 ("irqchip/irq-sifive-plic: Add syscore callbacks for hibernation")
> Co-developed-by: Jia Wang <wangjia@ultrarisc.com>
> Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
> Signed-off-by: Lucas Zampieri <lzampier@redhat.com>
>
> [1] https://github.com/riscv/riscv-plic-spec/releases/tag/1.0.0

Link: .....

This [1] stuff is just annoying.

> ---
>  drivers/irqchip/irq-sifive-plic.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> index bf69a4802b71..1c2b4d2575ac 100644
> --- a/drivers/irqchip/irq-sifive-plic.c
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -252,7 +252,7 @@ static int plic_irq_suspend(void)
>  
>  	priv = per_cpu_ptr(&plic_handlers, smp_processor_id())->priv;
>  
> -	for (i = 0; i < priv->nr_irqs; i++) {
> +	for (i = 1; i < priv->nr_irqs; i++) {

This lacks a comment explaining this non-obvious 'i = 1'.

Thanks,

        tglx

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

WARNING: multiple messages have this Message-ID (diff)
From: Thomas Gleixner <tglx@linutronix.de>
To: Lucas Zampieri <lzampier@redhat.com>, linux-kernel@vger.kernel.org
Cc: Lucas Zampieri <lzampier@redhat.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Samuel Holland <samuel.holland@sifive.com>,
	stable@vger.kernel.org, linux-riscv@lists.infradead.org,
	Jia Wang <wangjia@ultrarisc.com>
Subject: Re: [PATCH] irqchip/sifive-plic: avoid interrupt ID 0 handling during suspend/resume
Date: Sun, 21 Sep 2025 10:27:59 +0200	[thread overview]
Message-ID: <87ikhc5hww.ffs@tglx> (raw)
In-Reply-To: <20250915162847.103445-1-lzampier@redhat.com>

On Mon, Sep 15 2025 at 17:28, Lucas Zampieri wrote:

> To: linux-kernel@vger.kernel.org
> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> Cc: Samuel Holland <samuel.holland@sifive.com>
> Cc: stable@vger.kernel.org
> Cc: linux-riscv@lists.infradead.org
> Cc: Thomas Gleixner <tglx@linutronix.de>

How is this Cc list relevant in explaining the changes here?

> According to the PLIC specification[1], global interrupt sources are
> assigned small unsigned integer identifiers beginning at the value 1.
> An interrupt ID of 0 is reserved to mean "no interrupt".
>
> The current plic_irq_resume() and plic_irq_suspend() functions incorrectly
> starts the loop from index 0, which could access the reserved interrupt ID
> 0 register space.
> This fix changes the loop to start from index 1, skipping the reserved
> interrupt ID 0 as per the PLIC specification.

s/This fix changes/Change/

And please separate this from the explanation above.

https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#patch-submission-notes

> This prevents potential undefined behavior when accessing the reserved
> register space during suspend/resume cycles.
>
> Fixes: e80f0b6a2cf3 ("irqchip/irq-sifive-plic: Add syscore callbacks for hibernation")
> Co-developed-by: Jia Wang <wangjia@ultrarisc.com>
> Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
> Signed-off-by: Lucas Zampieri <lzampier@redhat.com>
>
> [1] https://github.com/riscv/riscv-plic-spec/releases/tag/1.0.0

Link: .....

This [1] stuff is just annoying.

> ---
>  drivers/irqchip/irq-sifive-plic.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> index bf69a4802b71..1c2b4d2575ac 100644
> --- a/drivers/irqchip/irq-sifive-plic.c
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -252,7 +252,7 @@ static int plic_irq_suspend(void)
>  
>  	priv = per_cpu_ptr(&plic_handlers, smp_processor_id())->priv;
>  
> -	for (i = 0; i < priv->nr_irqs; i++) {
> +	for (i = 1; i < priv->nr_irqs; i++) {

This lacks a comment explaining this non-obvious 'i = 1'.

Thanks,

        tglx

  reply	other threads:[~2025-09-21  8:28 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-15 16:28 [PATCH] irqchip/sifive-plic: avoid interrupt ID 0 handling during suspend/resume Lucas Zampieri
2025-09-15 16:28 ` Lucas Zampieri
2025-09-21  8:27 ` Thomas Gleixner [this message]
2025-09-21  8:27   ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ikhc5hww.ffs@tglx \
    --to=tglx@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=lzampier@redhat.com \
    --cc=paul.walmsley@sifive.com \
    --cc=samuel.holland@sifive.com \
    --cc=stable@vger.kernel.org \
    --cc=wangjia@ultrarisc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.