* [PATCH] drm/ttm: Refuse to fault (prime-) imported pages
@ 2014-01-03 10:22 Thomas Hellstrom
2014-01-07 9:14 ` Daniel Vetter
0 siblings, 1 reply; 3+ messages in thread
From: Thomas Hellstrom @ 2014-01-03 10:22 UTC (permalink / raw)
To: dri-devel; +Cc: Thomas Hellstrom
This is illegal for at least two reasons:
1) While it may work on some platforms / iommus, obtaining page pointers from
mapped sg-lists is illegal, since the DMA API allows page pointer information
to be destroyed in the sg mapping process.
2) TTM has no way of determining the linear kernel map caching state of the
underlying pages. PTEs with conflicting caching state pointing to the same
pfn is not allowed.
TTM operations touching pages of imported sg-tables should be redirected through
the proper dma-buf operations.
Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
---
drivers/gpu/drm/ttm/ttm_bo_vm.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index cdda784..12d7f53 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -132,6 +132,15 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
return VM_FAULT_NOPAGE;
}
+ /*
+ * Refuse to fault imported pages. This should be handled
+ * (if at all) by redirecting mmap to the exporter.
+ */
+ if (bo->ttm && (bo->ttm->page_flags & TTM_PAGE_FLAG_SG)) {
+ retval = VM_FAULT_SIGBUS;
+ goto out_unlock;
+ }
+
if (bdev->driver->fault_reserve_notify) {
ret = bdev->driver->fault_reserve_notify(bo);
switch (ret) {
--
1.7.10.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/ttm: Refuse to fault (prime-) imported pages
2014-01-03 10:22 [PATCH] drm/ttm: Refuse to fault (prime-) imported pages Thomas Hellstrom
@ 2014-01-07 9:14 ` Daniel Vetter
2014-01-07 9:45 ` Thomas Hellstrom
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Vetter @ 2014-01-07 9:14 UTC (permalink / raw)
To: Thomas Hellstrom; +Cc: dri-devel
On Fri, Jan 03, 2014 at 11:22:17AM +0100, Thomas Hellstrom wrote:
> This is illegal for at least two reasons:
>
> 1) While it may work on some platforms / iommus, obtaining page pointers from
> mapped sg-lists is illegal, since the DMA API allows page pointer information
> to be destroyed in the sg mapping process.
>
> 2) TTM has no way of determining the linear kernel map caching state of the
> underlying pages. PTEs with conflicting caching state pointing to the same
> pfn is not allowed.
>
> TTM operations touching pages of imported sg-tables should be redirected through
> the proper dma-buf operations.
>
> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
Shouldn't we do something similar in the kmap helpers ttm_bo_util.c like
ttm_bo_kmap and ttm_bo_move_memcpy? Maybe just a BUG to catch driver bugs.
Otherwise this sounds like the right thing to do, so Acked.
-Daniel
> ---
> drivers/gpu/drm/ttm/ttm_bo_vm.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> index cdda784..12d7f53 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> @@ -132,6 +132,15 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
> return VM_FAULT_NOPAGE;
> }
>
> + /*
> + * Refuse to fault imported pages. This should be handled
> + * (if at all) by redirecting mmap to the exporter.
> + */
> + if (bo->ttm && (bo->ttm->page_flags & TTM_PAGE_FLAG_SG)) {
> + retval = VM_FAULT_SIGBUS;
> + goto out_unlock;
> + }
> +
> if (bdev->driver->fault_reserve_notify) {
> ret = bdev->driver->fault_reserve_notify(bo);
> switch (ret) {
> --
> 1.7.10.4
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/ttm: Refuse to fault (prime-) imported pages
2014-01-07 9:14 ` Daniel Vetter
@ 2014-01-07 9:45 ` Thomas Hellstrom
0 siblings, 0 replies; 3+ messages in thread
From: Thomas Hellstrom @ 2014-01-07 9:45 UTC (permalink / raw)
To: Daniel Vetter; +Cc: dri-devel
On 01/07/2014 10:14 AM, Daniel Vetter wrote:
> On Fri, Jan 03, 2014 at 11:22:17AM +0100, Thomas Hellstrom wrote:
>> This is illegal for at least two reasons:
>>
>> 1) While it may work on some platforms / iommus, obtaining page pointers from
>> mapped sg-lists is illegal, since the DMA API allows page pointer information
>> to be destroyed in the sg mapping process.
>>
>> 2) TTM has no way of determining the linear kernel map caching state of the
>> underlying pages. PTEs with conflicting caching state pointing to the same
>> pfn is not allowed.
>>
>> TTM operations touching pages of imported sg-tables should be redirected through
>> the proper dma-buf operations.
>>
>> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
> Shouldn't we do something similar in the kmap helpers ttm_bo_util.c like
> ttm_bo_kmap and ttm_bo_move_memcpy? Maybe just a BUG to catch driver bugs.
> Otherwise this sounds like the right thing to do, so Acked.
> -Daniel
Indeed, although I think that TTM should by default reroute the
ttm_bo_kmap operations through the
correct dma-buf ops. Haven't gotten around to do that yet.
/Thomas
>> ---
>> drivers/gpu/drm/ttm/ttm_bo_vm.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
>> index cdda784..12d7f53 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
>> @@ -132,6 +132,15 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>> return VM_FAULT_NOPAGE;
>> }
>>
>> + /*
>> + * Refuse to fault imported pages. This should be handled
>> + * (if at all) by redirecting mmap to the exporter.
>> + */
>> + if (bo->ttm && (bo->ttm->page_flags & TTM_PAGE_FLAG_SG)) {
>> + retval = VM_FAULT_SIGBUS;
>> + goto out_unlock;
>> + }
>> +
>> if (bdev->driver->fault_reserve_notify) {
>> ret = bdev->driver->fault_reserve_notify(bo);
>> switch (ret) {
>> --
>> 1.7.10.4
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/mailman/listinfo/dri-devel&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=l5Ago9ekmVFZ3c4M6eauqrJWGwjf6fTb%2BP3CxbBFkVM%3D%0A&m=3HWzF0bywXXmhWUQ1pCmbecMKkpEmnzo1A%2FnuPHbTX8%3D%0A&s=7209cea9c0020dea424c53ca15e12218f0370076ed1105a55775ea3d39a2b267
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-01-07 9:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-03 10:22 [PATCH] drm/ttm: Refuse to fault (prime-) imported pages Thomas Hellstrom
2014-01-07 9:14 ` Daniel Vetter
2014-01-07 9:45 ` Thomas Hellstrom
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.