* [PATCH 0/6] Initial support for CVADP
@ 2019-02-06 13:31 Andrew Murray
2019-02-06 13:31 ` [PATCH 1/6] arm64: Handle trapped DC CVADP Andrew Murray
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Andrew Murray @ 2019-02-06 13:31 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon; +Cc: Szabolcs Nagy, dave.martin, linux-arm-kernel
ARMv8.5 introduces a DC CVADP instruction which cleans the data cache to
the point of deep persistence. This series makes the instruction
available to userspace and advertises the presence of this CPU feature.
At present when CONFIG_ARM64_PMEM is enabled and the CVAP feature is
present (ARMv8.2) the CVAP instruction is used (from memcpy_flushcache
and arch_wb_cache_pmem). No changes have been made to use CVADP in
these functions or similar.
As we have moved beyond 32 capabilities we now begin using ELF_HWCAP2
for userspace.
Tested as follows:
# dmesg | grep "Deep"
[ 0.166496] CPU features: detected: Data cache clean to Point of Deep Persistence
# LD_SHOW_AUXV=1 sleep 2>&1 | grep AT_HWCAP
AT_HWCAP: ef91ff87
AT_HWCAP2: 0x1
Andrew Murray (6):
arm64: Handle trapped DC CVADP
arm64: HWCAP: add support for ELF_HWCAP2
arm64: HWCAP: encapsulate elf_hwcap
arm64: Expose DC CVADP to userspace
arm64: add CVADP support to the cache maintenance helper
arm64: Advertise ARM64_HAS_DCPODP cpu feature
Documentation/arm64/elf_hwcaps.txt | 4 ++
arch/arm64/crypto/aes-ce-ccm-glue.c | 2 +-
arch/arm64/crypto/aes-neonbs-glue.c | 2 +-
arch/arm64/crypto/chacha-neon-glue.c | 2 +-
arch/arm64/crypto/crct10dif-ce-glue.c | 2 +-
arch/arm64/crypto/ghash-ce-glue.c | 6 +-
arch/arm64/crypto/nhpoly1305-neon-glue.c | 2 +-
arch/arm64/crypto/sha256-glue.c | 4 +-
arch/arm64/include/asm/assembler.h | 4 ++
arch/arm64/include/asm/cpucaps.h | 3 +-
arch/arm64/include/asm/cpufeature.h | 13 ++--
arch/arm64/include/asm/esr.h | 3 +-
arch/arm64/include/asm/hwcap.h | 42 +++++++++++-
arch/arm64/include/uapi/asm/hwcap.h | 7 +-
arch/arm64/kernel/cpufeature.c | 106 +++++++++++++++++++++----------
arch/arm64/kernel/cpuinfo.c | 3 +-
arch/arm64/kernel/fpsimd.c | 4 +-
arch/arm64/kernel/traps.c | 3 +
drivers/clocksource/arm_arch_timer.c | 8 +++
19 files changed, 160 insertions(+), 60 deletions(-)
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/6] arm64: Handle trapped DC CVADP
2019-02-06 13:31 [PATCH 0/6] Initial support for CVADP Andrew Murray
@ 2019-02-06 13:31 ` Andrew Murray
2019-02-06 13:31 ` [PATCH 2/6] arm64: HWCAP: add support for ELF_HWCAP2 Andrew Murray
` (4 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Andrew Murray @ 2019-02-06 13:31 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon; +Cc: Szabolcs Nagy, dave.martin, linux-arm-kernel
The ARMv8.5 DC CVADP instruction may be trapped to EL1 via
SCTLR_EL1.UCI therefore let's provide a handler for it.
Just like the CVAP instruction we use a 'sys' instruction instead of
the 'dc' alias to avoid build issues with older toolchains.
Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
arch/arm64/include/asm/esr.h | 3 ++-
arch/arm64/kernel/traps.c | 3 +++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index 52233f0..07d5c02 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -198,9 +198,10 @@
/*
* User space cache operations have the following sysreg encoding
* in System instructions.
- * op0=1, op1=3, op2=1, crn=7, crm={ 5, 10, 11, 12, 14 }, WRITE (L=0)
+ * op0=1, op1=3, op2=1, crn=7, crm={ 5, 10, 11, 12, 13, 14 }, WRITE (L=0)
*/
#define ESR_ELx_SYS64_ISS_CRM_DC_CIVAC 14
+#define ESR_ELx_SYS64_ISS_CRM_DC_CVADP 13
#define ESR_ELx_SYS64_ISS_CRM_DC_CVAP 12
#define ESR_ELx_SYS64_ISS_CRM_DC_CVAU 11
#define ESR_ELx_SYS64_ISS_CRM_DC_CVAC 10
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 4e2fb87..d1299b6 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -459,6 +459,9 @@ static void user_cache_maint_handler(unsigned int esr, struct pt_regs *regs)
case ESR_ELx_SYS64_ISS_CRM_DC_CVAC: /* DC CVAC, gets promoted */
__user_cache_maint("dc civac", address, ret);
break;
+ case ESR_ELx_SYS64_ISS_CRM_DC_CVADP: /* DC CVADP */
+ __user_cache_maint("sys 3, c7, c13, 1", address, ret);
+ break;
case ESR_ELx_SYS64_ISS_CRM_DC_CVAP: /* DC CVAP */
__user_cache_maint("sys 3, c7, c12, 1", address, ret);
break;
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/6] arm64: HWCAP: add support for ELF_HWCAP2
2019-02-06 13:31 [PATCH 0/6] Initial support for CVADP Andrew Murray
2019-02-06 13:31 ` [PATCH 1/6] arm64: Handle trapped DC CVADP Andrew Murray
@ 2019-02-06 13:31 ` Andrew Murray
2019-02-07 11:31 ` Dave Martin
2019-02-06 13:31 ` [PATCH 3/6] arm64: HWCAP: encapsulate elf_hwcap Andrew Murray
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Andrew Murray @ 2019-02-06 13:31 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon; +Cc: Szabolcs Nagy, dave.martin, linux-arm-kernel
As we will exhaust the first 32 bits of ELF_HWCAP let's start
exposing ELF_HWCAP2 to userspace.
Whilst it's possible to use the remaining 32 bits of ELF_HWCAP, we
prefer to expand into ELF_HWCAP2 in order to provide a consistent
view to userspace between ILP32 and LP64.
To reduce complexity and allow for future expansion, we now
represent hwcaps in the kernel as ordinals and use a
KERNEL_HWCAP_ prefix. This allows us to support automatic feature
based module loading for all our hwcaps.
We introduce cpu_set_feature to set hwcaps in the relevant
elf_hwcapX variable which compliments the existing cpu_have_feature
helper. These helpers allow us to clean up existing direct uses of
elf_hwcap.
Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
arch/arm64/crypto/aes-ce-ccm-glue.c | 2 +-
arch/arm64/crypto/aes-neonbs-glue.c | 2 +-
arch/arm64/crypto/chacha-neon-glue.c | 2 +-
arch/arm64/crypto/crct10dif-ce-glue.c | 2 +-
arch/arm64/crypto/ghash-ce-glue.c | 6 +--
arch/arm64/crypto/nhpoly1305-neon-glue.c | 2 +-
arch/arm64/crypto/sha256-glue.c | 4 +-
arch/arm64/include/asm/cpufeature.h | 18 +++++++--
arch/arm64/include/asm/hwcap.h | 41 ++++++++++++++++++-
arch/arm64/include/uapi/asm/hwcap.h | 2 +-
arch/arm64/kernel/cpufeature.c | 69 +++++++++++++++++---------------
arch/arm64/kernel/cpuinfo.c | 2 +-
arch/arm64/kernel/fpsimd.c | 4 +-
drivers/clocksource/arm_arch_timer.c | 8 ++++
14 files changed, 111 insertions(+), 53 deletions(-)
diff --git a/arch/arm64/crypto/aes-ce-ccm-glue.c b/arch/arm64/crypto/aes-ce-ccm-glue.c
index 68b11aa..8bca470 100644
--- a/arch/arm64/crypto/aes-ce-ccm-glue.c
+++ b/arch/arm64/crypto/aes-ce-ccm-glue.c
@@ -374,7 +374,7 @@ static struct aead_alg ccm_aes_alg = {
static int __init aes_mod_init(void)
{
- if (!(elf_hwcap & HWCAP_AES))
+ if (!cpu_have_feature(KERNEL_HWCAP_AES))
return -ENODEV;
return crypto_register_aead(&ccm_aes_alg);
}
diff --git a/arch/arm64/crypto/aes-neonbs-glue.c b/arch/arm64/crypto/aes-neonbs-glue.c
index e7a95a5..e5a8334 100644
--- a/arch/arm64/crypto/aes-neonbs-glue.c
+++ b/arch/arm64/crypto/aes-neonbs-glue.c
@@ -440,7 +440,7 @@ static int __init aes_init(void)
int err;
int i;
- if (!(elf_hwcap & HWCAP_ASIMD))
+ if (!cpu_have_feature(KERNEL_HWCAP_ASIMD))
return -ENODEV;
err = crypto_register_skciphers(aes_algs, ARRAY_SIZE(aes_algs));
diff --git a/arch/arm64/crypto/chacha-neon-glue.c b/arch/arm64/crypto/chacha-neon-glue.c
index bece1d8..54de986 100644
--- a/arch/arm64/crypto/chacha-neon-glue.c
+++ b/arch/arm64/crypto/chacha-neon-glue.c
@@ -173,7 +173,7 @@ static struct skcipher_alg algs[] = {
static int __init chacha_simd_mod_init(void)
{
- if (!(elf_hwcap & HWCAP_ASIMD))
+ if (!cpu_have_feature(KERNEL_HWCAP_ASIMD))
return -ENODEV;
return crypto_register_skciphers(algs, ARRAY_SIZE(algs));
diff --git a/arch/arm64/crypto/crct10dif-ce-glue.c b/arch/arm64/crypto/crct10dif-ce-glue.c
index b461d62..d78d123 100644
--- a/arch/arm64/crypto/crct10dif-ce-glue.c
+++ b/arch/arm64/crypto/crct10dif-ce-glue.c
@@ -88,7 +88,7 @@ static struct shash_alg crc_t10dif_alg = {
static int __init crc_t10dif_mod_init(void)
{
- if (elf_hwcap & HWCAP_PMULL)
+ if (cpu_have_feature(KERNEL_HWCAP_PMULL))
crc_t10dif_pmull = crc_t10dif_pmull_p64;
else
crc_t10dif_pmull = crc_t10dif_pmull_p8;
diff --git a/arch/arm64/crypto/ghash-ce-glue.c b/arch/arm64/crypto/ghash-ce-glue.c
index 067d893..f3aa5dd 100644
--- a/arch/arm64/crypto/ghash-ce-glue.c
+++ b/arch/arm64/crypto/ghash-ce-glue.c
@@ -646,10 +646,10 @@ static int __init ghash_ce_mod_init(void)
{
int ret;
- if (!(elf_hwcap & HWCAP_ASIMD))
+ if (!cpu_have_feature(KERNEL_HWCAP_ASIMD))
return -ENODEV;
- if (elf_hwcap & HWCAP_PMULL)
+ if (cpu_have_feature(KERNEL_HWCAP_PMULL))
pmull_ghash_update = pmull_ghash_update_p64;
else
@@ -659,7 +659,7 @@ static int __init ghash_ce_mod_init(void)
if (ret)
return ret;
- if (elf_hwcap & HWCAP_PMULL) {
+ if (cpu_have_feature(KERNEL_HWCAP_PMULL)) {
ret = crypto_register_aead(&gcm_aes_alg);
if (ret)
crypto_unregister_shash(&ghash_alg);
diff --git a/arch/arm64/crypto/nhpoly1305-neon-glue.c b/arch/arm64/crypto/nhpoly1305-neon-glue.c
index 22cc32a..7e6815b 100644
--- a/arch/arm64/crypto/nhpoly1305-neon-glue.c
+++ b/arch/arm64/crypto/nhpoly1305-neon-glue.c
@@ -56,7 +56,7 @@ static struct shash_alg nhpoly1305_alg = {
static int __init nhpoly1305_mod_init(void)
{
- if (!(elf_hwcap & HWCAP_ASIMD))
+ if (!cpu_have_feature(KERNEL_HWCAP_ASIMD))
return -ENODEV;
return crypto_register_shash(&nhpoly1305_alg);
diff --git a/arch/arm64/crypto/sha256-glue.c b/arch/arm64/crypto/sha256-glue.c
index 4aedeae..7799f4c 100644
--- a/arch/arm64/crypto/sha256-glue.c
+++ b/arch/arm64/crypto/sha256-glue.c
@@ -173,7 +173,7 @@ static int __init sha256_mod_init(void)
if (ret)
return ret;
- if (elf_hwcap & HWCAP_ASIMD) {
+ if (cpu_have_feature(KERNEL_HWCAP_ASIMD)) {
ret = crypto_register_shashes(neon_algs, ARRAY_SIZE(neon_algs));
if (ret)
crypto_unregister_shashes(algs, ARRAY_SIZE(algs));
@@ -183,7 +183,7 @@ static int __init sha256_mod_init(void)
static void __exit sha256_mod_fini(void)
{
- if (elf_hwcap & HWCAP_ASIMD)
+ if (cpu_have_feature(KERNEL_HWCAP_ASIMD))
crypto_unregister_shashes(neon_algs, ARRAY_SIZE(neon_algs));
crypto_unregister_shashes(algs, ARRAY_SIZE(algs));
}
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index dfcfba7..15d939e 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -18,11 +18,10 @@
* In the arm64 world (as in the ARM world), elf_hwcap is used both internally
* in the kernel and for user space to keep track of which optional features
* are supported by the current system. So let's map feature 'x' to HWCAP_x.
- * Note that HWCAP_x constants are bit fields so we need to take the log.
*/
-#define MAX_CPU_FEATURES (8 * sizeof(elf_hwcap))
-#define cpu_feature(x) ilog2(HWCAP_ ## x)
+#define MAX_CPU_FEATURES (2 * 8 * sizeof(unsigned long))
+#define cpu_feature(x) (KERNEL_HWCAP_ ## x)
#ifndef __ASSEMBLY__
@@ -396,9 +395,20 @@ extern struct static_key_false arm64_const_caps_ready;
bool this_cpu_has_cap(unsigned int cap);
+static inline void cpu_set_feature(unsigned int num)
+{
+ if (num < 32)
+ elf_hwcap |= BIT(num);
+ else
+ elf_hwcap2 |= BIT(num - 32);
+}
+
static inline bool cpu_have_feature(unsigned int num)
{
- return elf_hwcap & (1UL << num);
+ if (num < 32)
+ return elf_hwcap & BIT(num);
+ else
+ return elf_hwcap2 & BIT(num - 32);
}
/* System capability check for constant caps */
diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
index 400b80b..05ee9b9 100644
--- a/arch/arm64/include/asm/hwcap.h
+++ b/arch/arm64/include/asm/hwcap.h
@@ -39,12 +39,49 @@
#define COMPAT_HWCAP2_SHA2 (1 << 3)
#define COMPAT_HWCAP2_CRC32 (1 << 4)
+/*
+ * KERNEL_HWCAP flags - for elf_hwcap (in kernel)
+ */
+#define KERNEL_HWCAP_FP ilog2(HWCAP_FP)
+#define KERNEL_HWCAP_ASIMD ilog2(HWCAP_ASIMD)
+#define KERNEL_HWCAP_EVTSTRM ilog2(HWCAP_EVTSTRM)
+#define KERNEL_HWCAP_AES ilog2(HWCAP_AES)
+#define KERNEL_HWCAP_PMULL ilog2(HWCAP_PMULL)
+#define KERNEL_HWCAP_SHA1 ilog2(HWCAP_SHA1)
+#define KERNEL_HWCAP_SHA2 ilog2(HWCAP_SHA2)
+#define KERNEL_HWCAP_CRC32 ilog2(HWCAP_CRC32)
+#define KERNEL_HWCAP_ATOMICS ilog2(HWCAP_ATOMICS)
+#define KERNEL_HWCAP_FPHP ilog2(HWCAP_FPHP)
+#define KERNEL_HWCAP_ASIMDHP ilog2(HWCAP_ASIMDHP)
+#define KERNEL_HWCAP_CPUID ilog2(HWCAP_CPUID)
+#define KERNEL_HWCAP_ASIMDRDM ilog2(HWCAP_ASIMDRDM)
+#define KERNEL_HWCAP_JSCVT ilog2(HWCAP_JSCVT)
+#define KERNEL_HWCAP_FCMA ilog2(HWCAP_FCMA)
+#define KERNEL_HWCAP_LRCPC ilog2(HWCAP_LRCPC)
+#define KERNEL_HWCAP_DCPOP ilog2(HWCAP_DCPOP)
+#define KERNEL_HWCAP_SHA3 ilog2(HWCAP_SHA3)
+#define KERNEL_HWCAP_SM3 ilog2(HWCAP_SM3)
+#define KERNEL_HWCAP_SM4 ilog2(HWCAP_SM4)
+#define KERNEL_HWCAP_ASIMDDP ilog2(HWCAP_ASIMDDP)
+#define KERNEL_HWCAP_SHA512 ilog2(HWCAP_SHA512)
+#define KERNEL_HWCAP_SVE ilog2(HWCAP_SVE)
+#define KERNEL_HWCAP_ASIMDFHM ilog2(HWCAP_ASIMDFHM)
+#define KERNEL_HWCAP_DIT ilog2(HWCAP_DIT)
+#define KERNEL_HWCAP_USCAT ilog2(HWCAP_USCAT)
+#define KERNEL_HWCAP_ILRCPC ilog2(HWCAP_ILRCPC)
+#define KERNEL_HWCAP_FLAGM ilog2(HWCAP_FLAGM)
+#define KERNEL_HWCAP_SSBS ilog2(HWCAP_SSBS)
+#define KERNEL_HWCAP_SB ilog2(HWCAP_SB)
+#define KERNEL_HWCAP_PACA ilog2(HWCAP_PACA)
+#define KERNEL_HWCAP_PACG ilog2(HWCAP_PACG)
+
#ifndef __ASSEMBLY__
/*
* This yields a mask that user programs can use to figure out what
* instruction set this cpu supports.
*/
-#define ELF_HWCAP (elf_hwcap)
+#define ELF_HWCAP elf_hwcap
+#define ELF_HWCAP2 elf_hwcap2
#ifdef CONFIG_COMPAT
#define COMPAT_ELF_HWCAP (compat_elf_hwcap)
@@ -60,6 +97,6 @@ enum {
#endif
};
-extern unsigned long elf_hwcap;
+extern unsigned long elf_hwcap, elf_hwcap2;
#endif
#endif
diff --git a/arch/arm64/include/uapi/asm/hwcap.h b/arch/arm64/include/uapi/asm/hwcap.h
index 5f0750c..453b45a 100644
--- a/arch/arm64/include/uapi/asm/hwcap.h
+++ b/arch/arm64/include/uapi/asm/hwcap.h
@@ -18,7 +18,7 @@
#define _UAPI__ASM_HWCAP_H
/*
- * HWCAP flags - for elf_hwcap (in kernel) and AT_HWCAP
+ * HWCAP flags - for AT_HWCAP
*/
#define HWCAP_FP (1 << 0)
#define HWCAP_ASIMD (1 << 1)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index f6d84e2..d10a455 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -38,6 +38,9 @@
unsigned long elf_hwcap __read_mostly;
EXPORT_SYMBOL_GPL(elf_hwcap);
+unsigned long elf_hwcap2 __read_mostly;
+EXPORT_SYMBOL_GPL(elf_hwcap2);
+
#ifdef CONFIG_COMPAT
#define COMPAT_ELF_HWCAP_DEFAULT \
(COMPAT_HWCAP_HALF|COMPAT_HWCAP_THUMB|\
@@ -1536,39 +1539,39 @@ static const struct arm64_cpu_capabilities ptr_auth_hwcap_gen_matches[] = {
#endif
static const struct arm64_cpu_capabilities arm64_elf_hwcaps[] = {
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_AES_SHIFT, FTR_UNSIGNED, 2, CAP_HWCAP, HWCAP_PMULL),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_AES_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_AES),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SHA1_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_SHA1),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SHA2_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_SHA2),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SHA2_SHIFT, FTR_UNSIGNED, 2, CAP_HWCAP, HWCAP_SHA512),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_CRC32_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_CRC32),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_ATOMICS_SHIFT, FTR_UNSIGNED, 2, CAP_HWCAP, HWCAP_ATOMICS),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_RDM_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_ASIMDRDM),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SHA3_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_SHA3),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SM3_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_SM3),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SM4_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_SM4),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_DP_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_ASIMDDP),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_FHM_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_ASIMDFHM),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_TS_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_FLAGM),
- HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_FP_SHIFT, FTR_SIGNED, 0, CAP_HWCAP, HWCAP_FP),
- HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_FP_SHIFT, FTR_SIGNED, 1, CAP_HWCAP, HWCAP_FPHP),
- HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_ASIMD_SHIFT, FTR_SIGNED, 0, CAP_HWCAP, HWCAP_ASIMD),
- HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_ASIMD_SHIFT, FTR_SIGNED, 1, CAP_HWCAP, HWCAP_ASIMDHP),
- HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_DIT_SHIFT, FTR_SIGNED, 1, CAP_HWCAP, HWCAP_DIT),
- HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_DPB_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_DCPOP),
- HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_JSCVT_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_JSCVT),
- HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_FCMA_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_FCMA),
- HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_LRCPC_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_LRCPC),
- HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_LRCPC_SHIFT, FTR_UNSIGNED, 2, CAP_HWCAP, HWCAP_ILRCPC),
- HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_SB_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_SB),
- HWCAP_CAP(SYS_ID_AA64MMFR2_EL1, ID_AA64MMFR2_AT_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_USCAT),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_AES_SHIFT, FTR_UNSIGNED, 2, CAP_HWCAP, KERNEL_HWCAP_PMULL),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_AES_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_AES),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SHA1_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_SHA1),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SHA2_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_SHA2),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SHA2_SHIFT, FTR_UNSIGNED, 2, CAP_HWCAP, KERNEL_HWCAP_SHA512),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_CRC32_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_CRC32),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_ATOMICS_SHIFT, FTR_UNSIGNED, 2, CAP_HWCAP, KERNEL_HWCAP_ATOMICS),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_RDM_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_ASIMDRDM),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SHA3_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_SHA3),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SM3_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_SM3),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SM4_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_SM4),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_DP_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_ASIMDDP),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_FHM_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_ASIMDFHM),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_TS_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_FLAGM),
+ HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_FP_SHIFT, FTR_SIGNED, 0, CAP_HWCAP, KERNEL_HWCAP_FP),
+ HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_FP_SHIFT, FTR_SIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_FPHP),
+ HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_ASIMD_SHIFT, FTR_SIGNED, 0, CAP_HWCAP, KERNEL_HWCAP_ASIMD),
+ HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_ASIMD_SHIFT, FTR_SIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_ASIMDHP),
+ HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_DIT_SHIFT, FTR_SIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_DIT),
+ HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_DPB_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_DCPOP),
+ HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_JSCVT_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_JSCVT),
+ HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_FCMA_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_FCMA),
+ HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_LRCPC_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_LRCPC),
+ HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_LRCPC_SHIFT, FTR_UNSIGNED, 2, CAP_HWCAP, KERNEL_HWCAP_ILRCPC),
+ HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_SB_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_SB),
+ HWCAP_CAP(SYS_ID_AA64MMFR2_EL1, ID_AA64MMFR2_AT_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_USCAT),
#ifdef CONFIG_ARM64_SVE
- HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_SVE_SHIFT, FTR_UNSIGNED, ID_AA64PFR0_SVE, CAP_HWCAP, HWCAP_SVE),
+ HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_SVE_SHIFT, FTR_UNSIGNED, ID_AA64PFR0_SVE, CAP_HWCAP, KERNEL_HWCAP_SVE),
#endif
- HWCAP_CAP(SYS_ID_AA64PFR1_EL1, ID_AA64PFR1_SSBS_SHIFT, FTR_UNSIGNED, ID_AA64PFR1_SSBS_PSTATE_INSNS, CAP_HWCAP, HWCAP_SSBS),
+ HWCAP_CAP(SYS_ID_AA64PFR1_EL1, ID_AA64PFR1_SSBS_SHIFT, FTR_UNSIGNED, ID_AA64PFR1_SSBS_PSTATE_INSNS, CAP_HWCAP, KERNEL_HWCAP_SSBS),
#ifdef CONFIG_ARM64_PTR_AUTH
- HWCAP_MULTI_CAP(ptr_auth_hwcap_addr_matches, CAP_HWCAP, HWCAP_PACA),
- HWCAP_MULTI_CAP(ptr_auth_hwcap_gen_matches, CAP_HWCAP, HWCAP_PACG),
+ HWCAP_MULTI_CAP(ptr_auth_hwcap_addr_matches, CAP_HWCAP, KERNEL_HWCAP_PACA),
+ HWCAP_MULTI_CAP(ptr_auth_hwcap_gen_matches, CAP_HWCAP, KERNEL_HWCAP_PACG),
#endif
{},
};
@@ -1588,7 +1591,7 @@ static void __init cap_set_elf_hwcap(const struct arm64_cpu_capabilities *cap)
{
switch (cap->hwcap_type) {
case CAP_HWCAP:
- elf_hwcap |= cap->hwcap;
+ cpu_set_feature(cap->hwcap);
break;
#ifdef CONFIG_COMPAT
case CAP_COMPAT_HWCAP:
@@ -1611,7 +1614,7 @@ static bool cpus_have_elf_hwcap(const struct arm64_cpu_capabilities *cap)
switch (cap->hwcap_type) {
case CAP_HWCAP:
- rc = (elf_hwcap & cap->hwcap) != 0;
+ rc = cpu_have_feature(cap->hwcap);
break;
#ifdef CONFIG_COMPAT
case CAP_COMPAT_HWCAP:
@@ -1632,7 +1635,7 @@ static bool cpus_have_elf_hwcap(const struct arm64_cpu_capabilities *cap)
static void __init setup_elf_hwcaps(const struct arm64_cpu_capabilities *hwcaps)
{
/* We support emulation of accesses to CPU ID feature registers */
- elf_hwcap |= HWCAP_CPUID;
+ cpu_set_feature(KERNEL_HWCAP_CPUID);
for (; hwcaps->matches; hwcaps++)
if (hwcaps->matches(hwcaps, cpucap_default_scope(hwcaps)))
cap_set_elf_hwcap(hwcaps);
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index ca0685f..810db95 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -167,7 +167,7 @@ static int c_show(struct seq_file *m, void *v)
#endif /* CONFIG_COMPAT */
} else {
for (j = 0; hwcap_str[j]; j++)
- if (elf_hwcap & (1 << j))
+ if (cpu_have_feature(j))
seq_printf(m, " %s", hwcap_str[j]);
}
seq_puts(m, "\n");
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 5ebe73b..f28837c 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -1258,14 +1258,14 @@ static inline void fpsimd_hotplug_init(void) { }
*/
static int __init fpsimd_init(void)
{
- if (elf_hwcap & HWCAP_FP) {
+ if (cpu_have_feature(KERNEL_HWCAP_FP)) {
fpsimd_pm_init();
fpsimd_hotplug_init();
} else {
pr_notice("Floating-point is not implemented\n");
}
- if (!(elf_hwcap & HWCAP_ASIMD))
+ if (!cpu_have_feature(KERNEL_HWCAP_ASIMD))
pr_notice("Advanced SIMD is not implemented\n");
return sve_sysctl_init();
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 9a7d4dc..5b2d56d 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -778,7 +778,11 @@ static void arch_timer_evtstrm_enable(int divider)
cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
| ARCH_TIMER_VIRT_EVT_EN;
arch_timer_set_cntkctl(cntkctl);
+#ifdef CONFIG_ARM64
+ cpu_set_feature(KERNEL_HWCAP_EVTSTRM);
+#else
elf_hwcap |= HWCAP_EVTSTRM;
+#endif
#ifdef CONFIG_COMPAT
compat_elf_hwcap |= COMPAT_HWCAP_EVTSTRM;
#endif
@@ -1000,7 +1004,11 @@ static int arch_timer_cpu_pm_notify(struct notifier_block *self,
} else if (action == CPU_PM_ENTER_FAILED || action == CPU_PM_EXIT) {
arch_timer_set_cntkctl(__this_cpu_read(saved_cntkctl));
+#ifdef CONFIG_ARM64
+ if (cpu_have_feature(KERNEL_HWCAP_EVTSTRM))
+#else
if (elf_hwcap & HWCAP_EVTSTRM)
+#endif
cpumask_set_cpu(smp_processor_id(), &evtstrm_available);
}
return NOTIFY_OK;
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/6] arm64: HWCAP: encapsulate elf_hwcap
2019-02-06 13:31 [PATCH 0/6] Initial support for CVADP Andrew Murray
2019-02-06 13:31 ` [PATCH 1/6] arm64: Handle trapped DC CVADP Andrew Murray
2019-02-06 13:31 ` [PATCH 2/6] arm64: HWCAP: add support for ELF_HWCAP2 Andrew Murray
@ 2019-02-06 13:31 ` Andrew Murray
2019-02-07 11:47 ` Dave Martin
2019-02-06 13:31 ` [PATCH 4/6] arm64: Expose DC CVADP to userspace Andrew Murray
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Andrew Murray @ 2019-02-06 13:31 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon; +Cc: Szabolcs Nagy, dave.martin, linux-arm-kernel
The introduction of elf_hwcap2 introduced accessors which ensure that
features are set/tested in the appropriate elf_hwcapX variable.
Let's now mandate access to elf_hwcapX via these accessors by making
elf_hwcapX static within cpufeature.c.
Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
arch/arm64/include/asm/cpufeature.h | 19 ++++---------------
arch/arm64/include/asm/hwcap.h | 6 +++---
arch/arm64/kernel/cpufeature.c | 33 ++++++++++++++++++++++++++++-----
3 files changed, 35 insertions(+), 23 deletions(-)
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 15d939e..0e6132f 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -395,21 +395,10 @@ extern struct static_key_false arm64_const_caps_ready;
bool this_cpu_has_cap(unsigned int cap);
-static inline void cpu_set_feature(unsigned int num)
-{
- if (num < 32)
- elf_hwcap |= BIT(num);
- else
- elf_hwcap2 |= BIT(num - 32);
-}
-
-static inline bool cpu_have_feature(unsigned int num)
-{
- if (num < 32)
- return elf_hwcap & BIT(num);
- else
- return elf_hwcap2 & BIT(num - 32);
-}
+void cpu_set_feature(unsigned int num);
+bool cpu_have_feature(unsigned int num);
+unsigned long cpu_get_elf_hwcap(void);
+unsigned long cpu_get_elf_hwcap2(void);
/* System capability check for constant caps */
static inline bool __cpus_have_const_cap(int num)
diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
index 05ee9b9..ced87ad 100644
--- a/arch/arm64/include/asm/hwcap.h
+++ b/arch/arm64/include/asm/hwcap.h
@@ -17,6 +17,7 @@
#define __ASM_HWCAP_H
#include <uapi/asm/hwcap.h>
+#include <asm/cpufeature.h>
#define COMPAT_HWCAP_HALF (1 << 1)
#define COMPAT_HWCAP_THUMB (1 << 2)
@@ -80,8 +81,8 @@
* This yields a mask that user programs can use to figure out what
* instruction set this cpu supports.
*/
-#define ELF_HWCAP elf_hwcap
-#define ELF_HWCAP2 elf_hwcap2
+#define ELF_HWCAP cpu_get_elf_hwcap()
+#define ELF_HWCAP2 cpu_get_elf_hwcap2()
#ifdef CONFIG_COMPAT
#define COMPAT_ELF_HWCAP (compat_elf_hwcap)
@@ -97,6 +98,5 @@ enum {
#endif
};
-extern unsigned long elf_hwcap, elf_hwcap2;
#endif
#endif
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index d10a455..5b9620d 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -35,11 +35,8 @@
#include <asm/traps.h>
#include <asm/virt.h>
-unsigned long elf_hwcap __read_mostly;
-EXPORT_SYMBOL_GPL(elf_hwcap);
-
-unsigned long elf_hwcap2 __read_mostly;
-EXPORT_SYMBOL_GPL(elf_hwcap2);
+static unsigned long elf_hwcap __read_mostly;
+static unsigned long elf_hwcap2 __read_mostly;
#ifdef CONFIG_COMPAT
#define COMPAT_ELF_HWCAP_DEFAULT \
@@ -1912,6 +1909,32 @@ bool this_cpu_has_cap(unsigned int n)
return false;
}
+void cpu_set_feature(unsigned int num)
+{
+ if (num < 32)
+ elf_hwcap |= BIT(num);
+ else
+ elf_hwcap2 |= BIT(num - 32);
+}
+
+bool cpu_have_feature(unsigned int num)
+{
+ if (num < 32)
+ return elf_hwcap & BIT(num);
+ else
+ return elf_hwcap2 & BIT(num - 32);
+}
+
+unsigned long cpu_get_elf_hwcap(void)
+{
+ return elf_hwcap;
+}
+
+unsigned long cpu_get_elf_hwcap2(void)
+{
+ return elf_hwcap2;
+}
+
static void __init setup_system_capabilities(void)
{
/*
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/6] arm64: Expose DC CVADP to userspace
2019-02-06 13:31 [PATCH 0/6] Initial support for CVADP Andrew Murray
` (2 preceding siblings ...)
2019-02-06 13:31 ` [PATCH 3/6] arm64: HWCAP: encapsulate elf_hwcap Andrew Murray
@ 2019-02-06 13:31 ` Andrew Murray
2019-02-06 13:31 ` [PATCH 5/6] arm64: add CVADP support to the cache maintenance helper Andrew Murray
2019-02-06 13:31 ` [PATCH 6/6] arm64: Advertise ARM64_HAS_DCPODP cpu feature Andrew Murray
5 siblings, 0 replies; 14+ messages in thread
From: Andrew Murray @ 2019-02-06 13:31 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon; +Cc: Szabolcs Nagy, dave.martin, linux-arm-kernel
ARMv8.5 builds upon the ARMv8.2 DC CVAP instruction by introducing a DC
CVADP instruction which cleans the data cache to the point of deep
persistence. Let's expose this support via the arm64 ELF hwcaps.
Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
Documentation/arm64/elf_hwcaps.txt | 4 ++++
arch/arm64/include/asm/hwcap.h | 1 +
arch/arm64/include/uapi/asm/hwcap.h | 5 +++++
arch/arm64/kernel/cpufeature.c | 1 +
arch/arm64/kernel/cpuinfo.c | 1 +
5 files changed, 12 insertions(+)
diff --git a/Documentation/arm64/elf_hwcaps.txt b/Documentation/arm64/elf_hwcaps.txt
index 13d6691..7db6e13 100644
--- a/Documentation/arm64/elf_hwcaps.txt
+++ b/Documentation/arm64/elf_hwcaps.txt
@@ -135,6 +135,10 @@ HWCAP_DCPOP
Functionality implied by ID_AA64ISAR1_EL1.DPB == 0b0001.
+HWCAP_DCPODP
+
+ Functionality implied by ID_AA64ISAR1_EL1.DPB == 0b0010.
+
HWCAP_SHA3
Functionality implied by ID_AA64ISAR0_EL1.SHA3 == 0b0001.
diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
index ced87ad..8696db9 100644
--- a/arch/arm64/include/asm/hwcap.h
+++ b/arch/arm64/include/asm/hwcap.h
@@ -75,6 +75,7 @@
#define KERNEL_HWCAP_SB ilog2(HWCAP_SB)
#define KERNEL_HWCAP_PACA ilog2(HWCAP_PACA)
#define KERNEL_HWCAP_PACG ilog2(HWCAP_PACG)
+#define KERNEL_HWCAP_DCPODP (ilog2(HWCAP2_DCPODP) + 32)
#ifndef __ASSEMBLY__
/*
diff --git a/arch/arm64/include/uapi/asm/hwcap.h b/arch/arm64/include/uapi/asm/hwcap.h
index 453b45a..d64af39 100644
--- a/arch/arm64/include/uapi/asm/hwcap.h
+++ b/arch/arm64/include/uapi/asm/hwcap.h
@@ -53,4 +53,9 @@
#define HWCAP_PACA (1 << 30)
#define HWCAP_PACG (1UL << 31)
+/*
+ * HWCAP2 flags - for AT_HWCAP2
+ */
+#define HWCAP2_DCPODP (1 << 0)
+
#endif /* _UAPI__ASM_HWCAP_H */
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 5b9620d..4c0c5b9 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1556,6 +1556,7 @@ static const struct arm64_cpu_capabilities arm64_elf_hwcaps[] = {
HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_ASIMD_SHIFT, FTR_SIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_ASIMDHP),
HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_DIT_SHIFT, FTR_SIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_DIT),
HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_DPB_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_DCPOP),
+ HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_DPB_SHIFT, FTR_UNSIGNED, 2, CAP_HWCAP, KERNEL_HWCAP_DCPODP),
HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_JSCVT_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_JSCVT),
HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_FCMA_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_FCMA),
HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_LRCPC_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_LRCPC),
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index 810db95..093ca53 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -85,6 +85,7 @@ static const char *const hwcap_str[] = {
"sb",
"paca",
"pacg",
+ "dcpodp",
NULL
};
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 5/6] arm64: add CVADP support to the cache maintenance helper
2019-02-06 13:31 [PATCH 0/6] Initial support for CVADP Andrew Murray
` (3 preceding siblings ...)
2019-02-06 13:31 ` [PATCH 4/6] arm64: Expose DC CVADP to userspace Andrew Murray
@ 2019-02-06 13:31 ` Andrew Murray
2019-02-06 13:31 ` [PATCH 6/6] arm64: Advertise ARM64_HAS_DCPODP cpu feature Andrew Murray
5 siblings, 0 replies; 14+ messages in thread
From: Andrew Murray @ 2019-02-06 13:31 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon; +Cc: Szabolcs Nagy, dave.martin, linux-arm-kernel
Allow users of dcache_by_line_op to specify cvadp as an op.
Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
arch/arm64/include/asm/assembler.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index 4feb611..75a62d7 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -414,10 +414,14 @@ alternative_endif
.ifc \op, cvap
sys 3, c7, c12, 1, \kaddr // dc cvap
.else
+ .ifc \op, cvadp
+ sys 3, c7, c13, 1, \kaddr // dc cvadp
+ .else
dc \op, \kaddr
.endif
.endif
.endif
+ .endif
add \kaddr, \kaddr, \tmp1
cmp \kaddr, \size
b.lo 9998b
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 6/6] arm64: Advertise ARM64_HAS_DCPODP cpu feature
2019-02-06 13:31 [PATCH 0/6] Initial support for CVADP Andrew Murray
` (4 preceding siblings ...)
2019-02-06 13:31 ` [PATCH 5/6] arm64: add CVADP support to the cache maintenance helper Andrew Murray
@ 2019-02-06 13:31 ` Andrew Murray
5 siblings, 0 replies; 14+ messages in thread
From: Andrew Murray @ 2019-02-06 13:31 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon; +Cc: Szabolcs Nagy, dave.martin, linux-arm-kernel
Advertise ARM64_HAS_DCPOP when both DC CVAP and DC CVADP are supported.
Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
arch/arm64/include/asm/cpucaps.h | 3 ++-
arch/arm64/kernel/cpufeature.c | 9 +++++++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index 82e9099..4a2a77d 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -60,7 +60,8 @@
#define ARM64_HAS_ADDRESS_AUTH_IMP_DEF 39
#define ARM64_HAS_GENERIC_AUTH_ARCH 40
#define ARM64_HAS_GENERIC_AUTH_IMP_DEF 41
+#define ARM64_HAS_DCPODP 42
-#define ARM64_NCAPS 42
+#define ARM64_NCAPS 43
#endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 4c0c5b9..ae28f02 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1320,6 +1320,15 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
.field_pos = ID_AA64ISAR1_DPB_SHIFT,
.min_field_value = 1,
},
+ {
+ .desc = "Data cache clean to Point of Deep Persistence",
+ .capability = ARM64_HAS_DCPODP,
+ .type = ARM64_CPUCAP_SYSTEM_FEATURE,
+ .matches = has_cpuid_feature,
+ .sys_reg = SYS_ID_AA64ISAR1_EL1,
+ .field_pos = ID_AA64ISAR1_DPB_SHIFT,
+ .min_field_value = 2,
+ },
#endif
#ifdef CONFIG_ARM64_SVE
{
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/6] arm64: HWCAP: add support for ELF_HWCAP2
2019-02-06 13:31 ` [PATCH 2/6] arm64: HWCAP: add support for ELF_HWCAP2 Andrew Murray
@ 2019-02-07 11:31 ` Dave Martin
2019-02-18 15:32 ` Andrew Murray
0 siblings, 1 reply; 14+ messages in thread
From: Dave Martin @ 2019-02-07 11:31 UTC (permalink / raw)
To: Andrew Murray
Cc: Szabolcs Nagy, Catalin Marinas, Will Deacon, linux-arm-kernel
On Wed, Feb 06, 2019 at 01:31:04PM +0000, Andrew Murray wrote:
> As we will exhaust the first 32 bits of ELF_HWCAP let's start
> exposing ELF_HWCAP2 to userspace.
>
> Whilst it's possible to use the remaining 32 bits of ELF_HWCAP, we
> prefer to expand into ELF_HWCAP2 in order to provide a consistent
> view to userspace between ILP32 and LP64.
>
> To reduce complexity and allow for future expansion, we now
> represent hwcaps in the kernel as ordinals and use a
> KERNEL_HWCAP_ prefix. This allows us to support automatic feature
> based module loading for all our hwcaps.
>
> We introduce cpu_set_feature to set hwcaps in the relevant
> elf_hwcapX variable which compliments the existing cpu_have_feature
> helper. These helpers allow us to clean up existing direct uses of
> elf_hwcap.
>
> Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> ---
[...]
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index dfcfba7..15d939e 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -18,11 +18,10 @@
> * In the arm64 world (as in the ARM world), elf_hwcap is used both internally
> * in the kernel and for user space to keep track of which optional features
> * are supported by the current system. So let's map feature 'x' to HWCAP_x.
> - * Note that HWCAP_x constants are bit fields so we need to take the log.
> */
>
> -#define MAX_CPU_FEATURES (8 * sizeof(elf_hwcap))
> -#define cpu_feature(x) ilog2(HWCAP_ ## x)
> +#define MAX_CPU_FEATURES (2 * 8 * sizeof(unsigned long))
Doesn't this make 2 * 8 * 8 == 128?
Maybe just say "64". We open-code 32 later anyway.
> +#define cpu_feature(x) (KERNEL_HWCAP_ ## x)
>
> #ifndef __ASSEMBLY__
>
> @@ -396,9 +395,20 @@ extern struct static_key_false arm64_const_caps_ready;
>
> bool this_cpu_has_cap(unsigned int cap);
>
> +static inline void cpu_set_feature(unsigned int num)
> +{
> + if (num < 32)
> + elf_hwcap |= BIT(num);
> + else
> + elf_hwcap2 |= BIT(num - 32);
> +}
> +
> static inline bool cpu_have_feature(unsigned int num)
> {
> - return elf_hwcap & (1UL << num);
> + if (num < 32)
> + return elf_hwcap & BIT(num);
> + else
> + return elf_hwcap2 & BIT(num - 32);
> }
Nit: I worry a bit that people will blithely pass HWCAP_foo to these,
not realising that the interface has changed.
Can we do something like the following:
static inline bool __cpu_have_feature(unsigned int num)
{
if (WARN_ON(num >= 64))
return false;
if (num < 32)
return elf_hwcap & BIT(num);
else
return elf_hwcap2 & BIT(num);
}
#define cpu_have_feature(name) __cpu_have_feature(cpu_feature(name))
In compiletime-constant cases, the compiler should simply delete the
WARN. A couple of cases in the cpufeatures and /proc/cpuinfo code would
need to use __cpu_have_feature directly and would retain the WARN, but
that's not fastpath and probably not the end of the world, plus we
get an additional debugging aid if that code goes wrong.
Alternatively, we could bake cpu_feature() into HWCAP_CAP(), making
it impossible to specify invalid or user-encoded hwcaps there either.
Then we might justifiably omit the policing in __cpu_have_feature()
altogether.
>
> /* System capability check for constant caps */
> diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
> index 400b80b..05ee9b9 100644
> --- a/arch/arm64/include/asm/hwcap.h
> +++ b/arch/arm64/include/asm/hwcap.h
> @@ -39,12 +39,49 @@
> #define COMPAT_HWCAP2_SHA2 (1 << 3)
> #define COMPAT_HWCAP2_CRC32 (1 << 4)
>
> +/*
> + * KERNEL_HWCAP flags - for elf_hwcap (in kernel)
> + */
> +#define KERNEL_HWCAP_FP ilog2(HWCAP_FP)
> +#define KERNEL_HWCAP_ASIMD ilog2(HWCAP_ASIMD)
> +#define KERNEL_HWCAP_EVTSTRM ilog2(HWCAP_EVTSTRM)
> +#define KERNEL_HWCAP_AES ilog2(HWCAP_AES)
> +#define KERNEL_HWCAP_PMULL ilog2(HWCAP_PMULL)
> +#define KERNEL_HWCAP_SHA1 ilog2(HWCAP_SHA1)
> +#define KERNEL_HWCAP_SHA2 ilog2(HWCAP_SHA2)
> +#define KERNEL_HWCAP_CRC32 ilog2(HWCAP_CRC32)
> +#define KERNEL_HWCAP_ATOMICS ilog2(HWCAP_ATOMICS)
> +#define KERNEL_HWCAP_FPHP ilog2(HWCAP_FPHP)
> +#define KERNEL_HWCAP_ASIMDHP ilog2(HWCAP_ASIMDHP)
> +#define KERNEL_HWCAP_CPUID ilog2(HWCAP_CPUID)
> +#define KERNEL_HWCAP_ASIMDRDM ilog2(HWCAP_ASIMDRDM)
> +#define KERNEL_HWCAP_JSCVT ilog2(HWCAP_JSCVT)
> +#define KERNEL_HWCAP_FCMA ilog2(HWCAP_FCMA)
> +#define KERNEL_HWCAP_LRCPC ilog2(HWCAP_LRCPC)
> +#define KERNEL_HWCAP_DCPOP ilog2(HWCAP_DCPOP)
> +#define KERNEL_HWCAP_SHA3 ilog2(HWCAP_SHA3)
> +#define KERNEL_HWCAP_SM3 ilog2(HWCAP_SM3)
> +#define KERNEL_HWCAP_SM4 ilog2(HWCAP_SM4)
> +#define KERNEL_HWCAP_ASIMDDP ilog2(HWCAP_ASIMDDP)
> +#define KERNEL_HWCAP_SHA512 ilog2(HWCAP_SHA512)
> +#define KERNEL_HWCAP_SVE ilog2(HWCAP_SVE)
> +#define KERNEL_HWCAP_ASIMDFHM ilog2(HWCAP_ASIMDFHM)
> +#define KERNEL_HWCAP_DIT ilog2(HWCAP_DIT)
> +#define KERNEL_HWCAP_USCAT ilog2(HWCAP_USCAT)
> +#define KERNEL_HWCAP_ILRCPC ilog2(HWCAP_ILRCPC)
> +#define KERNEL_HWCAP_FLAGM ilog2(HWCAP_FLAGM)
> +#define KERNEL_HWCAP_SSBS ilog2(HWCAP_SSBS)
> +#define KERNEL_HWCAP_SB ilog2(HWCAP_SB)
> +#define KERNEL_HWCAP_PACA ilog2(HWCAP_PACA)
> +#define KERNEL_HWCAP_PACG ilog2(HWCAP_PACG)
Nit: Odd spacing? Personally I wouldn't indent with spaces just so that
(ilog2() + 32) lines up nicely, but that's just my opinion and not a
huge deal.
(The spacing can make subsequent diffs look jagged by default, which
can be a distraction for reviewers.)
> +
> #ifndef __ASSEMBLY__
> /*
> * This yields a mask that user programs can use to figure out what
> * instruction set this cpu supports.
> */
> -#define ELF_HWCAP (elf_hwcap)
> +#define ELF_HWCAP elf_hwcap
> +#define ELF_HWCAP2 elf_hwcap2
Do we need elf_hwcap2 at all?
We could just have these macros fish the bits out of a single variable,
or wherever.
However, it may be better to keep things this way if we want to maintain
the elf_hwcap in the kernel/module ABI. See note on EXPORT_SYMBOL_GPL()
removal for elf_hwcap in the subsequent patch.
>
> #ifdef CONFIG_COMPAT
> #define COMPAT_ELF_HWCAP (compat_elf_hwcap)
> @@ -60,6 +97,6 @@ enum {
> #endif
> };
>
> -extern unsigned long elf_hwcap;
> +extern unsigned long elf_hwcap, elf_hwcap2;
> #endif
> #endif
> diff --git a/arch/arm64/include/uapi/asm/hwcap.h b/arch/arm64/include/uapi/asm/hwcap.h
> index 5f0750c..453b45a 100644
> --- a/arch/arm64/include/uapi/asm/hwcap.h
> +++ b/arch/arm64/include/uapi/asm/hwcap.h
> @@ -18,7 +18,7 @@
> #define _UAPI__ASM_HWCAP_H
>
> /*
> - * HWCAP flags - for elf_hwcap (in kernel) and AT_HWCAP
> + * HWCAP flags - for AT_HWCAP
> */
> #define HWCAP_FP (1 << 0)
> #define HWCAP_ASIMD (1 << 1)
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index f6d84e2..d10a455 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -38,6 +38,9 @@
> unsigned long elf_hwcap __read_mostly;
> EXPORT_SYMBOL_GPL(elf_hwcap);
>
> +unsigned long elf_hwcap2 __read_mostly;
> +EXPORT_SYMBOL_GPL(elf_hwcap2);
> +
Historically nobody used this because it didn't exist, and we don't want
anybody to use it (since cpu_have_feature() etc. is the preferred
interface).
So lose EXPORT_SYMBOL_GPL() here. (You remove it in any case in a
subsequent patch, but having it even transiently sends mixed messages
about the intent.)
[...]
Otherwise looks reasonable.
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/6] arm64: HWCAP: encapsulate elf_hwcap
2019-02-06 13:31 ` [PATCH 3/6] arm64: HWCAP: encapsulate elf_hwcap Andrew Murray
@ 2019-02-07 11:47 ` Dave Martin
2019-02-18 15:46 ` Andrew Murray
0 siblings, 1 reply; 14+ messages in thread
From: Dave Martin @ 2019-02-07 11:47 UTC (permalink / raw)
To: Andrew Murray
Cc: Szabolcs Nagy, Catalin Marinas, Will Deacon, linux-arm-kernel
On Wed, Feb 06, 2019 at 01:31:05PM +0000, Andrew Murray wrote:
> The introduction of elf_hwcap2 introduced accessors which ensure that
> features are set/tested in the appropriate elf_hwcapX variable.
>
> Let's now mandate access to elf_hwcapX via these accessors by making
> elf_hwcapX static within cpufeature.c.
>
> Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> ---
> arch/arm64/include/asm/cpufeature.h | 19 ++++---------------
> arch/arm64/include/asm/hwcap.h | 6 +++---
> arch/arm64/kernel/cpufeature.c | 33 ++++++++++++++++++++++++++++-----
> 3 files changed, 35 insertions(+), 23 deletions(-)
[...]
> diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
> index 05ee9b9..ced87ad 100644
> --- a/arch/arm64/include/asm/hwcap.h
> +++ b/arch/arm64/include/asm/hwcap.h
[...]
> @@ -97,6 +98,5 @@ enum {
> #endif
> };
>
> -extern unsigned long elf_hwcap, elf_hwcap2;
> #endif
> #endif
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index d10a455..5b9620d 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -35,11 +35,8 @@
> #include <asm/traps.h>
> #include <asm/virt.h>
>
> -unsigned long elf_hwcap __read_mostly;
> -EXPORT_SYMBOL_GPL(elf_hwcap);
> -
Will this affect out-of-tree modules?
I also note an out-of-arch/ use in
drivers/clocksource/arm_arch_timer.c, which this series doesn't appear
to address (or I missed it).
We are allowed to break EXPORT_SYMBOL_GPL()s in general so long as it's
not done without any meaningful reason, so maybe this is not a huge
concern so long as we catch all the in-tree users.
[...]
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/6] arm64: HWCAP: add support for ELF_HWCAP2
2019-02-07 11:31 ` Dave Martin
@ 2019-02-18 15:32 ` Andrew Murray
2019-02-18 16:02 ` Dave Martin
0 siblings, 1 reply; 14+ messages in thread
From: Andrew Murray @ 2019-02-18 15:32 UTC (permalink / raw)
To: Dave Martin; +Cc: Szabolcs Nagy, Catalin Marinas, Will Deacon, linux-arm-kernel
On Thu, Feb 07, 2019 at 11:31:24AM +0000, Dave Martin wrote:
> On Wed, Feb 06, 2019 at 01:31:04PM +0000, Andrew Murray wrote:
> > As we will exhaust the first 32 bits of ELF_HWCAP let's start
> > exposing ELF_HWCAP2 to userspace.
> >
> > Whilst it's possible to use the remaining 32 bits of ELF_HWCAP, we
> > prefer to expand into ELF_HWCAP2 in order to provide a consistent
> > view to userspace between ILP32 and LP64.
> >
> > To reduce complexity and allow for future expansion, we now
> > represent hwcaps in the kernel as ordinals and use a
> > KERNEL_HWCAP_ prefix. This allows us to support automatic feature
> > based module loading for all our hwcaps.
> >
> > We introduce cpu_set_feature to set hwcaps in the relevant
> > elf_hwcapX variable which compliments the existing cpu_have_feature
> > helper. These helpers allow us to clean up existing direct uses of
> > elf_hwcap.
> >
> > Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> > ---
>
> [...]
>
> > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> > index dfcfba7..15d939e 100644
> > --- a/arch/arm64/include/asm/cpufeature.h
> > +++ b/arch/arm64/include/asm/cpufeature.h
> > @@ -18,11 +18,10 @@
> > * In the arm64 world (as in the ARM world), elf_hwcap is used both internally
> > * in the kernel and for user space to keep track of which optional features
> > * are supported by the current system. So let's map feature 'x' to HWCAP_x.
> > - * Note that HWCAP_x constants are bit fields so we need to take the log.
> > */
> >
> > -#define MAX_CPU_FEATURES (8 * sizeof(elf_hwcap))
> > -#define cpu_feature(x) ilog2(HWCAP_ ## x)
> > +#define MAX_CPU_FEATURES (2 * 8 * sizeof(unsigned long))
>
> Doesn't this make 2 * 8 * 8 == 128?
>
> Maybe just say "64". We open-code 32 later anyway.
I guess 128 would have been correct if we use all the bits of
each ELF_HWCAP{,2} - but we chose to limit each ELF_HWCAP to be
32 bits.
I'll set this to 64 as you suggest.
>
> > +#define cpu_feature(x) (KERNEL_HWCAP_ ## x)
> >
> > #ifndef __ASSEMBLY__
> >
> > @@ -396,9 +395,20 @@ extern struct static_key_false arm64_const_caps_ready;
> >
> > bool this_cpu_has_cap(unsigned int cap);
> >
> > +static inline void cpu_set_feature(unsigned int num)
> > +{
> > + if (num < 32)
> > + elf_hwcap |= BIT(num);
> > + else
> > + elf_hwcap2 |= BIT(num - 32);
> > +}
> > +
> > static inline bool cpu_have_feature(unsigned int num)
> > {
> > - return elf_hwcap & (1UL << num);
> > + if (num < 32)
> > + return elf_hwcap & BIT(num);
> > + else
> > + return elf_hwcap2 & BIT(num - 32);
> > }
>
> Nit: I worry a bit that people will blithely pass HWCAP_foo to these,
> not realising that the interface has changed.
Yeah this would be an easy mistake to make.
>
> Can we do something like the following:
>
> static inline bool __cpu_have_feature(unsigned int num)
> {
> if (WARN_ON(num >= 64))
> return false;
>
> if (num < 32)
> return elf_hwcap & BIT(num);
> else
> return elf_hwcap2 & BIT(num);
> }
>
> #define cpu_have_feature(name) __cpu_have_feature(cpu_feature(name))
It took me a while to figure out what the benefit of the above line
is - this means that callers of cpu_have_feature must use the unprefixed
feature name (e.g. SHA512), anything else such as (KERNEL_HWCAP_SHA512
or HWCAP_SHA512) would result in a compile time error. Whereas without
this users could pass KERNEL_HWCAP_x or HWCAP_x without compile errors
yet resulting in incorrectly passing a bitfield when using HWCAP_x.
>
>
> In compiletime-constant cases, the compiler should simply delete the
> WARN.
Is this really needed? Surely this would only come into play when a user
directly calls __cpu_have_feature. I think this is overkill, especially
as __cpu_have_feature is a new function.
> A couple of cases in the cpufeatures and /proc/cpuinfo code would
> need to use __cpu_have_feature directly and would retain the WARN, but
> that's not fastpath and probably not the end of the world, plus we
> get an additional debugging aid if that code goes wrong.
But even so, the WARN (for the cpuinfo case) is unlikely to ever be hit.
The challenge with this approach is print_cpu_modalias (drivers/base/cpu.c),
this also uses cpu_have_feature - this expects cpu_have_feature to take
an ordinal. How can we work around this?
>
> Alternatively, we could bake cpu_feature() into HWCAP_CAP(), making
> it impossible to specify invalid or user-encoded hwcaps there either.
> Then we might justifiably omit the policing in __cpu_have_feature()
> altogether.
This would certainly please checkpatch with respect to line lengths :)
However since this patchset, we now use cpu_have_feature elsewhere, such
as in the crypto drivers.
>
> >
> > /* System capability check for constant caps */
> > diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
> > index 400b80b..05ee9b9 100644
> > --- a/arch/arm64/include/asm/hwcap.h
> > +++ b/arch/arm64/include/asm/hwcap.h
> > @@ -39,12 +39,49 @@
> > #define COMPAT_HWCAP2_SHA2 (1 << 3)
> > #define COMPAT_HWCAP2_CRC32 (1 << 4)
> >
> > +/*
> > + * KERNEL_HWCAP flags - for elf_hwcap (in kernel)
> > + */
> > +#define KERNEL_HWCAP_FP ilog2(HWCAP_FP)
> > +#define KERNEL_HWCAP_ASIMD ilog2(HWCAP_ASIMD)
> > +#define KERNEL_HWCAP_EVTSTRM ilog2(HWCAP_EVTSTRM)
> > +#define KERNEL_HWCAP_AES ilog2(HWCAP_AES)
> > +#define KERNEL_HWCAP_PMULL ilog2(HWCAP_PMULL)
> > +#define KERNEL_HWCAP_SHA1 ilog2(HWCAP_SHA1)
> > +#define KERNEL_HWCAP_SHA2 ilog2(HWCAP_SHA2)
> > +#define KERNEL_HWCAP_CRC32 ilog2(HWCAP_CRC32)
> > +#define KERNEL_HWCAP_ATOMICS ilog2(HWCAP_ATOMICS)
> > +#define KERNEL_HWCAP_FPHP ilog2(HWCAP_FPHP)
> > +#define KERNEL_HWCAP_ASIMDHP ilog2(HWCAP_ASIMDHP)
> > +#define KERNEL_HWCAP_CPUID ilog2(HWCAP_CPUID)
> > +#define KERNEL_HWCAP_ASIMDRDM ilog2(HWCAP_ASIMDRDM)
> > +#define KERNEL_HWCAP_JSCVT ilog2(HWCAP_JSCVT)
> > +#define KERNEL_HWCAP_FCMA ilog2(HWCAP_FCMA)
> > +#define KERNEL_HWCAP_LRCPC ilog2(HWCAP_LRCPC)
> > +#define KERNEL_HWCAP_DCPOP ilog2(HWCAP_DCPOP)
> > +#define KERNEL_HWCAP_SHA3 ilog2(HWCAP_SHA3)
> > +#define KERNEL_HWCAP_SM3 ilog2(HWCAP_SM3)
> > +#define KERNEL_HWCAP_SM4 ilog2(HWCAP_SM4)
> > +#define KERNEL_HWCAP_ASIMDDP ilog2(HWCAP_ASIMDDP)
> > +#define KERNEL_HWCAP_SHA512 ilog2(HWCAP_SHA512)
> > +#define KERNEL_HWCAP_SVE ilog2(HWCAP_SVE)
> > +#define KERNEL_HWCAP_ASIMDFHM ilog2(HWCAP_ASIMDFHM)
> > +#define KERNEL_HWCAP_DIT ilog2(HWCAP_DIT)
> > +#define KERNEL_HWCAP_USCAT ilog2(HWCAP_USCAT)
> > +#define KERNEL_HWCAP_ILRCPC ilog2(HWCAP_ILRCPC)
> > +#define KERNEL_HWCAP_FLAGM ilog2(HWCAP_FLAGM)
> > +#define KERNEL_HWCAP_SSBS ilog2(HWCAP_SSBS)
> > +#define KERNEL_HWCAP_SB ilog2(HWCAP_SB)
> > +#define KERNEL_HWCAP_PACA ilog2(HWCAP_PACA)
> > +#define KERNEL_HWCAP_PACG ilog2(HWCAP_PACG)
>
> Nit: Odd spacing? Personally I wouldn't indent with spaces just so that
> (ilog2() + 32) lines up nicely, but that's just my opinion and not a
> huge deal.
>
> (The spacing can make subsequent diffs look jagged by default, which
> can be a distraction for reviewers.)
I'll convert to tabs, I probably copy pasted and the tabs got converted.
>
> > +
> > #ifndef __ASSEMBLY__
> > /*
> > * This yields a mask that user programs can use to figure out what
> > * instruction set this cpu supports.
> > */
> > -#define ELF_HWCAP (elf_hwcap)
> > +#define ELF_HWCAP elf_hwcap
> > +#define ELF_HWCAP2 elf_hwcap2
>
> Do we need elf_hwcap2 at all?
>
> We could just have these macros fish the bits out of a single variable,
> or wherever.
>
> However, it may be better to keep things this way if we want to maintain
> the elf_hwcap in the kernel/module ABI. See note on EXPORT_SYMBOL_GPL()
> removal for elf_hwcap in the subsequent patch.
OK, I'll leave it as it is.
>
> >
> > #ifdef CONFIG_COMPAT
> > #define COMPAT_ELF_HWCAP (compat_elf_hwcap)
> > @@ -60,6 +97,6 @@ enum {
> > #endif
> > };
> >
> > -extern unsigned long elf_hwcap;
> > +extern unsigned long elf_hwcap, elf_hwcap2;
> > #endif
> > #endif
> > diff --git a/arch/arm64/include/uapi/asm/hwcap.h b/arch/arm64/include/uapi/asm/hwcap.h
> > index 5f0750c..453b45a 100644
> > --- a/arch/arm64/include/uapi/asm/hwcap.h
> > +++ b/arch/arm64/include/uapi/asm/hwcap.h
> > @@ -18,7 +18,7 @@
> > #define _UAPI__ASM_HWCAP_H
> >
> > /*
> > - * HWCAP flags - for elf_hwcap (in kernel) and AT_HWCAP
> > + * HWCAP flags - for AT_HWCAP
> > */
> > #define HWCAP_FP (1 << 0)
> > #define HWCAP_ASIMD (1 << 1)
> > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > index f6d84e2..d10a455 100644
> > --- a/arch/arm64/kernel/cpufeature.c
> > +++ b/arch/arm64/kernel/cpufeature.c
> > @@ -38,6 +38,9 @@
> > unsigned long elf_hwcap __read_mostly;
> > EXPORT_SYMBOL_GPL(elf_hwcap);
> >
> > +unsigned long elf_hwcap2 __read_mostly;
> > +EXPORT_SYMBOL_GPL(elf_hwcap2);
> > +
>
> Historically nobody used this because it didn't exist, and we don't want
> anybody to use it (since cpu_have_feature() etc. is the preferred
> interface).
In that case I should probably EXPORT_SYMBOL_GPL cpu_have_feature.
>
> So lose EXPORT_SYMBOL_GPL() here. (You remove it in any case in a
> subsequent patch, but having it even transiently sends mixed messages
> about the intent.)
No problem.
>
> [...]
>
> Otherwise looks reasonable.
Thanks for the review.
Andrew Murray
>
> Cheers
> ---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/6] arm64: HWCAP: encapsulate elf_hwcap
2019-02-07 11:47 ` Dave Martin
@ 2019-02-18 15:46 ` Andrew Murray
2019-02-18 16:05 ` Dave Martin
0 siblings, 1 reply; 14+ messages in thread
From: Andrew Murray @ 2019-02-18 15:46 UTC (permalink / raw)
To: Dave Martin; +Cc: Szabolcs Nagy, Catalin Marinas, Will Deacon, linux-arm-kernel
On Thu, Feb 07, 2019 at 11:47:59AM +0000, Dave Martin wrote:
> On Wed, Feb 06, 2019 at 01:31:05PM +0000, Andrew Murray wrote:
> > The introduction of elf_hwcap2 introduced accessors which ensure that
> > features are set/tested in the appropriate elf_hwcapX variable.
> >
> > Let's now mandate access to elf_hwcapX via these accessors by making
> > elf_hwcapX static within cpufeature.c.
> >
> > Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> > ---
> > arch/arm64/include/asm/cpufeature.h | 19 ++++---------------
> > arch/arm64/include/asm/hwcap.h | 6 +++---
> > arch/arm64/kernel/cpufeature.c | 33 ++++++++++++++++++++++++++++-----
> > 3 files changed, 35 insertions(+), 23 deletions(-)
>
> [...]
>
> > diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
> > index 05ee9b9..ced87ad 100644
> > --- a/arch/arm64/include/asm/hwcap.h
> > +++ b/arch/arm64/include/asm/hwcap.h
>
> [...]
>
> > @@ -97,6 +98,5 @@ enum {
> > #endif
> > };
> >
> > -extern unsigned long elf_hwcap, elf_hwcap2;
> > #endif
> > #endif
> > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > index d10a455..5b9620d 100644
> > --- a/arch/arm64/kernel/cpufeature.c
> > +++ b/arch/arm64/kernel/cpufeature.c
> > @@ -35,11 +35,8 @@
> > #include <asm/traps.h>
> > #include <asm/virt.h>
> >
> > -unsigned long elf_hwcap __read_mostly;
> > -EXPORT_SYMBOL_GPL(elf_hwcap);
> > -
>
> Will this affect out-of-tree modules?
>
> I also note an out-of-arch/ use in
> drivers/clocksource/arm_arch_timer.c, which this series doesn't appear
> to address (or I missed it).
It was in "arm64: HWCAP: add support for ELF_HWCAP2" - though due to it
being used by arm32 code I had to use #ifdef CONFIG_ARM64 as
cpu_set_feature doesn't exist there.
>
> We are allowed to break EXPORT_SYMBOL_GPL()s in general so long as it's
> not done without any meaningful reason, so maybe this is not a huge
> concern so long as we catch all the in-tree users.
The benefit of removing the EXPORT_SYMBOL_GPL is that we can encapsulate
the elf_hwcap variables with getters/setters that ensure they are modified
correctly. This is only relevant now as we split bits between elf_hwcap and
elf_hwcap2.
My suggestion is to remove the EXPORT_SYMBOL_GPL and then also stick with
using just one elf_hwcap variable. As you previously suggested we can then
update the ELF_HWCAP{,2} marcos to produce the relevant bit fields. This
also has the added benefit of simplifying cpu_{have,set}_feature functions
as the number of hwcaps grow.
Thanks,
Andrew Murray
>
> [...]
>
> Cheers
> ---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/6] arm64: HWCAP: add support for ELF_HWCAP2
2019-02-18 15:32 ` Andrew Murray
@ 2019-02-18 16:02 ` Dave Martin
2019-02-19 16:52 ` Andrew Murray
0 siblings, 1 reply; 14+ messages in thread
From: Dave Martin @ 2019-02-18 16:02 UTC (permalink / raw)
To: Andrew Murray
Cc: Szabolcs Nagy, Catalin Marinas, Will Deacon, linux-arm-kernel
On Mon, Feb 18, 2019 at 03:32:06PM +0000, Andrew Murray wrote:
> On Thu, Feb 07, 2019 at 11:31:24AM +0000, Dave Martin wrote:
> > On Wed, Feb 06, 2019 at 01:31:04PM +0000, Andrew Murray wrote:
> > > As we will exhaust the first 32 bits of ELF_HWCAP let's start
> > > exposing ELF_HWCAP2 to userspace.
> > >
> > > Whilst it's possible to use the remaining 32 bits of ELF_HWCAP, we
> > > prefer to expand into ELF_HWCAP2 in order to provide a consistent
> > > view to userspace between ILP32 and LP64.
> > >
> > > To reduce complexity and allow for future expansion, we now
> > > represent hwcaps in the kernel as ordinals and use a
> > > KERNEL_HWCAP_ prefix. This allows us to support automatic feature
> > > based module loading for all our hwcaps.
> > >
> > > We introduce cpu_set_feature to set hwcaps in the relevant
> > > elf_hwcapX variable which compliments the existing cpu_have_feature
> > > helper. These helpers allow us to clean up existing direct uses of
> > > elf_hwcap.
> > >
> > > Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> > > ---
> >
> > [...]
> >
> > > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> > > index dfcfba7..15d939e 100644
> > > --- a/arch/arm64/include/asm/cpufeature.h
> > > +++ b/arch/arm64/include/asm/cpufeature.h
> > > @@ -18,11 +18,10 @@
> > > * In the arm64 world (as in the ARM world), elf_hwcap is used both internally
> > > * in the kernel and for user space to keep track of which optional features
> > > * are supported by the current system. So let's map feature 'x' to HWCAP_x.
> > > - * Note that HWCAP_x constants are bit fields so we need to take the log.
> > > */
> > >
> > > -#define MAX_CPU_FEATURES (8 * sizeof(elf_hwcap))
> > > -#define cpu_feature(x) ilog2(HWCAP_ ## x)
> > > +#define MAX_CPU_FEATURES (2 * 8 * sizeof(unsigned long))
> >
> > Doesn't this make 2 * 8 * 8 == 128?
> >
> > Maybe just say "64". We open-code 32 later anyway.
>
> I guess 128 would have been correct if we use all the bits of
> each ELF_HWCAP{,2} - but we chose to limit each ELF_HWCAP to be
> 32 bits.
>
> I'll set this to 64 as you suggest.
>
> >
> > > +#define cpu_feature(x) (KERNEL_HWCAP_ ## x)
> > >
> > > #ifndef __ASSEMBLY__
> > >
> > > @@ -396,9 +395,20 @@ extern struct static_key_false arm64_const_caps_ready;
> > >
> > > bool this_cpu_has_cap(unsigned int cap);
> > >
> > > +static inline void cpu_set_feature(unsigned int num)
> > > +{
> > > + if (num < 32)
> > > + elf_hwcap |= BIT(num);
> > > + else
> > > + elf_hwcap2 |= BIT(num - 32);
> > > +}
> > > +
> > > static inline bool cpu_have_feature(unsigned int num)
> > > {
> > > - return elf_hwcap & (1UL << num);
> > > + if (num < 32)
> > > + return elf_hwcap & BIT(num);
> > > + else
> > > + return elf_hwcap2 & BIT(num - 32);
> > > }
> >
> > Nit: I worry a bit that people will blithely pass HWCAP_foo to these,
> > not realising that the interface has changed.
>
> Yeah this would be an easy mistake to make.
>
> >
> > Can we do something like the following:
> >
> > static inline bool __cpu_have_feature(unsigned int num)
> > {
> > if (WARN_ON(num >= 64))
> > return false;
> >
> > if (num < 32)
> > return elf_hwcap & BIT(num);
> > else
> > return elf_hwcap2 & BIT(num);
> > }
> >
> > #define cpu_have_feature(name) __cpu_have_feature(cpu_feature(name))
>
> It took me a while to figure out what the benefit of the above line
> is - this means that callers of cpu_have_feature must use the unprefixed
> feature name (e.g. SHA512), anything else such as (KERNEL_HWCAP_SHA512
> or HWCAP_SHA512) would result in a compile time error. Whereas without
> this users could pass KERNEL_HWCAP_x or HWCAP_x without compile errors
> yet resulting in incorrectly passing a bitfield when using HWCAP_x.
That was the idea (apologies for being cryptic...)
> > In compiletime-constant cases, the compiler should simply delete the
> > WARN.
>
> Is this really needed? Surely this would only come into play when a user
> directly calls __cpu_have_feature. I think this is overkill, especially
> as __cpu_have_feature is a new function.
>
> > A couple of cases in the cpufeatures and /proc/cpuinfo code would
> > need to use __cpu_have_feature directly and would retain the WARN, but
> > that's not fastpath and probably not the end of the world, plus we
> > get an additional debugging aid if that code goes wrong.
>
> But even so, the WARN (for the cpuinfo case) is unlikely to ever be hit.
Sure, since we don't expect general purpose callers to use
__cpu_have_feature() directly it is a bit paranoid to include the WARN.
Given the contexts in which this is used, I wouldn't lose sleep if it
weren't there.
> The challenge with this approach is print_cpu_modalias (drivers/base/cpu.c),
> this also uses cpu_have_feature - this expects cpu_have_feature to take
> an ordinal. How can we work around this?
>
> >
> > Alternatively, we could bake cpu_feature() into HWCAP_CAP(), making
> > it impossible to specify invalid or user-encoded hwcaps there either.
> > Then we might justifiably omit the policing in __cpu_have_feature()
> > altogether.
>
> This would certainly please checkpatch with respect to line lengths :)
>
> However since this patchset, we now use cpu_have_feature elsewhere, such
> as in the crypto drivers.
Hmmm, those are irksome issues.
It would be nice to split the ordinal versus symbolic cpu_have_feature()
into separate macros, but that would involve a change to code outside
arch/arm64. It's almost certainly not worth the pain.
Can you take a look at arch/arm? That already has hwcap2.
It may makes some sense to follow a common approach, although I'm not
sure how
#define __hwcap_feature(x) ilog2(HWCAP_ ## x)
#define __hwcap2_feature(x) (32 + ilog2(HWCAP2_ ## x))
#define cpu_feature(x) __hwcap2_feature(x)
is supposed to work for cpu_feature(SWP) for example... Maybe I missed
something.
> > > /* System capability check for constant caps */
> > > diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
> > > index 400b80b..05ee9b9 100644
> > > --- a/arch/arm64/include/asm/hwcap.h
> > > +++ b/arch/arm64/include/asm/hwcap.h
> > > @@ -39,12 +39,49 @@
[...]
> > > +#define KERNEL_HWCAP_SSBS ilog2(HWCAP_SSBS)
> > > +#define KERNEL_HWCAP_SB ilog2(HWCAP_SB)
> > > +#define KERNEL_HWCAP_PACA ilog2(HWCAP_PACA)
> > > +#define KERNEL_HWCAP_PACG ilog2(HWCAP_PACG)
> >
> > Nit: Odd spacing? Personally I wouldn't indent with spaces just so that
> > (ilog2() + 32) lines up nicely, but that's just my opinion and not a
> > huge deal.
> >
> > (The spacing can make subsequent diffs look jagged by default, which
> > can be a distraction for reviewers.)
>
> I'll convert to tabs, I probably copy pasted and the tabs got converted.
>
> >
> > > +
> > > #ifndef __ASSEMBLY__
> > > /*
> > > * This yields a mask that user programs can use to figure out what
> > > * instruction set this cpu supports.
> > > */
> > > -#define ELF_HWCAP (elf_hwcap)
> > > +#define ELF_HWCAP elf_hwcap
> > > +#define ELF_HWCAP2 elf_hwcap2
> >
> > Do we need elf_hwcap2 at all?
> >
> > We could just have these macros fish the bits out of a single variable,
> > or wherever.
> >
> > However, it may be better to keep things this way if we want to maintain
> > the elf_hwcap in the kernel/module ABI. See note on EXPORT_SYMBOL_GPL()
> > removal for elf_hwcap in the subsequent patch.
>
> OK, I'll leave it as it is.
>
> >
> > >
> > > #ifdef CONFIG_COMPAT
> > > #define COMPAT_ELF_HWCAP (compat_elf_hwcap)
> > > @@ -60,6 +97,6 @@ enum {
> > > #endif
> > > };
> > >
> > > -extern unsigned long elf_hwcap;
> > > +extern unsigned long elf_hwcap, elf_hwcap2;
> > > #endif
> > > #endif
> > > diff --git a/arch/arm64/include/uapi/asm/hwcap.h b/arch/arm64/include/uapi/asm/hwcap.h
> > > index 5f0750c..453b45a 100644
> > > --- a/arch/arm64/include/uapi/asm/hwcap.h
> > > +++ b/arch/arm64/include/uapi/asm/hwcap.h
> > > @@ -18,7 +18,7 @@
> > > #define _UAPI__ASM_HWCAP_H
> > >
> > > /*
> > > - * HWCAP flags - for elf_hwcap (in kernel) and AT_HWCAP
> > > + * HWCAP flags - for AT_HWCAP
> > > */
> > > #define HWCAP_FP (1 << 0)
> > > #define HWCAP_ASIMD (1 << 1)
> > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > > index f6d84e2..d10a455 100644
> > > --- a/arch/arm64/kernel/cpufeature.c
> > > +++ b/arch/arm64/kernel/cpufeature.c
> > > @@ -38,6 +38,9 @@
> > > unsigned long elf_hwcap __read_mostly;
> > > EXPORT_SYMBOL_GPL(elf_hwcap);
> > >
> > > +unsigned long elf_hwcap2 __read_mostly;
> > > +EXPORT_SYMBOL_GPL(elf_hwcap2);
> > > +
> >
> > Historically nobody used this because it didn't exist, and we don't want
> > anybody to use it (since cpu_have_feature() etc. is the preferred
> > interface).
>
> In that case I should probably EXPORT_SYMBOL_GPL cpu_have_feature.
>
> >
> > So lose EXPORT_SYMBOL_GPL() here. (You remove it in any case in a
> > subsequent patch, but having it even transiently sends mixed messages
> > about the intent.)
>
> No problem.
Hmmm, since this inline in a header we'd have to EXPORT_SYMBOL_GPL the
things it depends on (elf_hwcap, elf_hwcap2) rather than the (inline)
function itself.
So maybe we can't do better than the current situation after all.
If so, I guess exporting elf_hwcap2 as GPL is OK.
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/6] arm64: HWCAP: encapsulate elf_hwcap
2019-02-18 15:46 ` Andrew Murray
@ 2019-02-18 16:05 ` Dave Martin
0 siblings, 0 replies; 14+ messages in thread
From: Dave Martin @ 2019-02-18 16:05 UTC (permalink / raw)
To: Andrew Murray
Cc: Szabolcs Nagy, Catalin Marinas, Will Deacon, linux-arm-kernel
On Mon, Feb 18, 2019 at 03:46:02PM +0000, Andrew Murray wrote:
> On Thu, Feb 07, 2019 at 11:47:59AM +0000, Dave Martin wrote:
> > On Wed, Feb 06, 2019 at 01:31:05PM +0000, Andrew Murray wrote:
> > > The introduction of elf_hwcap2 introduced accessors which ensure that
> > > features are set/tested in the appropriate elf_hwcapX variable.
> > >
> > > Let's now mandate access to elf_hwcapX via these accessors by making
> > > elf_hwcapX static within cpufeature.c.
> > >
> > > Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> > > ---
> > > arch/arm64/include/asm/cpufeature.h | 19 ++++---------------
> > > arch/arm64/include/asm/hwcap.h | 6 +++---
> > > arch/arm64/kernel/cpufeature.c | 33 ++++++++++++++++++++++++++++-----
> > > 3 files changed, 35 insertions(+), 23 deletions(-)
> >
> > [...]
> >
> > > diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
> > > index 05ee9b9..ced87ad 100644
> > > --- a/arch/arm64/include/asm/hwcap.h
> > > +++ b/arch/arm64/include/asm/hwcap.h
> >
> > [...]
> >
> > > @@ -97,6 +98,5 @@ enum {
> > > #endif
> > > };
> > >
> > > -extern unsigned long elf_hwcap, elf_hwcap2;
> > > #endif
> > > #endif
> > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > > index d10a455..5b9620d 100644
> > > --- a/arch/arm64/kernel/cpufeature.c
> > > +++ b/arch/arm64/kernel/cpufeature.c
> > > @@ -35,11 +35,8 @@
> > > #include <asm/traps.h>
> > > #include <asm/virt.h>
> > >
> > > -unsigned long elf_hwcap __read_mostly;
> > > -EXPORT_SYMBOL_GPL(elf_hwcap);
> > > -
> >
> > Will this affect out-of-tree modules?
> >
> > I also note an out-of-arch/ use in
> > drivers/clocksource/arm_arch_timer.c, which this series doesn't appear
> > to address (or I missed it).
>
> It was in "arm64: HWCAP: add support for ELF_HWCAP2" - though due to it
> being used by arm32 code I had to use #ifdef CONFIG_ARM64 as
> cpu_set_feature doesn't exist there.
Ah, I guess I missed that.
> >
> > We are allowed to break EXPORT_SYMBOL_GPL()s in general so long as it's
> > not done without any meaningful reason, so maybe this is not a huge
> > concern so long as we catch all the in-tree users.
>
> The benefit of removing the EXPORT_SYMBOL_GPL is that we can encapsulate
> the elf_hwcap variables with getters/setters that ensure they are modified
> correctly. This is only relevant now as we split bits between elf_hwcap and
> elf_hwcap2.
>
> My suggestion is to remove the EXPORT_SYMBOL_GPL and then also stick with
> using just one elf_hwcap variable. As you previously suggested we can then
> update the ELF_HWCAP{,2} marcos to produce the relevant bit fields. This
> also has the added benefit of simplifying cpu_{have,set}_feature functions
> as the number of hwcaps grow.
I guess this depends on what is decided re the other patches.
While it would be nice to avoid exposing elf_hwcap and elf_hwcap2
directly, I'm starting to feel it may be easier to expose them as-is.
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/6] arm64: HWCAP: add support for ELF_HWCAP2
2019-02-18 16:02 ` Dave Martin
@ 2019-02-19 16:52 ` Andrew Murray
0 siblings, 0 replies; 14+ messages in thread
From: Andrew Murray @ 2019-02-19 16:52 UTC (permalink / raw)
To: Dave Martin; +Cc: Szabolcs Nagy, Catalin Marinas, Will Deacon, linux-arm-kernel
On Mon, Feb 18, 2019 at 04:02:38PM +0000, Dave Martin wrote:
> On Mon, Feb 18, 2019 at 03:32:06PM +0000, Andrew Murray wrote:
> > On Thu, Feb 07, 2019 at 11:31:24AM +0000, Dave Martin wrote:
> > > On Wed, Feb 06, 2019 at 01:31:04PM +0000, Andrew Murray wrote:
> > > > As we will exhaust the first 32 bits of ELF_HWCAP let's start
> > > > exposing ELF_HWCAP2 to userspace.
> > > >
> > > > Whilst it's possible to use the remaining 32 bits of ELF_HWCAP, we
> > > > prefer to expand into ELF_HWCAP2 in order to provide a consistent
> > > > view to userspace between ILP32 and LP64.
> > > >
> > > > To reduce complexity and allow for future expansion, we now
> > > > represent hwcaps in the kernel as ordinals and use a
> > > > KERNEL_HWCAP_ prefix. This allows us to support automatic feature
> > > > based module loading for all our hwcaps.
> > > >
> > > > We introduce cpu_set_feature to set hwcaps in the relevant
> > > > elf_hwcapX variable which compliments the existing cpu_have_feature
> > > > helper. These helpers allow us to clean up existing direct uses of
> > > > elf_hwcap.
> > > >
> > > > Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> > > > ---
> > >
> > > [...]
> > >
> > > > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> > > > index dfcfba7..15d939e 100644
> > > > --- a/arch/arm64/include/asm/cpufeature.h
> > > > +++ b/arch/arm64/include/asm/cpufeature.h
> > > > @@ -18,11 +18,10 @@
> > > > * In the arm64 world (as in the ARM world), elf_hwcap is used both internally
> > > > * in the kernel and for user space to keep track of which optional features
> > > > * are supported by the current system. So let's map feature 'x' to HWCAP_x.
> > > > - * Note that HWCAP_x constants are bit fields so we need to take the log.
> > > > */
> > > >
> > > > -#define MAX_CPU_FEATURES (8 * sizeof(elf_hwcap))
> > > > -#define cpu_feature(x) ilog2(HWCAP_ ## x)
> > > > +#define MAX_CPU_FEATURES (2 * 8 * sizeof(unsigned long))
> > >
> > > Doesn't this make 2 * 8 * 8 == 128?
> > >
> > > Maybe just say "64". We open-code 32 later anyway.
> >
> > I guess 128 would have been correct if we use all the bits of
> > each ELF_HWCAP{,2} - but we chose to limit each ELF_HWCAP to be
> > 32 bits.
> >
> > I'll set this to 64 as you suggest.
> >
> > >
> > > > +#define cpu_feature(x) (KERNEL_HWCAP_ ## x)
> > > >
> > > > #ifndef __ASSEMBLY__
> > > >
> > > > @@ -396,9 +395,20 @@ extern struct static_key_false arm64_const_caps_ready;
> > > >
> > > > bool this_cpu_has_cap(unsigned int cap);
> > > >
> > > > +static inline void cpu_set_feature(unsigned int num)
> > > > +{
> > > > + if (num < 32)
> > > > + elf_hwcap |= BIT(num);
> > > > + else
> > > > + elf_hwcap2 |= BIT(num - 32);
> > > > +}
> > > > +
> > > > static inline bool cpu_have_feature(unsigned int num)
> > > > {
> > > > - return elf_hwcap & (1UL << num);
> > > > + if (num < 32)
> > > > + return elf_hwcap & BIT(num);
> > > > + else
> > > > + return elf_hwcap2 & BIT(num - 32);
> > > > }
> > >
> > > Nit: I worry a bit that people will blithely pass HWCAP_foo to these,
> > > not realising that the interface has changed.
> >
> > Yeah this would be an easy mistake to make.
> >
> > >
> > > Can we do something like the following:
> > >
> > > static inline bool __cpu_have_feature(unsigned int num)
> > > {
> > > if (WARN_ON(num >= 64))
> > > return false;
> > >
> > > if (num < 32)
> > > return elf_hwcap & BIT(num);
> > > else
> > > return elf_hwcap2 & BIT(num);
> > > }
> > >
> > > #define cpu_have_feature(name) __cpu_have_feature(cpu_feature(name))
> >
> > It took me a while to figure out what the benefit of the above line
> > is - this means that callers of cpu_have_feature must use the unprefixed
> > feature name (e.g. SHA512), anything else such as (KERNEL_HWCAP_SHA512
> > or HWCAP_SHA512) would result in a compile time error. Whereas without
> > this users could pass KERNEL_HWCAP_x or HWCAP_x without compile errors
> > yet resulting in incorrectly passing a bitfield when using HWCAP_x.
>
> That was the idea (apologies for being cryptic...)
:)
>
> > > In compiletime-constant cases, the compiler should simply delete the
> > > WARN.
> >
> > Is this really needed? Surely this would only come into play when a user
> > directly calls __cpu_have_feature. I think this is overkill, especially
> > as __cpu_have_feature is a new function.
> >
> > > A couple of cases in the cpufeatures and /proc/cpuinfo code would
> > > need to use __cpu_have_feature directly and would retain the WARN, but
> > > that's not fastpath and probably not the end of the world, plus we
> > > get an additional debugging aid if that code goes wrong.
> >
> > But even so, the WARN (for the cpuinfo case) is unlikely to ever be hit.
>
> Sure, since we don't expect general purpose callers to use
> __cpu_have_feature() directly it is a bit paranoid to include the WARN.
> Given the contexts in which this is used, I wouldn't lose sleep if it
> weren't there.
>
> > The challenge with this approach is print_cpu_modalias (drivers/base/cpu.c),
> > this also uses cpu_have_feature - this expects cpu_have_feature to take
> > an ordinal. How can we work around this?
> >
> > >
> > > Alternatively, we could bake cpu_feature() into HWCAP_CAP(), making
> > > it impossible to specify invalid or user-encoded hwcaps there either.
> > > Then we might justifiably omit the policing in __cpu_have_feature()
> > > altogether.
> >
> > This would certainly please checkpatch with respect to line lengths :)
> >
> > However since this patchset, we now use cpu_have_feature elsewhere, such
> > as in the crypto drivers.
>
> Hmmm, those are irksome issues.
>
> It would be nice to split the ordinal versus symbolic cpu_have_feature()
> into separate macros, but that would involve a change to code outside
> arch/arm64. It's almost certainly not worth the pain.
>
>
> Can you take a look at arch/arm? That already has hwcap2.
> It may makes some sense to follow a common approach, although I'm not
> sure how
>
> #define __hwcap_feature(x) ilog2(HWCAP_ ## x)
> #define __hwcap2_feature(x) (32 + ilog2(HWCAP2_ ## x))
> #define cpu_feature(x) __hwcap2_feature(x)
>
> is supposed to work for cpu_feature(SWP) for example... Maybe I missed
> something.
The limitation with the arm32 approach is that you can only support automatic
module loading with features covered in HWCAP2. We can avoid this.
For arm32 cpu_feature(AES) will only match within HWCAP2, you would get
unintended behaviour if you used a feature name from HWCAP such as NEON.
Thus cpu_have_feature(cpu_feature(X)) is limited to HWCAP2.
Prior to this patchset cpu_have_feature had a sole user and that is
print_cpu_modalias. CPU features are normally tested by directly comparing
against elf_hwcap.
>
> > > > /* System capability check for constant caps */
> > > > diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
> > > > index 400b80b..05ee9b9 100644
> > > > --- a/arch/arm64/include/asm/hwcap.h
> > > > +++ b/arch/arm64/include/asm/hwcap.h
> > > > @@ -39,12 +39,49 @@
>
> [...]
>
> > > > +#define KERNEL_HWCAP_SSBS ilog2(HWCAP_SSBS)
> > > > +#define KERNEL_HWCAP_SB ilog2(HWCAP_SB)
> > > > +#define KERNEL_HWCAP_PACA ilog2(HWCAP_PACA)
> > > > +#define KERNEL_HWCAP_PACG ilog2(HWCAP_PACG)
> > >
> > > Nit: Odd spacing? Personally I wouldn't indent with spaces just so that
> > > (ilog2() + 32) lines up nicely, but that's just my opinion and not a
> > > huge deal.
> > >
> > > (The spacing can make subsequent diffs look jagged by default, which
> > > can be a distraction for reviewers.)
> >
> > I'll convert to tabs, I probably copy pasted and the tabs got converted.
> >
> > >
> > > > +
> > > > #ifndef __ASSEMBLY__
> > > > /*
> > > > * This yields a mask that user programs can use to figure out what
> > > > * instruction set this cpu supports.
> > > > */
> > > > -#define ELF_HWCAP (elf_hwcap)
> > > > +#define ELF_HWCAP elf_hwcap
> > > > +#define ELF_HWCAP2 elf_hwcap2
> > >
> > > Do we need elf_hwcap2 at all?
> > >
> > > We could just have these macros fish the bits out of a single variable,
> > > or wherever.
> > >
> > > However, it may be better to keep things this way if we want to maintain
> > > the elf_hwcap in the kernel/module ABI. See note on EXPORT_SYMBOL_GPL()
> > > removal for elf_hwcap in the subsequent patch.
> >
> > OK, I'll leave it as it is.
> >
> > >
> > > >
> > > > #ifdef CONFIG_COMPAT
> > > > #define COMPAT_ELF_HWCAP (compat_elf_hwcap)
> > > > @@ -60,6 +97,6 @@ enum {
> > > > #endif
> > > > };
> > > >
> > > > -extern unsigned long elf_hwcap;
> > > > +extern unsigned long elf_hwcap, elf_hwcap2;
> > > > #endif
> > > > #endif
> > > > diff --git a/arch/arm64/include/uapi/asm/hwcap.h b/arch/arm64/include/uapi/asm/hwcap.h
> > > > index 5f0750c..453b45a 100644
> > > > --- a/arch/arm64/include/uapi/asm/hwcap.h
> > > > +++ b/arch/arm64/include/uapi/asm/hwcap.h
> > > > @@ -18,7 +18,7 @@
> > > > #define _UAPI__ASM_HWCAP_H
> > > >
> > > > /*
> > > > - * HWCAP flags - for elf_hwcap (in kernel) and AT_HWCAP
> > > > + * HWCAP flags - for AT_HWCAP
> > > > */
> > > > #define HWCAP_FP (1 << 0)
> > > > #define HWCAP_ASIMD (1 << 1)
> > > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > > > index f6d84e2..d10a455 100644
> > > > --- a/arch/arm64/kernel/cpufeature.c
> > > > +++ b/arch/arm64/kernel/cpufeature.c
> > > > @@ -38,6 +38,9 @@
> > > > unsigned long elf_hwcap __read_mostly;
> > > > EXPORT_SYMBOL_GPL(elf_hwcap);
> > > >
> > > > +unsigned long elf_hwcap2 __read_mostly;
> > > > +EXPORT_SYMBOL_GPL(elf_hwcap2);
> > > > +
> > >
> > > Historically nobody used this because it didn't exist, and we don't want
> > > anybody to use it (since cpu_have_feature() etc. is the preferred
> > > interface).
> >
> > In that case I should probably EXPORT_SYMBOL_GPL cpu_have_feature.
> >
> > >
> > > So lose EXPORT_SYMBOL_GPL() here. (You remove it in any case in a
> > > subsequent patch, but having it even transiently sends mixed messages
> > > about the intent.)
> >
> > No problem.
>
> Hmmm, since this inline in a header we'd have to EXPORT_SYMBOL_GPL the
> things it depends on (elf_hwcap, elf_hwcap2) rather than the (inline)
> function itself.
>
> So maybe we can't do better than the current situation after all.
>
> If so, I guess exporting elf_hwcap2 as GPL is OK.
Actually my patchset moves the cpu_have_feature function a C file. So I could
add a EXPORT_SYMBOL_GPL to this and the other functions I introduce
(cpu_set_feature, cpu_get_elf_hwcap{,2}).
Unless you have objections I'll respin with this approach.
Andrew Murray
>
> Cheers
> ---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-02-19 16:53 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-06 13:31 [PATCH 0/6] Initial support for CVADP Andrew Murray
2019-02-06 13:31 ` [PATCH 1/6] arm64: Handle trapped DC CVADP Andrew Murray
2019-02-06 13:31 ` [PATCH 2/6] arm64: HWCAP: add support for ELF_HWCAP2 Andrew Murray
2019-02-07 11:31 ` Dave Martin
2019-02-18 15:32 ` Andrew Murray
2019-02-18 16:02 ` Dave Martin
2019-02-19 16:52 ` Andrew Murray
2019-02-06 13:31 ` [PATCH 3/6] arm64: HWCAP: encapsulate elf_hwcap Andrew Murray
2019-02-07 11:47 ` Dave Martin
2019-02-18 15:46 ` Andrew Murray
2019-02-18 16:05 ` Dave Martin
2019-02-06 13:31 ` [PATCH 4/6] arm64: Expose DC CVADP to userspace Andrew Murray
2019-02-06 13:31 ` [PATCH 5/6] arm64: add CVADP support to the cache maintenance helper Andrew Murray
2019-02-06 13:31 ` [PATCH 6/6] arm64: Advertise ARM64_HAS_DCPODP cpu feature Andrew Murray
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).