2020-01-13 23:38:23

by Suzuki K Poulose

[permalink] [raw]
Subject: [PATCH v3 0/7] arm64: Fix support for no FP/SIMD

This series fixes the support for systems without FP/SIMD unit.

We detect the absence of FP/SIMD after the SMP cpus are brought
online (i.e, SYSTEM scope). This means, we allow a hotplugged
CPU to boot successfully even if it doesn't have the FP/SIMD
when we have decided otherwise at boot and have now advertised
the ELF HWCAP for the userspace. Fix this by turning this to a
BOOT_RESTRICTED_CPU_LOCAL feature to allow the detection of the
feature the very moment a CPU turns up without FP/SIMD and also
prevent a conflict after SMP boot.

The COMPAT ELF_HWCAPs were statically set to indicate the
availability of VFP. Make it dynamic to set the appropriate
bits.

Also, some of the early kernel threads (including init) could run
with their TIF_FOREIGN_FPSTATE flag set which might be inherited
by applications forked by them (e.g, modprobe from initramfs).
Now, if we detect the absence of FP/SIMD we stop clearing the
TIF flag in fpsimd_restore_current_state(). This could cause
the applications stuck in do_notify_resume() looping forever
to clear the flag. Fix this by clearing the TIF flag in
fpsimd_restore_current_state() for the tasks that may
have it set.

This series also categorises the functions dealing with fpsimd
into two :

- Call permitted with missing FP/SIMD support. But we bail
out early in the function. This is for functions exposed
to the generic kernel code.

- Calls not permitted with missing FP/SIMD support. These
are functions which deal with the CPU/Task FP/SIMD registers
and/or meta-data. The callers must check for the support
before invoking them.

See the last patch in the series for details.

Also make sure that the SVE is initialised where supported,
before the FP/SIMD is used by the kernel.

Tested with debian armel initramfs and rootfs. The arm64 doesn't
have a soft-float ABI, thus haven't tested it with 64bit userspace.

Applies on linux-aarch64 for-next/core

Changes since v2:
- Rebase on to for-next/core, resolved conflict with the E0PD
handling changes
- Address comments from Catalin
- Remove warnings from static functions
- Add WARN_ON in may_use_simd() if called before initializing
capabilities.
- Add "active" hook for FP regset
- Remove dangerous WARN_ON from KVM, replaced with an additional
check to make sure that the FP/SIMD is always trapped.
- Added tags from Catalin, Marc

Suzuki K Poulose (7):
arm64: Introduce system_capabilities_finalized() marker
arm64: fpsimd: Make sure SVE setup is complete before SIMD is used
arm64: cpufeature: Fix the type of no FP/SIMD capability
arm64: cpufeature: Set the FP/SIMD compat HWCAP bits properly
arm64: ptrace: nofpsimd: Fail FP/SIMD regset operations
arm64: signal: nofpsimd: Handle fp/simd context for signal frames
arm64: nofpsmid: Handle TIF_FOREIGN_FPSTATE flag cleanly

arch/arm64/include/asm/cpufeature.h | 5 +++
arch/arm64/include/asm/kvm_host.h | 2 +-
arch/arm64/include/asm/simd.h | 8 +++-
arch/arm64/kernel/cpufeature.c | 67 +++++++++++++++++++----------
arch/arm64/kernel/fpsimd.c | 30 +++++++++++--
arch/arm64/kernel/process.c | 2 +-
arch/arm64/kernel/ptrace.c | 21 +++++++++
arch/arm64/kernel/signal.c | 6 ++-
arch/arm64/kernel/signal32.c | 4 +-
arch/arm64/kvm/hyp/switch.c | 10 ++++-
10 files changed, 121 insertions(+), 34 deletions(-)

--
2.24.1


2020-01-13 23:38:55

by Suzuki K Poulose

[permalink] [raw]
Subject: [PATCH v3 1/7] arm64: Introduce system_capabilities_finalized() marker

We finalize the system wide capabilities after the SMP CPUs
are booted by the kernel. This is used as a marker for deciding
various checks in the kernel. e.g, sanity check the hotplugged
CPUs for missing mandatory features.

However there is no explicit helper available for this in the
kernel. There is sys_caps_initialised, which is not exposed.
The other closest one we have is the jump_label arm64_const_caps_ready
which denotes that the capabilities are set and the capability checks
could use the individual jump_labels for fast path. This is
performed before setting the ELF Hwcaps, which must be checked
against the new CPUs. We also perform some of the other initialization
e.g, SVE setup, which is important for the use of FP/SIMD
where SVE is supported. Normally userspace doesn't get to run
before we finish this. However the in-kernel users may
potentially start using the neon mode. So, we need to
reject uses of neon mode before we are set. Instead of defining
a new marker for the completion of SVE setup, we could simply
reuse the arm64_const_caps_ready and enable it once we have
finished all the setup. Also we could expose this to the
various users as "system_capabilities_finalized()" to make
it more meaningful than "const_caps_ready".

Cc: Ard Biesheuvel <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Mark Rutland <[email protected]>
Reviewed-by: Catalin Marinas <[email protected]>
Signed-off-by: Suzuki K Poulose <[email protected]>
---
arch/arm64/include/asm/cpufeature.h | 5 +++++
arch/arm64/include/asm/kvm_host.h | 2 +-
arch/arm64/kernel/cpufeature.c | 28 ++++++++++------------------
arch/arm64/kernel/process.c | 2 +-
4 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 4261d55e8506..92ef9539874a 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -613,6 +613,11 @@ static inline bool system_has_prio_mask_debugging(void)
system_uses_irq_prio_masking();
}

+static inline bool system_capabilities_finalized(void)
+{
+ return static_branch_likely(&arm64_const_caps_ready);
+}
+
#define ARM64_BP_HARDEN_UNKNOWN -1
#define ARM64_BP_HARDEN_WA_NEEDED 0
#define ARM64_BP_HARDEN_NOT_REQUIRED 1
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index c61260cf63c5..48ce54639eb5 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -547,7 +547,7 @@ static inline void __cpu_init_hyp_mode(phys_addr_t pgd_ptr,
* wrong, and hyp will crash and burn when it uses any
* cpus_have_const_cap() wrapper.
*/
- BUG_ON(!static_branch_likely(&arm64_const_caps_ready));
+ BUG_ON(!system_capabilities_finalized());
__kvm_call_hyp((void *)pgd_ptr, hyp_stack_ptr, vector_ptr, tpidr_el2);

/*
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index fa907b13dd55..1154f2d47b4b 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -56,13 +56,14 @@ EXPORT_SYMBOL(arm64_use_ng_mappings);
* will be used to determine if a new booting CPU should
* go through the verification process to make sure that it
* supports the system capabilities, without using a hotplug
- * notifier.
+ * notifier. This is also used to decide if we could use
+ * the fast path for checking constant CPU caps.
*/
-static bool sys_caps_initialised;
-
-static inline void set_sys_caps_initialised(void)
+DEFINE_STATIC_KEY_FALSE(arm64_const_caps_ready);
+EXPORT_SYMBOL(arm64_const_caps_ready);
+static inline void finalize_system_capabilities(void)
{
- sys_caps_initialised = true;
+ static_branch_enable(&arm64_const_caps_ready);
}

static int dump_cpu_hwcaps(struct notifier_block *self, unsigned long v, void *p)
@@ -789,7 +790,7 @@ void update_cpu_features(int cpu,

/* Probe vector lengths, unless we already gave up on SVE */
if (id_aa64pfr0_sve(read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1)) &&
- !sys_caps_initialised)
+ !system_capabilities_finalized())
sve_update_vq_map();
}

