From: Palmer Dabbelt <palmer@dabbelt.com>
To: Marc Zyngier <maz@kernel.org>
Cc: samuel@sholland.org, linux-kernel@vger.kernel.org,
Paul Walmsley <paul.walmsley@sifive.com>,
tglx@linutronix.de, linux-riscv@lists.infradead.org
Subject: Re: [PATCH] irqchip/sifive-plic: Avoid clearing the per-hart enable bits
Date: Wed, 20 Sep 2023 03:18:07 -0700 (PDT) [thread overview]
Message-ID: <mhng-3622fd7f-23c4-4709-9082-0a62d49762ce@palmer-ri-x1c9a> (raw)
In-Reply-To: <86zg3ttvsi.wl-maz@kernel.org>
On Tue, 18 Jul 2023 02:26:37 PDT (-0700), Marc Zyngier wrote:
> On Mon, 17 Jul 2023 19:58:40 +0100,
> Samuel Holland <samuel.holland@sifive.com> wrote:
>>
>> Writes to the PLIC completion register are ignored if the enable bit for
>> that (interrupt, hart) combination is cleared. This leaves the interrupt
>> in a claimed state, preventing it from being triggered again.
>>
>> Originally, the enable bit was cleared in the .irq_mask operation, and
>> commit 69ea463021be ("irqchip/sifive-plic: Fixup EOI failed when masked")
>> added a workaround for this issue. Later, commit a1706a1c5062
>> ("irqchip/sifive-plic: Separate the enable and mask operations") moved
>> toggling the enable bit to the .irq_enable/.irq_disable operations and
>> removed the workaround.
>>
>> However, there are still places where .irq_disable can be called from
>> inside the hard IRQ handler, for example in irq_pm_check_wakeup(). As a
>> result, this issue causes an interrupt to get stuck in a claimed state
>> after being used to wake the system from s2idle.
>>
>> There is no real benefit to implementing the .irq_enable/.irq_disable
>> operations using the enable bits. In fact, the existing mask/unmask
>> implementation using the threshold register is already more efficient,
>> as it requires no read/modify/write cycles. So let's leave the enable
>> bits set for the lifetime of the IRQ, using them only to control its
>> affinity.
>
> Side question, which doesn't affect this patch: what happens with
> interrupts that are firing while the interrupt is in a disabled state?
> It's fine for levels, but what of edge interrupts?
>
> My reading of the spec is that it is the role of the "gateway" to hold
> the signal, and that this is upstream of the PLIC itself, so it
> *should* be fine, but I'd like confirmation on that.
Which spec are you reading? I'm not seeing anything in
<https://github.com/riscv/riscv-plic-spec>, but I've sort of only
skimmed it. I don't remember us ever really figuring out edge triggered
interrupts, it was sort of just a "vendors should make sure they do
something reasonable" type plan.
> Thanks,
>
> M.
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
prev parent reply other threads:[~2023-09-20 10:18 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-17 18:58 [PATCH] irqchip/sifive-plic: Avoid clearing the per-hart enable bits Samuel Holland
2023-07-18 9:26 ` Marc Zyngier
2023-09-20 10:18 ` Palmer Dabbelt [this message]
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=mhng-3622fd7f-23c4-4709-9082-0a62d49762ce@palmer-ri-x1c9a \
--to=palmer@dabbelt.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=maz@kernel.org \
--cc=paul.walmsley@sifive.com \
--cc=samuel@sholland.org \
--cc=tglx@linutronix.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).