Re: [PATCH v2 5/5] ARM: spectre-v2: per-CPU vtables to work around big.Little systems - Russell King

From: Russell King - ARM Linux <linux@armlinux.org.uk>
To: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Julien Thierry <julien.thierry@arm.com>,
	Marc Zyngier <marc.zyngier@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Morten Rasmussen <Morten.Rasmussen@arm.com>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	Qais Yousef <Qais.Yousef@arm.com>
Subject: Re: [PATCH v2 5/5] ARM: spectre-v2: per-CPU vtables to work around big.Little systems
Date: Thu, 6 Dec 2018 10:00:52 +0000	[thread overview]
Message-ID: <20181206100052.GR30658@n2100.armlinux.org.uk> (raw)
In-Reply-To: <dda0be26-5ac8-a158-3210-4bc6861bede2@samsung.com>

On Thu, Dec 06, 2018 at 10:32:56AM +0100, Marek Szyprowski wrote:
> Hi All,
> 
> On 2018-11-06 13:39, Russell King wrote:
> > In big.Little systems, some CPUs require the Spectre workarounds in
> > paths such as the context switch, but other CPUs do not.  In order
> > to handle these differences, we need per-CPU vtables.
> >
> > We are unable to use the kernel's per-CPU variables to support this
> > as per-CPU is not initialised at times when we need access to the
> > vtables, so we have to use an array indexed by logical CPU number.
> >
> > We use an array-of-pointers to avoid having function pointers in
> > the kernel's read/write .data section.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> 
> Today I've noticed that this patch (which has landed in v4.20-rc3 as
> commit 383fb3ee8024) causes regression in CPU hotplug and suspend/resume
> handling on Samsung Exynos4412 SoC (4*CortexA9 SMP). After putting any
> non-zero CPU offline, it is no longer possible to get it online again.
> Same in system suspend/resume - after s2r cycle, only CPU0 is online.
> The regression can be observed on Hardkernel's Odroid U3 and Samsung
> Trats2 boards.
> 
> Any idea how to fix this issue?

This is getting _really_ annoying.  I send the patches out, I ask for
people to test, they seem to ignore the patches, and then they later
report problems.  This is the _second_ time that this has happened,
both times with Exynos, the first time it caused the patches to miss
the merge window.  Given that Spectre has to deal with every kernel
entry path, wouldn't it be logical to test every form of entry - boot,
CPU hotplug, suspend/resume etc?

Sorry, but at this point I have little sympathy.

I've no idea based on what you've supplied given that the SoC
maintainers are responsible for writing the code to deal with hotplug
etc, and Exynos's code there is something of a maze.  It's not clear
which bits are being used.  I think you at the very least need to debug
to find out whether the problem is at CPU down or CPU up.

From the ARM architecture point of view, for Cortex A9, all the
processor function instances should be identical.  The only difference
as a result of the patch is that we'll be calling smp_processor_id()
early (which should be fine), and indirecting through the cpu_vtable[]
array rather than merely dereferencing the processor struct.

What about checking dmesg - messages from offline CPUs do not appear
on the console(s) but are still logged in the kernel log.

You could try making PROC_VTABLE() the same as PROC_TABLE() (iow,
always access cpu_vtable[0]) to see whether it's the smp_processor_id()
that's causing your problem or not.  If it is, then try and work out
which of the processor functions is causing it by restoring
PROC_VTABLE() and then switching each from PROC_VTABLE() to
PROC_TABLE() until it does work.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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