linux-kernel - Re: [PATCH V5 3/3] irqchip/sifive-plic: Fixup thead,c900-plic request_threaded

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87a6ixbcse.wl-maz@kernel.org>
Date:   Mon, 25 Oct 2021 11:48:33 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     guoren@...nel.org
Cc:     anup@...infault.org, atish.patra@....com, tglx@...utronix.de,
        palmer@...belt.com, heiko@...ech.de, robh@...nel.org,
        linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org,
        Guo Ren <guoren@...ux.alibaba.com>
Subject: Re: [PATCH V5 3/3] irqchip/sifive-plic: Fixup thead,c900-plic request_threaded_irq with ONESHOT

On Sun, 24 Oct 2021 02:33:03 +0100,
guoren@...nel.org wrote:
> 
> From: Guo Ren <guoren@...ux.alibaba.com>
> 
> When using "devm_request_threaded_irq(,,,,IRQF_ONESHOT,,)" in the driver,
> only the first interrupt could be handled, and continue irq is blocked by
> hw. Because the thead,c900-plic couldn't complete masked irq source which
> has been disabled in enable register. Add thead_plic_chip which fix up
> c906-plic irq source completion problem by unmask/mask wrapper.
> 
> Here is the description of Interrupt Completion in PLIC spec [1]:
> 
> The PLIC signals it has completed executing an interrupt handler by
> writing the interrupt ID it received from the claim to the claim/complete
> register. The PLIC does not check whether the completion ID is the same
> as the last claim ID for that target. If the completion ID does not match
> an interrupt source that is currently enabled for the target, the
>                          ^^ ^^^^^^^^^ ^^^^^^^
> completion is silently ignored.

Given this bit of the spec...

> +static void plic_thead_irq_eoi(struct irq_data *d)
> +{
> +	struct plic_handler *handler = this_cpu_ptr(&plic_handlers);
> +
> +	if (irqd_irq_masked(d)) {
> +		plic_irq_unmask(d);
> +		writel(d->hwirq, handler->hart_base + CONTEXT_CLAIM);
> +		plic_irq_mask(d);
> +	} else {
> +		writel(d->hwirq, handler->hart_base + CONTEXT_CLAIM);
> +	}
> +}
> +

... it isn't obvious to me why this cannot happen on an SiFive PLIC.

And it isn't only for threaded interrupts in oneshot mode. Any driver
can mask an interrupt from its handler after having set the
IRQ_DISABLE_UNLAZY flag, and the interrupt would need the exact same
treatment.

	M.

-- 
Without deviation from the norm, progress is not possible.