[+ Christoffer]
On 10/10/17 19:38, Dave Martin wrote:
> Until KVM has full SVE support, guests must not be allowed to
> execute SVE instructions.
>
> This patch enables the necessary traps, and also ensures that the
> traps are disabled again on exit from the guest so that the host
> can still use SVE if it wants to.
>
> This patch introduces another instance of
> __this_cpu_write(fpsimd_last_state, NULL), so this flush operation
> is abstracted out as a separate helper fpsimd_flush_cpu_state().
> Other instances are ported appropriately.
>
> As a side effect of this refactoring, a this_cpu_write() in
> fpsimd_cpu_pm_notifier() is changed to __this_cpu_write(). This
> should be fine, since cpu_pm_enter() is supposed to be called only
> with interrupts disabled.
>
> Signed-off-by: Dave Martin <Dave.Martin@xxxxxxx>
> Reviewed-by: Alex Bennée <alex.bennee@xxxxxxxxxx>
> Cc: Marc Zyngier <marc.zyngier@xxxxxxx>
> Cc: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
> ---
> arch/arm/include/asm/kvm_host.h | 3 +++
> arch/arm64/include/asm/fpsimd.h | 1 +
> arch/arm64/include/asm/kvm_arm.h | 4 +++-
> arch/arm64/include/asm/kvm_host.h | 11 +++++++++++
> arch/arm64/kernel/fpsimd.c | 31 +++++++++++++++++++++++++++++--
> arch/arm64/kvm/hyp/switch.c | 6 +++---
> virt/kvm/arm/arm.c | 3 +++
> 7 files changed, 53 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 4a879f6..242151e 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -293,4 +293,7 @@ int kvm_arm_vcpu_arch_get_attr(struct kvm_vcpu *vcpu,
> int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu,
> struct kvm_device_attr *attr);
>
> +/* All host FP/SIMD state is restored on guest exit, so nothing to save: */
> +static inline void kvm_fpsimd_flush_cpu_state(void) {}
> +
> #endif /* __ARM_KVM_HOST_H__ */
> diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
> index 3cfdfbe..10b2824 100644
> --- a/arch/arm64/include/asm/fpsimd.h
> +++ b/arch/arm64/include/asm/fpsimd.h
> @@ -75,6 +75,7 @@ extern void fpsimd_restore_current_state(void);
> extern void fpsimd_update_current_state(struct fpsimd_state *state);
>
> extern void fpsimd_flush_task_state(struct task_struct *target);
> +extern void sve_flush_cpu_state(void);
>
> /* Maximum VL that SVE VL-agnostic software can transparently support */
> #define SVE_VL_ARCH_MAX 0x100
> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> index dbf0537..7f069ff 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -186,7 +186,8 @@
> #define CPTR_EL2_TTA (1 << 20)
> #define CPTR_EL2_TFP (1 << CPTR_EL2_TFP_SHIFT)
> #define CPTR_EL2_TZ (1 << 8)
> -#define CPTR_EL2_DEFAULT 0x000033ff
> +#define CPTR_EL2_RES1 0x000032ff /* known RES1 bits in CPTR_EL2 */
> +#define CPTR_EL2_DEFAULT CPTR_EL2_RES1
>
> /* Hyp Debug Configuration Register bits */
> #define MDCR_EL2_TPMS (1 << 14)
> @@ -237,5 +238,6 @@
>
> #define CPACR_EL1_FPEN (3 << 20)
> #define CPACR_EL1_TTA (1 << 28)
> +#define CPACR_EL1_DEFAULT (CPACR_EL1_FPEN | CPACR_EL1_ZEN_EL1EN)
>
> #endif /* __ARM64_KVM_ARM_H__ */
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index e923b58..674912d 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -25,6 +25,7 @@
> #include <linux/types.h>
> #include <linux/kvm_types.h>
> #include <asm/cpufeature.h>
> +#include <asm/fpsimd.h>
> #include <asm/kvm.h>
> #include <asm/kvm_asm.h>
> #include <asm/kvm_mmio.h>
> @@ -384,4 +385,14 @@ static inline void __cpu_init_stage2(void)
> "PARange is %d bits, unsupported configuration!", parange);
> }
>
> +/*
> + * All host FP/SIMD state is restored on guest exit, so nothing needs
> + * doing here except in the SVE case:
> +*/
> +static inline void kvm_fpsimd_flush_cpu_state(void)
> +{
> + if (system_supports_sve())
> + sve_flush_cpu_state();
Hmmm. How does this work if...
> +}
> +
> #endif /* __ARM64_KVM_HOST_H__ */
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index a9cb794..6ae3703 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -1073,6 +1073,33 @@ void fpsimd_flush_task_state(struct task_struct *t)
> t->thread.fpsimd_state.cpu = NR_CPUS;
> }
>
> +static inline void fpsimd_flush_cpu_state(void)
> +{
> + __this_cpu_write(fpsimd_last_state, NULL);
> +}
> +
> +/*
> + * Invalidate any task SVE state currently held in this CPU's regs.
> + *
> + * This is used to prevent the kernel from trying to reuse SVE register data
> + * that is detroyed by KVM guest enter/exit. This function should go away when
> + * KVM SVE support is implemented. Don't use it for anything else.
> + */
> +#ifdef CONFIG_ARM64_SVE
> +void sve_flush_cpu_state(void)
> +{
> + struct fpsimd_state *const fpstate = __this_cpu_read(fpsimd_last_state);
> + struct task_struct *tsk;
> +
> + if (!fpstate)
> + return;
> +
> + tsk = container_of(fpstate, struct task_struct, thread.fpsimd_state);
> + if (test_tsk_thread_flag(tsk, TIF_SVE))
> + fpsimd_flush_cpu_state();
> +}
> +#endif /* CONFIG_ARM64_SVE */
... CONFIG_ARM64_SVE is not set? Fixing this should just be a matter of
moving the #ifdef/#endif inside the function...
> +
> #ifdef CONFIG_KERNEL_MODE_NEON
>
> DEFINE_PER_CPU(bool, kernel_neon_busy);
> @@ -1113,7 +1140,7 @@ void kernel_neon_begin(void)
> }
>
> /* Invalidate any task state remaining in the fpsimd regs: */
> - __this_cpu_write(fpsimd_last_state, NULL);
> + fpsimd_flush_cpu_state();
>
> preempt_disable();
>
> @@ -1234,7 +1261,7 @@ static int fpsimd_cpu_pm_notifier(struct notifier_block *self,
> case CPU_PM_ENTER:
> if (current->mm)
> task_fpsimd_save();
> - this_cpu_write(fpsimd_last_state, NULL);
> + fpsimd_flush_cpu_state();
> break;
> case CPU_PM_EXIT:
> if (current->mm)
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 35a90b8..951f3eb 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -48,7 +48,7 @@ static void __hyp_text __activate_traps_vhe(void)
>
> val = read_sysreg(cpacr_el1);
> val |= CPACR_EL1_TTA;
> - val &= ~CPACR_EL1_FPEN;
> + val &= ~(CPACR_EL1_FPEN | CPACR_EL1_ZEN);
> write_sysreg(val, cpacr_el1);
>
> write_sysreg(__kvm_hyp_vector, vbar_el1);
> @@ -59,7 +59,7 @@ static void __hyp_text __activate_traps_nvhe(void)
> u64 val;
>
> val = CPTR_EL2_DEFAULT;
> - val |= CPTR_EL2_TTA | CPTR_EL2_TFP;
> + val |= CPTR_EL2_TTA | CPTR_EL2_TFP | CPTR_EL2_TZ;
> write_sysreg(val, cptr_el2);
> }
>
> @@ -117,7 +117,7 @@ static void __hyp_text __deactivate_traps_vhe(void)
>
> write_sysreg(mdcr_el2, mdcr_el2);
> write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
> - write_sysreg(CPACR_EL1_FPEN, cpacr_el1);
> + write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);
> write_sysreg(vectors, vbar_el1);
> }
>
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index b9f68e4..4d3cf9c 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -652,6 +652,9 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
> */
> preempt_disable();
>
> + /* Flush FP/SIMD state that can't survive guest entry/exit */
> + kvm_fpsimd_flush_cpu_state();
> +
> kvm_pmu_flush_hwstate(vcpu);
>
> kvm_timer_flush_hwstate(vcpu);
>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
