On Thu, Feb 25, 2021 at 5:45 PM Mathieu Poirier mathieu.poirier@linaro.org wrote:
On Thu, Feb 25, 2021 at 10:42:58AM +0100, Arnd Bergmann wrote:
index 15016f757828..4cccf874a602 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -691,13 +691,13 @@ static void etm4_disable_hw(void *info) "timeout while waiting for PM stable Trace Status\n");
/* read the status of the single shot comparators */
for (i = 0; i < drvdata->nr_ss_cmp; i++) {
for (i = 0; i < min_t(u32, drvdata->nr_ss_cmp, ETM_MAX_SS_CMP); i++) { config->ss_status[i] = etm4x_relaxed_read32(csa, TRCSSCSRn(i)); } /* read back the current counter values */
for (i = 0; i < drvdata->nr_cntr; i++) {
for (i = 0; i < min_t(u32, drvdata->nr_cntr, ETMv4_MAX_CNTR); i++) {This patch will work and I'd be happy to apply it if this was the only instance, but there are dozens of places in the coresight drivers where such patterns are used. Why are those not flagged as well? And shouldn't the real fix be with clang?
This is the only one I see in randconfig build tests. In fact I only actually see the second one of the two causing problems, but it seemed silly to change only one of them. I had not noticed the other similar loops that use etm4x_read32() instead of etm4x_relaxed_read32().
I first suspected that the memory clobber in __iormb() prevents the compiler from running into the same problem there, but changing all etm4x_read32() to etm4x_relaxed_read32() did not make it show up in other places either.
While I expect this will be fixed in mainline clang, the workaround should be harmless and let us use the existing releases as well.
Arnd