[PATCHv4 0/5] arm64: handle heterogeneous system register values

[PATCHv4 0/5] arm64: handle heterogeneous system register values

[Mark Rutland]

Currently the arm64 kernel assumes CPUs are homogeneous, which is a source of
several potential problems as this is not always true. This series is an
attempt to ameliorate the situation. The first posting [1] was titled as
"arm64: extend/fix /proc/cpuinfo + runtime sanity checks", but with the
addition of the I-cache policy fix this title was either going to become
unwieldy or misleading.

The major addition is a couple of hooks to log system register values, and
sanity checking of our assumptions regarding the underlying hardware. If these
are violated it is made very clear to the user that things may be broken.

Due to rework between v2 an 3 I've dropped Acks from some patches. A once-over
from those who have reviewed this before would be much appreciated.

Due to some off-list comments I've restored the printing of features per-cpu
(which was also my initial preference). While this is still based on the global
hwcaps, this will be correct where the sanity checks don't fail. Hopefully
no-one is insane enough to build CPUs with different instruction set support
anyhow.

I'd like to see at least patches 1-4 go in shortly as they've been stable for a
while now, have received no major complaints, and will be of use immediately
for those involved in CPU bringup. I understand people might not like patch 5,
but that shouldn't delay the rest of the series.

Since v3 [5]:
* Minor cleanups suggested by Will.
* Print hwcaps per-cpu again.
* Clean up other issues in /proc/cpuinfo.
* Move the cpuinfo patch to the end.
Since v2 [4]:
* Print the hwcaps once in /proc/cpinfo, per Ard's comments.
* Apply fixupps per Will's comments.
* Sanity check additional registers: dczid id_aa64{isar,mmfr,pfr}1.
* We care about GICv3 support reporting, don't mask it out.
* Set TAINT_CPU_OUT_OF_SPEC upon sanity check failure.
* Store boot CPU values separately.
* Separate value stashing into new patch.
* Rebased the branch to v3.16-rc2.
Since v1 [1]:
* Add fix for differing I-cache policies
* Make read_cpuid_part_number and read_cpu_implementor return extracted fields.

This series adds infrastructure to record various system registers upon the
booting of each CPU into a new per-CPU struct cpuinfo_arm64. This data can then
be used to solve various problems:

* 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 can't
  discover what CPU implementations we have. Using the newly recorded
  information, we can print correct information per-CPU in /proc/cpuinfo.

* Level 1 instruction cache policy can differ across CPU micro-architectures in
  big.LITTLE configurations, which can lead to a subset of cores reporting
  aliasing I-caches in CTR_EL0.L1Ip. If any CPUs in the system have VIPT or
  AIVIVT I-caches, the appropriate maintenance must be performed even from those
  CPUs whose caches are non-aliasing. We can use the newly recorded information
  to detect when this is the case.

* Sometimes CPUs are erroneously configured differently by firmware such that a
  feature is only enabled on a subset of CPUs. Even in the presence of
  big.LITTLE systems we do not generally expect this, and such problems can be
  difficult to detect. To enable early detection of mismatched capabilities in
  SMP systems, we can use the newly recorded information to compare CPUs
  feature-wise and identify potential issues.

The series is based on v3.16-rc2, and can be found in my
arm64/heterogeneous-sysreg branch [2,3]. It should cleanly rebase to later
release candidates.

Cheers,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/255389.html
[2] http://linux-arm.org/git?p=linux-mr.git;a=shortlog;h=refs/heads/arm64/heterogeneous-sysreg
[3] git://linux-arm.org/linux-mr.git arm64/heterogeneous-sysreg
[4] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/264387.html
[5] lists.infradead.org/pipermail/linux-arm-kernel/2014-June/266973.html

Mark Rutland (5):
  arm64: add MIDR_EL1 field accessors
  arm64: cpuinfo: record cpu system register values
  arm64: cachetype: report weakest cache policy
  arm64: add runtime system sanity checks
  arm64: cpuinfo: print info for all CPUs

 arch/arm64/include/asm/cachetype.h |  16 ++--
 arch/arm64/include/asm/cpu.h       |  59 ++++++++++++
 arch/arm64/include/asm/cputype.h   |  33 +++++--
 arch/arm64/kernel/Makefile         |   3 +-
 arch/arm64/kernel/cpuinfo.c        | 192 +++++++++++++++++++++++++++++++++++++
 arch/arm64/kernel/setup.c          |  46 ++++-----
 arch/arm64/kernel/smp.c            |   6 ++
 7 files changed, 316 insertions(+), 39 deletions(-)
 create mode 100644 arch/arm64/include/asm/cpu.h
 create mode 100644 arch/arm64/kernel/cpuinfo.c

-- 
1.9.1

[PATCHv4 1/5] 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.

The read_cpuid_part_number macro is modified to return the extracted
bitfield rather than returning the value in-place with all other fields
(including revision) masked out, to better match the other accessors.
As the value is only used in comparison with the *_CPU_PART_* macros
which are similarly updated, and these values are never exposed to
userspace, this change should not affect any functionality.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/cputype.h | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index 27f54a7..ec5e41c 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -36,15 +36,34 @@
 	__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
 
-#define ARM_CPU_PART_AEM_V8	0xD0F0
-#define ARM_CPU_PART_FOUNDATION	0xD000
-#define ARM_CPU_PART_CORTEX_A53	0xD030
-#define ARM_CPU_PART_CORTEX_A57	0xD070
+#define ARM_CPU_PART_AEM_V8	0xD0F
+#define ARM_CPU_PART_FOUNDATION	0xD00
+#define ARM_CPU_PART_CORTEX_A57	0xD07
+#define ARM_CPU_PART_CORTEX_A53	0xD03
 
-#define APM_CPU_PART_POTENZA	0x0000
+#define APM_CPU_PART_POTENZA	0x000
 
 #ifndef __ASSEMBLY__
 
@@ -65,12 +84,12 @@ 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);
+	return MIDR_PARTNUM(read_cpuid_id());
 }
 
 static inline u32 __attribute_const__ read_cpuid_cachetype(void)
-- 
1.9.1

[PATCHv4 2/5] arm64: cpuinfo: record cpu system register values

[Mark Rutland]

Several kernel subsystems need to know details about CPU system register
values, sometimes for CPUs other than that they are executing on. Rather
than hard-coding system register accesses and cross-calls for these
cases, this patch adds logic to record various system register values at
boot-time. This may be used for feature reporting, firmware bug
detection, etc.

Separate hooks are added for the boot and hotplug paths to enable
one-time intialisation and cold/warm boot value mismatch detection in
later patches.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/cpu.h | 59 +++++++++++++++++++++++++++++++++++
 arch/arm64/kernel/Makefile   |  3 +-
 arch/arm64/kernel/cpuinfo.c  | 73 ++++++++++++++++++++++++++++++++++++++++++++
 arch/arm64/kernel/setup.c    |  7 +++--
 arch/arm64/kernel/smp.c      |  6 ++++
 5 files changed, 144 insertions(+), 4 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..0564430
--- /dev/null
+++ b/arch/arm64/include/asm/cpu.h
@@ -0,0 +1,59 @@
+/*
+  * 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/>.
+ */
+#ifndef __ASM_CPU_H
+#define __ASM_CPU_H
+
+#include <linux/cpu.h>
+#include <linux/init.h>
+#include <linux/percpu.h>
+
+/*
+ * Records attributes of an individual CPU.
+ */
+struct cpuinfo_arm64 {
+	struct cpu	cpu;
+	u32		reg_ctr;
+	u32		reg_cntfrq;
+	u32		reg_dczid;
+	u32		reg_midr;
+
+	u64		reg_id_aa64isar0;
+	u64		reg_id_aa64isar1;
+	u64		reg_id_aa64mmfr0;
+	u64		reg_id_aa64mmfr1;
+	u64		reg_id_aa64pfr0;
+	u64		reg_id_aa64pfr1;
+
+	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);
+
+void cpuinfo_store_cpu(void);
+void __init cpuinfo_store_boot_cpu(void);
+
+#endif /* __ASM_CPU_H */
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index cdaedad..27c72ef 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -15,7 +15,8 @@ CFLAGS_REMOVE_return_address.o = -pg
 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 return_address.o
