From: mark.rutland@arm.com (Mark Rutland)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv2] ARM64: Add AT_ARM64_MIDR to the aux vector
Date: Tue, 1 Sep 2015 17:33:04 +0100 [thread overview]
Message-ID: <20150901163304.GC16430@leverpostej> (raw)
In-Reply-To: <1440873982-44062-1-git-send-email-apinski@cavium.com>
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@cavium.com>
> ---
> 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
WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com>
To: Andrew Pinski <apinski@cavium.com>
Cc: "linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"pinskia@gmail.com" <pinskia@gmail.com>,
suzuki.poulose@arm.com, steve.capper@linaro.org
Subject: Re: [PATCHv2] ARM64: Add AT_ARM64_MIDR to the aux vector
Date: Tue, 1 Sep 2015 17:33:04 +0100 [thread overview]
Message-ID: <20150901163304.GC16430@leverpostej> (raw)
In-Reply-To: <1440873982-44062-1-git-send-email-apinski@cavium.com>
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@cavium.com>
> ---
> 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
next prev parent reply other threads:[~2015-09-01 16:33 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-29 18:46 [PATCHv2] ARM64: Add AT_ARM64_MIDR to the aux vector Andrew Pinski
2015-08-29 18:46 ` Andrew Pinski
2015-09-01 16:33 ` Mark Rutland [this message]
2015-09-01 16:33 ` Mark Rutland
2015-09-01 16:51 ` pinskia at gmail.com
2015-09-01 16:51 ` pinskia
2015-09-01 17:06 ` Pinski, Andrew
2015-09-01 17:06 ` Pinski, Andrew
2015-09-01 17:30 ` Mark Rutland
2015-09-01 17:30 ` Mark Rutland
2015-09-01 17:58 ` pinskia at gmail.com
2015-09-01 17:58 ` pinskia
2015-09-01 19:12 ` Siarhei Siamashka
2015-09-01 19:12 ` Siarhei Siamashka
2015-09-02 0:28 ` Pinski, Andrew
2015-09-02 0:28 ` Pinski, Andrew
2015-09-02 13:57 ` Siarhei Siamashka
2015-09-02 13:57 ` Siarhei Siamashka
2015-09-02 14:52 ` Andrew Pinski
2015-09-02 14:52 ` Andrew Pinski
2015-09-02 17:11 ` Catalin Marinas
2015-09-02 17:11 ` Catalin Marinas
2015-09-02 17:21 ` Pinski, Andrew
2015-09-02 17:21 ` Pinski, Andrew
2015-09-02 20:11 ` Thomas Gleixner
2015-09-02 20:11 ` Thomas Gleixner
2015-09-03 17:14 ` Catalin Marinas
2015-09-03 17:14 ` Catalin Marinas
2015-09-01 17:19 ` Mark Rutland
2015-09-01 17:19 ` Mark Rutland
2015-09-01 17:29 ` Pinski, Andrew
2015-09-01 17:29 ` Pinski, Andrew
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150901163304.GC16430@leverpostej \
--to=mark.rutland@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.