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 5572CC54798 for ; Thu, 7 Mar 2024 21:04:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D63456B02B7; Thu, 7 Mar 2024 16:04:37 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D13106B02B8; Thu, 7 Mar 2024 16:04:37 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BDA9E6B02B9; Thu, 7 Mar 2024 16:04:37 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id AC1666B02B7 for ; Thu, 7 Mar 2024 16:04:37 -0500 (EST) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 7B720C010E for ; Thu, 7 Mar 2024 21:04:37 +0000 (UTC) X-FDA: 81871471794.23.2C0B52D Received: from mail-yw1-f201.google.com (mail-yw1-f201.google.com [209.85.128.201]) by imf18.hostedemail.com (Postfix) with ESMTP id 88B341C001D for ; Thu, 7 Mar 2024 21:04:35 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=dNdaiNiD; spf=pass (imf18.hostedemail.com: domain of 34ivqZQoKCBkNDHGNz6B325DD5A3.1DBA7CJM-BB9Kz19.DG5@flex--yosryahmed.bounces.google.com designates 209.85.128.201 as permitted sender) smtp.mailfrom=34ivqZQoKCBkNDHGNz6B325DD5A3.1DBA7CJM-BB9Kz19.DG5@flex--yosryahmed.bounces.google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1709845475; 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=abcHkgaJlKdL9AQhodGhEJEYkKj/UXzrFfTMg7S4USk=; b=6zEp/q8HPE+ePmMiA1EYyoDPNmJH6zG1HEZymyZHcK+KTGtup5gsR+pTv63NZQFIBeROIW YKCF8fG5I7ZEnwQwKFNaxvv6Zm23E/nBav9HY1poaQd9BjKO3SQ13h/Z8Wafej0K8Aq7QC A9LdOHaMOzpmkrWkTa7/Wpufx51GW38= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1709845475; a=rsa-sha256; cv=none; b=61pVaXWlmXE7/ryMIjXEHMzGkHkyx7GN90J+MwRKTpbCdNWBY0M5jJt0QVniqfLTGZY2Vi /elVF4yc7NR4gQfsmQQZyaSACrabyrbtgtUju+R979dWsKaZphSb/G72MG0KMU/+8IK23l 2bYZfOc4IslN3AbTv7Y46EW1PWkb9tQ= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=dNdaiNiD; spf=pass (imf18.hostedemail.com: domain of 34ivqZQoKCBkNDHGNz6B325DD5A3.1DBA7CJM-BB9Kz19.DG5@flex--yosryahmed.bounces.google.com designates 209.85.128.201 as permitted sender) smtp.mailfrom=34ivqZQoKCBkNDHGNz6B325DD5A3.1DBA7CJM-BB9Kz19.DG5@flex--yosryahmed.bounces.google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-yw1-f201.google.com with SMTP id 00721157ae682-60a0151f194so3524157b3.1 for ; Thu, 07 Mar 2024 13:04:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1709845475; x=1710450275; darn=kvack.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=abcHkgaJlKdL9AQhodGhEJEYkKj/UXzrFfTMg7S4USk=; b=dNdaiNiDkd5dpcW1gEuMszmXk7DzUsWUgTul+fn0pKZTuC8z0BKSBXgAkti3Pc9+wg xfzNaCskATlslYaBg6NnxqYx80PWghAtGigk3aOT8ArdOui5Ti7GZytQw0QVD+nV2Vuh Zjbdn4+z7A2HJVsP1X1Y5XYezJ+Zm8CG3TrHuKMgHMs77zWrhnACSTN2rIcD6zRVJWMl AzRsXI2qOMQqZOUDzHaNw5q71feOYK9+00wdfMv1DXHU/oaLKdxRUx7CrzjdZeK3Lm+a Zf93unKXQCJcBm0J+QPP3oXdzD4OQuXiBrpNaykVJa71SZ5CADWJ9s6RZ+aIgcOmpFWu BKOQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709845475; x=1710450275; 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=abcHkgaJlKdL9AQhodGhEJEYkKj/UXzrFfTMg7S4USk=; b=n8rlHnk2DyXkRuRH1w4QmghwaEFk1bGcR7+zkhBx9UK+2U0DxCe8PpX0cnzHJd4QTy a1cBVwxF9hrlcCmC1g9HSPb97d/eYMJDNoHQNVLEk91miLVwhSNFlpJtZpAHJGINuTIU xmIGQJKngWtflWNWpP061qdiNZmk3IQZNTW7DZXrcoz0cjpdHFCGinb2PvasGRsHfggN VxIIunnUzf4+9oQLjy6ozmRQZIqCdajlu1p7BDzzo2mX7L0mXbn5k7kfVdrhl1aG17kI v8ThSlrgY8l884zbA7g2cHnxl0mR0HGSgXrrElwsGlqy6d3/2enMIazNR/Y1vmBp0Vbw GPog== X-Forwarded-Encrypted: i=1; AJvYcCUwJVpz6B7jgyjur8qh+RdS2dKKQp+nS/IukiOrC+JLw7MvqcdjBgG40J++tTmLjkOXyyjwG33NBhhyVtqk4TVnUg4= X-Gm-Message-State: AOJu0YxVH3KsKw/OjfX1IvVFeSuw866mwsuXKVwJ9vEoQ5OB+P56GGuo V0S4VKhLPodIjJPAHaPb3FgZMAtstrzJ4hm0/+6NrcUmCFZGNLaK64i0+eO0Y1w+C1ZYBOAtNGo 0aqmIgVS1fP1P6CaB6w== X-Google-Smtp-Source: AGHT+IE8Sm8RJ83g5TnEYwVTx6zL2GcqFCr63p/hfZyRIx4GyskOlcNRsPAvrRyFlxHBpp5sVjMNiBqUc/W5XILv X-Received: from yosry.c.googlers.com ([fda3:e722:ac3:cc00:20:ed76:c0a8:29b4]) (user=yosryahmed job=sendgmr) by 2002:a05:690c:85:b0:609:3cdf:c737 with SMTP id be5-20020a05690c008500b006093cdfc737mr4815606ywb.10.1709845474926; Thu, 07 Mar 2024 13:04:34 -0800 (PST) Date: Thu, 7 Mar 2024 21:04:32 +0000 In-Reply-To: <83b019e2-1b84-491a-b0b9-beb02e45d80c@intel.com> Mime-Version: 1.0 References: <20240307133916.3782068-1-yosryahmed@google.com> <20240307133916.3782068-3-yosryahmed@google.com> <83b019e2-1b84-491a-b0b9-beb02e45d80c@intel.com> Message-ID: Subject: Re: [RFC PATCH 2/3] x86/mm: make sure LAM is up-to-date during context switching From: Yosry Ahmed To: Dave Hansen Cc: Andrew Morton , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Peter Zijlstra , Andy Lutomirski , "Kirill A. Shutemov" , x86@kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="us-ascii" X-Stat-Signature: obcgsgwcfzpocik854jfch3xdo6ep94c X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 88B341C001D X-Rspam-User: X-HE-Tag: 1709845475-498551 X-HE-Meta: U2FsdGVkX18/8ANA8OvQpUczLSFHhy4Snhn/GUQz5L8KKXCz1WJboRWIIYy1waJUhVrhyLYK6eo1rr1yGys5F9qfj4sp6qyT25WACcwZiNOJkDsMDai4j2xaESAnUs2tpLHpooMwTD+/yFQ6r3iRzyvi39qwsGj3cOrPMJWn5f989hcwvbhm5KU/bRg30XvuWUiC819FoWqnYErbyAJPbapkNqSlmb8xy3txA7u0YXq9tpQS60zbM14UTQKX/RZWpftrNiy7BVbTSbO4uTJzr2DQpeTj4b9/jlhdDbJK13vpcoDemwAIQL2pf4h+y6t22GqgM4dx79cMPLsHv6WRif0pYTSnArtl7uMi/WfbpmDBTsEb4jwIMReXXaTtkPt4DsFUx9rBLk2U7V7R5qGWUL1OcQLFcIgfyJBRChNKrmDy9GJJwLIkEgSMGHe3jPF7yjejaZSDsvomGvShVx/lXRPQ5x2Dv9YuWIHa92tx7EHdv7zqn2gbsbWM+KRkblMGn5p1O57N4IMJUjhJKn1Fg8NZLW3p6/on9LprhfbQvSNl1TOy04tyFv9ETSnVFLxsdMO0/1iE2K8QE2HG2AK5MUlNJASSz3uFo7GCkfDs6Uha/mf2455CMRZBpVFpulbMKS/GOTS94Oyx7rE0akUKKWeEbgRJCScMBWGE/vv4OBIfs6c3itgzk6JGPqazC/+qb+JTgyMEeeCoOOZUsUllSHG0QAp/XkBZv9OoQyOlcE03ic3ZFjyOQ93qLF2fARxNgc/Ipabe5d9sLiW8hYKXPNTqf77N3svtuAEk6dkOZQQ9zcp0gYab2e8RZt7mBti8q9mId/g/PPlOZEO7SB94sXNRz3wc36IhqucNk8Qq61bD/nOlj/40OLr7j2MVvaLj01pdWKR9rEHnwjepd/xjFRZTaPZzK3BdcNeBjilMR+YdQkaGwF1yo5p5Z60SNCIbaPXywVADUVqzUCx2QVl /sO0kV9j mhNY7MTsmPC4YR4/XsXKAlXQ82zO9AryzNycVN6ivLGD64gjd44gVNlzgV4g6u0rSjMj6nFTIHEyFYGpM9innjEhEX3T6kcltQGzCMeTUfXbiSiGx8KLC7vgqcdbcG5gigwqMk2VOo0BYlaQ13pyAY27N5I5QfHn6RvsZmyCidj98kwyEIXTlQsVlhhZkdzTPCfsxHuZbPyBiZASqxXaWHt7IRKERZReMu6Rpm7eLWs50ym7M+15NEf3utclld+t2p3aA/HTZRUk0gqmFylQxdaBB8PWP5k2/Eh1qakMmytz+U+nsKKFtRtEOxvn2G3pP7vN0jjpyuzlH3nYTMwQyKAi/yR4CVjF8wC1GD1E3p3KUkMuciJSwsVfk00bkoRmhXrr06oQBq/OSUZQHUTtkGyCHoeu1ni+pjf5FT8DJ5NzkqijELpQ6vsrmfdvIzVVd5YYLb+Lbs78uWUS6UJZyK4ItShix1uqyqEVHBD8PD9AxhvDjXPuY2aEy9Pz29pvbidrf20d9cI7mWuLJ3Ud2H4RuMf1/rZdhXavd3cWg3/k7v45R4bwB2M/i5I4AdqpSf06qspmRQgCPS89ZamBpA+hPIgx7rxx/5Q0YLQmyH/xOjblxcsjfK6ihaK4iXXAVuQRPMSupf73T/hE= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000001, 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 Thu, Mar 07, 2024 at 07:29:34AM -0800, Dave Hansen wrote: > On 3/7/24 05:39, Yosry Ahmed wrote: > > - /* > > - * Read the tlb_gen to check whether a flush is needed. > > - * If the TLB is up to date, just use it. > > - * The barrier synchronizes with the tlb_gen increment in > > - * the TLB shootdown code. > > - */ > > - smp_mb(); > > - next_tlb_gen = atomic64_read(&next->context.tlb_gen); > > - if (this_cpu_read(cpu_tlbstate.ctxs[prev_asid].tlb_gen) == > > - next_tlb_gen) > > + if (!need_flush && !need_lam_update) > > return; > > Instead of all this new complexity, why not just inc_mm_tlb_gen() at the > site where LAM is enabled? That should signal to any CPU thread that > its TLB is out of date and it needs to do a full CR3 reload. It's not really a lot of complexity imo, most of the patch is reverting the if conditions that return if a TLB flush is not needed to have a single if block that sets need_flush to true. I can split this to a different patch if it helps review, or I can do something like this to keep the diff concise: diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c index 2975d3f89a5de..17ab105522287 100644 --- a/arch/x86/mm/tlb.c +++ b/arch/x86/mm/tlb.c @@ -576,7 +576,7 @@ void switch_mm_irqs_off(struct mm_struct *unused, struct mm_struct *next, * process. No TLB flush required. */ if (!was_lazy) - return; + goto check_return; /* * Read the tlb_gen to check whether a flush is needed. @@ -588,7 +588,7 @@ void switch_mm_irqs_off(struct mm_struct *unused, struct mm_struct *next, next_tlb_gen = atomic64_read(&next->context.tlb_gen); if (this_cpu_read(cpu_tlbstate.ctxs[prev_asid].tlb_gen) == next_tlb_gen) - return; + goto check_return; /* * TLB contents went out of date while we were in lazy @@ -596,6 +596,9 @@ void switch_mm_irqs_off(struct mm_struct *unused, struct mm_struct *next, */ new_asid = prev_asid; need_flush = true; +check_return: + if (!need_flush && /* LAM up-to-date */) + return; } else { /* * Apply process to process speculation vulnerability ..but I prefer the current patch though to avoid the goto. I think the flow is easier to follow. I thought about doing inc_mm_tlb_gen() when LAM is updated, but it felt hacky and more importantly doesn't make it clear in switch_mm_irqs_off() that we correctly handle LAM updates. We can certainly add a comment, but I think an explicit check for CPU LAM vs. mm LAM is much clearer. WDYT? > > Also, have you been able to actually catch this scenario in practice? Nope, by code inspection (as I admitted in the commit log). > Considering how fun this code path is, a little effort at an actual > reproduction would be really appreciated. I tried reproducing it but gave up quickly. We need a certain sequence of events to happen: CPU 1 CPU 2 kthread_use_mm() /* user thread enables LAM */ context_switch() context_switch() /* to user thread */ IIUC we don't really need kthread_use_mm(), we need the kthread to be scheduled on the CPU directly after the user thread, so maybe something like: CPU 1 CPU 2 /* user thread running */ context_switch() /* to kthread */ /* user thread enables LAM */ context_switch() context_switch() /* to user thread */ It doesn't seem easy to reproduce. WDYT?