Re: [PATCHv2] ARM64: Add AT_ARM64_MIDR to the aux vector

[Mark Rutland]

Hi,

On Sat, Aug 29, 2015 at 07:46:22PM +0100, Andrew Pinski wrote:
> It is useful to pass down MIDR register down to userland if all of
> the online cores are all the same type.  This adds AT_ARM64_MIDR
> aux vector type and passes down the midr system register.
> 
> This is alternative to MIDR_EL1 part of
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/358995.html.
> It allows for faster access to midr_el1 than going through a trap and
> does not exist if the set of cores are not the same.

I'm not sure I follow the rationale. If speed is important the
application can cache the value the first time it reads it with a trap.

This also means that the behaviour is different across homogeneous and
heterogeneous systems.

> Changes from v1:
> Forgot to include the auxvec.h part.
> 
> Signed-off-by: Andrew Pinski <apinski@xxxxxxxxxx>
> ---
>  arch/arm64/include/asm/cpu.h         |    1 +
>  arch/arm64/include/asm/elf.h         |    6 ++++++
>  arch/arm64/include/uapi/asm/auxvec.h |    3 +++
>  arch/arm64/kernel/cpuinfo.c          |   22 ++++++++++++++++++++++
>  4 files changed, 32 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/cpu.h b/arch/arm64/include/asm/cpu.h
> index 8e797b2..fab0aa1 100644
> --- a/arch/arm64/include/asm/cpu.h
> +++ b/arch/arm64/include/asm/cpu.h
> @@ -62,5 +62,6 @@ DECLARE_PER_CPU(struct cpuinfo_arm64, cpu_data);
>  
>  void cpuinfo_store_cpu(void);
>  void __init cpuinfo_store_boot_cpu(void);
> +u32 get_arm64_midr(void);
>  
>  #endif /* __ASM_CPU_H */
> diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
> index faad6df..d3549de 100644
> --- a/arch/arm64/include/asm/elf.h
> +++ b/arch/arm64/include/asm/elf.h
> @@ -17,6 +17,7 @@
>  #define __ASM_ELF_H
>  
>  #include <asm/hwcap.h>
> +#include <asm/cpu.h>
>  
>  /*
>   * ELF register definitions..
> @@ -138,8 +139,13 @@ typedef struct user_fpsimd_state elf_fpregset_t;
>  
>  #define ARCH_DLINFO							\
>  do {									\
> +	u32 midr;							\
> +									\
>  	NEW_AUX_ENT(AT_SYSINFO_EHDR,					\
>  		    (elf_addr_t)current->mm->context.vdso);		\
> +	midr = get_arm64_midr();					\
> +	if (midr != 0)							\
> +		NEW_AUX_ENT(AT_ARM64_MIDR, (elf_addr_t)midr);		\
>  } while (0)
>  
>  #define ARCH_HAS_SETUP_ADDITIONAL_PAGES
> diff --git a/arch/arm64/include/uapi/asm/auxvec.h b/arch/arm64/include/uapi/asm/auxvec.h
> index 22d6d88..dc55c56 100644
> --- a/arch/arm64/include/uapi/asm/auxvec.h
> +++ b/arch/arm64/include/uapi/asm/auxvec.h
> @@ -19,4 +19,7 @@
>  /* vDSO location */
>  #define AT_SYSINFO_EHDR	33
>  
> +/* Machine IDenfier Register (MDIR). */
> +#define AT_ARM64_MIDR 38
> +
>  #endif
> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> index 75d5a86..b14c87d 100644
> --- a/arch/arm64/kernel/cpuinfo.c
> +++ b/arch/arm64/kernel/cpuinfo.c
> @@ -254,3 +254,25 @@ void __init cpuinfo_store_boot_cpu(void)
>  
>  	boot_cpu_data = *info;
>  }
> +
> +u32 get_arm64_midr(void)
> +{
> +	int i;
> +	u32 midr = 0;
> +
> +	for_each_online_cpu(i) {
> +		struct cpuinfo_arm64 *cpuinfo = &per_cpu(cpu_data, i);
> +		u32 oldmidr = midr;
> +
> +		midr = cpuinfo->reg_midr;
> +		/*
> +		 * If there are cpus which have a different
> +		 * midr just return 0.
> +		 */
> +		if (oldmidr && oldmidr != midr)
> +			return 0;
> +	}
> +
> +	return midr;
> +}

If I have a big.LITTLE system where all the big CPUs are currently
offline, this will leave the MIDR the little CPUs in the auxvec.
However, at any point after this has run, I could hotplug the big CPUs
on and the little CPUs off, leaving this reporting a MIDR that
represents none of the online CPUs.

Given big.LITTLE and the potential for physical/dynamic hotplug (where
we won't know all the MIDRs in advance), I don't think that we can
generally expose a common MIDR in this fashion, and I don't think that
we should give the impression that we can.

I think that the only things we can do are expose the MIDR for CPU the
code is currently executing on (as Suzuki's patches do), and/or expose
all the MIDRs for currently online CPUs (as Steve's [1] patch does).
Anything else leaves us trying to provide semantics that we cannot
guarantee.

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/359127.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/