Re: [PATCH V5 3/3] irqchip/sifive-plic: Fixup thead,c900-plic request_threaded_irq with ONESHOT

[Marc Zyngier <maz@kernel.org>]

On Sun, 24 Oct 2021 02:33:03 +0100,
guoren@kernel.org wrote:
> 
> From: Guo Ren <guoren@linux.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.