[PATCH 0/3] arm64: extend/fix /proc/cpuinfo + runtime sanity checks

[PATCH 0/3] arm64: extend/fix /proc/cpuinfo + runtime sanity checks

[Mark Rutland]

On arm64 /proc/cpuinfo only contains information regarding the current
CPU (i.e. that which issued the read), due to use of read_cpuid_id()
within c_show.  This isn't fantastic as it doesn't match arm and x86
(which print information per-cpu), and on big.LITTLE AArch64 systems it
will mean we don't get accurate information regarding each CPU.

This series adds code to record the relevant system registers upon the
booting of each CPU, and uses this information to output correct
information per-CPU in /proc/cpuinfo. I have not seen this cause issues
so far with a 64-bit filesystem, but would appreciate any testing people
are willing to perform.

I've built some run-time sanity/consistency checking atop of the
cpuinfo_arm64 infrastructure, which hopefully should help to identify
when a system's CPUs vary in ways we do not expect and/or cannot
support.

The series is based on the arm64 for-next/core branch [1] due to reading
of the cache registers in sanity checks code, but patches 1 and 2 should
apply to v3.15-rc5.

Cheers,
Mark.


[1] https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/log/?h=for-next/core

Mark Rutland (3):
  arm64: add MIDR_EL1 field accessors
  arm64: cpuinfo: print info for all CPUs
  arm64: add runtime system sanity checks

 arch/arm64/include/asm/cpu.h     |  49 +++++++++++++++
 arch/arm64/include/asm/cputype.h |  27 ++++++++-
 arch/arm64/kernel/Makefile       |   2 +-
 arch/arm64/kernel/cpuinfo.c      | 125 +++++++++++++++++++++++++++++++++++++++
 arch/arm64/kernel/setup.c        |  46 +++++++-------
 arch/arm64/kernel/smp.c          |   6 ++
 6 files changed, 230 insertions(+), 25 deletions(-)
 create mode 100644 arch/arm64/include/asm/cpu.h
 create mode 100644 arch/arm64/kernel/cpuinfo.c

-- 
1.8.1.1

[PATCH 1/3] arm64: add MIDR_EL1 field accessors

[Mark Rutland]

The MIDR_EL1 register is composed of a number of bitfields, and uses of
the fields has so far involved open-coding of the shifts and masks
required.

This patch adds shifts and masks for each of the MIDR_EL1 subfields, and
also provides accessors built atop of these. Existing uses within
cputype.h are updated to use these accessors.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/include/asm/cputype.h | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index c404fb0..541b829 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -36,6 +36,25 @@
 	__val;								\
 })
 
+#define MIDR_REVISION_MASK	0xf
+#define MIDR_REVISION(midr)	((midr) & MIDR_REVISION_MASK)
+#define MIDR_PARTNUM_SHIFT	4
+#define MIDR_PARTNUM_MASK	(0xfff << MIDR_PARTNUM_SHIFT)
+#define MIDR_PARTNUM(midr)	\
+	(((midr) & MIDR_PARTNUM_MASK) >> MIDR_PARTNUM_SHIFT)
+#define MIDR_ARCHITECTURE_SHIFT	16
+#define MIDR_ARCHITECTURE_MASK	(0xf << MIDR_ARCHITECTURE_SHIFT)
+#define MIDR_ARCHITECTURE(midr)	\
+	(((midr) & MIDR_ARCHITECTURE_MASK) >> MIDR_ARCHITECTURE_SHIFT)
+#define MIDR_VARIANT_SHIFT	20
+#define MIDR_VARIANT_MASK	(0xf << MIDR_VARIANT_SHIFT)
+#define MIDR_VARIANT(midr)	\
+	(((midr) & MIDR_VARIANT_MASK) >> MIDR_VARIANT_SHIFT)
+#define MIDR_IMPLEMENTOR_SHIFT	24
+#define MIDR_IMPLEMENTOR_MASK	(0xff << MIDR_IMPLEMENTOR_SHIFT)
+#define MIDR_IMPLEMENTOR(midr)	\
+	(((midr) & MIDR_IMPLEMENTOR_MASK) >> MIDR_IMPLEMENTOR_SHIFT)
+
 #define ARM_CPU_IMP_ARM		0x41
 #define ARM_CPU_IMP_APM		0x50
 
@@ -64,12 +83,16 @@ static inline u64 __attribute_const__ read_cpuid_mpidr(void)
 
 static inline unsigned int __attribute_const__ read_cpuid_implementor(void)
 {
-	return (read_cpuid_id() & 0xFF000000) >> 24;
+	return MIDR_IMPLEMENTOR(read_cpuid_id());
 }
 
 static inline unsigned int __attribute_const__ read_cpuid_part_number(void)
 {
-	return (read_cpuid_id() & 0xFFF0);
+	/*
+	 * The part number field is used in-place as a userspace ABI, so we
+	 * cannot use MIDR_PARTNUM() here.
+	 */
+	return (read_cpuid_id() & MIDR_PARTNUM_MASK);
 }
 
 static inline u32 __attribute_const__ read_cpuid_cachetype(void)
-- 
1.8.1.1

[PATCH 1/3] arm64: add MIDR_EL1 field accessors

[Will Deacon]

