[{"id":1768510,"web_url":"http://patchwork.ozlabs.org/comment/1768510/","msgid":"<87d16ta6ih.fsf@linaro.org>","list_archive_url":null,"date":"2017-09-14T11:01:26","subject":"Re: [PATCH v2 18/28] arm64/sve: Preserve SVE registers around EFI\n\truntime service calls","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> The EFI runtime services ABI allows EFI to make free use of the\n> FPSIMD registers during EFI runtime service calls, subject to the\n> callee-save requirements of the AArch64 procedure call standard.\n>\n> However, the SVE architecture allows upper bits of the SVE vector\n> registers to be zeroed as a side-effect of FPSIMD V-register\n> writes. This means that the SVE vector registers must be saved in\n> their entirety in order to avoid data loss: non-SVE-aware EFI\n> implementations cannot restore them correctly.\n>\n> The non-IRQ case is already handled gracefully by\n> kernel_neon_begin(). For the IRQ case, this patch allocates a\n> suitable per-CPU stash buffer for the full SVE register state and\n> uses it to preserve the affected registers around EFI calls. It is\n> currently unclear how the EFI runtime services ABI will be\n> clarified with respect to SVE, so it safest to assume that the\n> predicate registers and FFR must be saved and restored too.\n>\n> No attempt is made to restore the restore the vector length after\n> a call, for now. It is deemed rather insane for EFI to change it,\n> and contemporary EFI implementations certainly won't.\n>\n> Signed-off-by: Dave Martin \n> Cc: Ard Biesheuvel \n\nReviewed-by: Alex Bennée \n\n>\n> ---\n>\n> Changes since v1\n> ----------------\n>\n> Requested by Ard Biesheuvel:\n>\n> * Fix unbalanced ifelse bracing to conform to the kernel coding style.\n>\n> * Make efi_sve_state_used static.\n>\n> Changes related to Alex Bennée's comments:\n>\n> * Migrate away from magic numbers for SVE_VQ_BYTES.\n>\n> Other:\n>\n> * Rename sve_kernel_mode_neon_setup() to sve_efi_setup().\n> The EFI FPSIMD code is something semi-independent from kernel-mode NEON\n> now, so the \"neon\" in the names no longer really makes sense.\n>\n> * Make the EFI FPSIMD setup code dependent on CONFIG_EFI (it's not\n> supposed to work with CONFIG_EFI=n anyway).\n> ---\n> arch/arm64/kernel/fpsimd.c | 60 +++++++++++++++++++++++++++++++++++++++++-----\n> 1 file changed, 54 insertions(+), 6 deletions(-)\n>\n> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c\n> index dd89acf..fff9fcf 100644\n> --- a/arch/arm64/kernel/fpsimd.c\n> +++ b/arch/arm64/kernel/fpsimd.c\n> @@ -118,11 +118,13 @@ static int sve_default_vl = -1;\n> int __ro_after_init sve_max_vl = -1;\n> /* Set of available vector lengths, as vq_to_bit(vq): */\n> static __ro_after_init DECLARE_BITMAP(sve_vq_map, SVE_VQ_MAX);\n> +static void __percpu *efi_sve_state;\n>\n> #else /* ! CONFIG_ARM64_SVE */\n>\n> /* Dummy declaration for code that will be optimised out: */\n> extern __ro_after_init DECLARE_BITMAP(sve_vq_map, SVE_VQ_MAX);\n> +extern void __percpu *efi_sve_state;\n>\n> #endif /* ! CONFIG_ARM64_SVE */\n>\n> @@ -447,6 +449,23 @@ int sve_verify_vq_map(void)\n> \treturn ret;\n> }\n>\n> +static void __init sve_efi_setup(void)\n> +{\n> +\tif (!IS_ENABLED(CONFIG_EFI))\n> +\t\treturn;\n> +\n> +\t/*\n> +\t * alloc_percpu() warns and prints a backtrace if this goes wrong.\n> +\t * This is evidence of a crippled system and we are returning void,\n> +\t * so no attempt is made to handle this situation here.\n> +\t */\n> +\tBUG_ON(!sve_vl_valid(sve_max_vl));\n> +\tefi_sve_state = __alloc_percpu(\n> +\t\tSVE_SIG_REGS_SIZE(sve_vq_from_vl(sve_max_vl)), SVE_VQ_BYTES);\n> +\tif (!efi_sve_state)\n> +\t\tpanic(\"Cannot allocate percpu memory for EFI SVE save/restore\");\n> +}\n> +\n> void __init sve_setup(void)\n> {\n> \tu64 zcr;\n> @@ -482,6 +501,8 @@ void __init sve_setup(void)\n> \t\tsve_max_vl);\n> \tpr_info(\"SVE: default vector length %u bytes per vector\\n\",\n> \t\tsve_default_vl);\n> +\n> +\tsve_efi_setup();\n> }\n>\n> void fpsimd_release_thread(struct task_struct *dead_task)\n> @@ -783,6 +804,7 @@ EXPORT_SYMBOL(kernel_neon_end);\n>\n> static DEFINE_PER_CPU(struct fpsimd_state, efi_fpsimd_state);\n> static DEFINE_PER_CPU(bool, efi_fpsimd_state_used);\n> +static DEFINE_PER_CPU(bool, efi_sve_state_used);\n>\n> /*\n> * EFI runtime services support functions\n> @@ -808,10 +830,24 @@ void __efi_fpsimd_begin(void)\n>\n> \tWARN_ON(preemptible());\n>\n> -\tif (may_use_simd())\n> +\tif (may_use_simd()) {\n> \t\tkernel_neon_begin();\n> -\telse {\n> -\t\tfpsimd_save_state(this_cpu_ptr(&efi_fpsimd_state));\n> +\t} else {\n> +\t\t/*\n> +\t\t * If !efi_sve_state, SVE can't be in use yet and doesn't need\n> +\t\t * preserving:\n> +\t\t */\n> +\t\tif (system_supports_sve() && likely(efi_sve_state)) {\n> +\t\t\tchar *sve_state = this_cpu_ptr(efi_sve_state);\n> +\n> +\t\t\t__this_cpu_write(efi_sve_state_used, true);\n> +\n> +\t\t\tsve_save_state(sve_state + sve_ffr_offset(sve_max_vl),\n> +\t\t\t\t &this_cpu_ptr(&efi_fpsimd_state)->fpsr);\n> +\t\t} else {\n> +\t\t\tfpsimd_save_state(this_cpu_ptr(&efi_fpsimd_state));\n> +\t\t}\n> +\n> \t\t__this_cpu_write(efi_fpsimd_state_used, true);\n> \t}\n> }\n> @@ -824,10 +860,22 @@ void __efi_fpsimd_end(void)\n> \tif (!system_supports_fpsimd())\n> \t\treturn;\n>\n> -\tif (__this_cpu_xchg(efi_fpsimd_state_used, false))\n> -\t\tfpsimd_load_state(this_cpu_ptr(&efi_fpsimd_state));\n> -\telse\n> +\tif (!__this_cpu_xchg(efi_fpsimd_state_used, false)) {\n> \t\tkernel_neon_end();\n> +\t} else {\n> +\t\tif (system_supports_sve() &&\n> +\t\t likely(__this_cpu_read(efi_sve_state_used))) {\n> +\t\t\tchar const *sve_state = this_cpu_ptr(efi_sve_state);\n> +\n> +\t\t\tsve_load_state(sve_state + sve_ffr_offset(sve_max_vl),\n> +\t\t\t\t &this_cpu_ptr(&efi_fpsimd_state)->fpsr,\n> +\t\t\t\t sve_vq_from_vl(sve_get_vl()) - 1);\n> +\n> +\t\t\t__this_cpu_write(efi_sve_state_used, false);\n> +\t\t} else {\n> +\t\t\tfpsimd_load_state(this_cpu_ptr(&efi_fpsimd_state));\n> +\t\t}\n> +\t}\n> }\n>\n> #endif /* CONFIG_KERNEL_MODE_NEON */\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-84605-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=\"B0Fw3vo3\"; 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 3xtFw76mhLz9s06\n\tfor ;\n\tThu, 14 Sep 2017 21:01:43 +1000 (AEST)","(qmail 76193 invoked by alias); 14 Sep 2017 11:01:37 -0000","(qmail 75929 invoked by uid 89); 14 Sep 2017 11:01:36 -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=xIuFdi7Ppt1AxTrP+DvhwG4fU8VAi717P40pzH2EyL0\n\tPGmyUBgnKDcwC3LBku3F6v9j+xEd/Fc86kR0OuGlEqgBnJKd3qtY65lz2QpczU0t\n\toGUM9gMzIf2Bkw9pSs1zzfPUJCm/P3TYr/cZSASe7zrfIJNpi5yzHDzhXoaofFxY\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=bwJpeJYeHDhi3fYVuITihjqw70w=; b=B0Fw3vo3bCXbJylam\n\tTr6xJFTGjvNYWQRjblzI593JEh2y3yK5pgC1etWsRYdzsoLIMngYUx3TNWYeSt3k\n\tquCzj3WU/K6Wo0GQlPhdBBxMvqkJxZBhZnBUdxJqhJMPWerrkGreHSeqYE8FtuN5\n\tjSF30HXbVS9lgDBlGmbU9CNvlo=","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.4 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,\n\tSPF_PASS autolearn=ham version=3.3.2 spammy=contemporary,\n\tsafest","X-HELO":"mail-wr0-f172.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=VaS6vfIzC5WVqhvWm1bFaD+oqwtBnI03uGz0bFD3RoI=;\n\tb=B/bVcBvL+D2XV3meJchAFFZvivfGE+SjmvQZsjFRLxYPL1ZbH4s/GUa9fSIN4VpiuC\n\tYQaCFFONqAqQjXWWjh5oZXh6UfPtaV3LacuD2JQufkJw9h42nlxnMk5M+LBtcNd5vHxh\n\taOErFC9grpn3f9mvShE6ri7UUMpKFvy4chsVbR5vGmGamC8jrzIFhVH24bq15jpHGZEH\n\trBe5tOZUnx7K5oL6liDjnGtUsXlpHylW/C+BpBuGihYM6qDE3tt+0PixpVkJQXuyIisj\n\t5TdTALM8FrcGYcTsFW8mhI0vUpFmJK/x1LqzKC7vexW4plU0JFGXR/jjAt4EZ/lhg+Ad\n\tHfkg==","X-Gm-Message-State":"AHPjjUi1cyzPJbgrgWl8sfSHXr7V5u91vwXldyYiy7W16qFnbLs03nDq\n\tEopNGoNyi1QKRn48","X-Google-Smtp-Source":"AOwi7QDO3D7nGpE64UrOd45PqOqLSGFYzKgJDokWyUjUC2OYwZbgyDur/88gTovJR42frh4xbrWjzg==","X-Received":"by 10.223.186.142 with SMTP id p14mr9322216wrg.169.1505386887758;\n\tThu, 14 Sep 2017 04:01:27 -0700 (PDT)","References":"<1504198860-12951-1-git-send-email-Dave.Martin@arm.com>\n\t<1504198860-12951-19-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","Subject":"Re: [PATCH v2 18/28] arm64/sve: Preserve SVE registers around EFI\n\truntime service calls","In-reply-to":"<1504198860-12951-19-git-send-email-Dave.Martin@arm.com>","Date":"Thu, 14 Sep 2017 12:01:26 +0100","Message-ID":"<87d16ta6ih.fsf@linaro.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"8bit"}}]