linux-kernel - Re: [PATCH] arm64: Run enable method for errata work arounds on late CPUs

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] [day] [month] [year] [list]

Message-ID: <20180117162959.GI22781@e103592.cambridge.arm.com>
Date:   Wed, 17 Jan 2018 16:29:59 +0000
From:   Dave Martin <Dave.Martin@....com>
To:     Suzuki K Poulose <Suzuki.Poulose@....com>
Cc:     Mark Rutland <mark.rutland@....com>, catalin.marinas@....com,
        will.deacon@....com, linux-kernel@...r.kernel.org,
        Andre Przywara <andre.przywara@....com>,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] arm64: Run enable method for errata work arounds on late
 CPUs

On Wed, Jan 17, 2018 at 02:52:09PM +0000, Suzuki K Poulose wrote:
> On 17/01/18 14:38, Dave Martin wrote:
> >On Wed, Jan 17, 2018 at 01:22:19PM +0000, Suzuki K Poulose wrote:
> >>On 17/01/18 12:25, Dave Martin wrote:
> >>>On Wed, Jan 17, 2018 at 10:05:56AM +0000, Suzuki K Poulose wrote:

[...]

> >>>>+	for (; caps->matches; caps++) {
> >>>>+		if (cpus_have_cap(caps->capability)) {
> >>>>+			if (caps->enable)
> >>>>+				caps->enable((void *)caps);
> >>>
> >>>Do we really need this cast?
> >>
> >>Yes, otherwise we would be passing a "const *" where a "void *" is expected,
> >>and the compiler warns. Or we could simply change the prototype of the
> >>enable() method to accept a const capability ptr.
> >
> >Hmmm, what is this argument for exactly?  cpufeature.h doesn't explain
> >what it is.
> 
> This was introduced by commit 0a0d111d40fd1 ("arm64: cpufeature: Pass capability
> structure to ->enable callback").
> 
> The idea is to enable multiple entries in the table for a single capability.
> Some capabilities (read errata) could be detected in multiple ways. e.g, different
> MIDR ranges. (e.g, ARM64_HARDEN_BRANCH_PREDICTOR, ARM64_WORKAROUND_QCOM_FALKOR_E1003.
> Now, even though the errata is the same, there might be different work arounds
> for them in each "matching" cases. So, we need the "caps" passed on to the
> enable() method to see if the "specific work around" should be applied
> to the system/CPU (since CPU hwcap could be set by one of the cpu_capabilities
> entry.) (as we invoke enable() for all "available" capabilities.)
> 
> Passing the caps information makes it easier to decide, by using the caps->matches().

This makes sense (it's a common OOP idiom anyway:
object->method(object, ...))

> >
> >Does any enable method use this for anything other than a struct
> >arm64_cpu_capabilities const * ?
> 
> No, we use it only for const struct arm64_cpu_capability *.
> 
> >If not, it would be better to specifiy that.
> 
> Yes, that could be done.

OK, I vote for doing that.

Cheers
---Dave