On Mon, May 12, 2014 at 03:37:48PM +0100, Mark Rutland wrote:
> The MIDR_EL1 register is composed of a number of bitfields, and uses of
> the fields has so far involved open-coding of the shifts and masks
> required.
> 
> This patch adds shifts and masks for each of the MIDR_EL1 subfields, and
> also provides accessors built atop of these. Existing uses within
> cputype.h are updated to use these accessors.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> ---
>  arch/arm64/include/asm/cputype.h | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
> index c404fb0..541b829 100644
> --- a/arch/arm64/include/asm/cputype.h
> +++ b/arch/arm64/include/asm/cputype.h
> @@ -36,6 +36,25 @@
>  	__val;								\
>  })
>  
> +#define MIDR_REVISION_MASK	0xf
> +#define MIDR_REVISION(midr)	((midr) & MIDR_REVISION_MASK)
> +#define MIDR_PARTNUM_SHIFT	4
> +#define MIDR_PARTNUM_MASK	(0xfff << MIDR_PARTNUM_SHIFT)
> +#define MIDR_PARTNUM(midr)	\
> +	(((midr) & MIDR_PARTNUM_MASK) >> MIDR_PARTNUM_SHIFT)
> +#define MIDR_ARCHITECTURE_SHIFT	16
> +#define MIDR_ARCHITECTURE_MASK	(0xf << MIDR_ARCHITECTURE_SHIFT)
> +#define MIDR_ARCHITECTURE(midr)	\
> +	(((midr) & MIDR_ARCHITECTURE_MASK) >> MIDR_ARCHITECTURE_SHIFT)
> +#define MIDR_VARIANT_SHIFT	20
> +#define MIDR_VARIANT_MASK	(0xf << MIDR_VARIANT_SHIFT)
> +#define MIDR_VARIANT(midr)	\
> +	(((midr) & MIDR_VARIANT_MASK) >> MIDR_VARIANT_SHIFT)
> +#define MIDR_IMPLEMENTOR_SHIFT	24
> +#define MIDR_IMPLEMENTOR_MASK	(0xff << MIDR_IMPLEMENTOR_SHIFT)
> +#define MIDR_IMPLEMENTOR(midr)	\
> +	(((midr) & MIDR_IMPLEMENTOR_MASK) >> MIDR_IMPLEMENTOR_SHIFT)
> +
>  #define ARM_CPU_IMP_ARM		0x41
>  #define ARM_CPU_IMP_APM		0x50
>  
> @@ -64,12 +83,16 @@ static inline u64 __attribute_const__ read_cpuid_mpidr(void)
>  
>  static inline unsigned int __attribute_const__ read_cpuid_implementor(void)
>  {
> -	return (read_cpuid_id() & 0xFF000000) >> 24;
> +	return MIDR_IMPLEMENTOR(read_cpuid_id());
>  }
>  
>  static inline unsigned int __attribute_const__ read_cpuid_part_number(void)
>  {
> -	return (read_cpuid_id() & 0xFFF0);
> +	/*
> +	 * The part number field is used in-place as a userspace ABI, so we
> +	 * cannot use MIDR_PARTNUM() here.
> +	 */
> +	return (read_cpuid_id() & MIDR_PARTNUM_MASK);

Where is this used as ABI? c_show is open-coded and kvm_target_cpu could
just add a shift left.

Will

[PATCH 1/3] arm64: add MIDR_EL1 field accessors

[Mark Rutland]

On Mon, May 12, 2014 at 04:05:14PM +0100, Will Deacon wrote:
> On Mon, May 12, 2014 at 03:37:48PM +0100, Mark Rutland wrote:
> > The MIDR_EL1 register is composed of a number of bitfields, and uses of
> > the fields has so far involved open-coding of the shifts and masks
> > required.
> > 
> > This patch adds shifts and masks for each of the MIDR_EL1 subfields, and
> > also provides accessors built atop of these. Existing uses within
> > cputype.h are updated to use these accessors.
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > ---
> >  arch/arm64/include/asm/cputype.h | 27 +++++++++++++++++++++++++--
> >  1 file changed, 25 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
> > index c404fb0..541b829 100644
> > --- a/arch/arm64/include/asm/cputype.h
> > +++ b/arch/arm64/include/asm/cputype.h
> > @@ -36,6 +36,25 @@
> >  	__val;								\
> >  })
> >  
> > +#define MIDR_REVISION_MASK	0xf
> > +#define MIDR_REVISION(midr)	((midr) & MIDR_REVISION_MASK)
> > +#define MIDR_PARTNUM_SHIFT	4
> > +#define MIDR_PARTNUM_MASK	(0xfff << MIDR_PARTNUM_SHIFT)
> > +#define MIDR_PARTNUM(midr)	\
> > +	(((midr) & MIDR_PARTNUM_MASK) >> MIDR_PARTNUM_SHIFT)
> > +#define MIDR_ARCHITECTURE_SHIFT	16
> > +#define MIDR_ARCHITECTURE_MASK	(0xf << MIDR_ARCHITECTURE_SHIFT)
> > +#define MIDR_ARCHITECTURE(midr)	\
> > +	(((midr) & MIDR_ARCHITECTURE_MASK) >> MIDR_ARCHITECTURE_SHIFT)
> > +#define MIDR_VARIANT_SHIFT	20
> > +#define MIDR_VARIANT_MASK	(0xf << MIDR_VARIANT_SHIFT)
> > +#define MIDR_VARIANT(midr)	\
> > +	(((midr) & MIDR_VARIANT_MASK) >> MIDR_VARIANT_SHIFT)
> > +#define MIDR_IMPLEMENTOR_SHIFT	24
> > +#define MIDR_IMPLEMENTOR_MASK	(0xff << MIDR_IMPLEMENTOR_SHIFT)
> > +#define MIDR_IMPLEMENTOR(midr)	\
> > +	(((midr) & MIDR_IMPLEMENTOR_MASK) >> MIDR_IMPLEMENTOR_SHIFT)
> > +
> >  #define ARM_CPU_IMP_ARM		0x41
> >  #define ARM_CPU_IMP_APM		0x50
> >  
> > @@ -64,12 +83,16 @@ static inline u64 __attribute_const__ read_cpuid_mpidr(void)
> >  
> >  static inline unsigned int __attribute_const__ read_cpuid_implementor(void)
> >  {
> > -	return (read_cpuid_id() & 0xFF000000) >> 24;
> > +	return MIDR_IMPLEMENTOR(read_cpuid_id());
> >  }
> >  
> >  static inline unsigned int __attribute_const__ read_cpuid_part_number(void)
> >  {
> > -	return (read_cpuid_id() & 0xFFF0);
> > +	/*
> > +	 * The part number field is used in-place as a userspace ABI, so we
> > +	 * cannot use MIDR_PARTNUM() here.
> > +	 */
> > +	return (read_cpuid_id() & MIDR_PARTNUM_MASK);
> 
> Where is this used as ABI? c_show is open-coded and kvm_target_cpu could
> just add a shift left.

For some reason I thought this was called by some code included from
arch/arm, but that doesn't seem to be the case.

I'll add the left shift in kvm_target_cpu instead.

Cheers,
Mark.

[PATCH 2/3] arm64: cpuinfo: print info for all CPUs

[Mark Rutland]

Currently reading /proc/cpuinfo will result in information being read
out of the MIDR_EL1 of the current CPU, and the information is not
associated with any particular logical CPU number.

This is problematic for systems with heterogeneous CPUs (i.e.
big.LITTLE) where fields will vary across CPUs, and the output will
differ depending on the executing CPU. Additionally the output is
different in format to the 32-bit ARM Linux port, where information is
printed out for each CPU.

This patch adds the necessary infrastructure to log the relevant
registers (currently just MPIDR_EL1) and print out the logged
information.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Jacob Bramley <jacob.bramley@arm.com>
---
 arch/arm64/include/asm/cpu.h | 30 +++++++++++++++++++++++++++++
 arch/arm64/kernel/Makefile   |  2 +-
 arch/arm64/kernel/cpuinfo.c  | 30 +++++++++++++++++++++++++++++
 arch/arm64/kernel/setup.c    | 46 +++++++++++++++++++++++---------------------
 arch/arm64/kernel/smp.c      |  6 ++++++
 5 files changed, 91 insertions(+), 23 deletions(-)
 create mode 100644 arch/arm64/include/asm/cpu.h
 create mode 100644 arch/arm64/kernel/cpuinfo.c

diff --git a/arch/arm64/include/asm/cpu.h b/arch/arm64/include/asm/cpu.h
new file mode 100644
index 0000000..74bf9bb
--- /dev/null
+++ b/arch/arm64/include/asm/cpu.h
@@ -0,0 +1,30 @@
+/*
+ *  arch/arm/include/asm/cpu.h
+ *
+ *  Copyright (C) 2004-2014 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef __ASM_ARM64_CPU_H
+#define __ASM_ARM64_CPU_H
+
+#include <linux/percpu.h>
+#include <linux/cpu.h>
+
+/*
+ * Records attributes of an individual CPU.
+ *
+ * This is used to cache data for /proc/cpuinfo.
+ */
+struct cpuinfo_arm64 {
+	struct cpu	cpu;
+	u32		reg_midr;
+};
+
+DECLARE_PER_CPU(struct cpuinfo_arm64, cpu_data);
+
+void cpuinfo_store_cpu(void);
+
+#endif
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 7d811d9..0e0431f 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -9,7 +9,7 @@ AFLAGS_head.o		:= -DTEXT_OFFSET=$(TEXT_OFFSET)
 arm64-obj-y		:= cputable.o debug-monitors.o entry.o irq.o fpsimd.o	\
 			   entry-fpsimd.o process.o ptrace.o setup.o signal.o	\
 			   sys.o stacktrace.o time.o traps.o io.o vdso.o	\
-			   hyp-stub.o psci.o cpu_ops.o insn.o
+			   hyp-stub.o psci.o cpu_ops.o insn.o cpuinfo.o
 
 arm64-obj-$(CONFIG_COMPAT)		+= sys32.o kuser32.o signal32.o 	\
 					   sys_compat.o
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
new file mode 100644
index 0000000..b9e18c5
--- /dev/null
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -0,0 +1,30 @@
+/*
+ * Record CPU attributes for later retrieval
+ *
+ * Copyright (C) 2014 ARM Ltd.
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <asm/cpu.h>
+
+#include <linux/smp.h>
+
+DEFINE_PER_CPU(struct cpuinfo_arm64, cpu_data);
+
+void cpuinfo_store_cpu(void)
+{
+	int cpu = smp_processor_id();
+	struct cpuinfo_arm64 *cpuinfo = &per_cpu(cpu_data, cpu);
+
+	cpuinfo->reg_midr = read_cpuid_id();
+}
+
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 7450c58..b2c0554 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -44,6 +44,7 @@
 #include <linux/of_platform.h>
 
 #include <asm/fixmap.h>
+#include <asm/cpu.h>
 #include <asm/cputype.h>
 #include <asm/elf.h>
 #include <asm/cputable.h>
@@ -391,6 +392,7 @@ void __init setup_arch(char **cmdline_p)
 
 	cpu_logical_map(0) = read_cpuid_mpidr() & MPIDR_HWID_BITMASK;
 	cpu_read_bootcpu_ops();
+	cpuinfo_store_cpu();
 #ifdef CONFIG_SMP
 	smp_init_cpus();
 	smp_build_mpidr_hash();
@@ -412,14 +414,12 @@ static int __init arm64_device_init(void)
 }
 arch_initcall_sync(arm64_device_init);
 
-static DEFINE_PER_CPU(struct cpu, cpu_data);
-
 static int __init topology_init(void)
 {
 	int i;
 
 	for_each_possible_cpu(i) {
-		struct cpu *cpu = &per_cpu(cpu_data, i);
+		struct cpu *cpu = &per_cpu(cpu_data.cpu, i);
 		cpu->hotpluggable = 1;
 		register_cpu(cpu, i);
 	}
@@ -442,38 +442,40 @@ static const char *hwcap_str[] = {
 
 static int c_show(struct seq_file *m, void *v)
 {
-	int i;
+	int c;
 
-	seq_printf(m, "Processor\t: %s rev %d (%s)\n",
-		   cpu_name, read_cpuid_id() & 15, ELF_PLATFORM);
+	for_each_online_cpu(c) {
+		int i;
+		struct cpuinfo_arm64 *cpuinfo = &per_cpu(cpu_data, c);
+		u32 midr = cpuinfo->reg_midr;
 
-	for_each_online_cpu(i) {
 		/*
 		 * glibc reads /proc/cpuinfo to determine the number of
 		 * online processors, looking for lines beginning with
 		 * "processor".  Give glibc what it expects.
 		 */
 #ifdef CONFIG_SMP
-		seq_printf(m, "processor\t: %d\n", i);
+		seq_printf(m, "processor\t: %d\n", c);
 #endif
-	}
+		seq_printf(m, "Type\t\t: %s rev %d (%s)\n",
+			   cpu_name, MIDR_REVISION(midr), ELF_PLATFORM);
 
