Messages in this thread | | | Subject | Re: [PATCH v2 06/18] arm64: arch_timer: Add infrastructure for multiple erratum detection methods | From | Marc Zyngier <> | Date | Tue, 28 Mar 2017 15:48:23 +0100 |
| |
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...
| |