* Fence wait in mmu_interval_notifier_ops::invalidate @ 2020-12-09 16:36 Thomas Hellström (Intel) 2020-12-09 16:37 ` Jason Gunthorpe 0 siblings, 1 reply; 11+ messages in thread From: Thomas Hellström (Intel) @ 2020-12-09 16:36 UTC (permalink / raw) To: Jason Gunthorpe, Christian König; +Cc: DRI Development Jason, Christian In most implementations of the callback mentioned in the subject there's a fence wait. What exactly is it needed for? Thanks, Thomas _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Fence wait in mmu_interval_notifier_ops::invalidate 2020-12-09 16:36 Fence wait in mmu_interval_notifier_ops::invalidate Thomas Hellström (Intel) @ 2020-12-09 16:37 ` Jason Gunthorpe 2020-12-09 16:46 ` Thomas Hellström (Intel) 0 siblings, 1 reply; 11+ messages in thread From: Jason Gunthorpe @ 2020-12-09 16:37 UTC (permalink / raw) To: Thomas Hellström (Intel); +Cc: Christian König, DRI Development On Wed, Dec 09, 2020 at 05:36:16PM +0100, Thomas Hellström (Intel) wrote: > Jason, Christian > > In most implementations of the callback mentioned in the subject there's a > fence wait. > What exactly is it needed for? Invalidate must stop DMA before returning, so presumably drivers using a dma fence are relying on a dma fence mechanism to stop DMA. Jason _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Fence wait in mmu_interval_notifier_ops::invalidate 2020-12-09 16:37 ` Jason Gunthorpe @ 2020-12-09 16:46 ` Thomas Hellström (Intel) 2020-12-10 10:53 ` Christian König 0 siblings, 1 reply; 11+ messages in thread From: Thomas Hellström (Intel) @ 2020-12-09 16:46 UTC (permalink / raw) To: Jason Gunthorpe; +Cc: Christian König, DRI Development On 12/9/20 5:37 PM, Jason Gunthorpe wrote: > On Wed, Dec 09, 2020 at 05:36:16PM +0100, Thomas Hellström (Intel) wrote: >> Jason, Christian >> >> In most implementations of the callback mentioned in the subject there's a >> fence wait. >> What exactly is it needed for? > Invalidate must stop DMA before returning, so presumably drivers using > a dma fence are relying on a dma fence mechanism to stop DMA. Yes, so far I follow, but what's the reason drivers need to stop DMA? Is it for invlidation before breaking COW after fork or something related? Thanks, Thomas > > Jason _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Fence wait in mmu_interval_notifier_ops::invalidate 2020-12-09 16:46 ` Thomas Hellström (Intel) @ 2020-12-10 10:53 ` Christian König 2020-12-11 7:50 ` Thomas Hellström (Intel) 0 siblings, 1 reply; 11+ messages in thread From: Christian König @ 2020-12-10 10:53 UTC (permalink / raw) To: Thomas Hellström (Intel), Jason Gunthorpe; +Cc: DRI Development Am 09.12.20 um 17:46 schrieb Thomas Hellström (Intel): > > On 12/9/20 5:37 PM, Jason Gunthorpe wrote: >> On Wed, Dec 09, 2020 at 05:36:16PM +0100, Thomas Hellström (Intel) >> wrote: >>> Jason, Christian >>> >>> In most implementations of the callback mentioned in the subject >>> there's a >>> fence wait. >>> What exactly is it needed for? >> Invalidate must stop DMA before returning, so presumably drivers using >> a dma fence are relying on a dma fence mechanism to stop DMA. > > Yes, so far I follow, but what's the reason drivers need to stop DMA? Well in general an invalidation means that the specified part of the page tables are updated, either with new addresses or new access flags. In both cases you need to stop the DMA because you could otherwise work with stale data, e.g. read/write with the wrong addresses or write to a read only region etc... > Is it for invlidation before breaking COW after fork or something > related? This is just one of many use cases which could invalidate a range. But there are many more, both from the kernel as well as userspace. Just imaging that userspace first mmaps() some anonymous memory r/w, starts a DMA to it and while the DMA is ongoing does a readonly mmap() of libc to the same location. Since most hardware doesn't have recoverable page faults guess what would happen if we don't wait for the DMA to finish? That would be a security hole you can push an elephant through :) Cheers, Christian. > > Thanks, > > Thomas > >> >> Jason _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Fence wait in mmu_interval_notifier_ops::invalidate 2020-12-10 10:53 ` Christian König @ 2020-12-11 7:50 ` Thomas Hellström (Intel) 2020-12-11 8:57 ` Christian König 2020-12-11 12:46 ` Jason Gunthorpe 0 siblings, 2 replies; 11+ messages in thread From: Thomas Hellström (Intel) @ 2020-12-11 7:50 UTC (permalink / raw) To: Christian König, Jason Gunthorpe; +Cc: DRI Development Hi, Christian Thanks for the reply. On 12/10/20 11:53 AM, Christian König wrote: > Am 09.12.20 um 17:46 schrieb Thomas Hellström (Intel): >> >> On 12/9/20 5:37 PM, Jason Gunthorpe wrote: >>> On Wed, Dec 09, 2020 at 05:36:16PM +0100, Thomas Hellström (Intel) >>> wrote: >>>> Jason, Christian >>>> >>>> In most implementations of the callback mentioned in the subject >>>> there's a >>>> fence wait. >>>> What exactly is it needed for? >>> Invalidate must stop DMA before returning, so presumably drivers using >>> a dma fence are relying on a dma fence mechanism to stop DMA. >> >> Yes, so far I follow, but what's the reason drivers need to stop DMA? > > Well in general an invalidation means that the specified part of the > page tables are updated, either with new addresses or new access flags. > > In both cases you need to stop the DMA because you could otherwise > work with stale data, e.g. read/write with the wrong addresses or > write to a read only region etc... Yes. That's clear. I'm just trying to understand the complete implications of doing that. > >> Is it for invlidation before breaking COW after fork or something >> related? > > This is just one of many use cases which could invalidate a range. But > there are many more, both from the kernel as well as userspace. > > Just imaging that userspace first mmaps() some anonymous memory r/w, > starts a DMA to it and while the DMA is ongoing does a readonly mmap() > of libc to the same location. My understanding of this particular case is that hardware would continue to DMA to orphaned pages that are pinned until the driver is done with DMA, unless hardware would somehow in-flight pick up the new PTE addresses pointing to libc but not the protection? Thanks, Thomas > > Since most hardware doesn't have recoverable page faults guess what > would happen if we don't wait for the DMA to finish? That would be a > security hole you can push an elephant through :) > > Cheers, > Christian. > >> >> Thanks, >> >> Thomas >> >>> >>> Jason _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Fence wait in mmu_interval_notifier_ops::invalidate 2020-12-11 7:50 ` Thomas Hellström (Intel) @ 2020-12-11 8:57 ` Christian König 2020-12-11 9:37 ` Thomas Hellström (Intel) 2020-12-11 12:46 ` Jason Gunthorpe 1 sibling, 1 reply; 11+ messages in thread From: Christian König @ 2020-12-11 8:57 UTC (permalink / raw) To: Thomas Hellström (Intel), Jason Gunthorpe; +Cc: DRI Development Am 11.12.20 um 08:50 schrieb Thomas Hellström (Intel): > Hi, Christian > > Thanks for the reply. > > On 12/10/20 11:53 AM, Christian König wrote: >> Am 09.12.20 um 17:46 schrieb Thomas Hellström (Intel): >>> >>> On 12/9/20 5:37 PM, Jason Gunthorpe wrote: >>>> On Wed, Dec 09, 2020 at 05:36:16PM +0100, Thomas Hellström (Intel) >>>> wrote: >>>>> Jason, Christian >>>>> >>>>> In most implementations of the callback mentioned in the subject >>>>> there's a >>>>> fence wait. >>>>> What exactly is it needed for? >>>> Invalidate must stop DMA before returning, so presumably drivers using >>>> a dma fence are relying on a dma fence mechanism to stop DMA. >>> >>> Yes, so far I follow, but what's the reason drivers need to stop DMA? >> >> Well in general an invalidation means that the specified part of the >> page tables are updated, either with new addresses or new access flags. >> >> In both cases you need to stop the DMA because you could otherwise >> work with stale data, e.g. read/write with the wrong addresses or >> write to a read only region etc... > > Yes. That's clear. I'm just trying to understand the complete > implications of doing that. > >> >>> Is it for invlidation before breaking COW after fork or something >>> related? >> >> This is just one of many use cases which could invalidate a range. >> But there are many more, both from the kernel as well as userspace. >> >> Just imaging that userspace first mmaps() some anonymous memory r/w, >> starts a DMA to it and while the DMA is ongoing does a readonly >> mmap() of libc to the same location. > > My understanding of this particular case is that hardware would > continue to DMA to orphaned pages that are pinned until the driver is > done with DMA, unless hardware would somehow in-flight pick up the new > PTE addresses pointing to libc but not the protection? Exactly that is not guaranteed under all circumstances. Especially since HMM tries to avoid grabbing a reference to the underlying pages. And it depends when the destination addresses of the DMA are read and when the access flags are evaluated. But even when it causes no security problem the requirement we have to fulfill here is that the DMA is coherent. In other words we either have to delay updates to the page tables until the DMA operation is completed or apply both address and access flag changes in a way the DMA operation immediately sees it as well. Regards, Christian. > > Thanks, > > Thomas > > >> >> Since most hardware doesn't have recoverable page faults guess what >> would happen if we don't wait for the DMA to finish? That would be a >> security hole you can push an elephant through :) >> >> Cheers, >> Christian. >> >>> >>> Thanks, >>> >>> Thomas >>> >>>> >>>> Jason _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Fence wait in mmu_interval_notifier_ops::invalidate 2020-12-11 8:57 ` Christian König @ 2020-12-11 9:37 ` Thomas Hellström (Intel) 0 siblings, 0 replies; 11+ messages in thread From: Thomas Hellström (Intel) @ 2020-12-11 9:37 UTC (permalink / raw) To: Christian König, Jason Gunthorpe; +Cc: DRI Development On 12/11/20 9:57 AM, Christian König wrote: > Am 11.12.20 um 08:50 schrieb Thomas Hellström (Intel): >> Hi, Christian >> >> Thanks for the reply. >> >> On 12/10/20 11:53 AM, Christian König wrote: >>> Am 09.12.20 um 17:46 schrieb Thomas Hellström (Intel): >>>> >>>> On 12/9/20 5:37 PM, Jason Gunthorpe wrote: >>>>> On Wed, Dec 09, 2020 at 05:36:16PM +0100, Thomas Hellström (Intel) >>>>> wrote: >>>>>> Jason, Christian >>>>>> >>>>>> In most implementations of the callback mentioned in the subject >>>>>> there's a >>>>>> fence wait. >>>>>> What exactly is it needed for? >>>>> Invalidate must stop DMA before returning, so presumably drivers >>>>> using >>>>> a dma fence are relying on a dma fence mechanism to stop DMA. >>>> >>>> Yes, so far I follow, but what's the reason drivers need to stop DMA? >>> >>> Well in general an invalidation means that the specified part of the >>> page tables are updated, either with new addresses or new access flags. >>> >>> In both cases you need to stop the DMA because you could otherwise >>> work with stale data, e.g. read/write with the wrong addresses or >>> write to a read only region etc... >> >> Yes. That's clear. I'm just trying to understand the complete >> implications of doing that. >> >>> >>>> Is it for invlidation before breaking COW after fork or something >>>> related? >>> >>> This is just one of many use cases which could invalidate a range. >>> But there are many more, both from the kernel as well as userspace. >>> >>> Just imaging that userspace first mmaps() some anonymous memory r/w, >>> starts a DMA to it and while the DMA is ongoing does a readonly >>> mmap() of libc to the same location. >> >> My understanding of this particular case is that hardware would >> continue to DMA to orphaned pages that are pinned until the driver is >> done with DMA, unless hardware would somehow in-flight pick up the >> new PTE addresses pointing to libc but not the protection? > > Exactly that is not guaranteed under all circumstances. Especially > since HMM tries to avoid grabbing a reference to the underlying pages. > And it depends when the destination addresses of the DMA are read and > when the access flags are evaluated. > > But even when it causes no security problem the requirement we have to > fulfill here is that the DMA is coherent. In other words we either > have to delay updates to the page tables until the DMA operation is > completed or apply both address and access flag changes in a way the > DMA operation immediately sees it as well. > > Regards, > Christian. > Got it. Thanks! Thomas _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Fence wait in mmu_interval_notifier_ops::invalidate 2020-12-11 7:50 ` Thomas Hellström (Intel) 2020-12-11 8:57 ` Christian König @ 2020-12-11 12:46 ` Jason Gunthorpe 2020-12-13 15:09 ` Thomas Hellström (Intel) 1 sibling, 1 reply; 11+ messages in thread From: Jason Gunthorpe @ 2020-12-11 12:46 UTC (permalink / raw) To: Thomas Hellström (Intel); +Cc: Christian König, DRI Development On Fri, Dec 11, 2020 at 08:50:53AM +0100, Thomas Hellström (Intel) wrote: > My understanding of this particular case is that hardware would continue to > DMA to orphaned pages that are pinned until the driver is done with > DMA, mmu notifier replaces pinning as the locking mechanism. Drivers using mmu notifier should not be taking pins. Keep in mind this was all built for HW with real shadow page tables that can do fine grained manipulation. The GPU version of this to instead manipulate a command queue is a big aberration from what was intended. Jason _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Fence wait in mmu_interval_notifier_ops::invalidate 2020-12-11 12:46 ` Jason Gunthorpe @ 2020-12-13 15:09 ` Thomas Hellström (Intel) 2020-12-14 9:52 ` Daniel Vetter 0 siblings, 1 reply; 11+ messages in thread From: Thomas Hellström (Intel) @ 2020-12-13 15:09 UTC (permalink / raw) To: Jason Gunthorpe; +Cc: Christian König, DRI Development On 12/11/20 1:46 PM, Jason Gunthorpe wrote: > On Fri, Dec 11, 2020 at 08:50:53AM +0100, Thomas Hellström (Intel) wrote: > >> My understanding of this particular case is that hardware would continue to >> DMA to orphaned pages that are pinned until the driver is done with >> DMA, > mmu notifier replaces pinning as the locking mechanism. Drivers using > mmu notifier should not be taking pins. > > Keep in mind this was all built for HW with real shadow page tables > that can do fine grained manipulation. OK yes, that makes sense and in that context the fence wait is easier to understand. Looks like for example the radeon driver is using the notifier + get_user_pages() but there it looks like it's used to avoid having get_user_pages() clash with invalidation. /Thomas > > The GPU version of this to instead manipulate a command queue is a big > aberration from what was intended. > > Jason _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Fence wait in mmu_interval_notifier_ops::invalidate 2020-12-13 15:09 ` Thomas Hellström (Intel) @ 2020-12-14 9:52 ` Daniel Vetter 2020-12-14 10:21 ` Christian König 0 siblings, 1 reply; 11+ messages in thread From: Daniel Vetter @ 2020-12-14 9:52 UTC (permalink / raw) To: Thomas Hellström (Intel) Cc: DRI Development, Christian König, Jason Gunthorpe On Sun, Dec 13, 2020 at 04:09:25PM +0100, Thomas Hellström (Intel) wrote: > > On 12/11/20 1:46 PM, Jason Gunthorpe wrote: > > On Fri, Dec 11, 2020 at 08:50:53AM +0100, Thomas Hellström (Intel) wrote: > > > > > My understanding of this particular case is that hardware would continue to > > > DMA to orphaned pages that are pinned until the driver is done with > > > DMA, > > mmu notifier replaces pinning as the locking mechanism. Drivers using > > mmu notifier should not be taking pins. > > > > Keep in mind this was all built for HW with real shadow page tables > > that can do fine grained manipulation. > OK yes, that makes sense and in that context the fence wait is easier to > understand. Looks like for example the radeon driver is using the notifier + > get_user_pages() but there it looks like it's used to avoid having > get_user_pages() clash with invalidation. I think the radeon userptr implementation is bad enough that Christian wants to outright remove it. At least he keeps talking about doing that. So maybe not a good example to look at :-) -Daniel > > The GPU version of this to instead manipulate a command queue is a big > > aberration from what was intended. > > > > Jason > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Fence wait in mmu_interval_notifier_ops::invalidate 2020-12-14 9:52 ` Daniel Vetter @ 2020-12-14 10:21 ` Christian König 0 siblings, 0 replies; 11+ messages in thread From: Christian König @ 2020-12-14 10:21 UTC (permalink / raw) To: Daniel Vetter, Thomas Hellström (Intel) Cc: DRI Development, Jason Gunthorpe Am 14.12.20 um 10:52 schrieb Daniel Vetter: > On Sun, Dec 13, 2020 at 04:09:25PM +0100, Thomas Hellström (Intel) wrote: >> On 12/11/20 1:46 PM, Jason Gunthorpe wrote: >>> On Fri, Dec 11, 2020 at 08:50:53AM +0100, Thomas Hellström (Intel) wrote: >>> >>>> My understanding of this particular case is that hardware would continue to >>>> DMA to orphaned pages that are pinned until the driver is done with >>>> DMA, >>> mmu notifier replaces pinning as the locking mechanism. Drivers using >>> mmu notifier should not be taking pins. >>> >>> Keep in mind this was all built for HW with real shadow page tables >>> that can do fine grained manipulation. >> OK yes, that makes sense and in that context the fence wait is easier to >> understand. Looks like for example the radeon driver is using the notifier + >> get_user_pages() but there it looks like it's used to avoid having >> get_user_pages() clash with invalidation. > I think the radeon userptr implementation is bad enough that Christian > wants to outright remove it. At least he keeps talking about doing that. Oh, yes :) Key point is having time for that. Christian. > > So maybe not a good example to look at :-) > -Daniel > >>> The GPU version of this to instead manipulate a command queue is a big >>> aberration from what was intended. >>> >>> Jason >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=04%7C01%7Cchristian.koenig%40amd.com%7C4c7b622de2cf4b71c44a08d8a015ffc3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637435363637046663%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=qJXWXbMXoUfDqpAsK76TIBDj8nOu9xZ66GLXCjZcNQM%3D&reserved=0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-12-14 10:21 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-12-09 16:36 Fence wait in mmu_interval_notifier_ops::invalidate Thomas Hellström (Intel) 2020-12-09 16:37 ` Jason Gunthorpe 2020-12-09 16:46 ` Thomas Hellström (Intel) 2020-12-10 10:53 ` Christian König 2020-12-11 7:50 ` Thomas Hellström (Intel) 2020-12-11 8:57 ` Christian König 2020-12-11 9:37 ` Thomas Hellström (Intel) 2020-12-11 12:46 ` Jason Gunthorpe 2020-12-13 15:09 ` Thomas Hellström (Intel) 2020-12-14 9:52 ` Daniel Vetter 2020-12-14 10:21 ` Christian König
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.