-	/* dump out the processor features */
-	seq_puts(m, "Features\t: ");
+		/* dump out the processor features */
+		seq_puts(m, "Features\t: ");
 
-	for (i = 0; hwcap_str[i]; i++)
-		if (elf_hwcap & (1 << i))
-			seq_printf(m, "%s ", hwcap_str[i]);
+		for (i = 0; hwcap_str[i]; i++)
+			if (elf_hwcap & (1 << i))
+				seq_printf(m, "%s ", hwcap_str[i]);
 
-	seq_printf(m, "\nCPU implementer\t: 0x%02x\n", read_cpuid_id() >> 24);
-	seq_printf(m, "CPU architecture: AArch64\n");
-	seq_printf(m, "CPU variant\t: 0x%x\n", (read_cpuid_id() >> 20) & 15);
-	seq_printf(m, "CPU part\t: 0x%03x\n", (read_cpuid_id() >> 4) & 0xfff);
-	seq_printf(m, "CPU revision\t: %d\n", read_cpuid_id() & 15);
+		seq_printf(m, "\nCPU implementer\t: 0x%02x\n",
+			   MIDR_IMPLEMENTOR(midr));
+		seq_printf(m, "CPU architecture: AArch64\n");
+		seq_printf(m, "CPU variant\t: 0x%x\n", MIDR_VARIANT(midr));
+		seq_printf(m, "CPU part\t: 0x%03x\n", MIDR_PARTNUM(midr));
+		seq_printf(m, "CPU revision\t: %d\n", MIDR_REVISION(midr));
 
