From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id DEDF7C5AD4C for ; Thu, 23 Nov 2023 17:45:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:From:Subject:Message-ID: References:Mime-Version:In-Reply-To:Date:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=MTvSzytvfU43V+Pd5qMaC6u1BBWKo0D4AJy4NQtPZTU=; b=dw3kQQsLDGwdA/9leXTqFOg2C2 fmFGT9YesJv64EoVULqNj4tCqNC6BW8+4yaahP0TQEKzcq3wLckCa8pvP+LCRqmEk/nmUi8AkKeDk rfMKVYCf1e0GqXqrvD4/mXwrRPsNYIkmC4DpCEpQHjER5Q96OaeB74Q9dSZZCRdn2zzAs9LNSzsrM OKOyi7cShPHOol3DhaTsA6c4CGTsmQwUBIBhEzTsoJBDiaNEE0KB89hd4SD5Jyil/xXWEv/sdZO1x w1iIDXVyWL3nXRh0rwOJQSCjfWx85QQQ7KPZUd6FnYn9kOSfPRyMoWt0PjVUhkyUFCKNOV0ZKQJJx f+/XQTSA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1r6Dl0-005SEV-26; Thu, 23 Nov 2023 17:44:54 +0000 Received: from mail-yw1-x1149.google.com ([2607:f8b0:4864:20::1149]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1r6Dks-005SB3-3B for linux-arm-kernel@lists.infradead.org; Thu, 23 Nov 2023 17:44:49 +0000 Received: by mail-yw1-x1149.google.com with SMTP id 00721157ae682-5cd573c2cccso4355007b3.1 for ; Thu, 23 Nov 2023 09:44:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1700761486; x=1701366286; darn=lists.infradead.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=7Hq9cGdSc7skJrdkne0VvJXnbJxMtxVGxSuH4EOuQUI=; b=1esSgPS5Sm+jBCosGQxuPI4fqdeh6TX5ajCDoQW38X96XT3ez93IB/eSx/owF6KNaG mTZ6+iZ9dDZEZW/uE/MmRUhu/4s1Ej12RChqbzyhJnxihBR44eKOJUfsCe7O+AvUwkrv oYhlwj4rk/NW5zHUpxKZCtA0ho5MJu8o0N8YbnK7CRrG0eWIZT6icv4CcRvmD7YXM6U1 8W5Yhq2h4esCNgYN2vdrdtJ/VnWEqeYDA8OkHgRlbDvB5uK+XLFS7npAy01pqx1ZVOtE 2Fr8/36xq5zMrdrxVvB6GfuRol0fItO8k3V6bZy3F/NHaVyDEUGOWbmgk512iyNZ6YaF pK8w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700761486; x=1701366286; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=7Hq9cGdSc7skJrdkne0VvJXnbJxMtxVGxSuH4EOuQUI=; b=gZqj//HihxKlbWC/RO0p9J/9qXN1sl4RHGkPcYAoZbvWL8dhApCfVpR+mcCI7ra9GJ 0EhYHoNpFNfm/n99d3G25Z3iNmu4sS3Ljz5mWN19V0iXCb3MTzo5UH5EF1ykxnwp/S5b Z22J+KzESq/JogG+p8cUulbU1lL38msHSu9EDLWbKyXIZPXsbAWi2R2UVmCkcPfli/vv CaOPR2EpS8TJsuo2BBulwKw/xGm+SbwrStToxi4VfE188bYqjeUHC2zER5kNTIYhkbm7 nAm3T5t4T5+TVF7YIeI9d0Cvypd8faqRYLRXx0CH4mTfSqxZmbw8gth0tuXZoyvlUesX aOMg== X-Gm-Message-State: AOJu0YxbOfX1RktUkw/Q3O7z62wOnwllbP/uBdIqB/XpQkIg+Atp6BSX F4FWgdxAaPkF5C74Wok0QgoAcfcBp5+PN821FvQ566bbo2NLaTFAzCLyzoSk5rsqWCvETkhx6Ar 0C0vJOiI8s7/sxDtlfOOeULwjbNC3EcrdqBSj6CQWIMGl7hA8G/0kI2bXIiVpb3zas6G7DC1ydz E= X-Google-Smtp-Source: AGHT+IF4v5Xd8iIFu7b7sjO3vLaDgN3oLPPEH94kHEDbrT0Krf/9oA1IP071lNt1Wi5vp3ZpGNJHg7H5 X-Received: from palermo.c.googlers.com ([fda3:e722:ac3:cc00:28:9cb1:c0a8:118a]) (user=ardb job=sendgmr) by 2002:a05:690c:340c:b0:5cc:9611:7259 with SMTP id fn12-20020a05690c340c00b005cc96117259mr106639ywb.2.1700761485578; Thu, 23 Nov 2023 09:44:45 -0800 (PST) Date: Thu, 23 Nov 2023 18:44:35 +0100 In-Reply-To: <20231123174433.737171-6-ardb@google.com> Mime-Version: 1.0 References: <20231123174433.737171-6-ardb@google.com> X-Developer-Key: i=ardb@kernel.org; a=openpgp; fpr=F43D03328115A198C90016883D200E9CA6329909 X-Developer-Signature: v=1; a=openpgp-sha256; l=7220; i=ardb@kernel.org; h=from:subject; bh=mx3uFna/A5A8ISppRoJjA0j8lLev8TBTGDC1nQGsxRo=; b=owGbwMvMwCFmkMcZplerG8N4Wi2JITW+v9mP5Un3lSRp+dPt9yc/UzO/d2y7WN6uglMeMzm9Y 1RqNXk7SlkYxDgYZMUUWQRm/3238/REqVrnWbIwc1iZQIYwcHEKwES6hRj+qVhNc97+YPYD1ofq yxaZ6f6fuOeuucVjv4vpRxewbr5rm87wV4DphWvvyWtcu86+bDpr6aPX2LbTSFt3Bi+HjMBqPzs FLgA= X-Mailer: git-send-email 2.43.0.rc1.413.gea7ed67945-goog Message-ID: <20231123174433.737171-7-ardb@google.com> Subject: [PATCH v2 1/4] arm64: fpsimd: Drop unneeded 'busy' flag From: Ard Biesheuvel To: linux-arm-kernel@lists.infradead.org Cc: Ard Biesheuvel , Marc Zyngier , Will Deacon , Mark Rutland , Kees Cook , Catalin Marinas , Mark Brown , Eric Biggers X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231123_094447_025059_5A6FFB27 X-CRM114-Status: GOOD ( 21.85 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org From: Ard Biesheuvel Kernel mode NEON will preserve the user mode FPSIMD state by saving it into the task struct before clobbering the registers. In order to avoid the need for preserving kernel mode state too, we disallow nested use of kernel mode NEON, i..e, use in softirq context while the interrupted task context was using kernel mode NEON too. Originally, this policy was implemented using a per-CPU flag which was exposed via may_use_simd(), requiring the users of the kernel mode NEON to deal with the possibility that it might return false, and having NEON and non-NEON code paths. This policy was changed by commit 13150149aa6ded1 ("arm64: fpsimd: run kernel mode NEON with softirqs disabled"), and now, softirq processing is disabled entirely instead, and so may_use_simd() can never fail when called from task or softirq context. This means we can drop the fpsimd_context_busy flag entirely, and instead, ensure that we disable softirq processing in places where we formerly relied on the flag for preventing races in the FPSIMD preserve routines. Reviewed-by: Mark Brown Signed-off-by: Ard Biesheuvel --- arch/arm64/include/asm/simd.h | 11 +--- arch/arm64/kernel/fpsimd.c | 53 +++++--------------- 2 files changed, 13 insertions(+), 51 deletions(-) diff --git a/arch/arm64/include/asm/simd.h b/arch/arm64/include/asm/simd.h index 6a75d7ecdcaa..8e86c9e70e48 100644 --- a/arch/arm64/include/asm/simd.h +++ b/arch/arm64/include/asm/simd.h @@ -12,8 +12,6 @@ #include #include -DECLARE_PER_CPU(bool, fpsimd_context_busy); - #ifdef CONFIG_KERNEL_MODE_NEON /* @@ -28,17 +26,10 @@ 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 - * cannot change under our feet -- if it's set we cannot be - * migrated, and if it's clear we cannot be migrated to a CPU - * where it is set. */ return !WARN_ON(!system_capabilities_finalized()) && system_supports_fpsimd() && - !in_hardirq() && !irqs_disabled() && !in_nmi() && - !this_cpu_read(fpsimd_context_busy); + !in_hardirq() && !irqs_disabled() && !in_nmi(); } #else /* ! CONFIG_KERNEL_MODE_NEON */ diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c index 1559c706d32d..ccc4a78a70e4 100644 --- a/arch/arm64/kernel/fpsimd.c +++ b/arch/arm64/kernel/fpsimd.c @@ -85,13 +85,13 @@ * softirq kicks in. Upon vcpu_put(), KVM will save the vcpu FP state and * flag the register state as invalid. * - * In order to allow softirq handlers to use FPSIMD, kernel_neon_begin() may - * save the task's FPSIMD context back to task_struct from softirq context. - * To prevent this from racing with the manipulation of the task's FPSIMD state - * from task context and thereby corrupting the state, it is necessary to - * protect any manipulation of a task's fpsimd_state or TIF_FOREIGN_FPSTATE - * flag with {, __}get_cpu_fpsimd_context(). This will still allow softirqs to - * run but prevent them to use FPSIMD. + * In order to allow softirq handlers to use FPSIMD, kernel_neon_begin() may be + * called from softirq context, which will save the task's FPSIMD context back + * to task_struct. To prevent this from racing with the manipulation of the + * task's FPSIMD state from task context and thereby corrupting the state, it + * is necessary to protect any manipulation of a task's fpsimd_state or + * TIF_FOREIGN_FPSTATE flag with get_cpu_fpsimd_context(), which will suspend + * softirq servicing entirely until put_cpu_fpsimd_context() is called. * * For a certain task, the sequence may look something like this: * - the task gets scheduled in; if both the task's fpsimd_cpu field @@ -209,27 +209,14 @@ static inline void sme_free(struct task_struct *t) { } #endif -DEFINE_PER_CPU(bool, fpsimd_context_busy); -EXPORT_PER_CPU_SYMBOL(fpsimd_context_busy); - static void fpsimd_bind_task_to_cpu(void); -static void __get_cpu_fpsimd_context(void) -{ - bool busy = __this_cpu_xchg(fpsimd_context_busy, true); - - WARN_ON(busy); -} - /* * Claim ownership of the CPU FPSIMD context for use by the calling context. * * The caller may freely manipulate the FPSIMD context metadata until * put_cpu_fpsimd_context() is called. * - * The double-underscore version must only be called if you know the task - * can't be preempted. - * * On RT kernels local_bh_disable() is not sufficient because it only * serializes soft interrupt related sections via a local lock, but stays * preemptible. Disabling preemption is the right choice here as bottom @@ -242,14 +229,6 @@ static void get_cpu_fpsimd_context(void) local_bh_disable(); else preempt_disable(); - __get_cpu_fpsimd_context(); -} - -static void __put_cpu_fpsimd_context(void) -{ - bool busy = __this_cpu_xchg(fpsimd_context_busy, false); - - WARN_ON(!busy); /* No matching get_cpu_fpsimd_context()? */ } /* @@ -261,18 +240,12 @@ static void __put_cpu_fpsimd_context(void) */ static void put_cpu_fpsimd_context(void) { - __put_cpu_fpsimd_context(); if (!IS_ENABLED(CONFIG_PREEMPT_RT)) local_bh_enable(); else preempt_enable(); } -static bool have_cpu_fpsimd_context(void) -{ - return !preemptible() && __this_cpu_read(fpsimd_context_busy); -} - unsigned int task_get_vl(const struct task_struct *task, enum vec_type type) { return task->thread.vl[type]; @@ -383,7 +356,7 @@ static void task_fpsimd_load(void) bool restore_ffr; WARN_ON(!system_supports_fpsimd()); - WARN_ON(!have_cpu_fpsimd_context()); + WARN_ON(preemptible()); if (system_supports_sve() || system_supports_sme()) { switch (current->thread.fp_type) { @@ -467,7 +440,7 @@ static void fpsimd_save(void) unsigned int vl; WARN_ON(!system_supports_fpsimd()); - WARN_ON(!have_cpu_fpsimd_context()); + WARN_ON(preemptible()); if (test_thread_flag(TIF_FOREIGN_FPSTATE)) return; @@ -1507,7 +1480,7 @@ void fpsimd_thread_switch(struct task_struct *next) if (!system_supports_fpsimd()) return; - __get_cpu_fpsimd_context(); + WARN_ON_ONCE(!irqs_disabled()); /* Save unsaved fpsimd state, if any: */ fpsimd_save(); @@ -1523,8 +1496,6 @@ void fpsimd_thread_switch(struct task_struct *next) update_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE, wrong_task || wrong_cpu); - - __put_cpu_fpsimd_context(); } static void fpsimd_flush_thread_vl(enum vec_type type) @@ -1829,10 +1800,10 @@ void fpsimd_save_and_flush_cpu_state(void) if (!system_supports_fpsimd()) return; WARN_ON(preemptible()); - __get_cpu_fpsimd_context(); + get_cpu_fpsimd_context(); fpsimd_save(); fpsimd_flush_cpu_state(); - __put_cpu_fpsimd_context(); + put_cpu_fpsimd_context(); } #ifdef CONFIG_KERNEL_MODE_NEON -- 2.43.0.rc1.413.gea7ed67945-goog _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel