[PULL][Zesty] Errata workarounds for arm_arch_timer

Seth Forshee seth.forshee at canonical.com
Fri Mar 24 14:02:30 UTC 2017


On Thu, Mar 23, 2017 at 08:17:31PM -0600, dann frazier wrote:
> This addresses LP: #1675509.
> 
> The HiSilicon D05 board and systems based on Cortex-A73 each have
> errata for their architected timers which can cause reads to return
> bogus data. One of the arm_arch_timer maintainers has developed an
> errata infrastructure to enable workarounds for these. I've backported
> the v2 revision from the developer's tree to the zesty kernel and
> added the relevant ACKs from the parent tree maintainer from the
> mailing list. I'll continue to track the review process and watch for
> any fixes relevant to Ubuntu.
> 
> Tested on HiSilicon D05; regression tested on HP m400 (APM X-Gene) and
> a Cavium ThunderX CRB.

Are you fairly confident all of this is going to make it upstream?

+static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type type,
+                                           void *arg)
+{
+       const struct arch_timer_erratum_workaround *wa;
+       ate_match_fn_t match_fn = NULL;
+
+       if (static_branch_unlikely(&arch_timer_read_ool_enabled))
+               return;
+
+       switch (type) {
+       case ate_match_dt:
+               match_fn = arch_timer_check_dt_erratum;
+               break;
+       }
+
+       wa = arch_timer_iterate_errata(type, match_fn, arg);
+       if (!wa)
+               return;
+
+       arch_timer_enable_workaround(wa);
+       pr_info("Enabling global workaround for %s\n", wa->desc);
+}

This function makes me uncomfortable, because match_fn could conceivably
be NULL when arch_timer_iterate_errata() is called, and that function
doesn't check for NULL (I suspect that the initialization of match_fn to
NULL is here was added just to silence a compiler warning).  Not that it
looks like any of the callers ultimately end up calling it in a way
where it end up NULL, but it's not difficult to imagine that happening.

I'd feel a lot better about it if the switch had a default case which
just returned and possibly provided a warning, or something like that.

Thanks,
Seth




More information about the kernel-team mailing list