-	seq_puts(m, "\n");
-
-	seq_printf(m, "Hardware\t: %s\n", machine_name);
+		seq_puts(m, "\n");
+	}
 
 	return 0;
 }
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index f0a141d..f74866d 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -38,6 +38,7 @@
 
 #include <asm/atomic.h>
 #include <asm/cacheflush.h>
+#include <asm/cpu.h>
 #include <asm/cputype.h>
 #include <asm/cpu_ops.h>
 #include <asm/mmu_context.h>
@@ -160,6 +161,11 @@ asmlinkage void secondary_start_kernel(void)
 	smp_store_cpu_info(cpu);
 
 	/*
+	 * Log the CPU info before it is marked online and might get read.
+	 */
+	cpuinfo_store_cpu();
+
+	/*
 	 * OK, now it's safe to let the boot CPU continue.  Wait for
 	 * the CPU migration code to notice that the CPU is online
 	 * before we continue.
-- 
1.8.1.1

[PATCH 2/3] arm64: cpuinfo: print info for all CPUs

[Ard Biesheuvel]

Hi Mark,

On 12 May 2014 16:37, Mark Rutland <mark.rutland@arm.com> wrote:
> Currently reading /proc/cpuinfo will result in information being read
> out of the MIDR_EL1 of the current CPU, and the information is not
> associated with any particular logical CPU number.
>
> This is problematic for systems with heterogeneous CPUs (i.e.
> big.LITTLE) where fields will vary across CPUs, and the output will
> differ depending on the executing CPU. Additionally the output is
> different in format to the 32-bit ARM Linux port, where information is
> printed out for each CPU.
>
> This patch adds the necessary infrastructure to log the relevant
> registers (currently just MPIDR_EL1) and print out the logged
> information.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Jacob Bramley <jacob.bramley@arm.com>
> ---
>  arch/arm64/include/asm/cpu.h | 30 +++++++++++++++++++++++++++++
>  arch/arm64/kernel/Makefile   |  2 +-
>  arch/arm64/kernel/cpuinfo.c  | 30 +++++++++++++++++++++++++++++
>  arch/arm64/kernel/setup.c    | 46 +++++++++++++++++++++++---------------------
>  arch/arm64/kernel/smp.c      |  6 ++++++
>  5 files changed, 91 insertions(+), 23 deletions(-)
>  create mode 100644 arch/arm64/include/asm/cpu.h
>  create mode 100644 arch/arm64/kernel/cpuinfo.c
>
> diff --git a/arch/arm64/include/asm/cpu.h b/arch/arm64/include/asm/cpu.h
> new file mode 100644
> index 0000000..74bf9bb
> --- /dev/null
> +++ b/arch/arm64/include/asm/cpu.h
> @@ -0,0 +1,30 @@
> +/*
> + *  arch/arm/include/asm/cpu.h
> + *
> + *  Copyright (C) 2004-2014 ARM Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#ifndef __ASM_ARM64_CPU_H
> +#define __ASM_ARM64_CPU_H
> +
> +#include <linux/percpu.h>
> +#include <linux/cpu.h>
> +
> +/*
> + * Records attributes of an individual CPU.
> + *
> + * This is used to cache data for /proc/cpuinfo.
> + */
> +struct cpuinfo_arm64 {
> +       struct cpu      cpu;
> +       u32             reg_midr;
> +};
> +
> +DECLARE_PER_CPU(struct cpuinfo_arm64, cpu_data);
> +
> +void cpuinfo_store_cpu(void);
> +
> +#endif
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index 7d811d9..0e0431f 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -9,7 +9,7 @@ AFLAGS_head.o           := -DTEXT_OFFSET=$(TEXT_OFFSET)
>  arm64-obj-y            := cputable.o debug-monitors.o entry.o irq.o fpsimd.o   \
>                            entry-fpsimd.o process.o ptrace.o setup.o signal.o   \
>                            sys.o stacktrace.o time.o traps.o io.o vdso.o        \
> -                          hyp-stub.o psci.o cpu_ops.o insn.o
> +                          hyp-stub.o psci.o cpu_ops.o insn.o cpuinfo.o
>
>  arm64-obj-$(CONFIG_COMPAT)             += sys32.o kuser32.o signal32.o         \
>                                            sys_compat.o
> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> new file mode 100644
> index 0000000..b9e18c5
> --- /dev/null
> +++ b/arch/arm64/kernel/cpuinfo.c
> @@ -0,0 +1,30 @@
> +/*
> + * Record CPU attributes for later retrieval
> + *
> + * Copyright (C) 2014 ARM Ltd.
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <asm/cpu.h>
> +
> +#include <linux/smp.h>
> +
> +DEFINE_PER_CPU(struct cpuinfo_arm64, cpu_data);
> +
> +void cpuinfo_store_cpu(void)
> +{
> +       int cpu = smp_processor_id();
> +       struct cpuinfo_arm64 *cpuinfo = &per_cpu(cpu_data, cpu);
> +
> +       cpuinfo->reg_midr = read_cpuid_id();
> +}
> +
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index 7450c58..b2c0554 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -44,6 +44,7 @@
>  #include <linux/of_platform.h>
>
>  #include <asm/fixmap.h>
> +#include <asm/cpu.h>
>  #include <asm/cputype.h>
>  #include <asm/elf.h>
>  #include <asm/cputable.h>
> @@ -391,6 +392,7 @@ void __init setup_arch(char **cmdline_p)
>
>         cpu_logical_map(0) = read_cpuid_mpidr() & MPIDR_HWID_BITMASK;
>         cpu_read_bootcpu_ops();
> +       cpuinfo_store_cpu();
>  #ifdef CONFIG_SMP
>         smp_init_cpus();
>         smp_build_mpidr_hash();
> @@ -412,14 +414,12 @@ static int __init arm64_device_init(void)
>  }
>  arch_initcall_sync(arm64_device_init);
>
> -static DEFINE_PER_CPU(struct cpu, cpu_data);
> -
>  static int __init topology_init(void)
>  {
>         int i;
>
>         for_each_possible_cpu(i) {
> -               struct cpu *cpu = &per_cpu(cpu_data, i);
> +               struct cpu *cpu = &per_cpu(cpu_data.cpu, i);
>                 cpu->hotpluggable = 1;
>                 register_cpu(cpu, i);
>         }
> @@ -442,38 +442,40 @@ static const char *hwcap_str[] = {
>
>  static int c_show(struct seq_file *m, void *v)
>  {
> -       int i;
> +       int c;
>
> -       seq_printf(m, "Processor\t: %s rev %d (%s)\n",
> -                  cpu_name, read_cpuid_id() & 15, ELF_PLATFORM);
> +       for_each_online_cpu(c) {
> +               int i;
> +               struct cpuinfo_arm64 *cpuinfo = &per_cpu(cpu_data, c);
> +               u32 midr = cpuinfo->reg_midr;
>
> -       for_each_online_cpu(i) {
>                 /*
>                  * glibc reads /proc/cpuinfo to determine the number of
>                  * online processors, looking for lines beginning with
>                  * "processor".  Give glibc what it expects.
>                  */
>  #ifdef CONFIG_SMP
> -               seq_printf(m, "processor\t: %d\n", i);
> +               seq_printf(m, "processor\t: %d\n", c);
>  #endif
> -       }
> +               seq_printf(m, "Type\t\t: %s rev %d (%s)\n",
> +                          cpu_name, MIDR_REVISION(midr), ELF_PLATFORM);
>
> -       /* dump out the processor features */
> -       seq_puts(m, "Features\t: ");
> +               /* dump out the processor features */
> +               seq_puts(m, "Features\t: ");
>
> -       for (i = 0; hwcap_str[i]; i++)
> -               if (elf_hwcap & (1 << i))
> -                       seq_printf(m, "%s ", hwcap_str[i]);
> +               for (i = 0; hwcap_str[i]; i++)
> +                       if (elf_hwcap & (1 << i))
> +                               seq_printf(m, "%s ", hwcap_str[i]);
>

