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 7D425C77B75 for ; Tue, 23 May 2023 00:07:31 +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=IC8JzBmOs9IX8husTDC3mGPIBKqzprwCz2y4YCV6Uuk=; b=SvTNEm3LKWhiilLXp6C6d9symb GvTYg6bRE8id7FqMCPEx8jQIIAVEK1HSBPEwE9xhgcAyMAr0hryfQJ3CBXMtxIMZeN9kZV91PUG5+ tV9CYWC0F1izR4PGqh7RyqGtI2GndbsmdlHOvTKtY+F6ksRmAL3jEwsRSlRedgoHtuUV4zq5ZSa32 0QVcP7QsRnz9l9A7/WN2Us1eJX+EpRG1lz+oumgGkMOiSofinGa/KUoRlV4QZQJrUhSKZIM40mRin io/YFRjo69U6Md3TFh5BD+pybVCPVTmW4M25xUgLrd5ofPDeVj5M9GYC2VWHuT5NBfaCEq+MrrlQA MibqXkbA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q1FYQ-008QAL-0W; Tue, 23 May 2023 00:07:06 +0000 Received: from mail-pg1-x549.google.com ([2607:f8b0:4864:20::549]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1q1FYM-008Q8s-2B for linux-arm-kernel@lists.infradead.org; Tue, 23 May 2023 00:07:04 +0000 Received: by mail-pg1-x549.google.com with SMTP id 41be03b00d2f7-53b9eb7bda0so896961a12.0 for ; Mon, 22 May 2023 17:06:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1684800418; x=1687392418; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=nqpMDrC4KK3NQ6AuoTcuN/aH10d3Gs6pWZJ4L+6LYdI=; b=einRQC06NM+pLkPcWJKuFkA3JrqGZ4jSC+L44hYXq+E/rLb8tn1KKR5KGxdVgGH9fL z4wM40qWv+kR5vQgB2cbnU9DNxDeI2IC0tmcs88RbHAhC13vUSJeJwXY2E2Dld8hrPg3 MG3L4X+OVgZf3FszqPcq/K+aN6nVAet/JMgxgQXoHdcBP0my7P2nA29a9lYONLLhRYMZ s2S/qIxe2XDDMncs9mGy6cUgAaVEpju9PTe00dW7/KviwcI5VWtDKN+UuNKlHfU72jrL KU7Iq+8yaRoCE4WlHn45jJU/E5rxEradrcAdgs1h1bhJKao7Ozt805O7vAzm8fUMRQot nQfg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684800418; x=1687392418; 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=nqpMDrC4KK3NQ6AuoTcuN/aH10d3Gs6pWZJ4L+6LYdI=; b=PTvdrlpJzrGFAUWW+NxA58LjPKUgKc9WG43fUzng7j5epm1dENF4U80xVR3ftJHk9F QQ585POBSf4X5tF3LYFk/Ks+OP0AidYqUgEaEAp8InjG07CRVb2hXgFy9efCu6DEU5jN tOEX61dDDX11GdCAPvbPXWsnnX4cWWcNlGu79fNL6pLAF92ok+3sCFW5GM2J8fRy0LX5 bb+X0PFYyu21Icp0x3rTIXHjGQP07p4nXTysGMFUBAOM6Q3bv/bcc1CG36MhnrmDVmgx qruradS1rVnc4qpbP6oq/hSrke7OWRf3am8Lyp3yCN5LTvyW1DNWAuoB1j3o/lJjrASM 04Qg== X-Gm-Message-State: AC+VfDzm/4BuRXiDHPzY0vPcHnwCpMN28jW4GIPjJEoTYsYmVRdzL9zL XEY5lmvEyMn8YHf+89YjeyHzKYRYyfw= X-Google-Smtp-Source: ACHHUZ6HjllyZzTlaaKR4BoUDyFce04HZ9UhEoY+TcxZpDQZdfHOuNAQJ4T7tsiqCTcoicnwg0ExZ7H4Ffw= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a63:5f91:0:b0:52c:b362:1894 with SMTP id t139-20020a635f91000000b0052cb3621894mr2851167pgb.3.1684800417778; Mon, 22 May 2023 17:06:57 -0700 (PDT) Date: Mon, 22 May 2023 17:06:56 -0700 In-Reply-To: <875y8jappz.fsf@nvidia.com> Mime-Version: 1.0 References: <20230522063725.284686-1-apopple@nvidia.com> <875y8jappz.fsf@nvidia.com> Message-ID: Subject: Re: [PATCH] mmu_notifiers: Notify on pte permission upgrades From: Sean Christopherson To: Alistair Popple Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, robin.murphy@arm.com, will@kernel.org, nicolinc@nvidia.com, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, jgg@nvidia.com, John Hubbard X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230522_170702_716803_F052BD4E X-CRM114-Status: GOOD ( 37.70 ) 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 On Tue, May 23, 2023, Alistair Popple wrote: > > Sean Christopherson writes: > >> diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h > >> index d6c06e140277..f14d68f119d8 100644 > >> --- a/include/linux/mmu_notifier.h > >> +++ b/include/linux/mmu_notifier.h > >> @@ -31,6 +31,11 @@ struct mmu_interval_notifier; > >> * pages in the range so to mirror those changes the user must inspect the CPU > >> * page table (from the end callback). > >> * > >> + * @MMU_NOTIFY_PROTECTION_UPGRAGE: update is due to a change from read-only to > >> + * read-write for pages in the range. This must not be used to upgrade > >> + * permissions on secondary PTEs, rather it should only be used to invalidate > >> + * caches such as secondary TLBs that may cache old read-only entries. > > > > This is a poor fit for invalidate_range_{start,end}(). All other uses bookend > > the primary MMU update, i.e. call start() _before_ changing PTEs. The comments > > are somewhat stale as they talk only about "unmapped", but the contract between > > the primary MMU and the secondary MMU is otherwise quite clear on when the primary > > MMU will invoke start() and end(). > > > > * invalidate_range_start() is called when all pages in the > > * range are still mapped and have at least a refcount of one. > > * > > * invalidate_range_end() is called when all pages in the > > * range have been unmapped and the pages have been freed by > > * the VM. > > > > I'm also confused as to how this actually fixes ARM's SMMU. Unless I'm looking > > at the wrong SMMU implementation, the SMMU implemenents only invalidate_range(), > > not the start()/end() variants. > > mmu_invalidate_range_end() calls the invalidate_range() callback if > the start()/end() variants aren't set. Ahh, thanks. > > static const struct mmu_notifier_ops arm_smmu_mmu_notifier_ops = { > > .invalidate_range = arm_smmu_mm_invalidate_range, > > .release = arm_smmu_mm_release, > > .free_notifier = arm_smmu_mmu_notifier_free, > > }; > > > > Again from include/linux/mmu_notifier.h, not implementing the start()/end() hooks > > is perfectly valid. And AFAICT, the existing invalidate_range() hook is pretty > > much a perfect fit for what you want to achieve. > > Right, I didn't take that approach because it doesn't allow an event > type to be passed which would allow them to be filtered on platforms > which don't require this. > > I had also assumed the invalidate_range() callbacks were allowed to > sleep, hence couldn't be called under PTL. That's certainly true of mmu > interval notifier callbacks, but Catalin reminded me that calls such as > ptep_clear_flush_notify() already call invalidate_range() callback under > PTL so I guess we already assume drivers don't sleep in their > invalidate_range() callbacks. I will update the comments to reflect > that. > > > * If invalidate_range() is used to manage a non-CPU TLB with > > * shared page-tables, it not necessary to implement the > > * invalidate_range_start()/end() notifiers, as > > * invalidate_range() already catches the points in time when an > > * external TLB range needs to be flushed. For more in depth > > * discussion on this see Documentation/mm/mmu_notifier.rst > > > > Even worse, this change may silently regress performance for secondary MMUs that > > haven't yet taken advantage of the event type, e.g. KVM will zap all of KVM's PTEs > > in response to the upgrade, instead of waiting until the guest actually tries to > > utilize the new protections. > > Yeah, I like the idea of introducing a > ptep_set_access_flags_notify(). That way this won't regress performance > on platforms that don't need it. Note this isn't a new feature but > rather a bugfix. It's unclear to me why KVM on ARM hasn't already run > into this issue, but I'm no KVM expert. Thanks for the feedback. KVM manages its own page tables and so does its own TLB invalidations as needed, e.g. KVM can and does change KVM's stage-2 PTEs from read-only to read/write irrespective of mmu_notifiers. I assume the SMMU issue arises only because the SMMU is reusing the host kernel's (stage-1?) page tables. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel