From: Thomas Hellstrom <[email protected]>
The drm/ttm module is using a modified on-stack copy of the
struct vm_area_struct to be able to set a page protection with customized
caching. Fix that by adding a vmf_insert_mixed_prot() function similar
to the existing vmf_insert_pfn_prot() for use with drm/ttm.
I'd like to merge this through a drm tree.
Changes since v1:
*) Formatting fixes in patch 1
*) Updated commit message of patch 2.
Changes since v2:
*) Moved vmf_insert_mixed_prot() export to patch 2 (Michal Hocko)
*) Documented under which conditions it's safe to use a page protection
different from struct vm_area_struct::vm_page_prot. (Michal Hocko)
Changes since v3:
*) More documentation regarding under which conditions it's safe to use a
page protection different from struct vm_area_struct::vm_page_prot. This
time also in core vm. (Michal Hocko)
Cc: Andrew Morton <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: "Matthew Wilcox (Oracle)" <[email protected]>
Cc: "Kirill A. Shutemov" <[email protected]>
Cc: Ralph Campbell <[email protected]>
Cc: "Jérôme Glisse" <[email protected]>
Cc: "Christian König" <[email protected]>
--
2.21.0
From: Thomas Hellstrom <[email protected]>
TTM graphics buffer objects may, transparently to user-space, move
between IO and system memory. When that happens, all PTEs pointing to the
old location are zapped before the move and then faulted in again if
needed. When that happens, the page protection caching mode- and
encryption bits may change and be different from those of
struct vm_area_struct::vm_page_prot.
We were using an ugly hack to set the page protection correctly.
Fix that and instead export and use vmf_insert_mixed_prot() or use
vmf_insert_pfn_prot().
Also get the default page protection from
struct vm_area_struct::vm_page_prot rather than using vm_get_page_prot().
This way we catch modifications done by the vm system for drivers that
want write-notification.
Cc: Andrew Morton <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: "Matthew Wilcox (Oracle)" <[email protected]>
Cc: "Kirill A. Shutemov" <[email protected]>
Cc: Ralph Campbell <[email protected]>
Cc: "Jérôme Glisse" <[email protected]>
Cc: "Christian König" <[email protected]>
Signed-off-by: Thomas Hellstrom <[email protected]>
Reviewed-by: Christian König <[email protected]>
---
drivers/gpu/drm/ttm/ttm_bo_vm.c | 22 +++++++++++++++-------
mm/memory.c | 1 +
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 3e8c3de91ae4..78bfab81cf04 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -173,7 +173,6 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault *vmf,
pgoff_t num_prefault)
{
struct vm_area_struct *vma = vmf->vma;
- struct vm_area_struct cvma = *vma;
struct ttm_buffer_object *bo = vma->vm_private_data;
struct ttm_bo_device *bdev = bo->bdev;
unsigned long page_offset;
@@ -244,7 +243,7 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault *vmf,
goto out_io_unlock;
}
- cvma.vm_page_prot = ttm_io_prot(bo->mem.placement, prot);
+ prot = ttm_io_prot(bo->mem.placement, prot);
if (!bo->mem.bus.is_iomem) {
struct ttm_operation_ctx ctx = {
.interruptible = false,
@@ -260,7 +259,7 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault *vmf,
}
} else {
/* Iomem should not be marked encrypted */
- cvma.vm_page_prot = pgprot_decrypted(cvma.vm_page_prot);
+ prot = pgprot_decrypted(prot);
}
/*
@@ -283,11 +282,20 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault *vmf,
pfn = page_to_pfn(page);
}
+ /*
+ * Note that the value of @prot at this point may differ from
+ * the value of @vma->vm_page_prot in the caching- and
+ * encryption bits. This is because the exact location of the
+ * data may not be known at mmap() time and may also change
+ * at arbitrary times while the data is mmap'ed.
+ * See vmf_insert_mixed_prot() for a discussion.
+ */
if (vma->vm_flags & VM_MIXEDMAP)
- ret = vmf_insert_mixed(&cvma, address,
- __pfn_to_pfn_t(pfn, PFN_DEV));
+ ret = vmf_insert_mixed_prot(vma, address,
+ __pfn_to_pfn_t(pfn, PFN_DEV),
+ prot);
else
- ret = vmf_insert_pfn(&cvma, address, pfn);
+ ret = vmf_insert_pfn_prot(vma, address, pfn, prot);
/* Never error on prefaulted PTEs */
if (unlikely((ret & VM_FAULT_ERROR))) {
@@ -319,7 +327,7 @@ vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)
if (ret)
return ret;
- prot = vm_get_page_prot(vma->vm_flags);
+ prot = vma->vm_page_prot;
ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT);
if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
return ret;
diff --git a/mm/memory.c b/mm/memory.c
index 269a8a871e83..0f262acd1018 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1798,6 +1798,7 @@ vm_fault_t vmf_insert_mixed_prot(struct vm_area_struct *vma, unsigned long addr,
{
return __vm_insert_mixed(vma, addr, pfn, pgprot, false);
}
+EXPORT_SYMBOL(vmf_insert_mixed_prot);
vm_fault_t vmf_insert_mixed(struct vm_area_struct *vma, unsigned long addr,
pfn_t pfn)
--
2.21.0
Andrew,
On 12/12/19 9:47 AM, Thomas Hellström (VMware) wrote:
> From: Thomas Hellstrom <[email protected]>
>
> The drm/ttm module is using a modified on-stack copy of the
> struct vm_area_struct to be able to set a page protection with customized
> caching. Fix that by adding a vmf_insert_mixed_prot() function similar
> to the existing vmf_insert_pfn_prot() for use with drm/ttm.
>
> I'd like to merge this through a drm tree.
>
> Changes since v1:
> *) Formatting fixes in patch 1
> *) Updated commit message of patch 2.
> Changes since v2:
> *) Moved vmf_insert_mixed_prot() export to patch 2 (Michal Hocko)
> *) Documented under which conditions it's safe to use a page protection
> different from struct vm_area_struct::vm_page_prot. (Michal Hocko)
> Changes since v3:
> *) More documentation regarding under which conditions it's safe to use a
> page protection different from struct vm_area_struct::vm_page_prot. This
> time also in core vm. (Michal Hocko)
>
> Cc: Andrew Morton <[email protected]>
> Cc: Michal Hocko <[email protected]>
> Cc: "Matthew Wilcox (Oracle)" <[email protected]>
> Cc: "Kirill A. Shutemov" <[email protected]>
> Cc: Ralph Campbell <[email protected]>
> Cc: "Jérôme Glisse" <[email protected]>
> Cc: "Christian König" <[email protected]>
>
Seems all concerns with this series have been addressed. Could I have an
ack to merge this through a DRM tree?
Thanks,
Thomas
On Fri, 20 Dec 2019 09:06:08 +0100 Thomas Hellstr?m (VMware) <[email protected]> wrote:
> Andrew,
>
> On 12/12/19 9:47 AM, Thomas Hellstr?m (VMware) wrote:
> > From: Thomas Hellstrom <[email protected]>
> >
> > The drm/ttm module is using a modified on-stack copy of the
> > struct vm_area_struct to be able to set a page protection with customized
> > caching. Fix that by adding a vmf_insert_mixed_prot() function similar
> > to the existing vmf_insert_pfn_prot() for use with drm/ttm.
> >
> > I'd like to merge this through a drm tree.
> >
> > Changes since v1:
> > *) Formatting fixes in patch 1
> > *) Updated commit message of patch 2.
> > Changes since v2:
> > *) Moved vmf_insert_mixed_prot() export to patch 2 (Michal Hocko)
> > *) Documented under which conditions it's safe to use a page protection
> > different from struct vm_area_struct::vm_page_prot. (Michal Hocko)
> > Changes since v3:
> > *) More documentation regarding under which conditions it's safe to use a
> > page protection different from struct vm_area_struct::vm_page_prot. This
> > time also in core vm. (Michal Hocko)
> >
> > Cc: Andrew Morton <[email protected]>
> > Cc: Michal Hocko <[email protected]>
> > Cc: "Matthew Wilcox (Oracle)" <[email protected]>
> > Cc: "Kirill A. Shutemov" <[email protected]>
> > Cc: Ralph Campbell <[email protected]>
> > Cc: "J?r?me Glisse" <[email protected]>
> > Cc: "Christian K?nig" <[email protected]>
> >
> Seems all concerns with this series have been addressed. Could I have an
> ack to merge this through a DRM tree?
>
Yes, please do that.
Acked-by: Andrew Morton <[email protected]>
for both.