Considering that you are printing the contents of the same variable
every iteration, and the fact that your subsequent patch makes sure
elf_hwcap is in sync between all CPUs, doesn't it make more sense to
print this line only once?

-- 
Ard.



> -       seq_printf(m, "\nCPU implementer\t: 0x%02x\n", read_cpuid_id() >> 24);
> -       seq_printf(m, "CPU architecture: AArch64\n");
> -       seq_printf(m, "CPU variant\t: 0x%x\n", (read_cpuid_id() >> 20) & 15);
> -       seq_printf(m, "CPU part\t: 0x%03x\n", (read_cpuid_id() >> 4) & 0xfff);
> -       seq_printf(m, "CPU revision\t: %d\n", read_cpuid_id() & 15);
> +               seq_printf(m, "\nCPU implementer\t: 0x%02x\n",
> +                          MIDR_IMPLEMENTOR(midr));
> +               seq_printf(m, "CPU architecture: AArch64\n");
> +               seq_printf(m, "CPU variant\t: 0x%x\n", MIDR_VARIANT(midr));
> +               seq_printf(m, "CPU part\t: 0x%03x\n", MIDR_PARTNUM(midr));
> +               seq_printf(m, "CPU revision\t: %d\n", MIDR_REVISION(midr));
>
> -       seq_puts(m, "\n");
> -
> -       seq_printf(m, "Hardware\t: %s\n", machine_name);
> +               seq_puts(m, "\n");
> +       }
>
>         return 0;
>  }
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index f0a141d..f74866d 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -38,6 +38,7 @@
>
>  #include <asm/atomic.h>
>  #include <asm/cacheflush.h>
> +#include <asm/cpu.h>
>  #include <asm/cputype.h>
>  #include <asm/cpu_ops.h>
>  #include <asm/mmu_context.h>
> @@ -160,6 +161,11 @@ asmlinkage void secondary_start_kernel(void)
>         smp_store_cpu_info(cpu);
>
>         /*
> +        * Log the CPU info before it is marked online and might get read.
> +        */
> +       cpuinfo_store_cpu();
> +
> +       /*
>          * OK, now it's safe to let the boot CPU continue.  Wait for
>          * the CPU migration code to notice that the CPU is online
>          * before we continue.
> --
> 1.8.1.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] arm64: cpuinfo: print info for all CPUs

[Mark Rutland]

On Mon, May 12, 2014 at 03:54:50PM +0100, Ard Biesheuvel wrote:
> Hi Mark,
 
Hi Ard,

> On 12 May 2014 16:37, Mark Rutland <mark.rutland@arm.com> wrote:
> > Currently reading /proc/cpuinfo will result in information being read
> > out of the MIDR_EL1 of the current CPU, and the information is not
> > associated with any particular logical CPU number.
> >
> > This is problematic for systems with heterogeneous CPUs (i.e.
> > big.LITTLE) where fields will vary across CPUs, and the output will
> > differ depending on the executing CPU. Additionally the output is
> > different in format to the 32-bit ARM Linux port, where information is
> > printed out for each CPU.
> >
> > This patch adds the necessary infrastructure to log the relevant
> > registers (currently just MPIDR_EL1) and print out the logged
> > information.
> >
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Jacob Bramley <jacob.bramley@arm.com>
> > ---
> >  arch/arm64/include/asm/cpu.h | 30 +++++++++++++++++++++++++++++
> >  arch/arm64/kernel/Makefile   |  2 +-
> >  arch/arm64/kernel/cpuinfo.c  | 30 +++++++++++++++++++++++++++++
> >  arch/arm64/kernel/setup.c    | 46 +++++++++++++++++++++++---------------------
> >  arch/arm64/kernel/smp.c      |  6 ++++++
> >  5 files changed, 91 insertions(+), 23 deletions(-)
> >  create mode 100644 arch/arm64/include/asm/cpu.h
> >  create mode 100644 arch/arm64/kernel/cpuinfo.c
> >

[...]

> > -       /* dump out the processor features */
> > -       seq_puts(m, "Features\t: ");
> > +               /* dump out the processor features */
> > +               seq_puts(m, "Features\t: ");
> >
> > -       for (i = 0; hwcap_str[i]; i++)
> > -               if (elf_hwcap & (1 << i))
> > -                       seq_printf(m, "%s ", hwcap_str[i]);
> > +               for (i = 0; hwcap_str[i]; i++)
> > +                       if (elf_hwcap & (1 << i))
> > +                               seq_printf(m, "%s ", hwcap_str[i]);
> >
> 
> Considering that you are printing the contents of the same variable
> every iteration, and the fact that your subsequent patch makes sure
> elf_hwcap is in sync between all CPUs, doesn't it make more sense to
> print this line only once?

