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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 955FBC35FFA for ; Wed, 19 Mar 2025 18:47:47 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 92330280002; Wed, 19 Mar 2025 14:47:44 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8D333280001; Wed, 19 Mar 2025 14:47:44 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 79B7F280002; Wed, 19 Mar 2025 14:47:44 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 5C523280001 for ; Wed, 19 Mar 2025 14:47:44 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 567C41C81B4 for ; Wed, 19 Mar 2025 18:47:46 +0000 (UTC) X-FDA: 83239184532.12.B069254 Received: from out-179.mta0.migadu.com (out-179.mta0.migadu.com [91.218.175.179]) by imf07.hostedemail.com (Postfix) with ESMTP id 4E08940002 for ; Wed, 19 Mar 2025 18:47:44 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=xaQi4Ef4; spf=pass (imf07.hostedemail.com: domain of yosry.ahmed@linux.dev designates 91.218.175.179 as permitted sender) smtp.mailfrom=yosry.ahmed@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1742410064; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=zqyBVaYb9ts4lSaXiElKMyansdaMcGaJGQGVhzkplWo=; b=qB2qgZChHA+rfYuXYb2krPdFHu0Kys+mduwfM/4Kw33f6SmMoX9Uxnq1vcnpmruJDoxteD aJcCX+POmvfnAOB0HqwDUtftjPEfAl+d7ngTZU1Q3lfb01GPjzIuXgC8zYZAyAFyDq6loe vY8e8nv8sCA8g5WR6E5xMq8OL41p4XI= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1742410064; a=rsa-sha256; cv=none; b=sCeg3aUuZ4Hvua/DA2Ow3w6zEkXEZDiauQbVyBROv/EPgr5V8E5eC8Zoc0Hg7NOcC5rvuf o64hEAmHHP6kIT5SS7qEcAu+9X+2Z9PPi8Npmlni5+trxMEs30G4DxWD1bcrM00l48vaRC BMOP67f/3nlU/fS8KXmAMCOUajdMtXQ= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=xaQi4Ef4; spf=pass (imf07.hostedemail.com: domain of yosry.ahmed@linux.dev designates 91.218.175.179 as permitted sender) smtp.mailfrom=yosry.ahmed@linux.dev; dmarc=pass (policy=none) header.from=linux.dev Date: Wed, 19 Mar 2025 18:47:31 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1742410061; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=zqyBVaYb9ts4lSaXiElKMyansdaMcGaJGQGVhzkplWo=; b=xaQi4Ef4746B+A1AJBeekKoem5X9BsU2Zv43rk34K1b57B9N/UUB0WB7+ebMMyAm4dfIwQ po8r8m5IqUgNRKUb/IkSMFPDZP7ikmojvJbVFmFRwuujjWoxhQaSwuSjM3plvtnggGTY0s MOiuRmYvVaZE3dOZtPg1/9XMdLYCzcU= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Yosry Ahmed To: Borislav Petkov Cc: Brendan Jackman , Thomas Gleixner , Ingo Molnar , Dave Hansen , "H. Peter Anvin" , Andy Lutomirski , Peter Zijlstra , Josh Poimboeuf , Pawan Gupta , x86@kernel.org, linux-kernel@vger.kernel.org, linux-alpha@vger.kernel.org, linux-snps-arc@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-csky@vger.kernel.org, linux-hexagon@vger.kernel.org, loongarch@lists.linux.dev, linux-m68k@lists.linux-m68k.org, linux-mips@vger.kernel.org, linux-openrisc@vger.kernel.org, linux-parisc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org, linux-sh@vger.kernel.org, sparclinux@vger.kernel.org, linux-um@lists.infradead.org, linux-arch@vger.kernel.org, linux-mm@kvack.org, linux-trace-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, kvm@vger.kernel.org, linux-efi@vger.kernel.org, Junaid Shahid Subject: Re: [PATCH RFC v2 04/29] mm: asi: Add infrastructure for boot-time enablement Message-ID: References: <20250110-asi-rfc-v2-v2-0-8419288bc805@google.com> <20250110-asi-rfc-v2-v2-4-8419288bc805@google.com> <20250319172935.GMZ9r-_zzXhyhHBLfj@fat_crate.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250319172935.GMZ9r-_zzXhyhHBLfj@fat_crate.local> X-Migadu-Flow: FLOW_OUT X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 4E08940002 X-Stat-Signature: dxykrxpd8xcsqwu1rhdx9n7am6hcosuk X-HE-Tag: 1742410064-915488 X-HE-Meta: U2FsdGVkX1/BKiQtn2USi9OZpJlo7etai6QsYmglvqiVQNUvfL/U+rmA77ftM1CYwpiLRdcqXCBqFEOBJeRRYO5yqMsV3FhGYYNd7QlBHarSBcdIkdbW7rjGDqx9HLYDQd0OCwyfPeMITK9qxc0cniiQnSYh+pEwE3ebECGRrxvyE3o3Vr5KwKNucOzXUCxMGZx5bK9tcUdnRxjvcN3Nz7SzSs/RhSPwMubR+ILUXpcpOLO/XYBVlbbpZSbR86iQqJO48NnF9UYEGz9MDBToXWJQMIXea3AQFDizR7PJ8B609xP/4rGax6mheCbJ5AuF2D3v9s1txdVqX5f11PvknO2HiQgDM4Iy61DETYkWptjK1Pyhe0kt7/EI1KTR6I3l8FEvg46nRNYQL7YZpS0dQWZgHyX0LiHoPDYtZyQdQmkHureSht9fbblRpV6My3977LO9JsOFuoI2KrHw60Q6NlvhFKl+NysL+PI7PG1nSwkHN8e2CTHoyasR7Gnw4d5f2we9fryyR8aM55f87scA9JWhzse3HxWYrQdngo/HxLoAvSs4HohUMXTFLB44MJ1t6gSICODPMmLHlEYK7Kix2SYni3tI91Tw6HFu+mYtshEtW+8vnb1BWPtzDzBkbwkPJzzVW9HUMaVhCTKdOyXxgx6J/VrzGMnS/KlPfzzYsthdY/XkDk/CM2I2uUfRbaA4m2yS3Xhwgsf0THJTj7Er96f2qE0yNn+guYgUuKjuG2Iho1bQR74rA75TFtBTVUCuGQyZWqBowi5LeZbV00Zf4pzmLTZWNJzNQbWobteZ/LojkoscwcyN70lMFcn8OKN7CVRzPUeDrVzxNpGPV2ZINSGKe+02n6uZx0IuuEVmKehJZK2+jFcv18M9mtNK+wBLVKBjo/X57TxR4FgHLkOcI5De477rbrjOzleYRDeSanTRb7zilE/M7M1+k0Q0DsUGXsrPEIfvufWtS1Wph9a 9UPX34IJ bt3ATsPuByTMUhFUL6rjEkOdvf5YPIqhfe07xxOxRbrSYcuFdXoIYCUbCRUc3j4QUD8ufirINPs2eHEs+Rar+txvO3yKyqvwdEDHAu32wU5fmHnefabq56WT5lJEVHEZQCwY4RkV02+wTLBJK+Z7ka5a055/4Ci3lDcfjbBmaAJsuzYBYZHx9FkxZTi+l+khyWi7Kr8JjY8JC6ShD9oHNVqGcfCGOBOjrXMwMi3qsZl23MekvFhOJpxUxfA== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Wed, Mar 19, 2025 at 06:29:35PM +0100, Borislav Petkov wrote: > On Fri, Jan 10, 2025 at 06:40:30PM +0000, Brendan Jackman wrote: > > Add a boot time parameter to control the newly added X86_FEATURE_ASI. > > "asi=on" or "asi=off" can be used in the kernel command line to enable > > or disable ASI at boot time. If not specified, ASI enablement depends > > on CONFIG_ADDRESS_SPACE_ISOLATION_DEFAULT_ON, which is off by default. > > I don't know yet why we need this default-on thing... It's a convenience to avoid needing to set asi=on if you want ASI to be on by default. It's similar to HUGETLB_PAGE_OPTIMIZE_VMEMMAP_DEFAULT_ON or ZSWAP_DEFAULT_ON. [..] > > @@ -175,7 +184,11 @@ static __always_inline bool asi_is_restricted(void) > > return (bool)asi_get_current(); > > } > > > > -/* If we exit/have exited, can we stay that way until the next asi_enter? */ > > +/* > > + * If we exit/have exited, can we stay that way until the next asi_enter? > > What is that supposed to mean here? asi_is_relaxed() checks if the thread is outside an ASI critical section. I say "the thread" because it will also return true if we are executing an interrupt that arrived during the critical section, even though the interrupt handler is not technically part of the critical section. Now the reason it says "if we exit we stay that way" is probably referring to the fact that an asi_exit() when interrupting a critical section will be undone in the interrupt epilogue by re-entering ASI. I agree the wording here is confusing. We should probably describe this more explicitly and probably rename the function after the API discussions you had in the previous patch. > > > + * > > + * When ASI is disabled, this returns true. > > + */ > > static __always_inline bool asi_is_relaxed(void) > > { > > return !asi_get_target(current); [..] > > @@ -66,10 +73,36 @@ const char *asi_class_name(enum asi_class_id class_id) > > return asi_class_names[class_id]; > > } > > > > +void __init asi_check_boottime_disable(void) > > +{ > > + bool enabled = IS_ENABLED(CONFIG_MITIGATION_ADDRESS_SPACE_ISOLATION_DEFAULT_ON); > > + char arg[4]; > > + int ret; > > + > > + ret = cmdline_find_option(boot_command_line, "asi", arg, sizeof(arg)); > > + if (ret == 3 && !strncmp(arg, "off", 3)) { > > + enabled = false; > > + pr_info("ASI disabled through kernel command line.\n"); > > + } else if (ret == 2 && !strncmp(arg, "on", 2)) { > > + enabled = true; > > + pr_info("Ignoring asi=on param while ASI implementation is incomplete.\n"); > > + } else { > > + pr_info("ASI %s by default.\n", > > + enabled ? "enabled" : "disabled"); > > + } > > + > > + if (enabled) > > + pr_info("ASI enablement ignored due to incomplete implementation.\n"); > > Incomplete how? This is referring to the fact that ASI is still not fully/correctly functional, but it will be after the following patches. I think it will be clearer if we just add the feature flag here so that we have something to check for in the following patches, but add the infrastructure for boot-time enablement at the end of the series when the impelemntation is complete. Basically start by a feature flag that has no way of being enabled, use it in the implmentation, then add means of enabling it. > > > +} > > + > > static void __asi_destroy(struct asi *asi) > > { > > - lockdep_assert_held(&asi->mm->asi_init_lock); > > + WARN_ON_ONCE(asi->ref_count <= 0); > > + if (--(asi->ref_count) > 0) > > Switch that to > > include/linux/kref.h > > It gives you a sanity-checking functionality too so you don't need the WARN... I think we hve internal changes that completely get rid of this ref_count and simplifies the lifetime handling that we can squash here. We basically keep ASI objects around until the process is torn down, which makes this simpler and avoids the need for complex synchronization when we try to context switch or run userspace without exiting ASI (spoiler alert :) ). > > > + return; > > > > + free_pages((ulong)asi->pgd, PGD_ALLOCATION_ORDER); > > + memset(asi, 0, sizeof(struct asi)); > > And then you can do: > > if (kref_put()) > free_pages... > > and so on. > > Thx. > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette >