Re: [PATCH v2 07/17] irqchip/gic: Atomically update affinity

From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: "Marc Zyngier" <maz@kernel.org>,
	"Linux ARM" <linux-arm-kernel@lists.infradead.org>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"Will Deacon" <will@kernel.org>,
	"Catalin Marinas" <catalin.marinas@arm.com>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Jason Cooper" <jason@lakedaemon.net>,
	"Sumit Garg" <sumit.garg@linaro.org>,
	"Valentin Schneider" <Valentin.Schneider@arm.com>,
	"Florian Fainelli" <f.fainelli@gmail.com>,
	"Gregory Clement" <gregory.clement@bootlin.com>,
	"Andrew Lunn" <andrew@lunn.ch>,
	"Android Kernel Team" <kernel-team@android.com>,
	stable <stable@vger.kernel.org>,
	"Magnus Damm" <damm+renesas@opensource.se>,
	"Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>
Subject: Re: [PATCH v2 07/17] irqchip/gic: Atomically update affinity
Date: Thu, 9 Sep 2021 16:37:03 +0100	[thread overview]
Message-ID: <YToqH4phGwq4/MPQ@shell.armlinux.org.uk> (raw)
In-Reply-To: <CAMuHMdV+Ev47K5NO8XHsanSq5YRMCHn2gWAQyV-q2LpJVy9HiQ@mail.gmail.com>

On Thu, Sep 09, 2021 at 05:22:01PM +0200, Geert Uytterhoeven wrote:
> Despite the ARM Generic Interrupt Controller Architecture Specification
> (both version 1.0 and 2.0) stating that the Interrupt Processor Targets
> Registers are byte-accessible, the EMMA Mobile EV2 User's Manual
> states that the interrupt registers can be accessed via the APB bus,
> in 32-bit units.  Using byte accesses locks up the system.

Fun. Seems someone can't read ARMs documentation. Even the old
ARM IHI 0048B.b document I have for the GIC from 2013 states
"In addition, the GICD_IPRIORITYRn, GICD_ITARGETSRn, GICD_CPENDSGIRn,
and GICD_SPENDSGIRn registers support byte accesses."

However, this kind of thing is sadly not uncommon. There's been a
similar issue with the PL011 UART driver as well - some platforms
require 16-bit accesses instead of normal 32-bit accesses.

> Unfortunately I only have remote access to the board showing the
> issue.  I did check that adding the writeb_relaxed() before the
> writel_relaxed() that was used before also causes a lock-up, so the
> issue is not an endian mismatch.
> Looking at the driver history, these registers have always been
> accessed using 32-bit accesses before.  As byte accesses lead
> indeed to simpler code, I'm wondering if they had been tried before,
> and caused issues before?
> 
> Since you said the locking was bogus before, due to the reliance on
> the BL_SWITCHER option, I'm not suggesting a plain revert, but I'm
> wondering what kind of locking you suggest to use instead?

If byte accesses are not going to be workable, then the only
answer _is_ a read-modify-write with working locking.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

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