Update cpu feature extraction

Quoted 60 lines · Russell King - ARM Linux

Quoted 7 lines · Vladimir Murzin

Quoted 6 lines · Vladimir Murzin

Hi,

This has been started as a fix in cpu feature extractor, but Suzuki suggested syncing-up the whole cpu feature handling with the recent updates on arm64 side [1,2].

Quoted 8 lines

Quoted 6 lines

I've kept fixes separately, so they can go as the are in case there is concern on updating cpu feature handling.

[1] http://comments.gmane.org/gmane.linux.ports.arm.kernel/455568
[2] https://lkml.org/lkml/2015/11/18/570

Russell, is it OK for patch tracker?

I'd really like to hear that someone has validated these changes or at least someone in ARM Ltd such as Will or Catalin has reviewed them; I remember the subject of whether certain fields are signed or unsigned being almost a personal opinion - we used to treat all fields as unsigned, but that was declared to be wrong, so we then went to treating them all as signed fields... See:

commit b8c9592b4a6c93211c8163888a97880d608503b5

Author: Ard Biesheuvel <[email protected]>
Date:   Thu Mar 19 19:03:25 2015 +0100

ARM: 8318/1: treat CPU feature register fields as signed quantities

The various CPU feature registers consist of 4-bit blocks that represent signed quantities, whose positive values represent incremental features, and whose negative values are reserved.

To improve forward compatibility, update the feature detection code to take possible future higher values into account, but ignore negative values.

Signed-off-by Ard Biesheuvel <[email protected]>

Signed-off-by Russell King <[email protected]>

For instance:

(quoted diff) +3 -3

-       sync_prim = ((read_cpuid_ext(CPUID_EXT_ISAR3) >> 8) & 0xf0) |
-                   ((read_cpuid_ext(CPUID_EXT_ISAR4) >> 20) & 0x0f);
-       if (sync_prim >= 0x13)
+       if (cpuid_feature_extract(CPUID_EXT_ISAR3, 12) > 1 ||
+           (cpuid_feature_extract(CPUID_EXT_ISAR3, 12) == 1 &&
+            cpuid_feature_extract(CPUID_EXT_ISAR3, 20) >= 3))
                elf_hwcap &= ~HWCAP_SWP;

(let's ignore the ISAR4 vs ISAR3 problem). What if ISAR4 20:23 have bit 23 set? In the original code, that would be treated as being ">= 3" but in the new code, that's less than zero, so would fail the test.

I think we had much the same questions back in March 2015 when this was last discussed, when I raised the issue of there being a documentation bug in that the documentation doesn't make it clear how reserved 4-bit CPU ID fields should be intepreted.

Quoted 6 lines

What I'd like to see is some kind of positive concensus on this (and a doc update); what I don't want to do is to apply these patches and then get another round of patches converting some of them back to signed tests, and then later another set of patches doing the reverse, because that seems to be what's now happening.

It is fair point. Since this topic is quite fuzzy and discussion can take some time I'd be happy if only fixes (the first two patches) can find their way to mainline independent of the rest of the series.

Is it OK for patches 1/4 and 2/4 to go in patch tracker?

Thanks
Vladimir

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel