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 BC268C3DA63 for ; Tue, 23 Jul 2024 16:05:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0D22C6B00A2; Tue, 23 Jul 2024 12:05:58 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 081EC6B00D1; Tue, 23 Jul 2024 12:05:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E8C446B00D2; Tue, 23 Jul 2024 12:05:57 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id CE8F76B00A2 for ; Tue, 23 Jul 2024 12:05:57 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 706B2A158B for ; Tue, 23 Jul 2024 16:05:57 +0000 (UTC) X-FDA: 82371493554.12.83BFEB7 Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by imf12.hostedemail.com (Postfix) with ESMTP id 04AA34002B for ; Tue, 23 Jul 2024 16:05:53 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=clnKmAvQ; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf12.hostedemail.com: domain of will@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=will@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721750720; a=rsa-sha256; cv=none; b=fEUg0O/L96A+Dr4cxoiXAEzaUM0xLlVGxV9crDC5J7q1JbwVZE9K4k/3XsgBGyCfuW4/M7 PWo4zMscUN9yF3vSCY/AYl23syatmZW00/1RKMOGffTK3/by5uS++uGzrZz8zRrX/QgKLs s46NdZ35+3/dDcS9nDDgQv9PMI3j0KQ= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=clnKmAvQ; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf12.hostedemail.com: domain of will@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=will@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1721750720; 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=KPUYVxQoK1e/ZnPJySiUYC6c7jJZZCsAsDb8yMClwZU=; b=U7W9awMd5DwPIh8+nCq/o70R/uTOJbmZee+Wz56hEogHmRSjBDnak5EZuU8MmCs4wSeKBq sxuwOVaqLNQVlkLaY+DX4jcjAHFQIpwBaEp4ajlOTJVwpkVnus+TLjATCW4Z8mpn8E+V5D tzoZuZPdkOPFBqa5MZ1iDiIjG1FT32U= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 85F34CE0F14; Tue, 23 Jul 2024 16:05:49 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E7680C4AF09; Tue, 23 Jul 2024 16:05:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1721750748; bh=3KMdKirR+5uJeBNuZveA3AnyH88+bYKyW66Km+vghS8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=clnKmAvQOwXBe7A73/OF/NwP18wPGliYAOYYDSu2siqYSHC4x0hIlKiYIrsUUGY82 M/Rug8d/gNl1EKBR3QKb8CdNwNGCvKc+n65cL0l1n1H4RFjT6RZinQN9s6TXSi+JY/ 7kbcFiQTgJMVsBEaP0hnZLxNDOLNosGo5ZxBFdMhKPvdjCNJAxQSXmy5hCQEjLcmj9 dx9fqCH3Uh/uTj1/fGigXkEwc5ug1E9rxXj5lEExsgfuYD14xMDAZNZJz6u45WmsF5 LoiKanwaKsCf1yW6mPY6PNfL0w4dwk2F7ilflop7GBlx76toS13qTwgFob3DhaOEl4 c0btHCb5nx4sQ== Date: Tue, 23 Jul 2024 17:05:43 +0100 From: Will Deacon To: Ard Biesheuvel Cc: Asahi Lina , linux-mm@kvack.org, linux-kernel@vger.kernel.org, asahi@lists.linux.dev, linux-arm-kernel@lists.infradead.org, Catalin Marinas , ryan.roberts@arm.com, mark.rutland@arm.com Subject: Re: LPA2 on non-LPA2 hardware broken with 16K pages Message-ID: <20240723160543.GA26546@willie-the-truck> References: <50360968-13fb-4e6f-8f52-1725b3177215@asahilina.net> <20240718131428.GA21243@willie-the-truck> <20240723145214.GA26403@willie-the-truck> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-Rspamd-Queue-Id: 04AA34002B X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: u4ibsok3cwm6iqxna6qdfnwu8a8u6zpq X-HE-Tag: 1721750753-456525 X-HE-Meta: U2FsdGVkX1/TNzMhfhCvytLevhm/pRYmcHBnmyt17urdVSkCQSXHffm2yFDOybjAH4/WwGO/u1zGm3KgWCYvE5Kk7NFT/PVDpt+jboyRjRhiCWl3iPerYSQ9fOuGHu2FtjXi2qaZ+W6Z8x2WgmGLwAdbWCbJMbp7KH7G8wKYzDepYKgo68kGqPl2GYXsSJSRL/EkKDsR87u1ETdkT7rqyi5+vZNEITsHu3dzs7+x1LAgH7EWt41W6dkwD5j1LSirMgwxf8OAL3F3eNe74r1rDd1xx6kkxDvzsLfvVoWM3eJEcCJcF9RYbnPMP9+UwXS1jY1fXOkcwWGyxdVHQjKe1rZk/iL6EhXhHjrPkSIsM4KRlv+dE3/5DtsYSwb3ABM2T4A82vI9aSK0DaNjftWZbxE6Tes+vLDvllH0ePGXmTajuyLOwxKfG6D9nZthxoDsSckJ2Sc/GtTccE5NbC0AtJJKPKUSvMWiXkP2/AM35utUKiwqlGAVSpqytuLKy9GdKg7XO4lDRObkX/IsVaH/fr9I+Ck/YMBG5g5oCEs6oITxLAGPXXGEVN3TFJTDj9D4UrqDL4jABFdZBokSSlsUgIgMacwrpjZkxJn+Nt19/US/U1QNne01StSj6gdw6FIZFdBdKrEi1/HNC3BMOreUkeJ3mEUXqvuDGL8jb9zjfFhwJ9LWqerl6NiVidB8oY75p2w7XqmN1ojcSj6c9OiB9DR+Vm76Pns9/1pSrZ+giRTLNCmqE7JU/Am+vDYCAcNYwuFgln8JXRSssHnbIOWYAl4eqlhJXSYQ/tVluT4oDwUD6YmJ0HhpmcYl7K8wsPlaw6T7Qru8H+vHxeVJl1RIBWMHAscfk8/HTLOkwmd3qjL8zKmoMPw5K9Y4xw9h5UQ+0n3vL8F3Go99vh6rzoC2M4V0ze2wNcIJJ25R4Z2MHS2u42nKW5R43grX9ZnDbSO/p/tMZ8P1+f8PXryL0zG ipWYWd5S 7aN33Y0zhOzx7ZATueGrFJbug1zy/TqdQIKLNS0U6ja1TcYFlcjCq22Nh9EUbh9p2GgaQ5FwMahcHoDESXfgK3HKmeYDZJgOXFpehtzrtOohWA9wuc3zpZrk0Jaq+TaCBLiEIZ6KEsA+MGuMcpzKauw1XSlnVkV8afOcEl6TkPLnknituvBUZ1DMkFdH3b8Zfle7J38i6O3P3Erf2gqo7birGRgbiaGCcQxGVkxR1KifVjhUzk/MEp16lHw== 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 Tue, Jul 23, 2024 at 05:02:15PM +0200, Ard Biesheuvel wrote: > On Tue, 23 Jul 2024 at 16:52, Will Deacon wrote: > > On Fri, Jul 19, 2024 at 11:02:29AM -0700, Ard Biesheuvel wrote: > > > Thanks for the cc, and thanks to Lina for the excellent diagnosis - > > > this is really helpful. > > > > > > > diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h > > > > index f8efbc128446..3afe624a39e1 100644 > > > > --- a/arch/arm64/include/asm/pgtable.h > > > > +++ b/arch/arm64/include/asm/pgtable.h > > > > @@ -1065,6 +1065,13 @@ static inline bool pgtable_l5_enabled(void) { return false; } > > > > > > > > #define p4d_offset_kimg(dir,addr) ((p4d_t *)dir) > > > > > > > > +static inline > > > > +p4d_t *p4d_offset_lockless(pgd_t *pgdp, pgd_t pgd, unsigned long addr) > > > > > > This is in the wrong place, I think - we already define this for the > > > 5-level case (around line 1760). > > > > Hmm, I'm a bit confused. In my tree, we have one definition at line 1012, > > which is for the 5-level case (i.e. guarded by > > '#if CONFIG_PGTABLE_LEVELS > 4'). I'm adding a new one at line 1065, > > which puts it in the '#else' block and means we use an override instead > > of the problematic generic version when we're folding. > > > > Indeed. I failed to spot from the context (which is there in the diff) > that this is in the else branch. No worries. > > > > +{ > > > > > > We might add > > > > > > if (pgtable_l4_enabled()) > > > pgdp = &pgd; > > > > > > here to preserve the existing 'lockless' behavior when PUDs are not > > > folded. > > > > The code still needs to be 'lockless' for the 5-level case, so I don't > > think this is necessary. > > The 5-level case is never handled here. Urgh, yes, sorry. I've done a fantasticly bad job of explaining myself. > There is the 3-level case, where the runtime PUD folding needs the > actual address in order to recalculate the descriptor address using > the correct shift. In this case, we don't dereference the pointer > anyway so the 'lockless' thing doesn't matter (afaict) > > In the 4-level case, we want to preserve the original behavior, where > pgd is not reloaded from pgdp. Setting pgdp to &pgd achieves that. Right. What I'm trying to get at is the case where we have folding. For example, with my patch applied, if we have 3 levels then the lockless GUP walk looks like: pgd_t pgd = READ_ONCE(*pgdp); p4dp = p4d_offset_lockless(pgdp, pgd, addr); => Returns pgdp p4d_t p4d = READ_ONCE(*p4dp); pudp = pud_offset_lockless(p4dp, p4d, addr); => Returns &p4d, which is again the pgdp pud_t pud = READ_ONCE(*pudp); So here we're reloading the same pointer multiple times and my argument is that if we need to add logic to avoid this for the pgtable_l4_enabled() case, then we have bigger problems. > > Yes, we'll load the same entry multiple times, > > but it should be fine because they're in the context of a different > > (albeit folded) level. > > > > I don't understand what you are saying here. Why is that fine? I think it's fine because (a) the CPU guarantees same address read-after-read ordering and (b) We only evaluate the most recently read value. It would be a problem if we mixed data from different reads but, because the use is confined to that 'level', we don't end up doing that. Dunno, am I making any sense? Will