From: Jason Gunthorpe <jgg@nvidia.com> To: Sean Christopherson <seanjc@google.com> Cc: Robin Murphy <robin.murphy@arm.com>, Alistair Popple <apopple@nvidia.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: Tue, 30 May 2023 20:08:48 -0300 [thread overview] Message-ID: <ZHaCAJI+OgIfDWSx@nvidia.com> (raw) In-Reply-To: <ZHZuSDp6ioPqI272@google.com> On Tue, May 30, 2023 at 02:44:40PM -0700, Sean Christopherson wrote: > > KVM already has locking for invalidate_start/end - it has to check > > mmu_notifier_retry_cache() with the sequence numbers/etc around when > > it does does hva_to_pfn() > > > > The bug is that the kvm_vcpu_reload_apic_access_page() path is > > ignoring this locking so it ignores in-progress range > > invalidations. It should spin until the invalidation clears like other > > places in KVM. > > > > The comment is kind of misleading because drivers shouldn't be abusing > > the iommu centric invalidate_range() thing to fix missing locking in > > start/end users. :\ > > > > So if KVM could be fixed up we could make invalidate_range defined to > > be an arch specific callback to synchronize the iommu TLB. > > And maybe rename invalidate_range() and/or invalidate_range_{start,end}() to make > it super obvious that they are intended for two different purposes? E.g. instead > of invalidate_range(), something like invalidate_secondary_tlbs(). 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.. > FWIW, PPC's OpenCAPI support (drivers/misc/ocxl/link.c) also uses invalidate_range(). > Though IIUC, the use case is the same as a "traditional" IOMMU, where a device can > share the CPU's page tables, so maybe the devices can be considered IOMMUs in practice, > if not in name? OpenCAPI is an IOMMU HW for sure. PPC just doesn't have integration with the drivers/iommu infrastructure. > I have patches coded up. Assuming testing goes well, I'll post them regardless > of the OCXL side of things. I've disliked KVM's one-off use of invalidate_range() > for a long time, this is a good excuse to get rid of it before KVM gains more usage. Nice! Thanks, Jason
WARNING: multiple messages have this Message-ID (diff)
From: Jason Gunthorpe <jgg@nvidia.com> To: Sean Christopherson <seanjc@google.com> Cc: Robin Murphy <robin.murphy@arm.com>, Alistair Popple <apopple@nvidia.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: Tue, 30 May 2023 20:08:48 -0300 [thread overview] Message-ID: <ZHaCAJI+OgIfDWSx@nvidia.com> (raw) In-Reply-To: <ZHZuSDp6ioPqI272@google.com> On Tue, May 30, 2023 at 02:44:40PM -0700, Sean Christopherson wrote: > > KVM already has locking for invalidate_start/end - it has to check > > mmu_notifier_retry_cache() with the sequence numbers/etc around when > > it does does hva_to_pfn() > > > > The bug is that the kvm_vcpu_reload_apic_access_page() path is > > ignoring this locking so it ignores in-progress range > > invalidations. It should spin until the invalidation clears like other > > places in KVM. > > > > The comment is kind of misleading because drivers shouldn't be abusing > > the iommu centric invalidate_range() thing to fix missing locking in > > start/end users. :\ > > > > So if KVM could be fixed up we could make invalidate_range defined to > > be an arch specific callback to synchronize the iommu TLB. > > And maybe rename invalidate_range() and/or invalidate_range_{start,end}() to make > it super obvious that they are intended for two different purposes? E.g. instead > of invalidate_range(), something like invalidate_secondary_tlbs(). 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.. > FWIW, PPC's OpenCAPI support (drivers/misc/ocxl/link.c) also uses invalidate_range(). > Though IIUC, the use case is the same as a "traditional" IOMMU, where a device can > share the CPU's page tables, so maybe the devices can be considered IOMMUs in practice, > if not in name? OpenCAPI is an IOMMU HW for sure. PPC just doesn't have integration with the drivers/iommu infrastructure. > I have patches coded up. Assuming testing goes well, I'll post them regardless > of the OCXL side of things. I've disliked KVM's one-off use of invalidate_range() > for a long time, this is a good excuse to get rid of it before KVM gains more usage. Nice! Thanks, 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-30 23:09 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 [this message] 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 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=ZHaCAJI+OgIfDWSx@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.