+			   hyp-stub.o psci.o cpu_ops.o insn.o return_address.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..1f350fe
--- /dev/null
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -0,0 +1,73 @@
+/*
+ * Record and handle CPU attributes.
+ *
+ * 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/arch_timer.h>
+#include <asm/cachetype.h>
+#include <asm/cpu.h>
+#include <asm/cputype.h>
+
+#include <linux/init.h>
+#include <linux/smp.h>
+
+/*
+ * In case the boot CPU is hotpluggable, we record its initial state and
+ * current state separately. Certain system registers may contain different
+ * values depending on configuration at or after reset.
+ */
+DEFINE_PER_CPU(struct cpuinfo_arm64, cpu_data);
+static struct cpuinfo_arm64 boot_cpu_data;
+
+static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info)
+{
+	info->reg_cntfrq = arch_timer_get_cntfrq();
+	info->reg_ctr = read_cpuid_cachetype();
+	info->reg_dczid = read_cpuid(DCZID_EL0);
+	info->reg_midr = read_cpuid_id();
+
+	info->reg_id_aa64isar0 = read_cpuid(ID_AA64ISAR0_EL1);
+	info->reg_id_aa64isar1 = read_cpuid(ID_AA64ISAR1_EL1);
+	info->reg_id_aa64mmfr0 = read_cpuid(ID_AA64MMFR0_EL1);
+	info->reg_id_aa64mmfr1 = read_cpuid(ID_AA64MMFR1_EL1);
+	info->reg_id_aa64pfr0 = read_cpuid(ID_AA64PFR0_EL1);
+	info->reg_id_aa64pfr1 = read_cpuid(ID_AA64PFR1_EL1);
+
+	info->reg_id_isar0 = read_cpuid(ID_ISAR0_EL1);
+	info->reg_id_isar1 = read_cpuid(ID_ISAR1_EL1);
+	info->reg_id_isar2 = read_cpuid(ID_ISAR2_EL1);
+	info->reg_id_isar3 = read_cpuid(ID_ISAR3_EL1);
+	info->reg_id_isar4 = read_cpuid(ID_ISAR4_EL1);
+	info->reg_id_isar5 = read_cpuid(ID_ISAR5_EL1);
+	info->reg_id_mmfr0 = read_cpuid(ID_MMFR0_EL1);
+	info->reg_id_mmfr1 = read_cpuid(ID_MMFR1_EL1);
+	info->reg_id_mmfr2 = read_cpuid(ID_MMFR2_EL1);
+	info->reg_id_mmfr3 = read_cpuid(ID_MMFR3_EL1);
+	info->reg_id_pfr0 = read_cpuid(ID_PFR0_EL1);
+	info->reg_id_pfr1 = read_cpuid(ID_PFR1_EL1);
+}
+
+void cpuinfo_store_cpu(void)
+{
+	struct cpuinfo_arm64 *info = this_cpu_ptr(&cpu_data);
+	__cpuinfo_store_cpu(info);
+}
+
+void __init cpuinfo_store_boot_cpu(void)
+{
+	struct cpuinfo_arm64 *info = &per_cpu(cpu_data, 0);
+	__cpuinfo_store_cpu(info);
+
+	boot_cpu_data = *info;
+}
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 46d1125..edb146d 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -45,6 +45,7 @@
 #include <linux/efi.h>
 
 #include <asm/fixmap.h>
+#include <asm/cpu.h>
 #include <asm/cputype.h>
 #include <asm/elf.h>
 #include <asm/cputable.h>
@@ -219,6 +220,8 @@ static void __init setup_processor(void)
 	sprintf(init_utsname()->machine, ELF_PLATFORM);
 	elf_hwcap = 0;
 
+	cpuinfo_store_boot_cpu();
+
 	/*
 	 * Check for sane CTR_EL0.CWG value.
 	 */
@@ -417,14 +420,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);
 	}
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 40f38f4..3e2f5eb 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -39,6 +39,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>
@@ -155,6 +156,11 @@ asmlinkage void secondary_start_kernel(void)
 		cpu_ops[cpu]->cpu_postboot();
 
 	/*
+	 * Log the CPU info before it is marked online and might get read.
+	 */
+	cpuinfo_store_cpu();
+
+	/*
 	 * Enable GIC and timers.
 	 */
 	notify_cpu_starting(cpu);
-- 
1.9.1

[PATCHv4 3/5] arm64: cachetype: report weakest cache policy

[Mark Rutland]

In big.LITTLE systems, the I-cache policy may differ across CPUs, and
thus we must always meet the most stringent maintenance requirements of
any I-cache in the system when performing maintenance to ensure
correctness. Unfortunately this requirement is not met as we always look
at the current CPU's cache type register to determine the maintenance
requirements.

This patch causes the I-cache policy of all CPUs to be taken into
account for icache_is_aliasing and icache_is_aivivt. If any I-cache in
the system is aliasing or AIVIVT, the respective function will return
true. At boot each CPU may set flags to identify that at least one
I-cache in the system is aliasing and/or AIVIVT.

The now unused and potentially misleading icache_policy function is
removed.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/cachetype.h | 16 ++++++++++------
 arch/arm64/kernel/cpuinfo.c        | 26 ++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/cachetype.h b/arch/arm64/include/asm/cachetype.h
index 4b23e75..7a2e076 100644
--- a/arch/arm64/include/asm/cachetype.h
+++ b/arch/arm64/include/asm/cachetype.h
@@ -30,10 +30,14 @@
 
 #ifndef __ASSEMBLY__
 
-static inline u32 icache_policy(void)
-{
-	return (read_cpuid_cachetype() >> CTR_L1IP_SHIFT) & CTR_L1IP_MASK;
-}
+#include <linux/bitops.h>
+
+#define CTR_L1IP(ctr)	(((ctr) >> CTR_L1IP_SHIFT) & CTR_L1IP_MASK)
+
+#define ICACHEF_ALIASING	BIT(0)
+#define ICACHEF_AIVIVT		BIT(1)
+
+extern unsigned long __icache_flags;
 
 /*
  * Whilst the D-side always behaves as PIPT on AArch64, aliasing is
@@ -41,12 +45,12 @@ static inline u32 icache_policy(void)
  */
 static inline int icache_is_aliasing(void)
 {
-	return icache_policy() != ICACHE_POLICY_PIPT;
+	return test_bit(ICACHEF_ALIASING, &__icache_flags);
 }
 
 static inline int icache_is_aivivt(void)
 {
-	return icache_policy() == ICACHE_POLICY_AIVIVT;
+	return test_bit(ICACHEF_AIVIVT, &__icache_flags);
 }
 
 static inline u32 cache_type_cwg(void)
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index 1f350fe..3ce99fc 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -19,7 +19,9 @@
 #include <asm/cpu.h>
 #include <asm/cputype.h>
 
+#include <linux/bitops.h>
 #include <linux/init.h>
