| Message ID | 20221017210119.3581-1-scott@os.amperecomputing.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: Fix bit-shifting UB in MIDR_AMPERE1 | expand |
Hi Scott, On Mon, Oct 17, 2022 at 11:01 PM D Scott Phillips <scott@os.amperecomputing.com> wrote: > CONFIG_UBSAN_SHIFT with gcc-5 complains that shifting ARM_CPU_IMP_AMPERE > (0xC0) into bits [31:24] is undefined behavior. Well, sort of, it actually > spells the error as: > > /kisskb/src/arch/arm64/kernel/proton-pack.c: In function 'spectre_bhb_loop_affected': > /kisskb/src/arch/arm64/include/asm/cputype.h:44:2: error: initializer element is not constant > (((imp) << MIDR_IMPLEMENTOR_SHIFT) | \ > ^ > > This isn't an issue for other Implementor codes, as all the other codes > have zero in the top bit. > > Fixes: 0e5d5ae837c8ce04 ("arm64: Add AMPERE1 to the Spectre-BHB affected list") > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> > Signed-off-by: D Scott Phillips <scott@os.amperecomputing.com> Thanks for your patch! > --- a/arch/arm64/include/asm/cputype.h > +++ b/arch/arm64/include/asm/cputype.h > @@ -60,7 +60,7 @@ > #define ARM_CPU_IMP_FUJITSU 0x46 > #define ARM_CPU_IMP_HISI 0x48 > #define ARM_CPU_IMP_APPLE 0x61 > -#define ARM_CPU_IMP_AMPERE 0xC0 > +#define ARM_CPU_IMP_AMPERE 0xC0u This makes the entry inconsistent with all others. So we can either add the "u" suffix everywhere, or add a cast to "u32" to the definitions[1] of MIDR_CPU_MODEL(). [1] There are two: in arch/arm64/include/asm/cputype.h and in tools/arch/arm64/include/asm/cputype.h. Looks like the second file is a copy of the first, but still lacking the AMPERE definitions. I guess it should be replaced by a symlink, to keep them in sync. > #define ARM_CPU_PART_AEM_V8 0xD0F > #define ARM_CPU_PART_FOUNDATION 0xD00 Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h index abc418650fec..b28db64799eb 100644 --- a/arch/arm64/include/asm/cputype.h +++ b/arch/arm64/include/asm/cputype.h @@ -60,7 +60,7 @@ #define ARM_CPU_IMP_FUJITSU 0x46 #define ARM_CPU_IMP_HISI 0x48 #define ARM_CPU_IMP_APPLE 0x61 -#define ARM_CPU_IMP_AMPERE 0xC0 +#define ARM_CPU_IMP_AMPERE 0xC0u #define ARM_CPU_PART_AEM_V8 0xD0F #define ARM_CPU_PART_FOUNDATION 0xD00
CONFIG_UBSAN_SHIFT with gcc-5 complains that shifting ARM_CPU_IMP_AMPERE (0xC0) into bits [31:24] is undefined behavior. Well, sort of, it actually spells the error as: /kisskb/src/arch/arm64/kernel/proton-pack.c: In function 'spectre_bhb_loop_affected': /kisskb/src/arch/arm64/include/asm/cputype.h:44:2: error: initializer element is not constant (((imp) << MIDR_IMPLEMENTOR_SHIFT) | \ ^ This isn't an issue for other Implementor codes, as all the other codes have zero in the top bit. Fixes: 0e5d5ae837c8ce04 ("arm64: Add AMPERE1 to the Spectre-BHB affected list") Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> Signed-off-by: D Scott Phillips <scott@os.amperecomputing.com> --- arch/arm64/include/asm/cputype.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)