linux-kernel - Re: [PATCH v2 06/18] arm64: arch_timer: Add infrastructure for multiple erratum detection methods

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: <20170328143633.GC2123@mai>
Date:   Tue, 28 Mar 2017 16:36:33 +0200
From:   Daniel Lezcano <daniel.lezcano@...aro.org>
To:     Marc Zyngier <marc.zyngier@....com>
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Mark Rutland <mark.rutland@....com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        Scott Wood <oss@...error.net>,
        Hanjun Guo <hanjun.guo@...aro.org>,
        Ding Tianhong <dingtianhong@...wei.com>,
        dann frazier <dann.frazier@...onical.com>
Subject: Re: [PATCH v2 06/18] arm64: arch_timer: Add infrastructure for
 multiple erratum detection methods

On Tue, Mar 28, 2017 at 03:07:52PM +0100, Marc Zyngier wrote:

[ ... ]

> >>> -bool arch_timer_check_global_cap_erratum(const struct arch_timer_erratum_workaround *wa,
> >>> -					 const void *arg)
> >>> +bool arch_timer_check_cap_erratum(const struct arch_timer_erratum_workaround *wa,
> >>> +				  const void *arg)
> >>>  {
> >>> -	return cpus_have_cap((uintptr_t)wa->id);
> >>> +	return cpus_have_cap((uintptr_t)wa->id) | this_cpu_has_cap((uintptr_t)wa->id);
> >>
> >> Not quite. Here, you're making all capability-based errata to be be
> >> global (if a single CPU in the system has a capability, then by
> >> transitivity cpus_have_cap returns true). If that's a big-little system,
> >> you end-up applying the workaround to all CPUs, including those unaffected.
> >>
> >> I'd rather drop cpus_have_cap altogether and rely on individual CPU
> >> matching (since we don't have a need for a global capability erratum
> >> handling yet).
> > 
> > Ok, thanks.
> 
> Quick update. I've just implemented this, and found out that getting rid
> of local/global has an unfortunate effect:
> 
> Since we only probe the global errata (using ACPI for example) on the
> boot CPU path, we lose propagation of the erratum across the secondary
> CPUs. One way of solving this is to convert the secondary boot path to
> be aware of DT vs ACPI vs detection method of the month. Which isn't
> easy, since by the time we boot secondary CPUs, we don't have the
> pointers to the various ACPI tables anymore. Also, assuming we were
> careful and saved the pointers, the tables may have been unmapped. Fun.

My proposal was supposed to prevent that. The detecion is done in the
subsystems, ACPI detects ACPI errata, DT detects DT errata and CPU detects CPU
errata. The drivers get the errata and enable the workaround. The id
association <-> errata self contains errata types (void *, char *, int). So
everything can be done in a CPU basis without local / global dance.

> Given that, I'm reintroducing the global/local flag for good. It's not
> pretty, but it doesn't require reinventing new ways of dealing with CPUs
> booting late.



-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog