From: Oleksandr Andrushchenko <[email protected]>
When GEM backing storage is allocated those are normal pages,
so there is no point using pgprot_writecombine while mmaping.
This fixes mismatch of buffer pages' memory attributes between
the frontend and backend which may cause screen artifacts.
Fixes: c575b7eeb89f ("drm/xen-front: Add support for Xen PV display frontend")
Signed-off-by: Oleksandr Andrushchenko <[email protected]>
Suggested-by: Julien Grall <[email protected]>
---
drivers/gpu/drm/xen/xen_drm_front_gem.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c b/drivers/gpu/drm/xen/xen_drm_front_gem.c
index d303a2e17f5e..9d5c03d7668d 100644
--- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
+++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
@@ -235,8 +235,7 @@ static int gem_mmap_obj(struct xen_gem_object *xen_obj,
vma->vm_flags &= ~VM_PFNMAP;
vma->vm_flags |= VM_MIXEDMAP;
vma->vm_pgoff = 0;
- vma->vm_page_prot =
- pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+ vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
/*
* vm_operations_struct.fault handler will be called if CPU access
@@ -283,7 +282,7 @@ void *xen_drm_front_gem_prime_vmap(struct drm_gem_object *gem_obj)
return NULL;
return vmap(xen_obj->pages, xen_obj->num_pages,
- VM_MAP, pgprot_writecombine(PAGE_KERNEL));
+ VM_MAP, PAGE_KERNEL);
}
void xen_drm_front_gem_prime_vunmap(struct drm_gem_object *gem_obj,
--
2.20.1
Hi Oleksandr,
On 1/29/19 3:04 PM, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <[email protected]>
>
> When GEM backing storage is allocated those are normal pages,
> so there is no point using pgprot_writecombine while mmaping.
> This fixes mismatch of buffer pages' memory attributes between
> the frontend and backend which may cause screen artifacts.
>
> Fixes: c575b7eeb89f ("drm/xen-front: Add support for Xen PV display frontend")
>
> Signed-off-by: Oleksandr Andrushchenko <[email protected]>
> Suggested-by: Julien Grall <[email protected]>
> ---
> drivers/gpu/drm/xen/xen_drm_front_gem.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c b/drivers/gpu/drm/xen/xen_drm_front_gem.c
> index d303a2e17f5e..9d5c03d7668d 100644
> --- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
> +++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
> @@ -235,8 +235,7 @@ static int gem_mmap_obj(struct xen_gem_object *xen_obj,
> vma->vm_flags &= ~VM_PFNMAP;
> vma->vm_flags |= VM_MIXEDMAP;
> vma->vm_pgoff = 0;
> - vma->vm_page_prot =
> - pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
> + vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
The patch looks good to me. It would be worth expanding the comment a
bit before to explain that we overwrite vm_page_prot to use cacheable
attribute as required by the Xen ABI.
With the comment updated:
Acked-by: Julien Grall <[email protected]>
Cheers,
--
Julien Grall
On 1/29/19 9:07 PM, Julien Grall wrote:
> Hi Oleksandr,
>
> On 1/29/19 3:04 PM, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko <[email protected]>
>>
>> When GEM backing storage is allocated those are normal pages,
>> so there is no point using pgprot_writecombine while mmaping.
>> This fixes mismatch of buffer pages' memory attributes between
>> the frontend and backend which may cause screen artifacts.
>>
>> Fixes: c575b7eeb89f ("drm/xen-front: Add support for Xen PV display
>> frontend")
>>
>> Signed-off-by: Oleksandr Andrushchenko
>> <[email protected]>
>> Suggested-by: Julien Grall <[email protected]>
>> ---
>> drivers/gpu/drm/xen/xen_drm_front_gem.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c
>> b/drivers/gpu/drm/xen/xen_drm_front_gem.c
>> index d303a2e17f5e..9d5c03d7668d 100644
>> --- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
>> +++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
>> @@ -235,8 +235,7 @@ static int gem_mmap_obj(struct xen_gem_object
>> *xen_obj,
>> vma->vm_flags &= ~VM_PFNMAP;
>> vma->vm_flags |= VM_MIXEDMAP;
>> vma->vm_pgoff = 0;
>> - vma->vm_page_prot =
>> - pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
>> + vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
>
> The patch looks good to me. It would be worth expanding the comment a
> bit before to explain that we overwrite vm_page_prot to use cacheable
> attribute as required by the Xen ABI.
>
Ok, then I'll put:
+ /*
+ * According to Xen on ARM ABI (xen/include/public/arch-arm.h):
+ * all memory which is shared with other entities in the system
+ * (including the hypervisor and other guests) must reside in memory
+ * which is mapped as Normal Inner Write-Back Outer Write-Back
+ * Inner-Shareable.
+ */
vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
Please let me know if this is not what you want
> With the comment updated:
>
> Acked-by: Julien Grall <[email protected]>
>
> Cheers,
>
Thank you,
Oleksandr
On 1/30/19 11:09 AM, Oleksandr Andrushchenko wrote:
> On 1/29/19 9:07 PM, Julien Grall wrote:
>> Hi Oleksandr,
>>
>> On 1/29/19 3:04 PM, Oleksandr Andrushchenko wrote:
>>> From: Oleksandr Andrushchenko <[email protected]>
>>>
>>> When GEM backing storage is allocated those are normal pages,
>>> so there is no point using pgprot_writecombine while mmaping.
>>> This fixes mismatch of buffer pages' memory attributes between
>>> the frontend and backend which may cause screen artifacts.
>>>
>>> Fixes: c575b7eeb89f ("drm/xen-front: Add support for Xen PV display
>>> frontend")
>>>
>>> Signed-off-by: Oleksandr Andrushchenko
>>> <[email protected]>
>>> Suggested-by: Julien Grall <[email protected]>
>>> ---
>>> drivers/gpu/drm/xen/xen_drm_front_gem.c | 5 ++---
>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c
>>> b/drivers/gpu/drm/xen/xen_drm_front_gem.c
>>> index d303a2e17f5e..9d5c03d7668d 100644
>>> --- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
>>> +++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
>>> @@ -235,8 +235,7 @@ static int gem_mmap_obj(struct xen_gem_object
>>> *xen_obj,
>>> vma->vm_flags &= ~VM_PFNMAP;
>>> vma->vm_flags |= VM_MIXEDMAP;
>>> vma->vm_pgoff = 0;
>>> - vma->vm_page_prot =
>>> - pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
>>> + vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
>>
>> The patch looks good to me. It would be worth expanding the comment a
>> bit before to explain that we overwrite vm_page_prot to use cacheable
>> attribute as required by the Xen ABI.
>>
> Ok, then I'll put:
>
> + /*
> + * According to Xen on ARM ABI (xen/include/public/arch-arm.h):
> + * all memory which is shared with other entities in the system
> + * (including the hypervisor and other guests) must reside in
> memory
> + * which is mapped as Normal Inner Write-Back Outer Write-Back
> + * Inner-Shareable.
> + */
> vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
> Please let me know if this is not what you want
>> With the comment updated:
>>
>> Acked-by: Julien Grall <[email protected]>
>>
If nobody objects I'll apply this to drm-misc-fixes next Monday
>> Cheers,
>>
> Thank you,
> Oleksandr
On 1/29/19 9:07 PM, Julien Grall wrote:
> Hi Oleksandr,
>
> On 1/29/19 3:04 PM, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko <[email protected]>
>>
>> When GEM backing storage is allocated those are normal pages,
>> so there is no point using pgprot_writecombine while mmaping.
>> This fixes mismatch of buffer pages' memory attributes between
>> the frontend and backend which may cause screen artifacts.
>>
>> Fixes: c575b7eeb89f ("drm/xen-front: Add support for Xen PV display
>> frontend")
>>
>> Signed-off-by: Oleksandr Andrushchenko
>> <[email protected]>
>> Suggested-by: Julien Grall <[email protected]>
>> ---
>> drivers/gpu/drm/xen/xen_drm_front_gem.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c
>> b/drivers/gpu/drm/xen/xen_drm_front_gem.c
>> index d303a2e17f5e..9d5c03d7668d 100644
>> --- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
>> +++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
>> @@ -235,8 +235,7 @@ static int gem_mmap_obj(struct xen_gem_object
>> *xen_obj,
>> vma->vm_flags &= ~VM_PFNMAP;
>> vma->vm_flags |= VM_MIXEDMAP;
>> vma->vm_pgoff = 0;
>> - vma->vm_page_prot =
>> - pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
>> + vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
>
> The patch looks good to me. It would be worth expanding the comment a
> bit before to explain that we overwrite vm_page_prot to use cacheable
> attribute as required by the Xen ABI.
>
> With the comment updated:
>
> Acked-by: Julien Grall <[email protected]>
>
> Cheers,
>
Applied to drm-misc-next