diff mbox

[3/7] drm/udl: import prime-fds with proper page-alignment

Message ID 1390245989-13280-3-git-send-email-dh.herrmann@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

David Herrmann Jan. 20, 2014, 7:26 p.m. UTC
Instead of rounding down to the next lower page-boundary, round up.
dma-buf guarantees that we can map buffers in multiples of a page, so if
an exporter does not page-align, do it ourselves.

This avoids issues if the exported buffer contains an unaligned size and
we crop it. In this case, the buffer is too small for the UDL CRTC. So we
round up to page-size now and avoid black borders. Worst case is we end up
reading out some random kernel memory, but we can never fault as the whole
page has the same access-rights. And in this case it's an issue of the
buggy exporting driver, not the importing one.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/udl/udl_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Daniel Vetter Jan. 21, 2014, 9:41 a.m. UTC | #1
On Mon, Jan 20, 2014 at 08:26:25PM +0100, David Herrmann wrote:
> Instead of rounding down to the next lower page-boundary, round up.
> dma-buf guarantees that we can map buffers in multiples of a page, so if
> an exporter does not page-align, do it ourselves.
> 
> This avoids issues if the exported buffer contains an unaligned size and
> we crop it. In this case, the buffer is too small for the UDL CRTC. So we
> round up to page-size now and avoid black borders. Worst case is we end up
> reading out some random kernel memory, but we can never fault as the whole
> page has the same access-rights. And in this case it's an issue of the
> buggy exporting driver, not the importing one.
> 
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---
>  drivers/gpu/drm/udl/udl_gem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
> index df963a1..1069e57 100644
> --- a/drivers/gpu/drm/udl/udl_gem.c
> +++ b/drivers/gpu/drm/udl/udl_gem.c
> @@ -227,7 +227,7 @@ static int udl_prime_create(struct drm_device *dev,
>  	struct udl_gem_object *obj;
>  	int npages;
>  
> -	npages = size / PAGE_SIZE;
> +	npages = PAGE_ALIGN(size) >> PAGE_SHIFT;

Imo the proper patch would be to reject exporting anything which isn't
page-aligned as a dma-buf in the driver core (together with a quick review
of the docs to make sure this requirement is clear). This is just a bug in
the exporter.
-Daniel

>  
>  	*obj_p = NULL;
>  	obj = udl_gem_alloc_object(dev, npages * PAGE_SIZE);
> -- 
> 1.8.5.3
>
David Herrmann Jan. 23, 2014, 12:51 p.m. UTC | #2
Hi

On Tue, Jan 21, 2014 at 10:41 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Jan 20, 2014 at 08:26:25PM +0100, David Herrmann wrote:
>> Instead of rounding down to the next lower page-boundary, round up.
>> dma-buf guarantees that we can map buffers in multiples of a page, so if
>> an exporter does not page-align, do it ourselves.
>>
>> This avoids issues if the exported buffer contains an unaligned size and
>> we crop it. In this case, the buffer is too small for the UDL CRTC. So we
>> round up to page-size now and avoid black borders. Worst case is we end up
>> reading out some random kernel memory, but we can never fault as the whole
>> page has the same access-rights. And in this case it's an issue of the
>> buggy exporting driver, not the importing one.
>>
>> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
>> ---
>>  drivers/gpu/drm/udl/udl_gem.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
>> index df963a1..1069e57 100644
>> --- a/drivers/gpu/drm/udl/udl_gem.c
>> +++ b/drivers/gpu/drm/udl/udl_gem.c
>> @@ -227,7 +227,7 @@ static int udl_prime_create(struct drm_device *dev,
>>       struct udl_gem_object *obj;
>>       int npages;
>>
>> -     npages = size / PAGE_SIZE;
>> +     npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
>
> Imo the proper patch would be to reject exporting anything which isn't
> page-aligned as a dma-buf in the driver core (together with a quick review
> of the docs to make sure this requirement is clear). This is just a bug in
> the exporter.

Ok, I've dropped this one for now. I think the best way to deal with
it is to make dma_buf disallow any non-aligned sizes during
initialization. So we should just assume it's always page-aligned.

Thanks
David
diff mbox

Patch

diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
index df963a1..1069e57 100644
--- a/drivers/gpu/drm/udl/udl_gem.c
+++ b/drivers/gpu/drm/udl/udl_gem.c
@@ -227,7 +227,7 @@  static int udl_prime_create(struct drm_device *dev,
 	struct udl_gem_object *obj;
 	int npages;
 
-	npages = size / PAGE_SIZE;
+	npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
 
 	*obj_p = NULL;
 	obj = udl_gem_alloc_object(dev, npages * PAGE_SIZE);