[PATCH 0/2] arm64: /proc/cpuinfo updates for compat tasks

[PATCH 0/2] arm64: /proc/cpuinfo updates for compat tasks

[Catalin Marinas]

The first patch is relatively simple, adding the "model name" line which
is present in the arm32 /proc/cpuinfo.

The second patch is up for discussion. When arm64 support was merged, we
concluded that "uname -m" should report "aarch64" even for compat tasks.
The only way to change the output to "armv8l" was to set the PER_LINUX32
personality (and I think this is in line with x86).

We followed a similar personality approach for /proc/cpuinfo where the
compat one is only exposed if the task sets/inherits PER_LINUX32. The
downside is that apparently some 32-bit applications started making use
of the 64-bit /proc/cpuinfo format. To prevent further spread, I'm
proposing the second patch in this series to force compat /proc/cpuinfo
for 32-bit tasks. This is a departure from the "uname -m" behaviour but
maybe they don't need to be coupled anyway.

Thoughts?

Catalin Marinas (2):
  arm64: Provide "model name" in /proc/cpuinfo for PER_LINUX32 tasks
  arm64: Always provide compat /proc/cpuinfo for 32-bit tasks

 arch/arm64/include/asm/elf.h | 4 ++--
 arch/arm64/kernel/cpuinfo.c  | 9 ++++++++-
 2 files changed, 10 insertions(+), 3 deletions(-)

[PATCH 1/2] arm64: Provide "model name" in /proc/cpuinfo for PER_LINUX32 tasks

[Catalin Marinas]

This patch brings the PER_LINUX32 /proc/cpuinfo format more in line with
the 32-bit ARM one by providing an additional line:

model name      : ARMv8 Processor rev X (v8l)

