[{"id":1768584,"web_url":"http://patchwork.ozlabs.org/comment/1768584/","msgid":"<877ex19zpf.fsf@linaro.org>","list_archive_url":null,"date":"2017-09-14T13:28:28","subject":"Re: [PATCH v2 22/28] arm64/sve: KVM: Prevent guests from using SVE","submitter":{"id":39532,"url":"http://patchwork.ozlabs.org/api/people/39532/","name":"Alex Bennée","email":"alex.bennee@linaro.org"},"content":"Dave Martin writes:\n\n> Until KVM has full SVE support, guests must not be allowed to\n> execute SVE instructions.\n>\n> This patch enables the necessary traps, and also ensures that the\n> traps are disabled again on exit from the guest so that the host\n> can still use SVE if it wants to.\n>\n> This patch introduces another instance of\n> __this_cpu_write(fpsimd_last_state, NULL), so this flush operation\n> is abstracted out as a separate helper fpsimd_flush_cpu_state().\n> Other instances are ported appropriately.\n>\n> As a side effect of this refactoring, a this_cpu_write() in\n> fpsimd_cpu_pm_notifier() is changed to __this_cpu_write(). This\n> should be fine, since cpu_pm_enter() is supposed to be called only\n> with interrupts disabled.\n>\n> Signed-off-by: Dave Martin \n> Cc: Marc Zyngier \n> Cc: Ard Biesheuvel \n\nReviewed-by: Alex Bennée \n\n>\n> ---\n>\n> Changes since v1\n> ----------------\n>\n> Requested by Marc Zyngier:\n>\n> * Avoid the verbose arithmetic for CPTR_EL2_DEFAULT, and just\n> describe it in terms of the set of bits known to be RES1 in\n> CPTR_EL2.\n>\n> Other:\n>\n> * Fixup to drop task SVE state cached in the CPU registers across\n> guest entry/exit.\n>\n> Without this, we may enter an EL0 process with wrong data in the\n> extended SVE bits and/or wrong trap configuration.\n>\n> This is not a problem for the FPSIMD part of the state because KVM\n> explicitly restores the host FPSIMD state on guest exit; but this\n> restore is sufficient to corrupt the extra SVE bits even if nothing\n> else does.\n>\n> * The fpsimd_flush_cpu_state() function, which was supposed to abstract\n> the underlying flush operation, wasn't used. [sparse]\n>\n> This patch is now ported to use it. Other users of the same idiom are\n> ported too (which was the original intention).\n>\n> fpsimd_flush_cpu_state() is marked inline, since all users are\n> ifdef'd and the function may be unused. Plus, it's trivially\n> suitable for inlining.\n> ---\n> arch/arm/include/asm/kvm_host.h | 3 +++\n> arch/arm64/include/asm/fpsimd.h | 1 +\n> arch/arm64/include/asm/kvm_arm.h | 4 +++-\n> arch/arm64/include/asm/kvm_host.h | 11 +++++++++++\n> arch/arm64/kernel/fpsimd.c | 31 +++++++++++++++++++++++++++++--\n> arch/arm64/kvm/hyp/switch.c | 6 +++---\n> virt/kvm/arm/arm.c | 3 +++\n> 7 files changed, 53 insertions(+), 6 deletions(-)\n>\n> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h\n> index 127e2dd..fa4a442 100644\n> --- a/arch/arm/include/asm/kvm_host.h\n> +++ b/arch/arm/include/asm/kvm_host.h\n> @@ -299,4 +299,7 @@ int kvm_arm_vcpu_arch_get_attr(struct kvm_vcpu *vcpu,\n> int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu,\n> \t\t\t struct kvm_device_attr *attr);\n>\n> +/* All host FP/SIMD state is restored on guest exit, so nothing to save: */\n> +static inline void kvm_fpsimd_flush_cpu_state(void) {}\n> +\n> #endif /* __ARM_KVM_HOST_H__ */\n> diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h\n> index d084968..5605fc1 100644\n> --- a/arch/arm64/include/asm/fpsimd.h\n> +++ b/arch/arm64/include/asm/fpsimd.h\n> @@ -74,6 +74,7 @@ extern void fpsimd_restore_current_state(void);\n> extern void fpsimd_update_current_state(struct fpsimd_state *state);\n>\n> extern void fpsimd_flush_task_state(struct task_struct *target);\n> +extern void sve_flush_cpu_state(void);\n>\n> /* Maximum VL that SVE VL-agnostic software can transparently support */\n> #define SVE_VL_ARCH_MAX 0x100\n> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h\n> index dbf0537..7f069ff 100644\n> --- a/arch/arm64/include/asm/kvm_arm.h\n> +++ b/arch/arm64/include/asm/kvm_arm.h\n> @@ -186,7 +186,8 @@\n> #define CPTR_EL2_TTA\t(1 << 20)\n> #define CPTR_EL2_TFP\t(1 << CPTR_EL2_TFP_SHIFT)\n> #define CPTR_EL2_TZ\t(1 << 8)\n> -#define CPTR_EL2_DEFAULT\t0x000033ff\n> +#define CPTR_EL2_RES1\t0x000032ff /* known RES1 bits in CPTR_EL2 */\n> +#define CPTR_EL2_DEFAULT\tCPTR_EL2_RES1\n>\n> /* Hyp Debug Configuration Register bits */\n> #define MDCR_EL2_TPMS\t\t(1 << 14)\n> @@ -237,5 +238,6 @@\n>\n> #define CPACR_EL1_FPEN\t\t(3 << 20)\n> #define CPACR_EL1_TTA\t\t(1 << 28)\n> +#define CPACR_EL1_DEFAULT\t(CPACR_EL1_FPEN | CPACR_EL1_ZEN_EL1EN)\n>\n> #endif /* __ARM64_KVM_ARM_H__ */\n> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h\n> index d686300..05d8373 100644\n> --- a/arch/arm64/include/asm/kvm_host.h\n> +++ b/arch/arm64/include/asm/kvm_host.h\n> @@ -25,6 +25,7 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -390,4 +391,14 @@ static inline void __cpu_init_stage2(void)\n> \t\t \"PARange is %d bits, unsupported configuration!\", parange);\n> }\n>\n> +/*\n> + * All host FP/SIMD state is restored on guest exit, so nothing needs\n> + * doing here except in the SVE case:\n> +*/\n> +static inline void kvm_fpsimd_flush_cpu_state(void)\n> +{\n> +\tif (system_supports_sve())\n> +\t\tsve_flush_cpu_state();\n> +}\n> +\n> #endif /* __ARM64_KVM_HOST_H__ */\n> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c\n> index b430ee0..7837ced 100644\n> --- a/arch/arm64/kernel/fpsimd.c\n> +++ b/arch/arm64/kernel/fpsimd.c\n> @@ -875,6 +875,33 @@ void fpsimd_flush_task_state(struct task_struct *t)\n> \tt->thread.fpsimd_state.cpu = NR_CPUS;\n> }\n>\n> +static inline void fpsimd_flush_cpu_state(void)\n> +{\n> +\t__this_cpu_write(fpsimd_last_state, NULL);\n> +}\n> +\n> +/*\n> + * Invalidate any task SVE state currently held in this CPU's regs.\n> + *\n> + * This is used to prevent the kernel from trying to reuse SVE register data\n> + * that is detroyed by KVM guest enter/exit. This function should go away when\n> + * KVM SVE support is implemented. Don't use it for anything else.\n> + */\n> +#ifdef CONFIG_ARM64_SVE\n> +void sve_flush_cpu_state(void)\n> +{\n> +\tstruct fpsimd_state *const fpstate = __this_cpu_read(fpsimd_last_state);\n> +\tstruct task_struct *tsk;\n> +\n> +\tif (!fpstate)\n> +\t\treturn;\n> +\n> +\ttsk = container_of(fpstate, struct task_struct, thread.fpsimd_state);\n> +\tif (test_tsk_thread_flag(tsk, TIF_SVE))\n> +\t\tfpsimd_flush_cpu_state();\n> +}\n> +#endif /* CONFIG_ARM64_SVE */\n> +\n> #ifdef CONFIG_KERNEL_MODE_NEON\n>\n> DEFINE_PER_CPU(bool, kernel_neon_busy);\n> @@ -915,7 +942,7 @@ void kernel_neon_begin(void)\n> \t}\n>\n> \t/* Invalidate any task state remaining in the fpsimd regs: */\n> -\t__this_cpu_write(fpsimd_last_state, NULL);\n> +\tfpsimd_flush_cpu_state();\n>\n> \tpreempt_disable();\n>\n> @@ -1032,7 +1059,7 @@ static int fpsimd_cpu_pm_notifier(struct notifier_block *self,\n> \tcase CPU_PM_ENTER:\n> \t\tif (current->mm)\n> \t\t\ttask_fpsimd_save();\n> -\t\tthis_cpu_write(fpsimd_last_state, NULL);\n> +\t\tfpsimd_flush_cpu_state();\n> \t\tbreak;\n> \tcase CPU_PM_EXIT:\n> \t\tif (current->mm)\n> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c\n> index 35a90b8..951f3eb 100644\n> --- a/arch/arm64/kvm/hyp/switch.c\n> +++ b/arch/arm64/kvm/hyp/switch.c\n> @@ -48,7 +48,7 @@ static void __hyp_text __activate_traps_vhe(void)\n>\n> \tval = read_sysreg(cpacr_el1);\n> \tval |= CPACR_EL1_TTA;\n> -\tval &= ~CPACR_EL1_FPEN;\n> +\tval &= ~(CPACR_EL1_FPEN | CPACR_EL1_ZEN);\n> \twrite_sysreg(val, cpacr_el1);\n>\n> \twrite_sysreg(__kvm_hyp_vector, vbar_el1);\n> @@ -59,7 +59,7 @@ static void __hyp_text __activate_traps_nvhe(void)\n> \tu64 val;\n>\n> \tval = CPTR_EL2_DEFAULT;\n> -\tval |= CPTR_EL2_TTA | CPTR_EL2_TFP;\n> +\tval |= CPTR_EL2_TTA | CPTR_EL2_TFP | CPTR_EL2_TZ;\n> \twrite_sysreg(val, cptr_el2);\n> }\n>\n> @@ -117,7 +117,7 @@ static void __hyp_text __deactivate_traps_vhe(void)\n>\n> \twrite_sysreg(mdcr_el2, mdcr_el2);\n> \twrite_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);\n> -\twrite_sysreg(CPACR_EL1_FPEN, cpacr_el1);\n> +\twrite_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);\n> \twrite_sysreg(vectors, vbar_el1);\n> }\n>\n> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c\n> index a39a1e1..af9f5da 100644\n> --- a/virt/kvm/arm/arm.c\n> +++ b/virt/kvm/arm/arm.c\n> @@ -647,6 +647,9 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)\n> \t\t */\n> \t\tpreempt_disable();\n>\n> +\t\t/* Flush FP/SIMD state that can't survive guest entry/exit */\n> +\t\tkvm_fpsimd_flush_cpu_state();\n> +\n> \t\tkvm_pmu_flush_hwstate(vcpu);\n>\n> \t\tkvm_timer_flush_hwstate(vcpu);\n\n\n--\nAlex Bennée","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","mailing list libc-alpha@sourceware.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=sourceware.org\n\t(client-ip=209.132.180.131; helo=sourceware.org;\n\tenvelope-from=libc-alpha-return-84614-incoming=patchwork.ozlabs.org@sourceware.org;\n\treceiver=)","ozlabs.org; dkim=pass (1024-bit key;\n\tsecure) header.d=sourceware.org header.i=@sourceware.org\n\theader.b=\"cAp1mbes\"; dkim-atps=neutral","sourceware.org; auth=none"],"Received":["from sourceware.org (server1.sourceware.org [209.132.180.131])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xtK9l3JVsz9s81\n\tfor ;\n\tThu, 14 Sep 2017 23:28:43 +1000 (AEST)","(qmail 88986 invoked by alias); 14 Sep 2017 13:28:35 -0000","(qmail 88964 invoked by uid 89); 14 Sep 2017 13:28:34 -0000"],"DomainKey-Signature":"a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id\n\t:list-unsubscribe:list-subscribe:list-archive:list-post\n\t:list-help:sender:references:from:to:cc:subject:in-reply-to:date\n\t:message-id:mime-version:content-type:content-transfer-encoding;\n\tq=dns; s=default; b=a1yrU09QoOH316SpdKCrhUe/3UmhHjU8imGMA+NFPfG\n\tBVwwE30q9du51y55XQrsvzzpsBXwHIXurLLnvv08zOhyqPr/XTK02B54JM5TI987\n\tO58+01W405bZfsO5yIGFhvcMYcVE8mjWS8dhm/u3GTRn+JL2CMFhOhdCd8vHqVNg\n\t=","DKIM-Signature":"v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id\n\t:list-unsubscribe:list-subscribe:list-archive:list-post\n\t:list-help:sender:references:from:to:cc:subject:in-reply-to:date\n\t:message-id:mime-version:content-type:content-transfer-encoding;\n\ts=default; bh=nSTd2Im5x0MJdLF5EaPzi/L8mcc=; b=cAp1mbesrevnG+mNb\n\t/2bEoy4UTzAfN9n2WMIHcWGPipCImdy0FaKMuLNqmqkc4XBXOCx+9Du5XnvpZ+Y5\n\t8/S9TG57Bf/3stXhmhNHY/vhvsd24KdSzPFIYKO9gDlZSWKKUls2HVF3h0kdnV9a\n\tXufdaz8pqQgNvfgvQLFjglba60=","Mailing-List":"contact libc-alpha-help@sourceware.org; run by ezmlm","Precedence":"bulk","List-Id":"","List-Unsubscribe":"","List-Subscribe":"","List-Archive":"","List-Post":"","List-Help":",\n\t","Sender":"libc-alpha-owner@sourceware.org","X-Virus-Found":"No","X-Spam-SWARE-Status":"No, score=-26.0 required=5.0 tests=AWL, BAYES_00,\n\tGIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3,\n\tRCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM,\n\tSPF_PASS autolearn=ham version=3.3.2 spammy=","X-HELO":"mail-wm0-f48.google.com","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:references:user-agent:from:to:cc:subject\n\t:in-reply-to:date:message-id:mime-version:content-transfer-encoding; \n\tbh=XNeQXaXDmartfDQfzyikWeI1mGfVtWZItHgIEJDgkuY=;\n\tb=NG7oxLuBZ7+MNP7FEqZ73rt4cpIwF2KE4/dLbwkFIkS3YCkDpm/bKokWUKuRT2e2jB\n\tUaVlcS41xKHBL2yCRtH8g84x3ywN4OI0UhMXmxm/DB2Eb/0xxzJxX5nL6hxhiCqZuOsq\n\tN7oMmjpqTFyq8wd6DnRurfBmoBebAu+ETUZY2gn5odeRGAFgW3MpsMiEm+zzrVV6OLTJ\n\t7Gf4dCoif7EhoeVgwAdwRNyxjfNSz4Ff1ybJG0JjUT1N2I/cC58GPQ2yGPAdov9b3zfx\n\tyDowDl6Aj0hPZLdV+ZFvE0Qc4alN5ZD5ZZ/7H4QGZ/6ur1TOWE2/y/wZYNR8Uz6icsFg\n\t55Fw==","X-Gm-Message-State":"AHPjjUjolIt2beVkBJMJ3+H2u+s36VxeQFt8J8OCdurIUigHqXkgvDDB\n\t61NdDi0jDmyaLXm2c/evbA==","X-Google-Smtp-Source":"AOwi7QCKh+FNPc9UAAiEvWx1s1JN0QyIL6MT0sQ+fhYzIl5YNEqU1elHyFjNWMiYVJbt81NpVqPVFA==","X-Received":"by 10.28.152.23 with SMTP id a23mr20617wme.45.1505395709516;\n\tThu, 14 Sep 2017 06:28:29 -0700 (PDT)","References":"<1504198860-12951-1-git-send-email-Dave.Martin@arm.com>\n\t<1504198860-12951-23-git-send-email-Dave.Martin@arm.com>","User-agent":"mu4e 0.9.19; emacs 25.2.50.3","From":"Alex =?utf-8?q?Benn=C3=A9e?= ","To":"Dave Martin ","Cc":"linux-arm-kernel@lists.infradead.org,\n\tCatalin Marinas ,\n\tWill Deacon ,\n\tArd Biesheuvel ,\n\tSzabolcs Nagy ,\n\tRichard Sandiford ,\n\tkvmarm@lists.cs.columbia.edu, libc-alpha@sourceware.org,\n\tlinux-arch@vger.kernel.org,\n\tChristoffer Dall ,\n\tMarc Zyngier ","Subject":"Re: [PATCH v2 22/28] arm64/sve: KVM: Prevent guests from using SVE","In-reply-to":"<1504198860-12951-23-git-send-email-Dave.Martin@arm.com>","Date":"Thu, 14 Sep 2017 14:28:28 +0100","Message-ID":"<877ex19zpf.fsf@linaro.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"8bit"}}]