lkml.org 
[lkml]   [2017]   [Mar]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 06/18] arm64: arch_timer: Add infrastructure for multiple erratum detection methods
From
Date
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.

Thanks,

M.
--
Jazz is not dead. It just smells funny...

\
 
 \ /
  Last update: 2017-03-28 16:51    [W:0.082 / U:0.276 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site