[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170328145524.GD2123@mai>
Date: Tue, 28 Mar 2017 16:55:24 +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:48:23PM +0100, Marc Zyngier wrote:
> On 28/03/17 15:36, Daniel Lezcano wrote:
> > 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.
>
> I'm sorry, but it feels like a Jumbo-Jet sized hammer to try and squash
> a fly (I'm staying away from the frozen shark metaphor here). You're
> willing to add a whole list of things with private ids that need
> matching to kill a flag? I don't think this buys us anything but extra
> complexity and another maintenance headache.
Well, it is like your approach except it is split in two steps.
Can you explain where is the extra complexity ? May be I am missing the point.
--
<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
Powered by blists - more mailing lists