+#include <linux/printk.h>
 #include <linux/smp.h>
 
 /*
@@ -30,6 +32,28 @@
 DEFINE_PER_CPU(struct cpuinfo_arm64, cpu_data);
 static struct cpuinfo_arm64 boot_cpu_data;
 
+static char *icache_policy_str[] = {
+	[ICACHE_POLICY_RESERVED] = "RESERVED/UNKNOWN",
+	[ICACHE_POLICY_AIVIVT] = "AIVIVT",
+	[ICACHE_POLICY_VIPT] = "VIPT",
+	[ICACHE_POLICY_PIPT] = "PIPT",
+};
+
+unsigned long __icache_flags;
+
+static void cpuinfo_detect_icache_policy(struct cpuinfo_arm64 *info)
+{
+	unsigned int cpu = smp_processor_id();
+	u32 l1ip = CTR_L1IP(info->reg_ctr);
+
+	if (l1ip != ICACHE_POLICY_PIPT)
+		set_bit(ICACHEF_ALIASING, &__icache_flags);
+	if (l1ip == ICACHE_POLICY_AIVIVT);
+		set_bit(ICACHEF_AIVIVT, &__icache_flags);
+
+	pr_info("Detected %s I-cache on CPU%d", icache_policy_str[l1ip], cpu);
+}
+
 static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info)
 {
 	info->reg_cntfrq = arch_timer_get_cntfrq();
@@ -56,6 +80,8 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info)
 	info->reg_id_mmfr3 = read_cpuid(ID_MMFR3_EL1);
 	info->reg_id_pfr0 = read_cpuid(ID_PFR0_EL1);
 	info->reg_id_pfr1 = read_cpuid(ID_PFR1_EL1);
+
+	cpuinfo_detect_icache_policy(info);
 }
 
 void cpuinfo_store_cpu(void)
-- 
1.9.1

[PATCHv4 4/5] 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 heterogeneous 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.

Where CPUs are fundamentally mismatched, set TAINT_CPU_OUT_OF_SPEC.
Given that the kernel assumes CPUs are identical feature wise, let's not
pretend that we expect such configurations to work. Supporting such
configurations would require massive rework, and hopefully they will
never exist.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/kernel/cpuinfo.c | 93 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index 3ce99fc..f82f7d1 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -21,6 +21,7 @@
 
 #include <linux/bitops.h>
 #include <linux/init.h>
+#include <linux/kernel.h>
 #include <linux/printk.h>
 #include <linux/smp.h>
 
@@ -54,6 +55,97 @@ static void cpuinfo_detect_icache_policy(struct cpuinfo_arm64 *info)
 	pr_info("Detected %s I-cache on CPU%d", icache_policy_str[l1ip], cpu);
 }
 
+static int check_reg_mask(char *name, u64 mask, u64 boot, u64 cur, int cpu)
+{
+	if ((boot & mask) == (cur & mask))
+		return 0;
+
+	pr_warn("SANITY CHECK: Unexpected variation in %s. Boot CPU: %#016lx, CPU%d: %#016lx\n",
+		name, (unsigned long)boot, cpu, (unsigned long)cur);
+
+	return 1;
+}
+
+#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, ~0ULL, boot, cur, cpu)
+
+/*
+ * Verify that CPUs don't have unexpected differences that will cause problems.
+ */
+static void cpuinfo_sanity_check(struct cpuinfo_arm64 *cur)
+{
+	unsigned int cpu = smp_processor_id();
+	struct cpuinfo_arm64 *boot = &boot_cpu_data;
+	unsigned int diff = 0;
+
+	/*
+	 * The kernel can handle differing I-cache policies, but otherwise
+	 * caches should look identical. Userspace JITs will make use of
+	 * *minLine.
+	 */
+	diff |= CHECK_MASK(ctr, 0xffff3fff, boot, cur, cpu);
+
+	/*
+	 * Userspace may perform DC ZVA instructions. Mismatched block sizes
+	 * could result in too much or too little memory being zeroed if a
+	 * process is preempted and migrated between CPUs.
+	 */
+	diff |= CHECK(dczid, boot, cur, cpu);
+
+	/* If different, timekeeping will be broken (especially with KVM) */
+	diff |= CHECK(cntfrq, boot, cur, cpu);
+
+	/*
+	 * Even in big.LITTLE, processors should be identical instruction-set
+	 * wise.
+	 */
+	diff |= CHECK(id_aa64isar0, boot, cur, cpu);
+	diff |= CHECK(id_aa64isar1, 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.
+	 * ID_AA64MMFR1 is currently RES0.
+	 */
+	diff |= CHECK_MASK(id_aa64mmfr0, 0xffffffffffff0ff0, boot, cur, cpu);
+	diff |= CHECK(id_aa64mmfr1, boot, cur, cpu);
+
+	/*
+	 * EL3 is not our concern.
+	 * ID_AA64PFR1 is currently RES0.
+	 */
+	diff |= CHECK_MASK(id_aa64pfr0, 0xffffffffffff0fff, boot, cur, cpu);
+	diff |= CHECK(id_aa64pfr1, boot, cur, cpu);
+
+	/*
+	 * If we have AArch32, we care about 32-bit features for compat. These
+	 * registers should be RES0 otherwise.
+	 */
+	diff |= CHECK(id_isar0, boot, cur, cpu);
+	diff |= CHECK(id_isar1, boot, cur, cpu);
+	diff |= CHECK(id_isar2, boot, cur, cpu);
+	diff |= CHECK(id_isar3, boot, cur, cpu);
+	diff |= CHECK(id_isar4, boot, cur, cpu);
+	diff |= CHECK(id_isar5, boot, cur, cpu);
+	diff |= CHECK(id_mmfr0, boot, cur, cpu);
+	diff |= CHECK(id_mmfr1, boot, cur, cpu);
+	diff |= CHECK(id_mmfr2, boot, cur, cpu);
+	diff |= CHECK(id_mmfr3, boot, cur, cpu);
+	diff |= CHECK(id_pfr0, boot, cur, cpu);
+	diff |= CHECK(id_pfr1, boot, cur, cpu);
+
+	/*
+	 * Mismatched CPU features are a recipe for disaster. Don't even
+	 * pretend to support them.
+	 */
+	WARN_TAINT_ONCE(diff, TAINT_CPU_OUT_OF_SPEC,
+			"Unsupported CPU feature variation.");
+}
+
 static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info)
 {
 	info->reg_cntfrq = arch_timer_get_cntfrq();
@@ -88,6 +180,7 @@ void cpuinfo_store_cpu(void)
 {
 	struct cpuinfo_arm64 *info = this_cpu_ptr(&cpu_data);
 	__cpuinfo_store_cpu(info);
+	cpuinfo_sanity_check(info);
 }
 
 void __init cpuinfo_store_boot_cpu(void)
-- 
1.9.1

[PATCHv4 5/5] 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 MIDR fields will vary across CPUs, and the output will
differ depending on the executing CPU.

This patch reorganises the code responsible for /proc/cpuinfo to print
information per-cpu. In the process, we perform several cleanups:

* Property names are coerced to lower-case (to match "processor" as per
  glibc's expectations).
* Property names are simplified and made to match the MIDR field names.
* Revision is changed to hex as with every other field.
* The meaningless Architecture property is removed.
* The ripe-for-abuse Machine field is removed.

The features are printed per-cpu to match the format used by other
architectures, and are derived from the (globally uniform) hwcaps. In
cases where this may report incorrect information, rework is required
elsewhere to function with varying instruction set support, and the
sanity checks should provide us with some advance notice (warnings and
TAINT_CPU_OUT_OF_SPEC). If we're lucky, such systems will never exist.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Marcus Shawcroft <marcus.shawcroft@arm.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/setup.c | 39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index edb146d..aa1b4f7 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -448,39 +448,34 @@ static const char *hwcap_str[] = {
 
 static int c_show(struct seq_file *m, void *v)
 {
-	int i;
+	int c, i;
 
-	seq_printf(m, "Processor\t: %s rev %d (%s)\n",
-		   cpu_name, read_cpuid_id() & 15, ELF_PLATFORM);
+	for_each_online_cpu(c) {
+		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, "implementer\t: 0x%02x\n",
+			   MIDR_IMPLEMENTOR(midr));
+		seq_printf(m, "variant\t\t: 0x%x\n", MIDR_VARIANT(midr));
+		seq_printf(m, "partnum\t\t: 0x%03x\n", MIDR_PARTNUM(midr));
+		seq_printf(m, "revision\t: 0x%x\n", MIDR_REVISION(midr));
+
+		/* 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]);
+		seq_puts(m, "\n\n");
 	}
 
-	/* 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]);
-
-	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_puts(m, "\n");
-
-	seq_printf(m, "Hardware\t: %s\n", machine_name);
-
 	return 0;
 }
 
-- 
1.9.1

[PATCHv4 0/5] arm64: handle heterogeneous system register values

[Will Deacon]

On Wed, Jul 16, 2014 at 04:32:42PM +0100, Mark Rutland wrote:
> Currently the arm64 kernel assumes CPUs are homogeneous, which is a source of
> several potential problems as this is not always true. This series is an
> attempt to ameliorate the situation. The first posting [1] was titled as
> "arm64: extend/fix /proc/cpuinfo + runtime sanity checks", but with the
> addition of the I-cache policy fix this title was either going to become
> unwieldy or misleading.

For 1-4:

  Reviewed-by: Will Deacon <will.deacon@arm.com>

Will

[PATCHv4 5/5] arm64: cpuinfo: print info for all CPUs

[Will Deacon]

On Wed, Jul 16, 2014 at 04:32:47PM +0100, Mark Rutland wrote:
> The features are printed per-cpu to match the format used by other
> architectures, and are derived from the (globally uniform) hwcaps. In
> cases where this may report incorrect information, rework is required
> elsewhere to function with varying instruction set support, and the
> sanity checks should provide us with some advance notice (warnings and
> TAINT_CPU_OUT_OF_SPEC). If we're lucky, such systems will never exist.

[...]

> -	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, "implementer\t: 0x%02x\n",
> +			   MIDR_IMPLEMENTOR(midr));
> +		seq_printf(m, "variant\t\t: 0x%x\n", MIDR_VARIANT(midr));
> +		seq_printf(m, "partnum\t\t: 0x%03x\n", MIDR_PARTNUM(midr));
> +		seq_printf(m, "revision\t: 0x%x\n", MIDR_REVISION(midr));
> +
> +		/* 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]);

I don't have hugely strong opinions about this, but I don't see why it's
useful to print exactly the same line out `n' times; once for each CPU. We
only pass one set of hwcaps to ELF executables via auxv, so why do we need
to duplicate things here?

Put another way, we're really treating the hwcaps as a system property
rather than a per-cpu property, so I think we should handle them as such.

Will

[PATCHv4 5/5] arm64: cpuinfo: print info for all CPUs

[Catalin Marinas]

On Wed, Jul 16, 2014 at 04:57:47PM +0100, Will Deacon wrote:
> On Wed, Jul 16, 2014 at 04:32:47PM +0100, Mark Rutland wrote:
> > The features are printed per-cpu to match the format used by other
> > architectures, and are derived from the (globally uniform) hwcaps. In
> > cases where this may report incorrect information, rework is required
> > elsewhere to function with varying instruction set support, and the
> > sanity checks should provide us with some advance notice (warnings and
> > TAINT_CPU_OUT_OF_SPEC). If we're lucky, such systems will never exist.
> 
> [...]
> 
> > -	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, "implementer\t: 0x%02x\n",
> > +			   MIDR_IMPLEMENTOR(midr));
> > +		seq_printf(m, "variant\t\t: 0x%x\n", MIDR_VARIANT(midr));
> > +		seq_printf(m, "partnum\t\t: 0x%03x\n", MIDR_PARTNUM(midr));
> > +		seq_printf(m, "revision\t: 0x%x\n", MIDR_REVISION(midr));
> > +
> > +		/* 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]);
> 
> I don't have hugely strong opinions about this, but I don't see why it's
> useful to print exactly the same line out `n' times; once for each CPU. We
> only pass one set of hwcaps to ELF executables via auxv, so why do we need
> to duplicate things here?
> 
> Put another way, we're really treating the hwcaps as a system property
> rather than a per-cpu property, so I think we should handle them as such.

I agree. An argument would be that we expose per-cpu hwcap in
/proc/cpuinfo so that whoever (human) looks at this can get an idea of
what features are missing on some CPUs. The elf_hwcap exported to user
via envp should only contain the common subset.

But my worry is that code will start reading /proc/cpuinfo and make the
wrong assumptions on heterogeneous systems, so I agree with Will's
proposal of only printing the (common) CPU features once as a system
property made available by the kernel.

-- 
Catalin

[PATCHv4 5/5] arm64: cpuinfo: print info for all CPUs

[Peter Maydell]

On 16 July 2014 16:57, Will Deacon <will.deacon@arm.com> wrote:
> I don't have hugely strong opinions about this, but I don't see why it's
> useful to print exactly the same line out `n' times; once for each CPU. We
> only pass one set of hwcaps to ELF executables via auxv, so why do we need
> to duplicate things here?

I think there are a couple of lines of argument here:

(1) Precedent from other architectures. Both 32-bit ARM and
x86 have the feature-info be per-core; we should do the same
for 64-bit ARM unless there's a really strong reason not to.

(2) Making this be not-per-CPU backs us into a corner. If we
have the feature flags per-core, and it turns out that they're
never ever different between cores even on heterogenous
CPUs, there's no problem. If we have the feature flags be
listed only once, and then in future we do want to support
some minor form of heterogeny between cores, then we're
stuck with a compatibility break because the format doesn't
let us express that. (Perhaps there might be a big.LITTLE
system where only the big cores had the crypto extensions,
to pick a random possibility where the kernel doesn't need to
care but userspace could take advantage by restricting itself
to running on only the big cores). I think this makes the
choice pretty straightforward, since there's basically no
benefit to having only a single features line.

(3) CPU features really are per-core, so collapsing them
down into a single line is not consistent with the line this
series otherwise takes of "we just report the information
the hardware provides without interpretation" (in particular
the refusal to provide nice human readable CPU names).

> Put another way, we're really treating the hwcaps as a system property
> rather than a per-cpu property, so I think we should handle them as such.

This is an implementation detail of the kernel, so you shouldn't
be gratuitously exposing it to userspace if you can easily
avoid doing so, as here. (The ELF HWCAP ABI obviously
does assume homogeny and would need fixing in a future
more heterogenous world.)

If you wanted you could print a single "lowest common
denominator features" line in addition to the per-CPU
features information.

thanks
-- PMM

[PATCHv4 5/5] arm64: cpuinfo: print info for all CPUs

[Marcus Shawcroft]

On 17/07/14 11:39, Peter Maydell wrote:
> On 16 July 2014 16:57, Will Deacon <will.deacon@arm.com> wrote:
>> I don't have hugely strong opinions about this, but I don't see why it's
>> useful to print exactly the same line out `n' times; once for each CPU. We
>> only pass one set of hwcaps to ELF executables via auxv, so why do we need
>> to duplicate things here?
>
> I think there are a couple of lines of argument here:
>
> (1) Precedent from other architectures. Both 32-bit ARM and
> x86 have the feature-info be per-core; we should do the same
> for 64-bit ARM unless there's a really strong reason not to.

Agreed.

> (2) Making this be not-per-CPU backs us into a corner. If we
> have the feature flags per-core, and it turns out that they're
> never ever different between cores even on heterogenous
> CPUs, there's no problem. If we have the feature flags be
> listed only once, and then in future we do want to support
> some minor form of heterogeny between cores, then we're
> stuck with a compatibility break because the format doesn't
> let us express that. (Perhaps there might be a big.LITTLE
> system where only the big cores had the crypto extensions,
> to pick a random possibility where the kernel doesn't need to
> care but userspace could take advantage by restricting itself
> to running on only the big cores). I think this makes the
> choice pretty straightforward, since there's basically no
> benefit to having only a single features line.

Agreed.

This patch is in itself an ABI break for anyone consuming /proc/cpuinfo. 
  Making this be not-per-cpu just sets us up for another ABI break in 
the future, not ideal.

Cheers
/Marcus

[PATCHv4 5/5] arm64: cpuinfo: print info for all CPUs

[Will Deacon]

On Thu, Jul 17, 2014 at 11:46:24AM +0100, Marcus Shawcroft wrote:
> On 17/07/14 11:39, Peter Maydell wrote:
> > On 16 July 2014 16:57, Will Deacon <will.deacon@arm.com> wrote:
> >> I don't have hugely strong opinions about this, but I don't see why it's
> >> useful to print exactly the same line out `n' times; once for each CPU. We
> >> only pass one set of hwcaps to ELF executables via auxv, so why do we need
> >> to duplicate things here?
> >
> > I think there are a couple of lines of argument here:
> >
> > (1) Precedent from other architectures. Both 32-bit ARM and
> > x86 have the feature-info be per-core; we should do the same
> > for 64-bit ARM unless there's a really strong reason not to.
> 
> Agreed.

I don't really see the benefits of pretending that /proc/cpuinfo is remotely
portable between architectures.

> > (2) Making this be not-per-CPU backs us into a corner. If we
> > have the feature flags per-core, and it turns out that they're
> > never ever different between cores even on heterogenous
> > CPUs, there's no problem. If we have the feature flags be
> > listed only once, and then in future we do want to support
> > some minor form of heterogeny between cores, then we're
> > stuck with a compatibility break because the format doesn't
> > let us express that. (Perhaps there might be a big.LITTLE
> > system where only the big cores had the crypto extensions,
> > to pick a random possibility where the kernel doesn't need to
> > care but userspace could take advantage by restricting itself
> > to running on only the big cores). I think this makes the
> > choice pretty straightforward, since there's basically no
> > benefit to having only a single features line.
> 
> Agreed.

So the real question is: do we want to allow Linux to support features that
exist only on a subset of cores in the system? The current thinking is that
we truncate the advertised features to the common system subset, which means
it will be the same on all cores, by definition. That allows hardware guys
to build crazy systems that we can at least use, without imparting a world
of pain onto software that needs to run on them.

I also think that we're backed into a corner regardless. We've seen how
people ignore hwcaps (e.g. SWP), so they're likely to parse the caps for
CPU0 and use that as an indication of the system capabilities.

Note that the features list *isn't* just the features supported by the CPU.
It also takes into account the set of features supported by the kernel (e.g.
we don't advertise VFP on ARMv7 if VFP context switching isn't enabled).

Will

[PATCHv4 0/5] arm64: handle heterogeneous system register values

[Catalin Marinas]

On Wed, Jul 16, 2014 at 04:32:42PM +0100, Mark Rutland wrote:
> Mark Rutland (5):
>   arm64: add MIDR_EL1 field accessors
>   arm64: cpuinfo: record cpu system register values
>   arm64: cachetype: report weakest cache policy
>   arm64: add runtime system sanity checks

For these 4 patches:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

>   arm64: cpuinfo: print info for all CPUs

This is still up for discussion.

-- 
Catalin

[PATCHv4 5/5] arm64: cpuinfo: print info for all CPUs

[Ard Biesheuvel]

> On 17 jul. 2014, at 12:54, Will Deacon <will.deacon@arm.com> wrote:
> 
>> On Thu, Jul 17, 2014 at 11:46:24AM +0100, Marcus Shawcroft wrote:
>>> On 17/07/14 11:39, Peter Maydell wrote:
>>>> On 16 July 2014 16:57, Will Deacon <will.deacon@arm.com> wrote:
>>>> I don't have hugely strong opinions about this, but I don't see why it's
>>>> useful to print exactly the same line out `n' times; once for each CPU. We
>>>> only pass one set of hwcaps to ELF executables via auxv, so why do we need
>>>> to duplicate things here?
>>> 
>>> I think there are a couple of lines of argument here:
>>> 
>>> (1) Precedent from other architectures. Both 32-bit ARM and
>>> x86 have the feature-info be per-core; we should do the same
>>> for 64-bit ARM unless there's a really strong reason not to.
>> 
>> Agreed.
> 
> I don't really see the benefits of pretending that /proc/cpuinfo is remotely
> portable between architectures.
> 
>>> (2) Making this be not-per-CPU backs us into a corner. If we
>>> have the feature flags per-core, and it turns out that they're
>>> never ever different between cores even on heterogenous
>>> CPUs, there's no problem. If we have the feature flags be
>>> listed only once, and then in future we do want to support
>>> some minor form of heterogeny between cores, then we're
>>> stuck with a compatibility break because the format doesn't
>>> let us express that. (Perhaps there might be a big.LITTLE
>>> system where only the big cores had the crypto extensions,
>>> to pick a random possibility where the kernel doesn't need to
>>> care but userspace could take advantage by restricting itself
>>> to running on only the big cores). I think this makes the
>>> choice pretty straightforward, since there's basically no
>>> benefit to having only a single features line.
>> 
>> Agreed.
> 
> So the real question is: do we want to allow Linux to support features that
> exist only on a subset of cores in the system? The current thinking is that
> we truncate the advertised features to the common system subset, which means
> it will be the same on all cores, by definition. That allows hardware guys
> to build crazy systems that we can at least use, without imparting a world
> of pain onto software that needs to run on them.
> 
> I also think that we're backed into a corner regardless. We've seen how
> people ignore hwcaps (e.g. SWP), so they're likely to parse the caps for
> CPU0 and use that as an indication of the system capabilities.
> 
> Note that the features list *isn't* just the features supported by the CPU.
> It also takes into account the set of features supported by the kernel (e.g.
> we don't advertise VFP on ARMv7 if VFP context switching isn't enabled).

So obviously, we shouldn't use hwcaps derived from cpu #0 to report cpu capabilities for all cpus. Why can't we have a single hwcaps line, and then a capabilties line for each cpu containing the 'raw' capability flags read straight from the cpu in question?

-- 
Ard.

[PATCHv4 5/5] arm64: cpuinfo: print info for all CPUs

[Peter Maydell]

On 17 July 2014 11:54, Will Deacon <will.deacon@arm.com> wrote:
> I don't really see the benefits of pretending that /proc/cpuinfo is remotely
> portable between architectures.

I'm not suggesting it's portable. I'm suggesting that you need
a good reason to push backwards against a design decision
made on other architectures. In particular, given that ARM is
in general *more* likely to be heterogenous than x86 (given
the existence of big.LITTLE), it seems baffling to try to move
in a direction that denies the possibility of further heterogeneity
in future.

> So the real question is: do we want to allow Linux to support features that
> exist only on a subset of cores in the system? The current thinking is that
> we truncate the advertised features to the common system subset, which means
> it will be the same on all cores, by definition. That allows hardware guys
> to build crazy systems that we can at least use, without imparting a world
> of pain onto software that needs to run on them.

I've already made one suggestion (non-pervasive crypto).
You could also envisage "feature bits" that effectively mean
"things will be faster on this core" even if they still work on
other cores too.

> I also think that we're backed into a corner regardless. We've seen how
> people ignore hwcaps (e.g. SWP), so they're likely to parse the caps for
> CPU0 and use that as an indication of the system capabilities.

Certainly userspace *can* do the wrong thing. That doesn't
necessarily mean that the kernel should only ever provide the
API equivalent of safety scissors...

> Note that the features list *isn't* just the features supported by the CPU.
> It also takes into account the set of features supported by the kernel (e.g.
> we don't advertise VFP on ARMv7 if VFP context switching isn't enabled).

This is an argument for also advertising the lowest-common-denominator
feature bits. (Do you want to argue for a single "features:" line
plus per-core "extra-features:" lines?)

thanks
-- PMM

[PATCHv4 5/5] arm64: cpuinfo: print info for all CPUs

[Will Deacon]

On Thu, Jul 17, 2014 at 12:12:48PM +0100, Peter Maydell wrote:
> On 17 July 2014 11:54, Will Deacon <will.deacon@arm.com> wrote:
> > I don't really see the benefits of pretending that /proc/cpuinfo is remotely
> > portable between architectures.
> 
> I'm not suggesting it's portable. I'm suggesting that you need
> a good reason to push backwards against a design decision
> made on other architectures. In particular, given that ARM is
> in general *more* likely to be heterogenous than x86 (given
> the existence of big.LITTLE), it seems baffling to try to move
> in a direction that denies the possibility of further heterogeneity
> in future.

We're not denying the possibility of heterogeneity, we're trying to expose a
consistent view of the system to userspace. Differences between cores should
be dealt with by the kernel (e.g. IKS, HMP scheduling), not blindly
passed off to userspace.

> > So the real question is: do we want to allow Linux to support features that
> > exist only on a subset of cores in the system? The current thinking is that
> > we truncate the advertised features to the common system subset, which means
> > it will be the same on all cores, by definition. That allows hardware guys
> > to build crazy systems that we can at least use, without imparting a world
> > of pain onto software that needs to run on them.
> 
> I've already made one suggestion (non-pervasive crypto).
> You could also envisage "feature bits" that effectively mean
> "things will be faster on this core" even if they still work on
> other cores too.

I don't believe that knowledge belongs in userspace. If you wanted to
support crypto on such a system, then you advertise it as a system feature
and migrate those tasks to the cores that support the instructions (in a
similar manner to migrating the FPU on architecture that can share one).
However, I'd prefer to start from a point where we *don't*' support such
systems and actively dissuage people from building them.

> > I also think that we're backed into a corner regardless. We've seen how
> > people ignore hwcaps (e.g. SWP), so they're likely to parse the caps for
> > CPU0 and use that as an indication of the system capabilities.
> 
> Certainly userspace *can* do the wrong thing. That doesn't
> necessarily mean that the kernel should only ever provide the
> API equivalent of safety scissors...

Userspace *will* do the wrong thing and it *will* turn the feature into
useless baggage that we have to carry.

> > Note that the features list *isn't* just the features supported by the CPU.
> > It also takes into account the set of features supported by the kernel (e.g.
> > we don't advertise VFP on ARMv7 if VFP context switching isn't enabled).
> 
> This is an argument for also advertising the lowest-common-denominator
> feature bits. (Do you want to argue for a single "features:" line
> plus per-core "extra-features:" lines?)

That's what Ard brought up and I think it makes more sense that printing the
same features line for each core. However, I question its usefulness and
think it's ripe for misuse.

Will

[PATCHv4 5/5] arm64: cpuinfo: print info for all CPUs

[Peter Maydell]

On 17 July 2014 13:35, Will Deacon <will.deacon@arm.com> wrote:
> We're not denying the possibility of heterogeneity, we're trying to expose a
> consistent view of the system to userspace. Differences between cores should
> be dealt with by the kernel (e.g. IKS, HMP scheduling), not blindly
> passed off to userspace.

On that basis, why report anything at all about invididual cores?
Just have /proc/cpuinfo report "number of processors: 4" and
no per-CPU information at all...

-- PMM

[PATCHv4 0/5] arm64: handle heterogeneous system register values

[Mark Rutland]

On Thu, Jul 17, 2014 at 12:03:33PM +0100, Catalin Marinas wrote:
> On Wed, Jul 16, 2014 at 04:32:42PM +0100, Mark Rutland wrote:
> > Mark Rutland (5):
> >   arm64: add MIDR_EL1 field accessors
> >   arm64: cpuinfo: record cpu system register values
> >   arm64: cachetype: report weakest cache policy
> >   arm64: add runtime system sanity checks
> 
> For these 4 patches:
> 
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

Thanks.

I take it you'll pick these up directly and apply the tags from you and
Will?

Or would you prefer I place these in a branch?

> 
> >   arm64: cpuinfo: print info for all CPUs
> 
> This is still up for discussion.

Sure, that's what I had expected. At least it's out in the open.

Cheers,
Mark.

[PATCHv4 0/5] arm64: handle heterogeneous system register values

[Will Deacon]

On Thu, Jul 17, 2014 at 03:21:33PM +0100, Mark Rutland wrote:
> On Thu, Jul 17, 2014 at 12:03:33PM +0100, Catalin Marinas wrote:
> > On Wed, Jul 16, 2014 at 04:32:42PM +0100, Mark Rutland wrote:
> > > Mark Rutland (5):
> > >   arm64: add MIDR_EL1 field accessors
> > >   arm64: cpuinfo: record cpu system register values
> > >   arm64: cachetype: report weakest cache policy
> > >   arm64: add runtime system sanity checks
> > 
> > For these 4 patches:
> > 
> > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> 
> Thanks.
> 
> I take it you'll pick these up directly and apply the tags from you and
> Will?
> 
> Or would you prefer I place these in a branch?

We'll grab them later today, I expect. If testing explodes, I'll let you
know ;)

Will

[PATCHv4 5/5] arm64: cpuinfo: print info for all CPUs

[Catalin Marinas]

On Thu, Jul 17, 2014 at 02:55:37PM +0100, Peter Maydell wrote:
> On 17 July 2014 13:35, Will Deacon <will.deacon@arm.com> wrote:
> > We're not denying the possibility of heterogeneity, we're trying to expose a
> > consistent view of the system to userspace. Differences between cores should
> > be dealt with by the kernel (e.g. IKS, HMP scheduling), not blindly
> > passed off to userspace.
> 
> On that basis, why report anything at all about invididual cores?
> Just have /proc/cpuinfo report "number of processors: 4" and
> no per-CPU information at all...

We lost a lot of time on this already (given the internal threads). So
my proposal is to go ahead with Mark's patch with per-CPU features. They
currently just include the same elf_hwcap multiple times. If we ever
need to present different features, the conditions would be:

1. Never report more than elf_hwcap
2. elf_hwcap can only include non-symmetric features *if* Linux gets a
   way to transparently handle migration or emulation

It basically means that Linux would not rely on the user space to make
informed decisions on where to run a thread and avoid SIGILL.

-- 
Catalin

[PATCHv4 5/5] arm64: cpuinfo: print info for all CPUs

[Will Deacon]

On Thu, Jul 17, 2014 at 06:10:58PM +0100, Catalin Marinas wrote:
> On Thu, Jul 17, 2014 at 02:55:37PM +0100, Peter Maydell wrote:
> > On 17 July 2014 13:35, Will Deacon <will.deacon@arm.com> wrote:
> > > We're not denying the possibility of heterogeneity, we're trying to expose a
> > > consistent view of the system to userspace. Differences between cores should
> > > be dealt with by the kernel (e.g. IKS, HMP scheduling), not blindly
> > > passed off to userspace.
> > 
> > On that basis, why report anything at all about invididual cores?
> > Just have /proc/cpuinfo report "number of processors: 4" and
> > no per-CPU information at all...
> 
> We lost a lot of time on this already (given the internal threads). So
> my proposal is to go ahead with Mark's patch with per-CPU features. They
> currently just include the same elf_hwcap multiple times. If we ever
> need to present different features, the conditions would be:
> 
> 1. Never report more than elf_hwcap
> 2. elf_hwcap can only include non-symmetric features *if* Linux gets a
>    way to transparently handle migration or emulation

... making the point of a per-cpu field entirely pointless ;)

Will

[PATCHv4 5/5] arm64: cpuinfo: print info for all CPUs

[Catalin Marinas]

On Thu, Jul 17, 2014 at 06:28:58PM +0100, Will Deacon wrote:
> On Thu, Jul 17, 2014 at 06:10:58PM +0100, Catalin Marinas wrote:
> > On Thu, Jul 17, 2014 at 02:55:37PM +0100, Peter Maydell wrote:
> > > On 17 July 2014 13:35, Will Deacon <will.deacon@arm.com> wrote:
> > > > We're not denying the possibility of heterogeneity, we're trying to expose a
> > > > consistent view of the system to userspace. Differences between cores should
> > > > be dealt with by the kernel (e.g. IKS, HMP scheduling), not blindly
> > > > passed off to userspace.
> > > 
> > > On that basis, why report anything at all about invididual cores?
> > > Just have /proc/cpuinfo report "number of processors: 4" and
> > > no per-CPU information at all...
> > 
> > We lost a lot of time on this already (given the internal threads). So
> > my proposal is to go ahead with Mark's patch with per-CPU features. They
> > currently just include the same elf_hwcap multiple times. If we ever
> > need to present different features, the conditions would be:
> > 
> > 1. Never report more than elf_hwcap
> > 2. elf_hwcap can only include non-symmetric features *if* Linux gets a
> >    way to transparently handle migration or emulation
> 
> ... making the point of a per-cpu field entirely pointless ;)

Well, if we can support such features in a transparent way,
/proc/cpuinfo becomes more informative (e.g. user wondering why a
process runs only on certain CPUs).

But to be clear (and I think we are aligned), I don't trust user space
to parse all processors in /proc/cpuinfo and make an informed selection
of CPU affinity to avoid SIGILL.

Yet another option would be to have a single features/hwcap line and
present the extra features in a human (and only human) readable form
(e.g. some haiku that changes with every kernel release ;)).

-- 
Catalin

[PATCHv4 5/5] arm64: cpuinfo: print info for all CPUs

[Will Deacon]

On Fri, Jul 18, 2014 at 10:27:44AM +0100, Catalin Marinas wrote:
> On Thu, Jul 17, 2014 at 06:28:58PM +0100, Will Deacon wrote:
> > On Thu, Jul 17, 2014 at 06:10:58PM +0100, Catalin Marinas wrote:
> > > On Thu, Jul 17, 2014 at 02:55:37PM +0100, Peter Maydell wrote:
> > > > On 17 July 2014 13:35, Will Deacon <will.deacon@arm.com> wrote:
> > > > > We're not denying the possibility of heterogeneity, we're trying to expose a
> > > > > consistent view of the system to userspace. Differences between cores should
> > > > > be dealt with by the kernel (e.g. IKS, HMP scheduling), not blindly
> > > > > passed off to userspace.
> > > > 
> > > > On that basis, why report anything at all about invididual cores?
> > > > Just have /proc/cpuinfo report "number of processors: 4" and
> > > > no per-CPU information at all...
> > > 
> > > We lost a lot of time on this already (given the internal threads). So
> > > my proposal is to go ahead with Mark's patch with per-CPU features. They
> > > currently just include the same elf_hwcap multiple times. If we ever
> > > need to present different features, the conditions would be:
> > > 
> > > 1. Never report more than elf_hwcap
> > > 2. elf_hwcap can only include non-symmetric features *if* Linux gets a
> > >    way to transparently handle migration or emulation
> > 
> > ... making the point of a per-cpu field entirely pointless ;)
> 
> Well, if we can support such features in a transparent way,
> /proc/cpuinfo becomes more informative (e.g. user wondering why a
> process runs only on certain CPUs).
> 
> But to be clear (and I think we are aligned), I don't trust user space
> to parse all processors in /proc/cpuinfo and make an informed selection
> of CPU affinity to avoid SIGILL.
> 
> Yet another option would be to have a single features/hwcap line and
> present the extra features in a human (and only human) readable form
> (e.g. some haiku that changes with every kernel release ;)).

Or just have the single features line, then the per-cpu line can be called
`flags' or something, like Ard suggested. If userspace decides to parse
flags, it deserves all the pain that it gets.

Will

[PATCHv4 5/5] arm64: cpuinfo: print info for all CPUs

[Mark Rutland]

On Fri, Jul 18, 2014 at 10:53:12AM +0100, Will Deacon wrote:
> On Fri, Jul 18, 2014 at 10:27:44AM +0100, Catalin Marinas wrote:
> > On Thu, Jul 17, 2014 at 06:28:58PM +0100, Will Deacon wrote:
> > > On Thu, Jul 17, 2014 at 06:10:58PM +0100, Catalin Marinas wrote:
> > > > On Thu, Jul 17, 2014 at 02:55:37PM +0100, Peter Maydell wrote:
> > > > > On 17 July 2014 13:35, Will Deacon <will.deacon@arm.com> wrote:
> > > > > > We're not denying the possibility of heterogeneity, we're trying to expose a
> > > > > > consistent view of the system to userspace. Differences between cores should
> > > > > > be dealt with by the kernel (e.g. IKS, HMP scheduling), not blindly
> > > > > > passed off to userspace.
> > > > > 
> > > > > On that basis, why report anything at all about invididual cores?
> > > > > Just have /proc/cpuinfo report "number of processors: 4" and
> > > > > no per-CPU information at all...
> > > > 
> > > > We lost a lot of time on this already (given the internal threads). So
> > > > my proposal is to go ahead with Mark's patch with per-CPU features. They
> > > > currently just include the same elf_hwcap multiple times. If we ever
> > > > need to present different features, the conditions would be:
> > > > 
> > > > 1. Never report more than elf_hwcap
> > > > 2. elf_hwcap can only include non-symmetric features *if* Linux gets a
> > > >    way to transparently handle migration or emulation
> > > 
> > > ... making the point of a per-cpu field entirely pointless ;)
> > 
> > Well, if we can support such features in a transparent way,
> > /proc/cpuinfo becomes more informative (e.g. user wondering why a
> > process runs only on certain CPUs).
> > 
> > But to be clear (and I think we are aligned), I don't trust user space
> > to parse all processors in /proc/cpuinfo and make an informed selection
> > of CPU affinity to avoid SIGILL.
> > 
> > Yet another option would be to have a single features/hwcap line and
> > present the extra features in a human (and only human) readable form
> > (e.g. some haiku that changes with every kernel release ;)).
> 
> Or just have the single features line, then the per-cpu line can be called
> `flags' or something, like Ard suggested. If userspace decides to parse
> flags, it deserves all the pain that it gets.

Ok. I think retaining the current (common) features line makes sense if
it's clearly separated from any particular CPU. If we need per-cpu
information later, this can be in addition to the common line.

That should enable software which only does mildy crazy things with
values from /proc/cpuinfo to work even if we print per-cpu information.
Anything trying to handle per-cpu differences will need rework
regardless, and that discussion can be had when we actually have crazily
heterogeneous systems.

Therefore, how about the below?

Thanks,
Mark.

---->8----
>From 7dc7b5e5c4a54f56a5a7e59d2dd0de009dc9b9d0 Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
Date: Thu, 26 Jun 2014 16:18:44 +0100
Subject: [PATCH] arm64: cpuinfo: print info for all CPUs

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 MIDR fields will vary across CPUs, and the output will
differ depending on the executing CPU.

This patch reorganises the code responsible for /proc/cpuinfo to print
information per-cpu. In the process, we perform several cleanups:

* Property names are coerced to lower-case (to match "processor" as per
  glibc's expectations).
* Property names are simplified and made to match the MIDR field names.
* Revision is changed to hex as with every other field.
* The meaningless Architecture property is removed.
* The ripe-for-abuse Machine field is removed.

The features field (a human-readable representation of the hwcaps)
remains printed once, as this is expected to remain in use as the
globally support CPU features. To enable the possibility of the addition
of per-cpu HW feature information later, this is printed before any
CPU-specific information.

Comments are added to guide userspace developers in the right direction
(using the hwcaps provided in auxval). Hopefully where userspace
applications parse /proc/cpuinfo rather than using the readily available
hwcaps, they limit themselves to reading said first line.

If CPU features differ from each other, the previously installed sanity
checks will give us some advance notice with warnings and
TAINT_CPU_OUT_OF_SPEC. If we are lucky, we will never see such systems.
Rework will be required in many places to support such systems anyway.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Marcus Shawcroft <marcus.shawcroft@arm.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/setup.c | 37 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index edb146d..aaf1ddb 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -450,10 +450,21 @@ static int c_show(struct seq_file *m, void *v)
 {
 	int i;
 
-	seq_printf(m, "Processor\t: %s rev %d (%s)\n",
-		   cpu_name, read_cpuid_id() & 15, ELF_PLATFORM);
+	/*
+	 * Dump out the common processor features in a single line. Userspace
+	 * should read the hwcaps with getauxval(AT_HWCAP) rather than
+	 * attempting to parse this.
+	 */
+	seq_puts(m, "features\t:");
+	for (i = 0; hwcap_str[i]; i++)
+		if (elf_hwcap & (1 << i))
+			seq_printf(m, " %s", hwcap_str[i]);
+	seq_puts(m, "\n\n");
 
 	for_each_online_cpu(i) {
+		struct cpuinfo_arm64 *cpuinfo = &per_cpu(cpu_data, i);
+		u32 midr = cpuinfo->reg_midr;
+
 		/*
 		 * glibc reads /proc/cpuinfo to determine the number of
 		 * online processors, looking for lines beginning with
@@ -462,25 +473,13 @@ static int c_show(struct seq_file *m, void *v)
 #ifdef CONFIG_SMP
 		seq_printf(m, "processor\t: %d\n", i);
 #endif
+		seq_printf(m, "implementer\t: 0x%02x\n",
+			   MIDR_IMPLEMENTOR(midr));
+		seq_printf(m, "variant\t\t: 0x%x\n", MIDR_VARIANT(midr));
+		seq_printf(m, "partnum\t\t: 0x%03x\n", MIDR_PARTNUM(midr));
+		seq_printf(m, "revision\t: 0x%x\n\n", MIDR_REVISION(midr));
 	}
 
-	/* 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]);
-
-	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_puts(m, "\n");
-
-	seq_printf(m, "Hardware\t: %s\n", machine_name);
-
 	return 0;
 }
 
-- 
1.9.1

[PATCHv4 5/5] arm64: cpuinfo: print info for all CPUs

[Peter Maydell]

On 18 July 2014 14:57, Mark Rutland <mark.rutland@arm.com> wrote:
> Comments are added to guide userspace developers in the right direction
> (using the hwcaps provided in auxval). Hopefully where userspace
> applications parse /proc/cpuinfo rather than using the readily available
> hwcaps, they limit themselves to reading said first line.

Comments in the kernel sources aren't going to guide
anybody except kernel developers. I was expecting from
this commit message that you were going to emit actual
comments in /proc/cpuinfo...

-- PMM

[PATCHv4 5/5] arm64: cpuinfo: print info for all CPUs

[Mark Rutland]

On Fri, Jul 18, 2014 at 04:52:37PM +0100, Peter Maydell wrote:
> On 18 July 2014 14:57, Mark Rutland <mark.rutland@arm.com> wrote:
> > Comments are added to guide userspace developers in the right direction
> > (using the hwcaps provided in auxval). Hopefully where userspace
> > applications parse /proc/cpuinfo rather than using the readily available
> > hwcaps, they limit themselves to reading said first line.
> 
> Comments in the kernel sources aren't going to guide
> anybody except kernel developers.

That's not entirely true, some people skim the kernel sources to figure
out how they're meant to use syscalls and such (though admitedly this
isn't all that common).

> I was expecting from this commit message that you were going to emit
> actual comments in /proc/cpuinfo...

I don't think that's a good idea, and I can only see that reading when I
squint quite hard. ;)

Thanks,
Mark

[PATCHv4 5/5] arm64: cpuinfo: print info for all CPUs

[Peter Maydell]

On 18 July 2014 16:58, Mark Rutland <mark.rutland@arm.com> wrote:
> On Fri, Jul 18, 2014 at 04:52:37PM +0100, Peter Maydell wrote:
>> Comments in the kernel sources aren't going to guide
>> anybody except kernel developers.
>
> That's not entirely true, some people skim the kernel sources to figure
> out how they're meant to use syscalls and such (though admitedly this
> isn't all that common).

Every time anybody has to do that it means you've failed to document
something...

>> I was expecting from this commit message that you were going to emit
>> actual comments in /proc/cpuinfo...
>
> I don't think that's a good idea, and I can only see that reading when I
> squint quite hard. ;)

You have to admit it would put the documentation right where
the people looking at cpuinfo can find it :-)

How about a patch to Documentation/filesystems/proc.txt ?

thanks
-- PMM

[PATCHv4 5/5] arm64: cpuinfo: print info for all CPUs

[Mark Rutland]

On Fri, Jul 18, 2014 at 05:18:27PM +0100, Peter Maydell wrote:
> On 18 July 2014 16:58, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Fri, Jul 18, 2014 at 04:52:37PM +0100, Peter Maydell wrote:
> >> Comments in the kernel sources aren't going to guide
> >> anybody except kernel developers.
> >
> > That's not entirely true, some people skim the kernel sources to figure
> > out how they're meant to use syscalls and such (though admitedly this
> > isn't all that common).
> 
> Every time anybody has to do that it means you've failed to document
> something...

...and in this case, the thing to document is the hwcaps.

> >> I was expecting from this commit message that you were going to emit
> >> actual comments in /proc/cpuinfo...
> >
> > I don't think that's a good idea, and I can only see that reading when I
> > squint quite hard. ;)
> 
> You have to admit it would put the documentation right where
> the people looking at cpuinfo can find it :-)

Sure :)

> How about a patch to Documentation/filesystems/proc.txt ?

Currently there seems to be a single relevant line, and it doesn't seem
to be up-to-date for SMP:

	cpuinfo     Info about the CPU

It might make sense to have something under Documentation/arm64, but I
don't know what precisely.

Cheers,
Mark.

[PATCHv4 5/5] arm64: cpuinfo: print info for all CPUs

[Christopher Covington]

On 07/18/2014 12:41 PM, Mark Rutland wrote:
> On Fri, Jul 18, 2014 at 05:18:27PM +0100, Peter Maydell wrote:
>> On 18 July 2014 16:58, Mark Rutland <mark.rutland@arm.com> wrote:
>>> On Fri, Jul 18, 2014 at 04:52:37PM +0100, Peter Maydell wrote:
>>>> Comments in the kernel sources aren't going to guide
>>>> anybody except kernel developers.
>>>
>>> That's not entirely true, some people skim the kernel sources to figure
>>> out how they're meant to use syscalls and such (though admitedly this
>>> isn't all that common).
>>
>> Every time anybody has to do that it means you've failed to document
>> something...
> 
> ...and in this case, the thing to document is the hwcaps.
> 
>>>> I was expecting from this commit message that you were going to emit
>>>> actual comments in /proc/cpuinfo...
>>>
>>> I don't think that's a good idea, and I can only see that reading when I
>>> squint quite hard. ;)
>>
>> You have to admit it would put the documentation right where
>> the people looking at cpuinfo can find it :-)
> 
> Sure :)
> 
>> How about a patch to Documentation/filesystems/proc.txt ?
> 
> Currently there seems to be a single relevant line, and it doesn't seem
> to be up-to-date for SMP:
> 
> 	cpuinfo     Info about the CPU
> 
> It might make sense to have something under Documentation/arm64, but I
> don't know what precisely.

If you're looking to communicate information to users I would highly recommend
including an update to the Linux man page.

http://man7.org/linux/man-pages/man5/proc.5.html

https://www.kernel.org/doc/man-pages/contributing.html

Regards,
Christopher

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by the Linux Foundation.