From: Jason Gunthorpe <jgg@nvidia.com> To: Alistair Popple <apopple@nvidia.com> Cc: Sean Christopherson <seanjc@google.com>, Robin Murphy <robin.murphy@arm.com>, Andrew Morton <akpm@linux-foundation.org>, will@kernel.org, catalin.marinas@arm.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, nicolinc@nvidia.com, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, John Hubbard <jhubbard@nvidia.com>, zhi.wang.linux@gmail.com Subject: Re: [PATCH 2/2] arm64: Notify on pte permission upgrades Date: Wed, 31 May 2023 12:30:00 -0300 [thread overview] Message-ID: <ZHdn+FsH6BWcK7C4@nvidia.com> (raw) In-Reply-To: <87ilc9qkuc.fsf@nvidia.com> On Wed, May 31, 2023 at 12:46:06PM +1000, Alistair Popple wrote: > > Jason Gunthorpe <jgg@nvidia.com> writes: > > > On Wed, May 31, 2023 at 10:30:48AM +1000, Alistair Popple wrote: > > > >> So I'd rather keep the invalidate in ptep_set_access_flags(). Would > >> renaming invalidate_range() to invalidate_arch_secondary_tlb() along > >> with clearing up the documentation make that more acceptable, at least > >> in the short term? > > > > Then we need to go through removing kvm first I think. > > Why? I don't think we need to hold up a fix for something that is an > issue today so we can rework a fix for an unrelated problem. I'm nervous about affecting KVM's weird usage if we go in and start making changes. Getting rid of it first is much safer > > Yeah, I think I would call it invalidate_arch_secondary_tlb() and > > document it as being an arch specific set of invalidations that match > > the architected TLB maintenance requrements. And maybe we can check it > > more carefully to make it be called in less places. Like I'm not sure > > it is right to call it from invalidate_range_end under this new > > definition.. > > I will look at this in more depth, but this comment reminded me there is > already an issue with calling .invalidate_range() from > invalidate_range_end(). We have seen slow downs when unmapping unused > ranges because unmap_vmas() will call .invalidate_range() via > .invalidate_range_end() flooding the SMMU with invalidates even though > zap_pte_range() skipped it because the PTEs were pte_none. Yes, if the new API is specifically about synchronizing an architected TLB then really the call to the op should be done near the architectures TLB flush points, and not higher in the MM. ie any flush of the CPU tlb should mirror 1:1 to a flush of the IOMMU TLB, no broadinging or narrowing. It is a very clean defintion and we can leap directly to it once we get kvm out of the way. Jason
WARNING: multiple messages have this Message-ID (diff)
From: Jason Gunthorpe <jgg@nvidia.com> To: Alistair Popple <apopple@nvidia.com> Cc: Sean Christopherson <seanjc@google.com>, Robin Murphy <robin.murphy@arm.com>, Andrew Morton <akpm@linux-foundation.org>, will@kernel.org, catalin.marinas@arm.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, nicolinc@nvidia.com, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, John Hubbard <jhubbard@nvidia.com>, zhi.wang.linux@gmail.com Subject: Re: [PATCH 2/2] arm64: Notify on pte permission upgrades Date: Wed, 31 May 2023 12:30:00 -0300 [thread overview] Message-ID: <ZHdn+FsH6BWcK7C4@nvidia.com> (raw) In-Reply-To: <87ilc9qkuc.fsf@nvidia.com> On Wed, May 31, 2023 at 12:46:06PM +1000, Alistair Popple wrote: > > Jason Gunthorpe <jgg@nvidia.com> writes: > > > On Wed, May 31, 2023 at 10:30:48AM +1000, Alistair Popple wrote: > > > >> So I'd rather keep the invalidate in ptep_set_access_flags(). Would > >> renaming invalidate_range() to invalidate_arch_secondary_tlb() along > >> with clearing up the documentation make that more acceptable, at least > >> in the short term? > > > > Then we need to go through removing kvm first I think. > > Why? I don't think we need to hold up a fix for something that is an > issue today so we can rework a fix for an unrelated problem. I'm nervous about affecting KVM's weird usage if we go in and start making changes. Getting rid of it first is much safer > > Yeah, I think I would call it invalidate_arch_secondary_tlb() and > > document it as being an arch specific set of invalidations that match > > the architected TLB maintenance requrements. And maybe we can check it > > more carefully to make it be called in less places. Like I'm not sure > > it is right to call it from invalidate_range_end under this new > > definition.. > > I will look at this in more depth, but this comment reminded me there is > already an issue with calling .invalidate_range() from > invalidate_range_end(). We have seen slow downs when unmapping unused > ranges because unmap_vmas() will call .invalidate_range() via > .invalidate_range_end() flooding the SMMU with invalidates even though > zap_pte_range() skipped it because the PTEs were pte_none. Yes, if the new API is specifically about synchronizing an architected TLB then really the call to the op should be done near the architectures TLB flush points, and not higher in the MM. ie any flush of the CPU tlb should mirror 1:1 to a flush of the IOMMU TLB, no broadinging or narrowing. It is a very clean defintion and we can leap directly to it once we get kvm out of the way. Jason _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-05-31 15:30 UTC|newest] Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-05-24 1:47 [PATCH 1/2] mmu_notifiers: Restore documentation for .invalidate_range() Alistair Popple 2023-05-24 1:47 ` Alistair Popple 2023-05-24 1:47 ` [PATCH 2/2] arm64: Notify on pte permission upgrades Alistair Popple 2023-05-24 1:47 ` Alistair Popple 2023-05-28 0:02 ` Jason Gunthorpe 2023-05-28 0:02 ` Jason Gunthorpe 2023-05-30 8:05 ` Alistair Popple 2023-05-30 8:05 ` Alistair Popple 2023-05-30 11:54 ` Jason Gunthorpe 2023-05-30 11:54 ` Jason Gunthorpe 2023-05-30 12:14 ` Robin Murphy 2023-05-30 12:14 ` Robin Murphy 2023-05-30 12:52 ` Jason Gunthorpe 2023-05-30 12:52 ` Jason Gunthorpe 2023-05-30 13:44 ` Robin Murphy 2023-05-30 13:44 ` Robin Murphy 2023-05-30 14:06 ` Jason Gunthorpe 2023-05-30 14:06 ` Jason Gunthorpe 2023-05-30 21:44 ` Sean Christopherson 2023-05-30 21:44 ` Sean Christopherson 2023-05-30 23:08 ` Jason Gunthorpe 2023-05-30 23:08 ` Jason Gunthorpe 2023-05-31 0:30 ` Alistair Popple 2023-05-31 0:30 ` Alistair Popple 2023-05-31 0:32 ` Jason Gunthorpe 2023-05-31 0:32 ` Jason Gunthorpe 2023-05-31 2:46 ` Alistair Popple 2023-05-31 2:46 ` Alistair Popple 2023-05-31 15:30 ` Jason Gunthorpe [this message] 2023-05-31 15:30 ` Jason Gunthorpe 2023-05-31 23:56 ` Alistair Popple 2023-05-31 23:56 ` Alistair Popple [not found] ` <31cdd164783fefad4c9ef4a6d33c1e0094405d0f03added523a82dd9febdf15f@mu.id> 2023-06-09 2:06 ` Alistair Popple 2023-06-09 2:06 ` Alistair Popple 2023-06-09 6:05 ` Alistair Popple 2023-06-09 6:05 ` Alistair Popple 2023-05-24 2:20 ` [PATCH 1/2] mmu_notifiers: Restore documentation for .invalidate_range() John Hubbard 2023-05-24 2:20 ` John Hubbard 2023-05-24 4:45 ` Alistair Popple 2023-05-24 4:45 ` Alistair Popple 2023-05-27 23:56 ` Jason Gunthorpe 2023-05-27 23:56 ` Jason Gunthorpe 2023-05-24 3:48 ` Zhi Wang 2023-05-24 3:48 ` Zhi Wang 2023-05-24 4:57 ` Alistair Popple 2023-05-24 4:57 ` Alistair Popple
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=ZHdn+FsH6BWcK7C4@nvidia.com \ --to=jgg@nvidia.com \ --cc=akpm@linux-foundation.org \ --cc=apopple@nvidia.com \ --cc=catalin.marinas@arm.com \ --cc=jhubbard@nvidia.com \ --cc=kvm@vger.kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=nicolinc@nvidia.com \ --cc=robin.murphy@arm.com \ --cc=seanjc@google.com \ --cc=will@kernel.org \ --cc=zhi.wang.linux@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.