Hmm, I'd meant to update this to parse hwcaps (or display purposes)
per-cpu.

Given everyone else (at least arm and x86) print features/flags per-cpu
even when they should be identical, I'm not sure I see the point of
breaking the pattern. I'd prefer to look the same, if nothing else it's
what users expect.

Cheers,
Mark.

[PATCH 3/3] arm64: add runtime system sanity checks

[Mark Rutland]

Unexpected variation in certain system register values across CPUs is an
indicator of potential problems with a system. The kernel expects CPUs
to be mostly identical in terms of supported features, even in systems
with homogeneous CPUs, with uniform instruction set support being
critical for the correct operation of userspace.

To help detect issues early where hardware violates the expectations of
the kernel, this patch adds simple runtime sanity checks on important ID
registers in the bring up path of each CPU.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/include/asm/cpu.h | 21 +++++++++-
 arch/arm64/kernel/cpuinfo.c  | 97 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 116 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/cpu.h b/arch/arm64/include/asm/cpu.h
index 74bf9bb..33a0e70 100644
--- a/arch/arm64/include/asm/cpu.h
+++ b/arch/arm64/include/asm/cpu.h
@@ -16,11 +16,30 @@
 /*
  * Records attributes of an individual CPU.
  *
- * This is used to cache data for /proc/cpuinfo.
+ * This is used to cache data for /proc/cpuinfo and run-time sanity checks.
  */
 struct cpuinfo_arm64 {
 	struct cpu	cpu;
+	u32		reg_ctr;
+	u32		reg_cntfrq;
 	u32		reg_midr;
+
+	u64		reg_id_aa64isar0;
+	u64		reg_id_aa64mmfr0;
+	u64		reg_id_aa64pfr0;
+
+	u32		reg_id_isar0;
+	u32		reg_id_isar1;
+	u32		reg_id_isar2;
+	u32		reg_id_isar3;
+	u32		reg_id_isar4;
+	u32		reg_id_isar5;
+	u32		reg_id_mmfr0;
+	u32		reg_id_mmfr1;
+	u32		reg_id_mmfr2;
+	u32		reg_id_mmfr3;
+	u32		reg_id_pfr0;
+	u32		reg_id_pfr1;
 };
 
 DECLARE_PER_CPU(struct cpuinfo_arm64, cpu_data);
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index b9e18c5..bccee60 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -1,5 +1,6 @@
 /*
- * Record CPU attributes for later retrieval
+ * Record CPU attributes for later retrieval, and sanity-check that processor
+ * features do not vary unexpectedly.
  *
  * Copyright (C) 2014 ARM Ltd.
  * This program is free software; you can redistribute it and/or modify
@@ -15,16 +16,110 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 #include <asm/cpu.h>
+#include <asm/arch_timer.h>
 
+#include <linux/printk.h>
 #include <linux/smp.h>
+#include <linux/types.h>
 
 DEFINE_PER_CPU(struct cpuinfo_arm64, cpu_data);
 
+static void check_reg_mask(char *name, u64 mask, u64 boot, u64 cur, int cpu)
+{
+	if ((boot & mask) == (cur & mask))
+		return;
+
+	pr_warn("SANITY CHECK: Unexpected variation in %s. cpu0: %#016lx, cpu%d: %#016lx\n",
+		name, (unsigned long)boot, cpu, (unsigned long)cur);
+}
+
+#define	CHECK_MASK(field, mask, boot, cur, cpu)	\
+	check_reg_mask(#field, mask, (boot)->reg_ ## field, (cur)->reg_ ## field, cpu)
+
+#define CHECK(field, boot, cur, cpu)	\
+	CHECK_MASK(field, (u64)-1, boot, cur, cpu)
+
+/*
+ * Verify that CPUs don't have unexpected differences that will cause problems.
+ */
+void cpuinfo_sanity_check(struct cpuinfo_arm64 *cur)
+{
+	struct cpuinfo_arm64 *boot = &per_cpu(cpu_data, 0);
+	int cpu = smp_processor_id();
+
+	/*
+	 * The kernel can handle differing I-cache policies, but otherwise
+	 * caches should look identical. Userspace JITs will make use of
+	 * *minLine.
+	 */
+	CHECK_MASK(ctr, 0xffff3fff, boot, cur, cpu);
+
+	/* If different, timekeeping will be broken (especially with KVM) */
+	CHECK(cntfrq, boot, cur, cpu);
+
+	/*
+	 * Even in big.LITTLE, processors should be identical instruction-set
+	 * wise.
+	 */
+	CHECK(id_aa64isar0, boot, cur, cpu);
+
+	/*
+	 * Differing PARange support is fine as long as all peripherals and
+	 * memory are mapped within the minimum PARange of all CPUs.
+	 * Linux should not care about secure memory.
+	 */
+	CHECK_MASK(id_aa64mmfr0, 0xffffffffffff0ff0, boot, cur, cpu);
+
+	/*
+	 * EL3 is not our concern, and GIC system register support only matters
+	 * if GICv3 is in use.
+	 */
+	CHECK_MASK(id_aa64pfr0, 0xfffffffff0ff0fff, boot, cur, cpu);
+
+	/*
+	 * If we have AArch32, we care about 32-bit features for compat. These
+	 * registers should be RES0 otherwise.
+	 */
+	CHECK(id_isar0, boot, cur, cpu);
+	CHECK(id_isar1, boot, cur, cpu);
+	CHECK(id_isar2, boot, cur, cpu);
+	CHECK(id_isar3, boot, cur, cpu);
+	CHECK(id_isar4, boot, cur, cpu);
+	CHECK(id_isar5, boot, cur, cpu);
+	CHECK(id_mmfr0, boot, cur, cpu);
+	CHECK(id_mmfr1, boot, cur, cpu);
+	CHECK(id_mmfr2, boot, cur, cpu);
+	CHECK(id_mmfr3, boot, cur, cpu);
+	CHECK(id_pfr0, boot, cur, cpu);
+	CHECK(id_pfr1, boot, cur, cpu);
+}
+
 void cpuinfo_store_cpu(void)
 {
 	int cpu = smp_processor_id();
 	struct cpuinfo_arm64 *cpuinfo = &per_cpu(cpu_data, cpu);
 
+	cpuinfo->reg_ctr = read_cpuid_cachetype();
+	cpuinfo->reg_cntfrq = arch_timer_get_cntfrq();
 	cpuinfo->reg_midr = read_cpuid_id();
+
+	cpuinfo->reg_id_aa64isar0 = read_cpuid(ID_AA64ISAR0_EL1);
+	cpuinfo->reg_id_aa64mmfr0 = read_cpuid(ID_AA64MMFR0_EL1);
+	cpuinfo->reg_id_aa64pfr0 = read_cpuid(ID_AA64PFR0_EL1);
+
+	cpuinfo->reg_id_isar0 = read_cpuid(ID_ISAR0_EL1);
+	cpuinfo->reg_id_isar1 = read_cpuid(ID_ISAR1_EL1);
+	cpuinfo->reg_id_isar2 = read_cpuid(ID_ISAR2_EL1);
+	cpuinfo->reg_id_isar3 = read_cpuid(ID_ISAR3_EL1);
+	cpuinfo->reg_id_isar4 = read_cpuid(ID_ISAR4_EL1);
+	cpuinfo->reg_id_isar5 = read_cpuid(ID_ISAR5_EL1);
+	cpuinfo->reg_id_mmfr0 = read_cpuid(ID_MMFR0_EL1);
+	cpuinfo->reg_id_mmfr1 = read_cpuid(ID_MMFR1_EL1);
+	cpuinfo->reg_id_mmfr2 = read_cpuid(ID_MMFR2_EL1);
+	cpuinfo->reg_id_mmfr3 = read_cpuid(ID_MMFR3_EL1);
+	cpuinfo->reg_id_pfr0 = read_cpuid(ID_PFR0_EL1);
+	cpuinfo->reg_id_pfr1 = read_cpuid(ID_PFR1_EL1);
+
+	cpuinfo_sanity_check(cpuinfo);
 }
 
-- 
1.8.1.1

[PATCH 3/3] arm64: add runtime system sanity checks

[Will Deacon]

On Mon, May 12, 2014 at 03:37:50PM +0100, Mark Rutland wrote:
> Unexpected variation in certain system register values across CPUs is an
> indicator of potential problems with a system. The kernel expects CPUs
> to be mostly identical in terms of supported features, even in systems
> with homogeneous CPUs, with uniform instruction set support being

You mean heterogeneous, right?

> critical for the correct operation of userspace.
> 
> To help detect issues early where hardware violates the expectations of
> the kernel, this patch adds simple runtime sanity checks on important ID
> registers in the bring up path of each CPU.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> ---
>  arch/arm64/include/asm/cpu.h | 21 +++++++++-
>  arch/arm64/kernel/cpuinfo.c  | 97 +++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 116 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/cpu.h b/arch/arm64/include/asm/cpu.h
> index 74bf9bb..33a0e70 100644
> --- a/arch/arm64/include/asm/cpu.h
> +++ b/arch/arm64/include/asm/cpu.h
> @@ -16,11 +16,30 @@
>  /*
>   * Records attributes of an individual CPU.
>   *
> - * This is used to cache data for /proc/cpuinfo.
> + * This is used to cache data for /proc/cpuinfo and run-time sanity checks.
>   */
>  struct cpuinfo_arm64 {
>  	struct cpu	cpu;
> +	u32		reg_ctr;
> +	u32		reg_cntfrq;
>  	u32		reg_midr;
> +
> +	u64		reg_id_aa64isar0;
> +	u64		reg_id_aa64mmfr0;
> +	u64		reg_id_aa64pfr0;
> +
> +	u32		reg_id_isar0;
> +	u32		reg_id_isar1;
> +	u32		reg_id_isar2;
> +	u32		reg_id_isar3;
> +	u32		reg_id_isar4;
> +	u32		reg_id_isar5;
> +	u32		reg_id_mmfr0;
> +	u32		reg_id_mmfr1;
> +	u32		reg_id_mmfr2;
> +	u32		reg_id_mmfr3;
> +	u32		reg_id_pfr0;
> +	u32		reg_id_pfr1;
>  };
>  
>  DECLARE_PER_CPU(struct cpuinfo_arm64, cpu_data);
> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> index b9e18c5..bccee60 100644
> --- a/arch/arm64/kernel/cpuinfo.c
> +++ b/arch/arm64/kernel/cpuinfo.c
> @@ -1,5 +1,6 @@
>  /*
> - * Record CPU attributes for later retrieval
> + * Record CPU attributes for later retrieval, and sanity-check that processor
> + * features do not vary unexpectedly.
>   *
>   * Copyright (C) 2014 ARM Ltd.
>   * This program is free software; you can redistribute it and/or modify
> @@ -15,16 +16,110 @@
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  #include <asm/cpu.h>
> +#include <asm/arch_timer.h>
>  
> +#include <linux/printk.h>
>  #include <linux/smp.h>
> +#include <linux/types.h>
>  
>  DEFINE_PER_CPU(struct cpuinfo_arm64, cpu_data);
>  
> +static void check_reg_mask(char *name, u64 mask, u64 boot, u64 cur, int cpu)
> +{
> +	if ((boot & mask) == (cur & mask))
> +		return;
> +
> +	pr_warn("SANITY CHECK: Unexpected variation in %s. cpu0: %#016lx, cpu%d: %#016lx\n",
> +		name, (unsigned long)boot, cpu, (unsigned long)cur);

Use could use pr_fmt for the prefix.

> +}
> +
> +#define	CHECK_MASK(field, mask, boot, cur, cpu)	\
> +	check_reg_mask(#field, mask, (boot)->reg_ ## field, (cur)->reg_ ## field, cpu)
> +
> +#define CHECK(field, boot, cur, cpu)	\
> +	CHECK_MASK(field, (u64)-1, boot, cur, cpu)
> +
> +/*
> + * Verify that CPUs don't have unexpected differences that will cause problems.
> + */
> +void cpuinfo_sanity_check(struct cpuinfo_arm64 *cur)
> +{
> +	struct cpuinfo_arm64 *boot = &per_cpu(cpu_data, 0);
> +	int cpu = smp_processor_id();
> +
> +	/*
> +	 * The kernel can handle differing I-cache policies, but otherwise
> +	 * caches should look identical. Userspace JITs will make use of
> +	 * *minLine.
> +	 */
> +	CHECK_MASK(ctr, 0xffff3fff, boot, cur, cpu);

Actually, if we have different I-cache policies we need to make sure that
icache_is_aliasing reports true on all CPUs, otherwise GDB will break
(via flush_ptrace_access).

Otherwise, looks like a good series to me.

Will

[PATCH 3/3] arm64: add runtime system sanity checks

[Mark Rutland]

On Mon, May 12, 2014 at 04:11:14PM +0100, Will Deacon wrote:
> On Mon, May 12, 2014 at 03:37:50PM +0100, Mark Rutland wrote:
> > Unexpected variation in certain system register values across CPUs is an
> > indicator of potential problems with a system. The kernel expects CPUs
> > to be mostly identical in terms of supported features, even in systems
> > with homogeneous CPUs, with uniform instruction set support being
> 
> You mean heterogeneous, right?

Yes, it appears I do.

> > critical for the correct operation of userspace.
> > 
> > To help detect issues early where hardware violates the expectations of
> > the kernel, this patch adds simple runtime sanity checks on important ID
> > registers in the bring up path of each CPU.
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > ---
> >  arch/arm64/include/asm/cpu.h | 21 +++++++++-
> >  arch/arm64/kernel/cpuinfo.c  | 97 +++++++++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 116 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/cpu.h b/arch/arm64/include/asm/cpu.h
> > index 74bf9bb..33a0e70 100644
> > --- a/arch/arm64/include/asm/cpu.h
> > +++ b/arch/arm64/include/asm/cpu.h
> > @@ -16,11 +16,30 @@
> >  /*
> >   * Records attributes of an individual CPU.
> >   *
> > - * This is used to cache data for /proc/cpuinfo.
> > + * This is used to cache data for /proc/cpuinfo and run-time sanity checks.
> >   */
> >  struct cpuinfo_arm64 {
> >  	struct cpu	cpu;
> > +	u32		reg_ctr;
> > +	u32		reg_cntfrq;
> >  	u32		reg_midr;
> > +
> > +	u64		reg_id_aa64isar0;
> > +	u64		reg_id_aa64mmfr0;
> > +	u64		reg_id_aa64pfr0;
> > +
> > +	u32		reg_id_isar0;
> > +	u32		reg_id_isar1;
> > +	u32		reg_id_isar2;
> > +	u32		reg_id_isar3;
> > +	u32		reg_id_isar4;
> > +	u32		reg_id_isar5;
> > +	u32		reg_id_mmfr0;
> > +	u32		reg_id_mmfr1;
> > +	u32		reg_id_mmfr2;
> > +	u32		reg_id_mmfr3;
> > +	u32		reg_id_pfr0;
> > +	u32		reg_id_pfr1;
> >  };
> >  
> >  DECLARE_PER_CPU(struct cpuinfo_arm64, cpu_data);
> > diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> > index b9e18c5..bccee60 100644
> > --- a/arch/arm64/kernel/cpuinfo.c
> > +++ b/arch/arm64/kernel/cpuinfo.c
> > @@ -1,5 +1,6 @@
> >  /*
> > - * Record CPU attributes for later retrieval
> > + * Record CPU attributes for later retrieval, and sanity-check that processor
> > + * features do not vary unexpectedly.
> >   *
> >   * Copyright (C) 2014 ARM Ltd.
> >   * This program is free software; you can redistribute it and/or modify
> > @@ -15,16 +16,110 @@
> >   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> >   */
> >  #include <asm/cpu.h>
> > +#include <asm/arch_timer.h>
> >  
> > +#include <linux/printk.h>
> >  #include <linux/smp.h>
> > +#include <linux/types.h>
> >  
> >  DEFINE_PER_CPU(struct cpuinfo_arm64, cpu_data);
> >  
> > +static void check_reg_mask(char *name, u64 mask, u64 boot, u64 cur, int cpu)
> > +{
> > +	if ((boot & mask) == (cur & mask))
> > +		return;
> > +
> > +	pr_warn("SANITY CHECK: Unexpected variation in %s. cpu0: %#016lx, cpu%d: %#016lx\n",
> > +		name, (unsigned long)boot, cpu, (unsigned long)cur);
> 
> Use could use pr_fmt for the prefix.

Originally I was going to fold the /proc/cpuinfo seq_file code in here
too (which I'd still like to do), for which I didn't want the "SANITY
CHECK" prefix. It doesn't look like that affects seq_printf though, so I
guess I can.

> 
> > +}
> > +
> > +#define	CHECK_MASK(field, mask, boot, cur, cpu)	\
> > +	check_reg_mask(#field, mask, (boot)->reg_ ## field, (cur)->reg_ ## field, cpu)
> > +
> > +#define CHECK(field, boot, cur, cpu)	\
> > +	CHECK_MASK(field, (u64)-1, boot, cur, cpu)
> > +
> > +/*
> > + * Verify that CPUs don't have unexpected differences that will cause problems.
> > + */
> > +void cpuinfo_sanity_check(struct cpuinfo_arm64 *cur)
> > +{
> > +	struct cpuinfo_arm64 *boot = &per_cpu(cpu_data, 0);
> > +	int cpu = smp_processor_id();
> > +
> > +	/*
> > +	 * The kernel can handle differing I-cache policies, but otherwise
> > +	 * caches should look identical. Userspace JITs will make use of
> > +	 * *minLine.
> > +	 */
> > +	CHECK_MASK(ctr, 0xffff3fff, boot, cur, cpu);
> 
> Actually, if we have different I-cache policies we need to make sure that
> icache_is_aliasing reports true on all CPUs, otherwise GDB will break
> (via flush_ptrace_access).

That's a point. I'll go and investigate that.

Cheers,
Mark.