@@ -1002,7 +1003,7 @@ bool kaslr_requires_kpti(void)
*/
if (!IS_ENABLED(CONFIG_CAVIUM_ERRATUM_27456)) {
tx1_bug = false;
- } else if (!static_branch_likely(&arm64_const_caps_ready)) {
+ } else if (!system_capabilities_finalized()) {
extern const struct midr_range cavium_erratum_27456_cpus[];

tx1_bug = is_midr_in_range_list(read_cpuid_id(),
@@ -2046,7 +2047,7 @@ void check_local_cpu_capabilities(void)
* Otherwise, this CPU should verify that it has all the system
* advertised capabilities.
*/
- if (!sys_caps_initialised)
+ if (!system_capabilities_finalized())
update_cpu_capabilities(SCOPE_LOCAL_CPU);
else
verify_local_cpu_capabilities();
@@ -2060,14 +2061,6 @@ static void __init setup_boot_cpu_capabilities(void)
enable_cpu_capabilities(SCOPE_BOOT_CPU);
}

-DEFINE_STATIC_KEY_FALSE(arm64_const_caps_ready);
-EXPORT_SYMBOL(arm64_const_caps_ready);
-
-static void __init mark_const_caps_ready(void)
-{
- static_branch_enable(&arm64_const_caps_ready);
-}
-
bool this_cpu_has_cap(unsigned int n)
{
if (!WARN_ON(preemptible()) && n < ARM64_NCAPS) {
@@ -2126,7 +2119,6 @@ void __init setup_cpu_features(void)
u32 cwg;

setup_system_capabilities();
- mark_const_caps_ready();
setup_elf_hwcaps(arm64_elf_hwcaps);

if (system_supports_32bit_el0())
@@ -2139,7 +2131,7 @@ void __init setup_cpu_features(void)
minsigstksz_setup();

/* Advertise that we have computed the system capabilities */
- set_sys_caps_initialised();
+ finalize_system_capabilities();

/*
* Check for sane CTR_EL0.CWG value.
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 71f788cd2b18..48a38144ea7b 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -646,6 +646,6 @@ asmlinkage void __sched arm64_preempt_schedule_irq(void)
* Only allow a task to be preempted once cpufeatures have been
* enabled.
*/
- if (static_branch_likely(&arm64_const_caps_ready))
+ if (system_capabilities_finalized())
preempt_schedule_irq();
}
--
2.24.1

2020-01-13 23:39:10

by Suzuki K Poulose

[permalink] [raw]
Subject: [PATCH v3 3/7] arm64: cpufeature: Fix the type of no FP/SIMD capability

The NO_FPSIMD capability is defined with scope SYSTEM, which implies
that the "absence" of FP/SIMD on at least one CPU is detected only
after all the SMP CPUs are brought up. However, we use the status
of this capability for every context switch. So, let us change
the scope to LOCAL_CPU to allow the detection of this capability
as and when the first CPU without FP is brought up.

Also, the current type allows hotplugged CPU to be brought up without
FP/SIMD when all the current CPUs have FP/SIMD and we have the userspace
up. Fix both of these issues by changing the capability to
BOOT_RESTRICTED_LOCAL_CPU_FEATURE.

Fixes: 82e0191a1aa11abf ("arm64: Support systems without FP/ASIMD")
Cc: Will Deacon <[email protected]>
Cc: Mark Rutland <[email protected]>
Reviewed-by: Catalin Marinas <[email protected]>
Signed-off-by: Suzuki K Poulose <[email protected]>
---
arch/arm64/kernel/cpufeature.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 1154f2d47b4b..7c373722f692 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1428,7 +1428,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
{
/* FP/SIMD is not implemented */
.capability = ARM64_HAS_NO_FPSIMD,
- .type = ARM64_CPUCAP_SYSTEM_FEATURE,
+ .type = ARM64_CPUCAP_BOOT_RESTRICTED_CPU_LOCAL_FEATURE,
.min_field_value = 0,
.matches = has_no_fpsimd,
},
--
2.24.1

2020-01-13 23:39:59

by Suzuki K Poulose

[permalink] [raw]
Subject: [PATCH v3 7/7] arm64: nofpsmid: Handle TIF_FOREIGN_FPSTATE flag cleanly

We detect the absence of FP/SIMD after an incapable CPU is brought up,
and by then we have kernel threads running already with TIF_FOREIGN_FPSTATE set
which could be set for early userspace applications (e.g, modprobe triggered
from initramfs) and init. This could cause the applications to loop forever in
do_nofity_resume() as we never clear the TIF flag, once we now know that
we don't support FP.

Fix this by making sure that we clear the TIF_FOREIGN_FPSTATE flag
for tasks which may have them set, as we would have done in the normal
case, but avoiding touching the hardware state (since we don't support any).

Also to make sure we handle the cases seemlessly we categorise the
helper functions to two :
1) Helpers for common core code, which calls into take appropriate
actions without knowing the current FPSIMD state of the CPU/task.

e.g fpsimd_restore_current_state(), fpsimd_flush_task_state(),
fpsimd_save_and_flush_cpu_state().

We bail out early for these functions, taking any appropriate actions
(e.g, clearing the TIF flag) where necessary to hide the handling
from core code.

2) Helpers used when the presence of FP/SIMD is apparent.
i.e, save/restore the FP/SIMD register state, modify the CPU/task
FP/SIMD state.
e.g,

fpsimd_save(), task_fpsimd_load() - save/restore task FP/SIMD registers

fpsimd_bind_task_to_cpu() \
- Update the "state" metadata for CPU/task.
fpsimd_bind_state_to_cpu() /

fpsimd_update_current_state() - Update the fp/simd state for the current
task from memory.

These must not be called in the absence of FP/SIMD. Put in a WARNING
to make sure they are not invoked in the absence of FP/SIMD.

KVM also uses the TIF_FOREIGN_FPSTATE flag to manage the FP/SIMD state
on the CPU. However, without FP/SIMD support we trap all accesses and
inject undefined instruction. Thus we should never "load" guest state.
Add a sanity check to make sure this is valid.

Fixes: 82e0191a1aa11abf ("arm64: Support systems without FP/ASIMD")
Cc: Will Deacon <[email protected]>
Cc: Mark Rutland <[email protected]>
Reviewed-by: Catalin Marinas <[email protected]>
Acked-by: Marc Zyngier <[email protected]>
Signed-off-by: Suzuki K Poulose <[email protected]>
---
Changes since v2:
- Fixed a nit, dont combine WARN_ON()s
---
arch/arm64/kernel/fpsimd.c | 30 +++++++++++++++++++++++++++---
arch/arm64/kvm/hyp/switch.c | 10 +++++++++-
2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 3eb338f14386..94289d126993 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -269,6 +269,7 @@ static void sve_free(struct task_struct *task)
*/
static void task_fpsimd_load(void)
{
+ WARN_ON(!system_supports_fpsimd());
WARN_ON(!have_cpu_fpsimd_context());

if (system_supports_sve() && test_thread_flag(TIF_SVE))
@@ -289,6 +290,7 @@ static void fpsimd_save(void)
this_cpu_ptr(&fpsimd_last_state);
/* set by fpsimd_bind_task_to_cpu() or fpsimd_bind_state_to_cpu() */

+ WARN_ON(!system_supports_fpsimd());
WARN_ON(!have_cpu_fpsimd_context());

if (!test_thread_flag(TIF_FOREIGN_FPSTATE)) {
@@ -1092,6 +1094,7 @@ void fpsimd_bind_task_to_cpu(void)
struct fpsimd_last_state_struct *last =
this_cpu_ptr(&fpsimd_last_state);

+ WARN_ON(!system_supports_fpsimd());
last->st = &current->thread.uw.fpsimd_state;
last->sve_state = current->thread.sve_state;
last->sve_vl = current->thread.sve_vl;
@@ -1114,6 +1117,7 @@ void fpsimd_bind_state_to_cpu(struct user_fpsimd_state *st, void *sve_state,
struct fpsimd_last_state_struct *last =
this_cpu_ptr(&fpsimd_last_state);

+ WARN_ON(!system_supports_fpsimd());
WARN_ON(!in_softirq() && !irqs_disabled());

last->st = st;
@@ -1128,8 +1132,19 @@ void fpsimd_bind_state_to_cpu(struct user_fpsimd_state *st, void *sve_state,
*/
void fpsimd_restore_current_state(void)
{
- if (!system_supports_fpsimd())
+ /*
+ * For the tasks that were created before we detected the absence of
+ * FP/SIMD, the TIF_FOREIGN_FPSTATE could be set via fpsimd_thread_switch(),
+ * e.g, init. This could be then inherited by the children processes.
+ * If we later detect that the system doesn't support FP/SIMD,
+ * we must clear the flag for all the tasks to indicate that the
+ * FPSTATE is clean (as we can't have one) to avoid looping for ever in
+ * do_notify_resume().
+ */
+ if (!system_supports_fpsimd()) {
+ clear_thread_flag(TIF_FOREIGN_FPSTATE);
return;
+ }

get_cpu_fpsimd_context();

@@ -1148,7 +1163,7 @@ void fpsimd_restore_current_state(void)
*/
void fpsimd_update_current_state(struct user_fpsimd_state const *state)
{
- if (!system_supports_fpsimd())
+ if (WARN_ON(!system_supports_fpsimd()))
return;

get_cpu_fpsimd_context();
@@ -1179,7 +1194,13 @@ void fpsimd_update_current_state(struct user_fpsimd_state const *state)
void fpsimd_flush_task_state(struct task_struct *t)
{
t->thread.fpsimd_cpu = NR_CPUS;
-
+ /*
+ * If we don't support fpsimd, bail out after we have
+ * reset the fpsimd_cpu for this task and clear the
+ * FPSTATE.
+ */
+ if (!system_supports_fpsimd())
+ return;
barrier();
set_tsk_thread_flag(t, TIF_FOREIGN_FPSTATE);

@@ -1193,6 +1214,7 @@ void fpsimd_flush_task_state(struct task_struct *t)
*/
static void fpsimd_flush_cpu_state(void)
{
+ WARN_ON(!system_supports_fpsimd());
__this_cpu_write(fpsimd_last_state.st, NULL);
set_thread_flag(TIF_FOREIGN_FPSTATE);
}
@@ -1203,6 +1225,8 @@ static void fpsimd_flush_cpu_state(void)
*/
void fpsimd_save_and_flush_cpu_state(void)
{
+ if (!system_supports_fpsimd())
+ return;
WARN_ON(preemptible());
__get_cpu_fpsimd_context();
fpsimd_save();
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 72fbbd86eb5e..e5816d885761 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -28,7 +28,15 @@
/* Check whether the FP regs were dirtied while in the host-side run loop: */
static bool __hyp_text update_fp_enabled(struct kvm_vcpu *vcpu)
{
- if (vcpu->arch.host_thread_info->flags & _TIF_FOREIGN_FPSTATE)
+ /*
+ * When the system doesn't support FP/SIMD, we cannot rely on
+ * the _TIF_FOREIGN_FPSTATE flag. However, we always inject an
+ * abort on the very first access to FP and thus we should never
+ * see KVM_ARM64_FP_ENABLED. For added safety, make sure we always
+ * trap the accesses.
+ */
+ if (!system_supports_fpsimd() ||
+ vcpu->arch.host_thread_info->flags & _TIF_FOREIGN_FPSTATE)
vcpu->arch.flags &= ~(KVM_ARM64_FP_ENABLED |
KVM_ARM64_FP_HOST);

--
2.24.1

2020-01-13 23:40:16

by Suzuki K Poulose

[permalink] [raw]
Subject: [PATCH v3 4/7] arm64: cpufeature: Set the FP/SIMD compat HWCAP bits properly

We set the compat_elf_hwcap bits unconditionally on arm64 to
include the VFP and NEON support. However, the FP/SIMD unit
is optional on Arm v8 and thus could be missing. We already
handle this properly in the kernel, but still advertise to
the COMPAT applications that the VFP is available. Fix this
to make sure we only advertise when we really have them.

Fixes: 82e0191a1aa11abf ("arm64: Support systems without FP/ASIMD")
Cc: Will Deacon <[email protected]>
Cc: Mark Rutland <[email protected]>
Reviewed-by: Catalin Marinas <[email protected]>
Signed-off-by: Suzuki K Poulose <[email protected]>
---
arch/arm64/kernel/cpufeature.c | 37 +++++++++++++++++++++++++++++++---
1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 7c373722f692..f0eb0f57ca8d 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -32,9 +32,7 @@ static unsigned long elf_hwcap __read_mostly;
#define COMPAT_ELF_HWCAP_DEFAULT \
(COMPAT_HWCAP_HALF|COMPAT_HWCAP_THUMB|\
COMPAT_HWCAP_FAST_MULT|COMPAT_HWCAP_EDSP|\
- COMPAT_HWCAP_TLS|COMPAT_HWCAP_VFP|\
- COMPAT_HWCAP_VFPv3|COMPAT_HWCAP_VFPv4|\
- COMPAT_HWCAP_NEON|COMPAT_HWCAP_IDIV|\
+ COMPAT_HWCAP_TLS|COMPAT_HWCAP_IDIV|\
COMPAT_HWCAP_LPAE)
unsigned int compat_elf_hwcap __read_mostly = COMPAT_ELF_HWCAP_DEFAULT;
unsigned int compat_elf_hwcap2 __read_mostly;
@@ -1669,6 +1667,12 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
.match_list = list, \
}

+#define HWCAP_CAP_MATCH(match, cap_type, cap) \
+ { \
+ __HWCAP_CAP(#cap, cap_type, cap) \
+ .matches = match, \
+ }
+
#ifdef CONFIG_ARM64_PTR_AUTH
static const struct arm64_cpu_capabilities ptr_auth_hwcap_addr_matches[] = {
{
@@ -1742,8 +1746,35 @@ static const struct arm64_cpu_capabilities arm64_elf_hwcaps[] = {
{},
};

+#ifdef CONFIG_COMPAT
+static bool compat_has_neon(const struct arm64_cpu_capabilities *cap, int scope)
+{
+ /*
+ * Check that all of MVFR1_EL1.{SIMDSP, SIMDInt, SIMDLS} are available,
+ * in line with that of arm32 as in vfp_init(). We make sure that the
+ * check is future proof, by making sure value is non-zero.
+ */
+ u32 mvfr1;
+
+ WARN_ON(scope == SCOPE_LOCAL_CPU && preemptible());
+ if (scope == SCOPE_SYSTEM)
+ mvfr1 = read_sanitised_ftr_reg(SYS_MVFR1_EL1);
+ else
+ mvfr1 = read_sysreg_s(SYS_MVFR1_EL1);
+
+ return cpuid_feature_extract_unsigned_field(mvfr1, MVFR1_SIMDSP_SHIFT) &&
+ cpuid_feature_extract_unsigned_field(mvfr1, MVFR1_SIMDINT_SHIFT) &&
+ cpuid_feature_extract_unsigned_field(mvfr1, MVFR1_SIMDLS_SHIFT);
+}
+#endif
+
static const struct arm64_cpu_capabilities compat_elf_hwcaps[] = {
#ifdef CONFIG_COMPAT
+ HWCAP_CAP_MATCH(compat_has_neon, CAP_COMPAT_HWCAP, COMPAT_HWCAP_NEON),
+ HWCAP_CAP(SYS_MVFR1_EL1, MVFR1_SIMDFMAC_SHIFT, FTR_UNSIGNED, 1, CAP_COMPAT_HWCAP, COMPAT_HWCAP_VFPv4),
+ /* Arm v8 mandates MVFR0.FPDP == {0, 2}. So, piggy back on this for the presence of VFP support */
+ HWCAP_CAP(SYS_MVFR0_EL1, MVFR0_FPDP_SHIFT, FTR_UNSIGNED, 2, CAP_COMPAT_HWCAP, COMPAT_HWCAP_VFP),
+ HWCAP_CAP(SYS_MVFR0_EL1, MVFR0_FPDP_SHIFT, FTR_UNSIGNED, 2, CAP_COMPAT_HWCAP, COMPAT_HWCAP_VFPv3),
HWCAP_CAP(SYS_ID_ISAR5_EL1, ID_ISAR5_AES_SHIFT, FTR_UNSIGNED, 2, CAP_COMPAT_HWCAP2, COMPAT_HWCAP2_PMULL),
HWCAP_CAP(SYS_ID_ISAR5_EL1, ID_ISAR5_AES_SHIFT, FTR_UNSIGNED, 1, CAP_COMPAT_HWCAP2, COMPAT_HWCAP2_AES),
HWCAP_CAP(SYS_ID_ISAR5_EL1, ID_ISAR5_SHA1_SHIFT, FTR_UNSIGNED, 1, CAP_COMPAT_HWCAP2, COMPAT_HWCAP2_SHA1),
--
2.24.1

2020-01-14 00:21:14

by Suzuki K Poulose

[permalink] [raw]
Subject: [PATCH v3 5/7] arm64: ptrace: nofpsimd: Fail FP/SIMD regset operations

When fp/simd is not supported on the system, fail the operations
of FP/SIMD regsets.

Fixes: 82e0191a1aa11abf ("arm64: Support systems without FP/ASIMD")
Cc: Will Deacon <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Catalin Marinas <[email protected]>
Signed-off-by: Suzuki K Poulose <[email protected]>
---
Changes since v2:
- Add active hook for fpsimd(AArch64)/vfp(AArch32) regsets
---
arch/arm64/kernel/ptrace.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)

diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 6771c399d40c..cd6e5fa48b9c 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -615,6 +615,13 @@ static int gpr_set(struct task_struct *target, const struct user_regset *regset,
return 0;
}

+static int fpr_active(struct task_struct *target, const struct user_regset *regset)
+{
+ if (!system_supports_fpsimd())
+ return -ENODEV;
+ return regset->n;
+}
+
/*
* TODO: update fp accessors for lazy context switching (sync/flush hwstate)
*/
@@ -637,6 +644,9 @@ static int fpr_get(struct task_struct *target, const struct user_regset *regset,
unsigned int pos, unsigned int count,
void *kbuf, void __user *ubuf)
{
+ if (!system_supports_fpsimd())
+ return -EINVAL;
+
if (target == current)
fpsimd_preserve_current_state();

@@ -676,6 +686,9 @@ static int fpr_set(struct task_struct *target, const struct user_regset *regset,
{
int ret;

+ if (!system_supports_fpsimd())
+ return -EINVAL;
+
ret = __fpr_set(target, regset, pos, count, kbuf, ubuf, 0);
if (ret)
return ret;
@@ -1134,6 +1147,7 @@ static const struct user_regset aarch64_regsets[] = {
*/
.size = sizeof(u32),
.align = sizeof(u32),
+ .active = fpr_active,
.get = fpr_get,
.set = fpr_set
},
@@ -1348,6 +1362,9 @@ static int compat_vfp_get(struct task_struct *target,
compat_ulong_t fpscr;
int ret, vregs_end_pos;

+ if (!system_supports_fpsimd())
+ return -EINVAL;
+
uregs = &target->thread.uw.fpsimd_state;

if (target == current)
@@ -1381,6 +1398,9 @@ static int compat_vfp_set(struct task_struct *target,
compat_ulong_t fpscr;
int ret, vregs_end_pos;

+ if (!system_supports_fpsimd())
+ return -EINVAL;
+
uregs = &target->thread.uw.fpsimd_state;

vregs_end_pos = VFP_STATE_SIZE - sizeof(compat_ulong_t);
@@ -1438,6 +1458,7 @@ static const struct user_regset aarch32_regsets[] = {
.n = VFP_STATE_SIZE / sizeof(compat_ulong_t),
.size = sizeof(compat_ulong_t),
.align = sizeof(compat_ulong_t),
+ .active = fpr_active,
.get = compat_vfp_get,
.set = compat_vfp_set
},
--
2.24.1

2020-01-14 00:21:15

by Suzuki K Poulose

[permalink] [raw]
Subject: [PATCH v3 6/7] arm64: signal: nofpsimd: Handle fp/simd context for signal frames

Make sure we try to save/restore the vfp/fpsimd context for signal
handling only when the fp/simd support is available. Otherwise, skip
the frames.

Cc: Will Deacon <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Catalin Marinas <[email protected]>
Signed-off-by: Suzuki K Poulose <[email protected]>
---
Changes since v2:
- Remove WARN_ON() from static functions already checked for
the presence of fpsmid.
---
arch/arm64/kernel/signal.c | 6 ++++--
arch/arm64/kernel/signal32.c | 4 ++--
2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index dd2cdc0d5be2..339882db5a91 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -371,6 +371,8 @@ static int parse_user_sigframe(struct user_ctxs *user,
goto done;

case FPSIMD_MAGIC:
+ if (!system_supports_fpsimd())
+ goto invalid;
if (user->fpsimd)
goto invalid;

@@ -506,7 +508,7 @@ static int restore_sigframe(struct pt_regs *regs,
if (err == 0)
err = parse_user_sigframe(&user, sf);

- if (err == 0) {
+ if (err == 0 && system_supports_fpsimd()) {
if (!user.fpsimd)
return -EINVAL;

@@ -623,7 +625,7 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,

err |= __copy_to_user(&sf->uc.uc_sigmask, set, sizeof(*set));

- if (err == 0) {
+ if (err == 0 && system_supports_fpsimd()) {
struct fpsimd_context __user *fpsimd_ctx =
apply_user_offset(user, user->fpsimd_offset);
err |= preserve_fpsimd_context(fpsimd_ctx);
diff --git a/arch/arm64/kernel/signal32.c b/arch/arm64/kernel/signal32.c
index 12a585386c2f..82feca6f7052 100644
--- a/arch/arm64/kernel/signal32.c
+++ b/arch/arm64/kernel/signal32.c
@@ -223,7 +223,7 @@ static int compat_restore_sigframe(struct pt_regs *regs,
err |= !valid_user_regs(&regs->user_regs, current);

aux = (struct compat_aux_sigframe __user *) sf->uc.uc_regspace;
- if (err == 0)
+ if (err == 0 && system_supports_fpsimd())
err |= compat_restore_vfp_context(&aux->vfp);

return err;
@@ -419,7 +419,7 @@ static int compat_setup_sigframe(struct compat_sigframe __user *sf,

aux = (struct compat_aux_sigframe __user *) sf->uc.uc_regspace;

- if (err == 0)
+ if (err == 0 && system_supports_fpsimd())
err |= compat_preserve_vfp_context(&aux->vfp);
__put_user_error(0, &aux->end_magic, err);

--
2.24.1

2020-01-14 00:36:00

by Suzuki K Poulose

[permalink] [raw]
Subject: [PATCH v3 2/7] arm64: fpsimd: Make sure SVE setup is complete before SIMD is used

In-kernel users of NEON rely on may_use_simd() to check if the SIMD
can be used. However, we must initialize the SVE before SIMD can
be used. Add a sanity check to make sure that we have completed the
SVE setup before anyone uses the SIMD.

Cc: Ard Biesheuvel <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Catalin Marinas <[email protected]>
Signed-off-by: Suzuki K Poulose <[email protected]>
---
Changes since v2:
- Add WARN_ON() in may_use_simd() if invoked before caps
are finalized()
---
arch/arm64/include/asm/simd.h | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/simd.h b/arch/arm64/include/asm/simd.h
index 7434844036d3..89cba2622b79 100644
--- a/arch/arm64/include/asm/simd.h
+++ b/arch/arm64/include/asm/simd.h
@@ -26,6 +26,8 @@ DECLARE_PER_CPU(bool, fpsimd_context_busy);
static __must_check inline bool may_use_simd(void)
{
/*
+ * We must make sure that the SVE has been initialized properly
+ * before using the SIMD in kernel.
* fpsimd_context_busy is only set while preemption is disabled,
* and is clear whenever preemption is enabled. Since
* this_cpu_read() is atomic w.r.t. preemption, fpsimd_context_busy
@@ -33,8 +35,10 @@ static __must_check inline bool may_use_simd(void)
* migrated, and if it's clear we cannot be migrated to a CPU
* where it is set.
*/
- return !in_irq() && !irqs_disabled() && !in_nmi() &&
- !this_cpu_read(fpsimd_context_busy);
+ return !WARN_ON(!system_capabilities_finalized()) &&
+ system_supports_fpsimd() &&
+ !in_irq() && !irqs_disabled() && !in_nmi() &&
+ !this_cpu_read(fpsimd_context_busy);
}

#else /* ! CONFIG_KERNEL_MODE_NEON */
--
2.24.1

2020-01-14 08:31:44

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v3 0/7] arm64: Fix support for no FP/SIMD

On Tue, 14 Jan 2020 at 00:30, Suzuki K Poulose <[email protected]> wrote:
>
> This series fixes the support for systems without FP/SIMD unit.
>
> We detect the absence of FP/SIMD after the SMP cpus are brought
> online (i.e, SYSTEM scope). This means, we allow a hotplugged
> CPU to boot successfully even if it doesn't have the FP/SIMD
> when we have decided otherwise at boot and have now advertised
> the ELF HWCAP for the userspace. Fix this by turning this to a
> BOOT_RESTRICTED_CPU_LOCAL feature to allow the detection of the
> feature the very moment a CPU turns up without FP/SIMD and also
> prevent a conflict after SMP boot.
>
> The COMPAT ELF_HWCAPs were statically set to indicate the
> availability of VFP. Make it dynamic to set the appropriate
> bits.
>
> Also, some of the early kernel threads (including init) could run
> with their TIF_FOREIGN_FPSTATE flag set which might be inherited
> by applications forked by them (e.g, modprobe from initramfs).
> Now, if we detect the absence of FP/SIMD we stop clearing the
> TIF flag in fpsimd_restore_current_state(). This could cause
> the applications stuck in do_notify_resume() looping forever
> to clear the flag. Fix this by clearing the TIF flag in
> fpsimd_restore_current_state() for the tasks that may
> have it set.
>
> This series also categorises the functions dealing with fpsimd
> into two :
>
> - Call permitted with missing FP/SIMD support. But we bail
> out early in the function. This is for functions exposed
> to the generic kernel code.
>
> - Calls not permitted with missing FP/SIMD support. These
> are functions which deal with the CPU/Task FP/SIMD registers
> and/or meta-data. The callers must check for the support
> before invoking them.
>
> See the last patch in the series for details.
>
> Also make sure that the SVE is initialised where supported,
> before the FP/SIMD is used by the kernel.
>
> Tested with debian armel initramfs and rootfs. The arm64 doesn't
> have a soft-float ABI, thus haven't tested it with 64bit userspace.
>
> Applies on linux-aarch64 for-next/core
>
> Changes since v2:
> - Rebase on to for-next/core, resolved conflict with the E0PD
> handling changes
> - Address comments from Catalin
> - Remove warnings from static functions
> - Add WARN_ON in may_use_simd() if called before initializing
> capabilities.
> - Add "active" hook for FP regset
> - Remove dangerous WARN_ON from KVM, replaced with an additional
> check to make sure that the FP/SIMD is always trapped.
> - Added tags from Catalin, Marc
>
> Suzuki K Poulose (7):
> arm64: Introduce system_capabilities_finalized() marker
> arm64: fpsimd: Make sure SVE setup is complete before SIMD is used
> arm64: cpufeature: Fix the type of no FP/SIMD capability
> arm64: cpufeature: Set the FP/SIMD compat HWCAP bits properly
> arm64: ptrace: nofpsimd: Fail FP/SIMD regset operations
> arm64: signal: nofpsimd: Handle fp/simd context for signal frames
> arm64: nofpsmid: Handle TIF_FOREIGN_FPSTATE flag cleanly
>

For the series,

Reviewed-by: Ard Biesheuvel <[email protected]>


> arch/arm64/include/asm/cpufeature.h | 5 +++
> arch/arm64/include/asm/kvm_host.h | 2 +-
> arch/arm64/include/asm/simd.h | 8 +++-
> arch/arm64/kernel/cpufeature.c | 67 +++++++++++++++++++----------
> arch/arm64/kernel/fpsimd.c | 30 +++++++++++--
> arch/arm64/kernel/process.c | 2 +-
> arch/arm64/kernel/ptrace.c | 21 +++++++++
> arch/arm64/kernel/signal.c | 6 ++-
> arch/arm64/kernel/signal32.c | 4 +-
> arch/arm64/kvm/hyp/switch.c | 10 ++++-
> 10 files changed, 121 insertions(+), 34 deletions(-)
>
> --
> 2.24.1
>

2020-01-14 11:57:46

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH v3 2/7] arm64: fpsimd: Make sure SVE setup is complete before SIMD is used

On Mon, Jan 13, 2020 at 11:30:18PM +0000, Suzuki K Poulose wrote:
> In-kernel users of NEON rely on may_use_simd() to check if the SIMD
> can be used. However, we must initialize the SVE before SIMD can
> be used. Add a sanity check to make sure that we have completed the
> SVE setup before anyone uses the SIMD.
>
> Cc: Ard Biesheuvel <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Signed-off-by: Suzuki K Poulose <[email protected]>

Reviewed-by: Catalin Marinas <[email protected]>

2020-01-14 11:59:04

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH v3 6/7] arm64: signal: nofpsimd: Handle fp/simd context for signal frames

On Mon, Jan 13, 2020 at 11:30:22PM +0000, Suzuki K Poulose wrote:
> Make sure we try to save/restore the vfp/fpsimd context for signal
> handling only when the fp/simd support is available. Otherwise, skip
> the frames.
>
> Cc: Will Deacon <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Signed-off-by: Suzuki K Poulose <[email protected]>

Reviewed-by: Catalin Marinas <[email protected]>

2020-01-14 11:59:05

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH v3 5/7] arm64: ptrace: nofpsimd: Fail FP/SIMD regset operations

On Mon, Jan 13, 2020 at 11:30:21PM +0000, Suzuki K Poulose wrote:
> When fp/simd is not supported on the system, fail the operations
> of FP/SIMD regsets.
>
> Fixes: 82e0191a1aa11abf ("arm64: Support systems without FP/ASIMD")
> Cc: Will Deacon <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Signed-off-by: Suzuki K Poulose <[email protected]>

Reviewed-by: Catalin Marinas <[email protected]>