* [PATCH v3 0/2] arm64: Support systems without FP/ASIMD
@ 2016-11-08 13:56 Suzuki K Poulose
2016-11-08 13:56 ` [PATCH v3 1/2] arm64: Add hypervisor safe helper for checking constant capabilities Suzuki K Poulose
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Suzuki K Poulose @ 2016-11-08 13:56 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-kernel, ard.biesheuvel, will.deacon, catalin.marinas,
mark.rutland, marc.zyngier, kvmarm, Suzuki K Poulose
This series adds supports to the kernel and KVM hyp to handle
systems without FP/ASIMD properly. At the moment the kernel
doesn't check if the FP unit is available before accessing
the registers (e.g during context switch). Also for KVM,
we trap the FP/ASIMD accesses and handle it by injecting an
undefined instruction into the VM on systems without FP.
Tested on a FVP_Base-AEM-v8A model by disabling VFP on at
least one CPU ( -C clusterX.cpuY.vfp-present=0 ).
Changes since V2:
- Dropped cleanup patch for arm64/crypto/aes-ce-ccm-glue.c
- Removed static_key check from cpus_have_cap. All users with
constant caps should use the new API to make use of static_keys.
- Removed a dedicated static_key used in irqchip-gic-v3.c for
Cavium errata with the new API.
Applies on v4.9-rc4 + [1] (which is pushed for rc5)
[1] http://marc.info/?l=linux-arm-kernel&m=147819889813214&w=2
Suzuki K Poulose (2):
arm64: Add hypervisor safe helper for checking constant capabilities
arm64: Support systems without FP/ASIMD
arch/arm64/include/asm/cpucaps.h | 3 ++-
arch/arm64/include/asm/cpufeature.h | 24 +++++++++++++++++-------
arch/arm64/include/asm/neon.h | 3 ++-
arch/arm64/kernel/cpufeature.c | 17 ++++++++++++++++-
arch/arm64/kernel/fpsimd.c | 14 ++++++++++++++
arch/arm64/kernel/process.c | 2 +-
arch/arm64/kvm/handle_exit.c | 11 +++++++++++
arch/arm64/kvm/hyp/hyp-entry.S | 9 ++++++++-
arch/arm64/kvm/hyp/switch.c | 5 ++++-
drivers/irqchip/irq-gic-v3.c | 13 +------------
10 files changed, 76 insertions(+), 25 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v3 1/2] arm64: Add hypervisor safe helper for checking constant capabilities 2016-11-08 13:56 [PATCH v3 0/2] arm64: Support systems without FP/ASIMD Suzuki K Poulose @ 2016-11-08 13:56 ` Suzuki K Poulose 2016-11-08 18:11 ` Will Deacon 2016-11-08 13:56 ` [PATCH v3 2/2] arm64: Support systems without FP/ASIMD Suzuki K Poulose ` (2 subsequent siblings) 3 siblings, 1 reply; 9+ messages in thread From: Suzuki K Poulose @ 2016-11-08 13:56 UTC (permalink / raw) To: linux-arm-kernel Cc: linux-kernel, ard.biesheuvel, will.deacon, catalin.marinas, mark.rutland, marc.zyngier, kvmarm, Suzuki K Poulose, Robert Ritcher, Tirumalesh Chalamarla The hypervisor may not have full access to the kernel data structures and hence cannot safely use cpus_have_cap() helper for checking the system capability. Add a safe helper for hypervisors to check a constant system capability, which *doesn't* fall back to checking the bitmap maintained by the kernel. With this, make the cpus_have_cap() only check the bitmask and force constant cap checks to use the new API for quicker checks. Cc: Robert Ritcher <rritcher@cavium.com> Cc: Tirumalesh Chalamarla <tchalamarla@cavium.com> Cc: Marc Zyngier <marc.zyngier@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will.deacon@arm.com> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> --- arch/arm64/include/asm/cpufeature.h | 19 ++++++++++++------- arch/arm64/kernel/cpufeature.c | 2 +- arch/arm64/kernel/process.c | 2 +- drivers/irqchip/irq-gic-v3.c | 13 +------------ 4 files changed, 15 insertions(+), 21 deletions(-) diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index 0bc0b1d..9890d20 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -9,8 +9,6 @@ #ifndef __ASM_CPUFEATURE_H #define __ASM_CPUFEATURE_H -#include <linux/jump_label.h> - #include <asm/cpucaps.h> #include <asm/hwcap.h> #include <asm/sysreg.h> @@ -27,6 +25,8 @@ #ifndef __ASSEMBLY__ +#include <linux/bug.h> +#include <linux/jump_label.h> #include <linux/kernel.h> /* CPU feature register tracking */ @@ -104,14 +104,19 @@ static inline bool cpu_have_feature(unsigned int num) return elf_hwcap & (1UL << num); } +/* System capability check for constant caps */ +static inline bool cpus_have_const_cap(int num) +{ + if (num >= ARM64_NCAPS) + return false; + return static_branch_unlikely(&cpu_hwcap_keys[num]); +} + static inline bool cpus_have_cap(unsigned int num) { if (num >= ARM64_NCAPS) return false; - if (__builtin_constant_p(num)) - return static_branch_unlikely(&cpu_hwcap_keys[num]); - else - return test_bit(num, cpu_hwcaps); + return test_bit(num, cpu_hwcaps); } static inline void cpus_set_cap(unsigned int num) @@ -200,7 +205,7 @@ static inline bool cpu_supports_mixed_endian_el0(void) static inline bool system_supports_32bit_el0(void) { - return cpus_have_cap(ARM64_HAS_32BIT_EL0); + return cpus_have_const_cap(ARM64_HAS_32BIT_EL0); } static inline bool system_supports_mixed_endian_el0(void) diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index c02504e..fc2bd19 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -1102,5 +1102,5 @@ void __init setup_cpu_features(void) static bool __maybe_unused cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry, int __unused) { - return (cpus_have_cap(ARM64_HAS_PAN) && !cpus_have_cap(ARM64_HAS_UAO)); + return (cpus_have_const_cap(ARM64_HAS_PAN) && !cpus_have_const_cap(ARM64_HAS_UAO)); } diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index 01753cd..18354f3 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -282,7 +282,7 @@ int copy_thread(unsigned long clone_flags, unsigned long stack_start, memset(childregs, 0, sizeof(struct pt_regs)); childregs->pstate = PSR_MODE_EL1h; if (IS_ENABLED(CONFIG_ARM64_UAO) && - cpus_have_cap(ARM64_HAS_UAO)) + cpus_have_const_cap(ARM64_HAS_UAO)) childregs->pstate |= PSR_UAO_BIT; p->thread.cpu_context.x19 = stack_start; p->thread.cpu_context.x20 = stk_sz; diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c index 19d642e..26e1d7f 100644 --- a/drivers/irqchip/irq-gic-v3.c +++ b/drivers/irqchip/irq-gic-v3.c @@ -120,11 +120,10 @@ static void gic_redist_wait_for_rwp(void) } #ifdef CONFIG_ARM64 -static DEFINE_STATIC_KEY_FALSE(is_cavium_thunderx); static u64 __maybe_unused gic_read_iar(void) { - if (static_branch_unlikely(&is_cavium_thunderx)) + if (cpus_have_const_cap(ARM64_WORKAROUND_CAVIUM_23154)) return gic_read_iar_cavium_thunderx(); else return gic_read_iar_common(); @@ -905,14 +904,6 @@ static const struct irq_domain_ops partition_domain_ops = { .select = gic_irq_domain_select, }; -static void gicv3_enable_quirks(void) -{ -#ifdef CONFIG_ARM64 - if (cpus_have_cap(ARM64_WORKAROUND_CAVIUM_23154)) - static_branch_enable(&is_cavium_thunderx); -#endif -} - static int __init gic_init_bases(void __iomem *dist_base, struct redist_region *rdist_regs, u32 nr_redist_regions, @@ -935,8 +926,6 @@ static int __init gic_init_bases(void __iomem *dist_base, gic_data.nr_redist_regions = nr_redist_regions; gic_data.redist_stride = redist_stride; - gicv3_enable_quirks(); - /* * Find out how many interrupts are supported. * The GIC only supports up to 1020 interrupt sources (SGI+PPI+SPI) -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/2] arm64: Add hypervisor safe helper for checking constant capabilities 2016-11-08 13:56 ` [PATCH v3 1/2] arm64: Add hypervisor safe helper for checking constant capabilities Suzuki K Poulose @ 2016-11-08 18:11 ` Will Deacon 0 siblings, 0 replies; 9+ messages in thread From: Will Deacon @ 2016-11-08 18:11 UTC (permalink / raw) To: Suzuki K Poulose Cc: linux-arm-kernel, linux-kernel, ard.biesheuvel, catalin.marinas, mark.rutland, marc.zyngier, kvmarm, Robert Ritcher, Tirumalesh Chalamarla On Tue, Nov 08, 2016 at 01:56:20PM +0000, Suzuki K Poulose wrote: > The hypervisor may not have full access to the kernel data structures > and hence cannot safely use cpus_have_cap() helper for checking the > system capability. Add a safe helper for hypervisors to check a constant > system capability, which *doesn't* fall back to checking the bitmap > maintained by the kernel. With this, make the cpus_have_cap() only > check the bitmask and force constant cap checks to use the new API > for quicker checks. > > Cc: Robert Ritcher <rritcher@cavium.com> > Cc: Tirumalesh Chalamarla <tchalamarla@cavium.com> > Cc: Marc Zyngier <marc.zyngier@arm.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will.deacon@arm.com> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > --- > arch/arm64/include/asm/cpufeature.h | 19 ++++++++++++------- > arch/arm64/kernel/cpufeature.c | 2 +- > arch/arm64/kernel/process.c | 2 +- > drivers/irqchip/irq-gic-v3.c | 13 +------------ It might be worth having the GIC changes as a separate patch, but either way: Reviewed-by: Will Deacon <will.deacon@arm.com> Will ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 2/2] arm64: Support systems without FP/ASIMD 2016-11-08 13:56 [PATCH v3 0/2] arm64: Support systems without FP/ASIMD Suzuki K Poulose 2016-11-08 13:56 ` [PATCH v3 1/2] arm64: Add hypervisor safe helper for checking constant capabilities Suzuki K Poulose @ 2016-11-08 13:56 ` Suzuki K Poulose 2016-11-14 11:48 ` Catalin Marinas 2016-11-11 13:41 ` [PATCH v3 0/2] " Marc Zyngier 2016-11-16 18:11 ` Catalin Marinas 3 siblings, 1 reply; 9+ messages in thread From: Suzuki K Poulose @ 2016-11-08 13:56 UTC (permalink / raw) To: linux-arm-kernel Cc: linux-kernel, ard.biesheuvel, will.deacon, catalin.marinas, mark.rutland, marc.zyngier, kvmarm, Suzuki K Poulose, Christoffer Dall The arm64 kernel assumes that FP/ASIMD units are always present and accesses the FP/ASIMD specific registers unconditionally. This could cause problems when they are absent. This patch adds the support for kernel handling systems without FP/ASIMD by skipping the register access within the kernel. For kvm, we trap the accesses to FP/ASIMD and inject an undefined instruction exception to the VM. The callers of the exported kernel_neon_begin_partial() should make sure that the FP/ASIMD is supported. Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will.deacon@arm.com> Cc: Christoffer Dall <christoffer.dall@linaro.org> Cc: Marc Zyngier <marc.zyngier@arm.com> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> --- arch/arm64/include/asm/cpucaps.h | 3 ++- arch/arm64/include/asm/cpufeature.h | 5 +++++ arch/arm64/include/asm/neon.h | 3 ++- arch/arm64/kernel/cpufeature.c | 15 +++++++++++++++ arch/arm64/kernel/fpsimd.c | 14 ++++++++++++++ arch/arm64/kvm/handle_exit.c | 11 +++++++++++ arch/arm64/kvm/hyp/hyp-entry.S | 9 ++++++++- arch/arm64/kvm/hyp/switch.c | 5 ++++- 8 files changed, 61 insertions(+), 4 deletions(-) diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h index 87b4465..4174f09 100644 --- a/arch/arm64/include/asm/cpucaps.h +++ b/arch/arm64/include/asm/cpucaps.h @@ -34,7 +34,8 @@ #define ARM64_HAS_32BIT_EL0 13 #define ARM64_HYP_OFFSET_LOW 14 #define ARM64_MISMATCHED_CACHE_LINE_SIZE 15 +#define ARM64_HAS_NO_FPSIMD 16 -#define ARM64_NCAPS 16 +#define ARM64_NCAPS 17 #endif /* __ASM_CPUCAPS_H */ diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index 9890d20..ce45770 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -213,6 +213,11 @@ static inline bool system_supports_mixed_endian_el0(void) return id_aa64mmfr0_mixed_endian_el0(read_system_reg(SYS_ID_AA64MMFR0_EL1)); } +static inline bool system_supports_fpsimd(void) +{ + return !cpus_have_const_cap(ARM64_HAS_NO_FPSIMD); +} + #endif /* __ASSEMBLY__ */ #endif diff --git a/arch/arm64/include/asm/neon.h b/arch/arm64/include/asm/neon.h index 13ce4cc..ad4cdc9 100644 --- a/arch/arm64/include/asm/neon.h +++ b/arch/arm64/include/asm/neon.h @@ -9,8 +9,9 @@ */ #include <linux/types.h> +#include <asm/fpsimd.h> -#define cpu_has_neon() (1) +#define cpu_has_neon() system_supports_fpsimd() #define kernel_neon_begin() kernel_neon_begin_partial(32) diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index fc2bd19..f89385d 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -746,6 +746,14 @@ static bool hyp_offset_low(const struct arm64_cpu_capabilities *entry, return idmap_addr > GENMASK(VA_BITS - 2, 0) && !is_kernel_in_hyp_mode(); } +static bool has_no_fpsimd(const struct arm64_cpu_capabilities *entry, int __unused) +{ + u64 pfr0 = read_system_reg(SYS_ID_AA64PFR0_EL1); + + return cpuid_feature_extract_signed_field(pfr0, + ID_AA64PFR0_FP_SHIFT) < 0; +} + static const struct arm64_cpu_capabilities arm64_features[] = { { .desc = "GIC system register CPU interface", @@ -829,6 +837,13 @@ static const struct arm64_cpu_capabilities arm64_features[] = { .def_scope = SCOPE_SYSTEM, .matches = hyp_offset_low, }, + { + /* FP/SIMD is not implemented */ + .capability = ARM64_HAS_NO_FPSIMD, + .def_scope = SCOPE_SYSTEM, + .min_field_value = 0, + .matches = has_no_fpsimd, + }, {}, }; diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c index 394c61d..b883f1f 100644 --- a/arch/arm64/kernel/fpsimd.c +++ b/arch/arm64/kernel/fpsimd.c @@ -127,6 +127,8 @@ void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs) void fpsimd_thread_switch(struct task_struct *next) { + if (!system_supports_fpsimd()) + return; /* * Save the current FPSIMD state to memory, but only if whatever is in * the registers is in fact the most recent userland FPSIMD state of @@ -157,6 +159,8 @@ void fpsimd_thread_switch(struct task_struct *next) void fpsimd_flush_thread(void) { + if (!system_supports_fpsimd()) + return; memset(¤t->thread.fpsimd_state, 0, sizeof(struct fpsimd_state)); fpsimd_flush_task_state(current); set_thread_flag(TIF_FOREIGN_FPSTATE); @@ -168,6 +172,8 @@ void fpsimd_flush_thread(void) */ void fpsimd_preserve_current_state(void) { + if (!system_supports_fpsimd()) + return; preempt_disable(); if (!test_thread_flag(TIF_FOREIGN_FPSTATE)) fpsimd_save_state(¤t->thread.fpsimd_state); @@ -181,6 +187,8 @@ void fpsimd_preserve_current_state(void) */ void fpsimd_restore_current_state(void) { + if (!system_supports_fpsimd()) + return; preempt_disable(); if (test_and_clear_thread_flag(TIF_FOREIGN_FPSTATE)) { struct fpsimd_state *st = ¤t->thread.fpsimd_state; @@ -199,6 +207,8 @@ void fpsimd_restore_current_state(void) */ void fpsimd_update_current_state(struct fpsimd_state *state) { + if (!system_supports_fpsimd()) + return; preempt_disable(); fpsimd_load_state(state); if (test_and_clear_thread_flag(TIF_FOREIGN_FPSTATE)) { @@ -228,6 +238,8 @@ static DEFINE_PER_CPU(struct fpsimd_partial_state, softirq_fpsimdstate); */ void kernel_neon_begin_partial(u32 num_regs) { + if (WARN_ON(!system_supports_fpsimd())) + return; if (in_interrupt()) { struct fpsimd_partial_state *s = this_cpu_ptr( in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate); @@ -252,6 +264,8 @@ EXPORT_SYMBOL(kernel_neon_begin_partial); void kernel_neon_end(void) { + if (!system_supports_fpsimd()) + return; if (in_interrupt()) { struct fpsimd_partial_state *s = this_cpu_ptr( in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate); diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c index a204adf..1bfe30d 100644 --- a/arch/arm64/kvm/handle_exit.c +++ b/arch/arm64/kvm/handle_exit.c @@ -57,6 +57,16 @@ static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run) return 1; } +/* + * Guest access to FP/ASIMD registers are routed to this handler only + * when the system doesn't support FP/ASIMD. + */ +static int handle_no_fpsimd(struct kvm_vcpu *vcpu, struct kvm_run *run) +{ + kvm_inject_undefined(vcpu); + return 1; +} + /** * kvm_handle_wfx - handle a wait-for-interrupts or wait-for-event * instruction executed by a guest @@ -144,6 +154,7 @@ static exit_handle_fn arm_exit_handlers[] = { [ESR_ELx_EC_BREAKPT_LOW]= kvm_handle_guest_debug, [ESR_ELx_EC_BKPT32] = kvm_handle_guest_debug, [ESR_ELx_EC_BRK64] = kvm_handle_guest_debug, + [ESR_ELx_EC_FP_ASIMD] = handle_no_fpsimd, }; static exit_handle_fn kvm_get_exit_handler(struct kvm_vcpu *vcpu) diff --git a/arch/arm64/kvm/hyp/hyp-entry.S b/arch/arm64/kvm/hyp/hyp-entry.S index 4e92399..5e9052f 100644 --- a/arch/arm64/kvm/hyp/hyp-entry.S +++ b/arch/arm64/kvm/hyp/hyp-entry.S @@ -106,9 +106,16 @@ el1_trap: * x0: ESR_EC */ - /* Guest accessed VFP/SIMD registers, save host, restore Guest */ + /* + * We trap the first access to the FP/SIMD to save the host context + * and restore the guest context lazily. + * If FP/SIMD is not implemented, handle the trap and inject an + * undefined instruction exception to the guest. + */ +alternative_if_not ARM64_HAS_NO_FPSIMD cmp x0, #ESR_ELx_EC_FP_ASIMD b.eq __fpsimd_guest_restore +alternative_else_nop_endif mrs x1, tpidr_el2 mov x0, #ARM_EXCEPTION_TRAP diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c index 83037cd..8bcae7b 100644 --- a/arch/arm64/kvm/hyp/switch.c +++ b/arch/arm64/kvm/hyp/switch.c @@ -21,6 +21,7 @@ #include <asm/kvm_asm.h> #include <asm/kvm_emulate.h> #include <asm/kvm_hyp.h> +#include <asm/fpsimd.h> static bool __hyp_text __fpsimd_enabled_nvhe(void) { @@ -76,9 +77,11 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu) * traps are only taken to EL2 if the operation would not otherwise * trap to EL1. Therefore, always make sure that for 32-bit guests, * we set FPEXC.EN to prevent traps to EL1, when setting the TFP bit. + * If FP/ASIMD is not implemented, FPEXC is UNDEFINED and any access to + * it will cause an exception. */ val = vcpu->arch.hcr_el2; - if (!(val & HCR_RW)) { + if (!(val & HCR_RW) && system_supports_fpsimd()) { write_sysreg(1 << 30, fpexc32_el2); isb(); } -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/2] arm64: Support systems without FP/ASIMD 2016-11-08 13:56 ` [PATCH v3 2/2] arm64: Support systems without FP/ASIMD Suzuki K Poulose @ 2016-11-14 11:48 ` Catalin Marinas 2016-11-15 10:42 ` Suzuki K Poulose 0 siblings, 1 reply; 9+ messages in thread From: Catalin Marinas @ 2016-11-14 11:48 UTC (permalink / raw) To: Suzuki K Poulose Cc: linux-arm-kernel, mark.rutland, ard.biesheuvel, marc.zyngier, will.deacon, linux-kernel, kvmarm, Christoffer Dall Hi Suzuki, On Tue, Nov 08, 2016 at 01:56:21PM +0000, Suzuki K. Poulose wrote: > diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h > index 87b4465..4174f09 100644 > --- a/arch/arm64/include/asm/cpucaps.h > +++ b/arch/arm64/include/asm/cpucaps.h > @@ -34,7 +34,8 @@ > #define ARM64_HAS_32BIT_EL0 13 > #define ARM64_HYP_OFFSET_LOW 14 > #define ARM64_MISMATCHED_CACHE_LINE_SIZE 15 > +#define ARM64_HAS_NO_FPSIMD 16 > > -#define ARM64_NCAPS 16 > +#define ARM64_NCAPS 17 > > #endif /* __ASM_CPUCAPS_H */ > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h > index 9890d20..ce45770 100644 > --- a/arch/arm64/include/asm/cpufeature.h > +++ b/arch/arm64/include/asm/cpufeature.h > @@ -213,6 +213,11 @@ static inline bool system_supports_mixed_endian_el0(void) > return id_aa64mmfr0_mixed_endian_el0(read_system_reg(SYS_ID_AA64MMFR0_EL1)); > } > > +static inline bool system_supports_fpsimd(void) > +{ > + return !cpus_have_const_cap(ARM64_HAS_NO_FPSIMD); > +} Any particular reason why using negation instead of a ARM64_HAS_FPSIMD? A potential problem would be the default cpus_have_const_cap() implementation and the default static key having a slight performance impact. > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > index fc2bd19..f89385d 100644 > --- a/arch/arm64/kernel/cpufeature.c > +++ b/arch/arm64/kernel/cpufeature.c > @@ -746,6 +746,14 @@ static bool hyp_offset_low(const struct arm64_cpu_capabilities *entry, > return idmap_addr > GENMASK(VA_BITS - 2, 0) && !is_kernel_in_hyp_mode(); > } > > +static bool has_no_fpsimd(const struct arm64_cpu_capabilities *entry, int __unused) > +{ > + u64 pfr0 = read_system_reg(SYS_ID_AA64PFR0_EL1); > + > + return cpuid_feature_extract_signed_field(pfr0, > + ID_AA64PFR0_FP_SHIFT) < 0; > +} > + > static const struct arm64_cpu_capabilities arm64_features[] = { > { > .desc = "GIC system register CPU interface", > @@ -829,6 +837,13 @@ static const struct arm64_cpu_capabilities arm64_features[] = { > .def_scope = SCOPE_SYSTEM, > .matches = hyp_offset_low, > }, > + { > + /* FP/SIMD is not implemented */ > + .capability = ARM64_HAS_NO_FPSIMD, > + .def_scope = SCOPE_SYSTEM, > + .min_field_value = 0, > + .matches = has_no_fpsimd, > + }, If we go for negation, I don't think we need a min_field_value at all, the matching is done by the has_no_fpsimd() function. -- Catalin ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/2] arm64: Support systems without FP/ASIMD 2016-11-14 11:48 ` Catalin Marinas @ 2016-11-15 10:42 ` Suzuki K Poulose 0 siblings, 0 replies; 9+ messages in thread From: Suzuki K Poulose @ 2016-11-15 10:42 UTC (permalink / raw) To: Catalin Marinas Cc: linux-arm-kernel, mark.rutland, ard.biesheuvel, marc.zyngier, will.deacon, linux-kernel, kvmarm, Christoffer Dall, nd On 14/11/16 11:48, Catalin Marinas wrote: > Hi Suzuki, > >> +static inline bool system_supports_fpsimd(void) >> +{ >> + return !cpus_have_const_cap(ARM64_HAS_NO_FPSIMD); >> +} > > Any particular reason why using negation instead of a ARM64_HAS_FPSIMD? > A potential problem would be the default cpus_have_const_cap() > implementation and the default static key having a slight performance > impact. The negation was chosen to avoid hotpatching in the most common case. But as you said, it has an impact on the other side. I think doing a one time hotpatching at boot time is more optimal than penalising a bunch of other users throughout the execution. I will take a look at changing it back to a ARM64_HAS_FPSIMD. >> }, >> + { >> + /* FP/SIMD is not implemented */ >> + .capability = ARM64_HAS_NO_FPSIMD, >> + .def_scope = SCOPE_SYSTEM, >> + .min_field_value = 0, >> + .matches = has_no_fpsimd, >> + }, > > If we go for negation, I don't think we need a min_field_value at all, > the matching is done by the has_no_fpsimd() function. You're right. Suzuki ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/2] arm64: Support systems without FP/ASIMD 2016-11-08 13:56 [PATCH v3 0/2] arm64: Support systems without FP/ASIMD Suzuki K Poulose 2016-11-08 13:56 ` [PATCH v3 1/2] arm64: Add hypervisor safe helper for checking constant capabilities Suzuki K Poulose 2016-11-08 13:56 ` [PATCH v3 2/2] arm64: Support systems without FP/ASIMD Suzuki K Poulose @ 2016-11-11 13:41 ` Marc Zyngier 2016-11-14 11:08 ` Catalin Marinas 2016-11-16 18:11 ` Catalin Marinas 3 siblings, 1 reply; 9+ messages in thread From: Marc Zyngier @ 2016-11-11 13:41 UTC (permalink / raw) To: Suzuki K Poulose, catalin.marinas Cc: linux-arm-kernel, linux-kernel, ard.biesheuvel, will.deacon, mark.rutland, kvmarm On 08/11/16 13:56, Suzuki K Poulose wrote: > This series adds supports to the kernel and KVM hyp to handle > systems without FP/ASIMD properly. At the moment the kernel > doesn't check if the FP unit is available before accessing > the registers (e.g during context switch). Also for KVM, > we trap the FP/ASIMD accesses and handle it by injecting an > undefined instruction into the VM on systems without FP. > > Tested on a FVP_Base-AEM-v8A model by disabling VFP on at > least one CPU ( -C clusterX.cpuY.vfp-present=0 ). > > Changes since V2: > - Dropped cleanup patch for arm64/crypto/aes-ce-ccm-glue.c > - Removed static_key check from cpus_have_cap. All users with > constant caps should use the new API to make use of static_keys. > - Removed a dedicated static_key used in irqchip-gic-v3.c for > Cavium errata with the new API. > > Applies on v4.9-rc4 + [1] (which is pushed for rc5) > > [1] http://marc.info/?l=linux-arm-kernel&m=147819889813214&w=2 > > > Suzuki K Poulose (2): > arm64: Add hypervisor safe helper for checking constant capabilities > arm64: Support systems without FP/ASIMD > > arch/arm64/include/asm/cpucaps.h | 3 ++- > arch/arm64/include/asm/cpufeature.h | 24 +++++++++++++++++------- > arch/arm64/include/asm/neon.h | 3 ++- > arch/arm64/kernel/cpufeature.c | 17 ++++++++++++++++- > arch/arm64/kernel/fpsimd.c | 14 ++++++++++++++ > arch/arm64/kernel/process.c | 2 +- > arch/arm64/kvm/handle_exit.c | 11 +++++++++++ > arch/arm64/kvm/hyp/hyp-entry.S | 9 ++++++++- > arch/arm64/kvm/hyp/switch.c | 5 ++++- > drivers/irqchip/irq-gic-v3.c | 13 +------------ > 10 files changed, 76 insertions(+), 25 deletions(-) > For the series: Reviewed-by: Marc Zyngier <marc.zyngier@arm.com> How do we plan on merging this? Catalin, are you willing to take it all? Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/2] arm64: Support systems without FP/ASIMD 2016-11-11 13:41 ` [PATCH v3 0/2] " Marc Zyngier @ 2016-11-14 11:08 ` Catalin Marinas 0 siblings, 0 replies; 9+ messages in thread From: Catalin Marinas @ 2016-11-14 11:08 UTC (permalink / raw) To: Marc Zyngier Cc: Suzuki K Poulose, mark.rutland, ard.biesheuvel, will.deacon, linux-kernel, kvmarm, linux-arm-kernel On Fri, Nov 11, 2016 at 01:41:37PM +0000, Marc Zyngier wrote: > On 08/11/16 13:56, Suzuki K Poulose wrote: > > This series adds supports to the kernel and KVM hyp to handle > > systems without FP/ASIMD properly. At the moment the kernel > > doesn't check if the FP unit is available before accessing > > the registers (e.g during context switch). Also for KVM, > > we trap the FP/ASIMD accesses and handle it by injecting an > > undefined instruction into the VM on systems without FP. > > > > Tested on a FVP_Base-AEM-v8A model by disabling VFP on at > > least one CPU ( -C clusterX.cpuY.vfp-present=0 ). > > > > Changes since V2: > > - Dropped cleanup patch for arm64/crypto/aes-ce-ccm-glue.c > > - Removed static_key check from cpus_have_cap. All users with > > constant caps should use the new API to make use of static_keys. > > - Removed a dedicated static_key used in irqchip-gic-v3.c for > > Cavium errata with the new API. > > > > Applies on v4.9-rc4 + [1] (which is pushed for rc5) > > > > [1] http://marc.info/?l=linux-arm-kernel&m=147819889813214&w=2 > > > > > > Suzuki K Poulose (2): > > arm64: Add hypervisor safe helper for checking constant capabilities > > arm64: Support systems without FP/ASIMD > > > > arch/arm64/include/asm/cpucaps.h | 3 ++- > > arch/arm64/include/asm/cpufeature.h | 24 +++++++++++++++++------- > > arch/arm64/include/asm/neon.h | 3 ++- > > arch/arm64/kernel/cpufeature.c | 17 ++++++++++++++++- > > arch/arm64/kernel/fpsimd.c | 14 ++++++++++++++ > > arch/arm64/kernel/process.c | 2 +- > > arch/arm64/kvm/handle_exit.c | 11 +++++++++++ > > arch/arm64/kvm/hyp/hyp-entry.S | 9 ++++++++- > > arch/arm64/kvm/hyp/switch.c | 5 ++++- > > drivers/irqchip/irq-gic-v3.c | 13 +------------ > > 10 files changed, 76 insertions(+), 25 deletions(-) > > For the series: > > Reviewed-by: Marc Zyngier <marc.zyngier@arm.com> > > How do we plan on merging this? Catalin, are you willing to take it all? Happy to take it all through the arm64 tree. Thanks for the review. -- Catalin ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/2] arm64: Support systems without FP/ASIMD 2016-11-08 13:56 [PATCH v3 0/2] arm64: Support systems without FP/ASIMD Suzuki K Poulose ` (2 preceding siblings ...) 2016-11-11 13:41 ` [PATCH v3 0/2] " Marc Zyngier @ 2016-11-16 18:11 ` Catalin Marinas 3 siblings, 0 replies; 9+ messages in thread From: Catalin Marinas @ 2016-11-16 18:11 UTC (permalink / raw) To: Suzuki K Poulose Cc: linux-arm-kernel, mark.rutland, ard.biesheuvel, marc.zyngier, will.deacon, linux-kernel, kvmarm On Tue, Nov 08, 2016 at 01:56:19PM +0000, Suzuki K. Poulose wrote: > This series adds supports to the kernel and KVM hyp to handle > systems without FP/ASIMD properly. At the moment the kernel > doesn't check if the FP unit is available before accessing > the registers (e.g during context switch). Also for KVM, > we trap the FP/ASIMD accesses and handle it by injecting an > undefined instruction into the VM on systems without FP. > > Tested on a FVP_Base-AEM-v8A model by disabling VFP on at > least one CPU ( -C clusterX.cpuY.vfp-present=0 ). > > Changes since V2: > - Dropped cleanup patch for arm64/crypto/aes-ce-ccm-glue.c > - Removed static_key check from cpus_have_cap. All users with > constant caps should use the new API to make use of static_keys. > - Removed a dedicated static_key used in irqchip-gic-v3.c for > Cavium errata with the new API. > > Applies on v4.9-rc4 + [1] (which is pushed for rc5) > > [1] http://marc.info/?l=linux-arm-kernel&m=147819889813214&w=2 I queued the patches for 4.10 with a slight modification as I haven't cherry-picked the patch above. I'll push the for-next/core branch out once I've done some testing. Thanks. -- Catalin ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-11-16 18:11 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-11-08 13:56 [PATCH v3 0/2] arm64: Support systems without FP/ASIMD Suzuki K Poulose 2016-11-08 13:56 ` [PATCH v3 1/2] arm64: Add hypervisor safe helper for checking constant capabilities Suzuki K Poulose 2016-11-08 18:11 ` Will Deacon 2016-11-08 13:56 ` [PATCH v3 2/2] arm64: Support systems without FP/ASIMD Suzuki K Poulose 2016-11-14 11:48 ` Catalin Marinas 2016-11-15 10:42 ` Suzuki K Poulose 2016-11-11 13:41 ` [PATCH v3 0/2] " Marc Zyngier 2016-11-14 11:08 ` Catalin Marinas 2016-11-16 18:11 ` Catalin Marinas
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).