Cc: <stable@vger.kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/elf.h | 4 ++--
 arch/arm64/kernel/cpuinfo.c  | 8 +++++++-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index 7a09c48c0475..579b6e654f2d 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -160,14 +160,14 @@ extern int arch_setup_additional_pages(struct linux_binprm *bprm,
 #define STACK_RND_MASK			(0x3ffff >> (PAGE_SHIFT - 12))
 #endif
 
-#ifdef CONFIG_COMPAT
-
 #ifdef __AARCH64EB__
 #define COMPAT_ELF_PLATFORM		("v8b")
 #else
 #define COMPAT_ELF_PLATFORM		("v8l")
 #endif
 
+#ifdef CONFIG_COMPAT
+
 #define COMPAT_ELF_ET_DYN_BASE		(2 * TASK_SIZE_32 / 3)
 
 /* AArch32 registers. */
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index 3808470486f3..c173d329397f 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -22,6 +22,8 @@
 
 #include <linux/bitops.h>
 #include <linux/bug.h>
+#include <linux/compat.h>
+#include <linux/elf.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/personality.h>
@@ -104,6 +106,7 @@ static const char *const compat_hwcap2_str[] = {
 static int c_show(struct seq_file *m, void *v)
 {
 	int i, j;
+	bool compat = personality(current->personality) == PER_LINUX32;
 
 	for_each_online_cpu(i) {
 		struct cpuinfo_arm64 *cpuinfo = &per_cpu(cpu_data, i);
@@ -115,6 +118,9 @@ static int c_show(struct seq_file *m, void *v)
 		 * "processor".  Give glibc what it expects.
 		 */
 		seq_printf(m, "processor\t: %d\n", i);
+		if (compat)
+			seq_printf(m, "model name\t: ARMv8 Processor rev %d (%s)\n",
+				   MIDR_REVISION(midr), COMPAT_ELF_PLATFORM);
 
 		seq_printf(m, "BogoMIPS\t: %lu.%02lu\n",
 			   loops_per_jiffy / (500000UL/HZ),
@@ -127,7 +133,7 @@ static int c_show(struct seq_file *m, void *v)
 		 * software which does already (at least for 32-bit).
 		 */
 		seq_puts(m, "Features\t:");
-		if (personality(current->personality) == PER_LINUX32) {
+		if (compat) {
 #ifdef CONFIG_COMPAT
 			for (j = 0; compat_hwcap_str[j]; j++)
 				if (compat_elf_hwcap & (1 << j))

[PATCH 2/2] arm64: Always provide compat /proc/cpuinfo for 32-bit tasks

[Catalin Marinas]

Currently, the compat /proc/cpuinfo is provided only if a task has the
PER_LINUX32 personality, either by setting it explicitly or by
inheriting it from the parent task. This is in line with the "uname -m"
behaviour.

However, there are 32-bit user applications making use of the
/proc/cpuinfo and unaware of a need to set the personality. This patch
changes the arm64 /proc/cpuinfo logic to provide the compat information
if the task is 32-bit _or_ the PER_LINUX32 personality is set.

Cc: <stable@vger.kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/kernel/cpuinfo.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index c173d329397f..bd6a122ea856 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -106,7 +106,8 @@ static const char *const compat_hwcap2_str[] = {
 static int c_show(struct seq_file *m, void *v)
 {
 	int i, j;
-	bool compat = personality(current->personality) == PER_LINUX32;
+	bool compat = is_compat_task() ||
+		personality(current->personality) == PER_LINUX32;
 
 	for_each_online_cpu(i) {
 		struct cpuinfo_arm64 *cpuinfo = &per_cpu(cpu_data, i);

[PATCH 1/2] arm64: Provide "model name" in /proc/cpuinfo for PER_LINUX32 tasks

[Will Deacon]

On Tue, May 31, 2016 at 03:55:03PM +0100, Catalin Marinas wrote:
> This patch brings the PER_LINUX32 /proc/cpuinfo format more in line with
> the 32-bit ARM one by providing an additional line:
> 
> model name      : ARMv8 Processor rev X (v8l)
> 
> Cc: <stable@vger.kernel.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
>  arch/arm64/include/asm/elf.h | 4 ++--
>  arch/arm64/kernel/cpuinfo.c  | 8 +++++++-
>  2 files changed, 9 insertions(+), 3 deletions(-)

Looks good to me:

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

Will

[PATCH 2/2] arm64: Always provide compat /proc/cpuinfo for 32-bit tasks

[Will Deacon]

On Tue, May 31, 2016 at 03:55:04PM +0100, Catalin Marinas wrote:
> Currently, the compat /proc/cpuinfo is provided only if a task has the
> PER_LINUX32 personality, either by setting it explicitly or by
> inheriting it from the parent task. This is in line with the "uname -m"
> behaviour.
> 
> However, there are 32-bit user applications making use of the
> /proc/cpuinfo and unaware of a need to set the personality. This patch
> changes the arm64 /proc/cpuinfo logic to provide the compat information
> if the task is 32-bit _or_ the PER_LINUX32 personality is set.
> 
> Cc: <stable@vger.kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
>  arch/arm64/kernel/cpuinfo.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> index c173d329397f..bd6a122ea856 100644
> --- a/arch/arm64/kernel/cpuinfo.c
> +++ b/arch/arm64/kernel/cpuinfo.c
> @@ -106,7 +106,8 @@ static const char *const compat_hwcap2_str[] = {
>  static int c_show(struct seq_file *m, void *v)
>  {
>  	int i, j;
> -	bool compat = personality(current->personality) == PER_LINUX32;
> +	bool compat = is_compat_task() ||
> +		personality(current->personality) == PER_LINUX32;

Hmm, I'm uneasy about this. It feels like an inconsistency with other
things like uname, where we require the personality to be changed explicitly
in order to get the AArch32 interface.

I'd rather go back to changing personality automatically based on the elf
machine, or continue to require that applications be run with the correct
personality (e.g. via linux32). This half way house is just confusing.

Arnd might have more thoughts...

Will

[PATCH 1/2] arm64: Provide "model name" in /proc/cpuinfo for PER_LINUX32 tasks

[Catalin Marinas]

On Tue, May 31, 2016 at 05:16:09PM +0100, Will Deacon wrote:
> On Tue, May 31, 2016 at 03:55:03PM +0100, Catalin Marinas wrote:
> > This patch brings the PER_LINUX32 /proc/cpuinfo format more in line with
> > the 32-bit ARM one by providing an additional line:
> > 
> > model name      : ARMv8 Processor rev X (v8l)
> > 
> > Cc: <stable@vger.kernel.org>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > ---
> >  arch/arm64/include/asm/elf.h | 4 ++--
> >  arch/arm64/kernel/cpuinfo.c  | 8 +++++++-
> >  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> Looks good to me:
> 
> Acked-by: Will Deacon <will.deacon@arm.com>

I guess you can take it as a fix for 4.7.

-- 
Catalin

[PATCH 2/2] arm64: Always provide compat /proc/cpuinfo for 32-bit tasks

[Brian Norris]

Hi all,

On Tue, May 31, 2016 at 03:55:04PM +0100, Catalin Marinas wrote:
> Currently, the compat /proc/cpuinfo is provided only if a task has the
> PER_LINUX32 personality, either by setting it explicitly or by
> inheriting it from the parent task. This is in line with the "uname -m"
> behaviour.
> 
> However, there are 32-bit user applications making use of the
> /proc/cpuinfo and unaware of a need to set the personality. This patch
> changes the arm64 /proc/cpuinfo logic to provide the compat information
> if the task is 32-bit _or_ the PER_LINUX32 personality is set.
> 
> Cc: <stable@vger.kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>

What's the status on this patch? The previous patch (which was accepted
already) is indeed confusing, because ARM32 processes on an ARM64 system
are not necessarily setting PER_LINUX32.

I'm also curious, why was 'model name' removed from ARM64 in the first
place? Plenty of other architectures support a similar property, and
it's useful for some tools that already parse this, such as coreutils
`uname -p` on Gentoo (and presumably others -- my Ubuntu machine must be
similarly patched, as it supports `uname -p` on x86_64).

If we were to just stick 'model name' back in unconditionally, we could
avoid the objections Will raised entirely :)

Regards,
Brian

> ---
>  arch/arm64/kernel/cpuinfo.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> index c173d329397f..bd6a122ea856 100644
> --- a/arch/arm64/kernel/cpuinfo.c
> +++ b/arch/arm64/kernel/cpuinfo.c
> @@ -106,7 +106,8 @@ static const char *const compat_hwcap2_str[] = {
>  static int c_show(struct seq_file *m, void *v)
>  {
>  	int i, j;
> -	bool compat = personality(current->personality) == PER_LINUX32;
> +	bool compat = is_compat_task() ||
> +		personality(current->personality) == PER_LINUX32;
>  
>  	for_each_online_cpu(i) {
>  		struct cpuinfo_arm64 *cpuinfo = &per_cpu(cpu_data, i);