* [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler
@ 2019-12-12 11:34 Abdiel Janulgue
2019-12-12 11:45 ` Chris Wilson
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Abdiel Janulgue @ 2019-12-12 11:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
Fault handler to handle missing pages for lmem objects.
v3: Add get_vm_cpu_ops, iterate over all memory regions in the
lmem selftest, use remap_io_mapping.
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 40 +++++
drivers/gpu/drm/i915/gem/i915_gem_lmem.h | 6 +
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 32 +++-
drivers/gpu/drm/i915/gem/i915_gem_mman.h | 1 +
.../drm/i915/gem/selftests/i915_gem_mman.c | 137 +++++++++++++++---
5 files changed, 188 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
index 0e2bf6b7e143..bbe625935005 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
@@ -6,8 +6,36 @@
#include "intel_memory_region.h"
#include "gem/i915_gem_region.h"
#include "gem/i915_gem_lmem.h"
+#include "gem/i915_gem_mman.h"
#include "i915_drv.h"
+vm_fault_t vm_fault_iomem(struct vm_fault *vmf)
+{
+ struct vm_area_struct *area = vmf->vma;
+ struct i915_mmap_offset *priv = area->vm_private_data;
+ struct drm_i915_gem_object *obj = priv->obj;
+ struct intel_memory_region *mem = obj->mm.region;
+ unsigned long size = area->vm_end - area->vm_start;
+ bool write = area->vm_flags & VM_WRITE;
+ int ret;
+
+ /* Sanity check that we allow writing into this object */
+ if (i915_gem_object_is_readonly(obj) && write)
+ return VM_FAULT_SIGBUS;
+
+ ret = i915_gem_object_pin_pages(obj);
+ if (ret)
+ return i915_error_to_vmf_fault(ret);
+
+ ret = remap_io_mapping(area, area->vm_start,
+ i915_gem_object_lmem_io_pfn(obj, 0), size,
+ &mem->iomap);
+
+ i915_gem_object_unpin_pages(obj);
+
+ return i915_error_to_vmf_fault(ret);
+}
+
const struct drm_i915_gem_object_ops i915_gem_lmem_obj_ops = {
.flags = I915_GEM_OBJECT_HAS_IOMEM,
@@ -56,6 +84,18 @@ i915_gem_object_lmem_io_map(struct drm_i915_gem_object *obj,
return io_mapping_map_wc(&obj->mm.region->iomap, offset, size);
}
+unsigned long i915_gem_object_lmem_io_pfn(struct drm_i915_gem_object *obj,
+ unsigned long n)
+{
+ struct intel_memory_region *mem = obj->mm.region;
+ resource_size_t offset;
+
+ offset = i915_gem_object_get_dma_address(obj, n);
+ offset -= mem->region.start;
+
+ return (mem->io_start + offset) >> PAGE_SHIFT;
+}
+
bool i915_gem_object_is_lmem(struct drm_i915_gem_object *obj)
{
return obj->ops == &i915_gem_lmem_obj_ops;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.h b/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
index 7c176b8b7d2f..36a412ace3cf 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
@@ -7,6 +7,7 @@
#define __I915_GEM_LMEM_H
#include <linux/types.h>
+#include <linux/mman.h>
struct drm_i915_private;
struct drm_i915_gem_object;
@@ -22,8 +23,13 @@ void __iomem *
i915_gem_object_lmem_io_map_page_atomic(struct drm_i915_gem_object *obj,
unsigned long n);
+unsigned long i915_gem_object_lmem_io_pfn(struct drm_i915_gem_object *obj,
+ unsigned long n);
+
bool i915_gem_object_is_lmem(struct drm_i915_gem_object *obj);
+vm_fault_t vm_fault_iomem(struct vm_fault *vmf);
+
struct drm_i915_gem_object *
i915_gem_object_create_lmem(struct drm_i915_private *i915,
resource_size_t size,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 879fff8adc48..958ca2033379 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -11,6 +11,7 @@
#include "gt/intel_gt.h"
#include "gt/intel_gt_requests.h"
+#include "i915_gem_lmem.h"
#include "i915_drv.h"
#include "i915_gem_gtt.h"
#include "i915_gem_ioctls.h"
@@ -203,7 +204,7 @@ compute_partial_view(const struct drm_i915_gem_object *obj,
return view;
}
-static vm_fault_t i915_error_to_vmf_fault(int err)
+vm_fault_t i915_error_to_vmf_fault(int err)
{
switch (err) {
default:
@@ -216,6 +217,7 @@ static vm_fault_t i915_error_to_vmf_fault(int err)
case -ENOSPC: /* shmemfs allocation failure */
case -ENOMEM: /* our allocation failure */
+ case -ENXIO:
return VM_FAULT_OOM;
case 0:
@@ -560,7 +562,8 @@ __assign_mmap_offset(struct drm_file *file,
}
if (mmap_type != I915_MMAP_TYPE_GTT &&
- !i915_gem_object_has_struct_page(obj)) {
+ !i915_gem_object_type_has(obj, I915_GEM_OBJECT_HAS_STRUCT_PAGE |
+ I915_GEM_OBJECT_HAS_IOMEM)) {
err = -ENODEV;
goto out;
}
@@ -694,6 +697,25 @@ static const struct vm_operations_struct vm_ops_cpu = {
.close = vm_close,
};
+static const struct vm_operations_struct vm_ops_iomem = {
+ .fault = vm_fault_iomem,
+ .open = vm_open,
+ .close = vm_close,
+};
+
+static const struct vm_operations_struct *
+get_vm_cpu_ops(struct drm_i915_gem_object *obj)
+{
+ if (i915_gem_object_type_has(obj, I915_GEM_OBJECT_HAS_STRUCT_PAGE))
+ return &vm_ops_cpu;
+
+ if (i915_gem_object_type_has(obj, I915_GEM_OBJECT_HAS_IOMEM))
+ return &vm_ops_iomem;
+
+ GEM_BUG_ON(1);
+ return NULL;
+}
+
/*
* This overcomes the limitation in drm_gem_mmap's assignment of a
* drm_gem_object as the vma->vm_private_data. Since we need to
@@ -762,18 +784,18 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
case I915_MMAP_TYPE_WC:
vma->vm_page_prot =
pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
- vma->vm_ops = &vm_ops_cpu;
+ vma->vm_ops = get_vm_cpu_ops(to_intel_bo(obj));
break;
case I915_MMAP_TYPE_WB:
vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
- vma->vm_ops = &vm_ops_cpu;
+ vma->vm_ops = get_vm_cpu_ops(to_intel_bo(obj));
break;
case I915_MMAP_TYPE_UC:
vma->vm_page_prot =
pgprot_noncached(vm_get_page_prot(vma->vm_flags));
- vma->vm_ops = &vm_ops_cpu;
+ vma->vm_ops = get_vm_cpu_ops(to_intel_bo(obj));
break;
case I915_MMAP_TYPE_GTT:
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
index 862e01b7cb69..aded4d0564c9 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
@@ -23,6 +23,7 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma);
int i915_gem_dumb_mmap_offset(struct drm_file *file_priv,
struct drm_device *dev,
u32 handle, u64 *offset);
+vm_fault_t i915_error_to_vmf_fault(int err);
void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 591435c5f368..db78ab33e7cb 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -9,6 +9,8 @@
#include "gt/intel_engine_pm.h"
#include "gt/intel_gt.h"
#include "gt/intel_gt_pm.h"
+#include "gem/i915_gem_lmem.h"
+#include "gem/i915_gem_region.h"
#include "huge_gem_object.h"
#include "i915_selftest.h"
#include "selftests/i915_random.h"
@@ -726,24 +728,23 @@ static int igt_mmap_offset_exhaustion(void *arg)
}
#define expand32(x) (((x) << 0) | ((x) << 8) | ((x) << 16) | ((x) << 24))
-static int igt_mmap(void *arg, enum i915_mmap_type type)
+static int igt_mmap(struct drm_i915_private *i915, struct drm_i915_gem_object *obj,
+ enum i915_mmap_type type)
{
- struct drm_i915_private *i915 = arg;
- struct drm_i915_gem_object *obj;
struct i915_mmap_offset *mmo;
struct vm_area_struct *area;
unsigned long addr;
void *vaddr;
int err = 0, i;
- if (!i915_ggtt_has_aperture(&i915->ggtt))
- return 0;
-
- obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
- if (IS_ERR(obj))
- return PTR_ERR(obj);
+ if (!i915_ggtt_has_aperture(&i915->ggtt) &&
+ type == I915_MMAP_TYPE_GTT) {
+ err = 0;
+ goto out;
+ }
- vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB);
+ vaddr = i915_gem_object_pin_map(obj, i915_gem_object_is_lmem(obj) ?
+ I915_MAP_WC : I915_MAP_WB);
if (IS_ERR(vaddr)) {
err = PTR_ERR(vaddr);
goto out;
@@ -827,12 +828,57 @@ static int igt_mmap(void *arg, enum i915_mmap_type type)
static int igt_mmap_gtt(void *arg)
{
- return igt_mmap(arg, I915_MMAP_TYPE_GTT);
+ struct drm_i915_private *i915 = arg;
+ struct drm_i915_gem_object *obj =
+ i915_gem_object_create_internal(i915, PAGE_SIZE);
+ if (IS_ERR(obj))
+ return PTR_ERR(obj);
+
+ return igt_mmap(i915, obj, I915_MMAP_TYPE_GTT);
}
static int igt_mmap_cpu(void *arg)
{
- return igt_mmap(arg, I915_MMAP_TYPE_WC);
+ struct drm_i915_private *i915 = arg;
+ struct drm_i915_gem_object *obj =
+ i915_gem_object_create_internal(i915, PAGE_SIZE);
+ if (IS_ERR(obj))
+ return PTR_ERR(obj);
+
+ return igt_mmap(i915, obj, I915_MMAP_TYPE_WC);
+}
+
+static int igt_mmap_lmem(void *arg)
+{
+ struct drm_i915_private *i915 = arg;
+ int i, err = 0;
+
+ if (!HAS_LMEM(i915)) {
+ pr_info("device lacks LMEM support, skipping\n");
+ return 0;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(i915->mm.regions); i++) {
+ struct intel_memory_region *mem = i915->mm.regions[i];
+ struct drm_i915_gem_object *obj;
+
+ if (!mem || mem->type != INTEL_MEMORY_LOCAL)
+ continue;
+
+ obj = i915_gem_object_create_region(mem, PAGE_SIZE,
+ I915_BO_ALLOC_CONTIGUOUS);
+ if (IS_ERR(obj)) {
+ err = PTR_ERR(obj);
+ break;
+ }
+
+ pr_info("%s: memory region %u", __func__, mem->instance);
+ err = igt_mmap(i915, obj, I915_MMAP_TYPE_WC);
+ if (err)
+ break;
+ }
+
+ return err;
}
static int check_present_pte(pte_t *pte, unsigned long addr, void *data)
@@ -887,20 +933,18 @@ static int prefault_range(u64 start, u64 len)
return __get_user(c, end - 1);
}
-static int igt_mmap_revoke(void *arg, enum i915_mmap_type type)
+static int igt_mmap_revoke(struct drm_i915_private *i915, struct drm_i915_gem_object *obj,
+ enum i915_mmap_type type)
{
- struct drm_i915_private *i915 = arg;
- struct drm_i915_gem_object *obj;
struct i915_mmap_offset *mmo;
unsigned long addr;
int err;
- if (!i915_ggtt_has_aperture(&i915->ggtt))
- return 0;
-
- obj = i915_gem_object_create_internal(i915, SZ_4M);
- if (IS_ERR(obj))
- return PTR_ERR(obj);
+ if (!i915_ggtt_has_aperture(&i915->ggtt) &&
+ type == I915_MMAP_TYPE_GTT) {
+ err = 0;
+ goto out;
+ }
mmo = mmap_offset_attach(obj, type, NULL);
if (IS_ERR(mmo)) {
@@ -959,12 +1003,57 @@ static int igt_mmap_revoke(void *arg, enum i915_mmap_type type)
static int igt_mmap_gtt_revoke(void *arg)
{
- return igt_mmap_revoke(arg, I915_MMAP_TYPE_GTT);
+ struct drm_i915_private *i915 = arg;
+ struct drm_i915_gem_object *obj =
+ i915_gem_object_create_internal(i915, PAGE_SIZE);
+ if (IS_ERR(obj))
+ return PTR_ERR(obj);
+
+ return igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_GTT);
}
static int igt_mmap_cpu_revoke(void *arg)
{
- return igt_mmap_revoke(arg, I915_MMAP_TYPE_WC);
+ struct drm_i915_private *i915 = arg;
+ struct drm_i915_gem_object *obj =
+ i915_gem_object_create_internal(i915, PAGE_SIZE);
+ if (IS_ERR(obj))
+ return PTR_ERR(obj);
+
+ return igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_WC);
+}
+
+static int igt_mmap_lmem_revoke(void *arg)
+{
+ struct drm_i915_private *i915 = arg;
+ int i, err = 0;
+
+ if (!HAS_LMEM(i915)) {
+ pr_info("device lacks LMEM support, skipping\n");
+ return 0;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(i915->mm.regions); i++) {
+ struct intel_memory_region *mem = i915->mm.regions[i];
+ struct drm_i915_gem_object *obj;
+
+ if (!mem || mem->type != INTEL_MEMORY_LOCAL)
+ continue;
+
+ obj = i915_gem_object_create_region(mem, PAGE_SIZE,
+ I915_BO_ALLOC_CONTIGUOUS);
+ if (IS_ERR(obj)) {
+ err = PTR_ERR(obj);
+ break;
+ }
+
+ pr_info("%s: memory region %u", __func__, mem->instance);
+ err = igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_WC);
+ if (err)
+ break;
+ }
+
+ return err;
}
int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
@@ -975,8 +1064,10 @@ int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
SUBTEST(igt_mmap_offset_exhaustion),
SUBTEST(igt_mmap_gtt),
SUBTEST(igt_mmap_cpu),
+ SUBTEST(igt_mmap_lmem),
SUBTEST(igt_mmap_gtt_revoke),
SUBTEST(igt_mmap_cpu_revoke),
+ SUBTEST(igt_mmap_lmem_revoke),
};
return i915_subtests(tests, i915);
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler
2019-12-12 11:34 [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler Abdiel Janulgue
@ 2019-12-12 11:45 ` Chris Wilson
2019-12-12 14:19 ` Chris Wilson
` (2 subsequent siblings)
3 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2019-12-12 11:45 UTC (permalink / raw)
To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld
Quoting Abdiel Janulgue (2019-12-12 11:34:38)
> Fault handler to handle missing pages for lmem objects.
>
> v3: Add get_vm_cpu_ops, iterate over all memory regions in the
> lmem selftest, use remap_io_mapping.
>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 40 +++++
> drivers/gpu/drm/i915/gem/i915_gem_lmem.h | 6 +
> drivers/gpu/drm/i915/gem/i915_gem_mman.c | 32 +++-
> drivers/gpu/drm/i915/gem/i915_gem_mman.h | 1 +
> .../drm/i915/gem/selftests/i915_gem_mman.c | 137 +++++++++++++++---
> 5 files changed, 188 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> index 0e2bf6b7e143..bbe625935005 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> @@ -6,8 +6,36 @@
> #include "intel_memory_region.h"
> #include "gem/i915_gem_region.h"
> #include "gem/i915_gem_lmem.h"
> +#include "gem/i915_gem_mman.h"
> #include "i915_drv.h"
>
> +vm_fault_t vm_fault_iomem(struct vm_fault *vmf)
I would push this back to being with the vm_ops and making it static,
over choosing a more appropriate name. I think we will prefer having the
fault routines next to each other for making common future changes.
> +{
> + struct vm_area_struct *area = vmf->vma;
> + struct i915_mmap_offset *priv = area->vm_private_data;
> + struct drm_i915_gem_object *obj = priv->obj;
> + struct intel_memory_region *mem = obj->mm.region;
> + unsigned long size = area->vm_end - area->vm_start;
> + bool write = area->vm_flags & VM_WRITE;
> + int ret;
> +
> + /* Sanity check that we allow writing into this object */
> + if (i915_gem_object_is_readonly(obj) && write)
> + return VM_FAULT_SIGBUS;
> +
> + ret = i915_gem_object_pin_pages(obj);
> + if (ret)
> + return i915_error_to_vmf_fault(ret);
> +
> + ret = remap_io_mapping(area, area->vm_start,
> + i915_gem_object_lmem_io_pfn(obj, 0), size,
> + &mem->iomap);
> +
> + i915_gem_object_unpin_pages(obj);
In terms of behaviour, I think this is correct. The PTE do not persist
past the lifetime of the lmem allocation so we cannot peek at other
user's data.
> +
> + return i915_error_to_vmf_fault(ret);
> +}
> +
> const struct drm_i915_gem_object_ops i915_gem_lmem_obj_ops = {
> .flags = I915_GEM_OBJECT_HAS_IOMEM,
>
> @@ -56,6 +84,18 @@ i915_gem_object_lmem_io_map(struct drm_i915_gem_object *obj,
> return io_mapping_map_wc(&obj->mm.region->iomap, offset, size);
> }
>
> +unsigned long i915_gem_object_lmem_io_pfn(struct drm_i915_gem_object *obj,
> + unsigned long n)
> +{
> + struct intel_memory_region *mem = obj->mm.region;
> + resource_size_t offset;
> +
> + offset = i915_gem_object_get_dma_address(obj, n);
> + offset -= mem->region.start;
> +
> + return (mem->io_start + offset) >> PAGE_SHIFT;
> +}
> +
> bool i915_gem_object_is_lmem(struct drm_i915_gem_object *obj)
> {
> return obj->ops == &i915_gem_lmem_obj_ops;
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.h b/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
> index 7c176b8b7d2f..36a412ace3cf 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
> @@ -7,6 +7,7 @@
> #define __I915_GEM_LMEM_H
>
> #include <linux/types.h>
> +#include <linux/mman.h>
>
> struct drm_i915_private;
> struct drm_i915_gem_object;
> @@ -22,8 +23,13 @@ void __iomem *
> i915_gem_object_lmem_io_map_page_atomic(struct drm_i915_gem_object *obj,
> unsigned long n);
>
> +unsigned long i915_gem_object_lmem_io_pfn(struct drm_i915_gem_object *obj,
> + unsigned long n);
> +
> bool i915_gem_object_is_lmem(struct drm_i915_gem_object *obj);
>
> +vm_fault_t vm_fault_iomem(struct vm_fault *vmf);
> +
> struct drm_i915_gem_object *
> i915_gem_object_create_lmem(struct drm_i915_private *i915,
> resource_size_t size,
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> index 879fff8adc48..958ca2033379 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> @@ -11,6 +11,7 @@
> #include "gt/intel_gt.h"
> #include "gt/intel_gt_requests.h"
>
> +#include "i915_gem_lmem.h"
> #include "i915_drv.h"
> #include "i915_gem_gtt.h"
> #include "i915_gem_ioctls.h"
> @@ -203,7 +204,7 @@ compute_partial_view(const struct drm_i915_gem_object *obj,
> return view;
> }
>
> -static vm_fault_t i915_error_to_vmf_fault(int err)
> +vm_fault_t i915_error_to_vmf_fault(int err)
> {
> switch (err) {
> default:
> @@ -216,6 +217,7 @@ static vm_fault_t i915_error_to_vmf_fault(int err)
>
> case -ENOSPC: /* shmemfs allocation failure */
> case -ENOMEM: /* our allocation failure */
> + case -ENXIO:
> return VM_FAULT_OOM;
>
> case 0:
> @@ -560,7 +562,8 @@ __assign_mmap_offset(struct drm_file *file,
> }
>
> if (mmap_type != I915_MMAP_TYPE_GTT &&
> - !i915_gem_object_has_struct_page(obj)) {
> + !i915_gem_object_type_has(obj, I915_GEM_OBJECT_HAS_STRUCT_PAGE |
> + I915_GEM_OBJECT_HAS_IOMEM)) {
> err = -ENODEV;
> goto out;
> }
> @@ -694,6 +697,25 @@ static const struct vm_operations_struct vm_ops_cpu = {
> .close = vm_close,
> };
>
> +static const struct vm_operations_struct vm_ops_iomem = {
> + .fault = vm_fault_iomem,
> + .open = vm_open,
> + .close = vm_close,
> +};
> +
> +static const struct vm_operations_struct *
> +get_vm_cpu_ops(struct drm_i915_gem_object *obj)
> +{
> + if (i915_gem_object_type_has(obj, I915_GEM_OBJECT_HAS_STRUCT_PAGE))
> + return &vm_ops_cpu;
> +
> + if (i915_gem_object_type_has(obj, I915_GEM_OBJECT_HAS_IOMEM))
> + return &vm_ops_iomem;
> +
> + GEM_BUG_ON(1);
GEM_BUG_ON("unknown object type");
> + return NULL;
> +}
> +
> /*
> * This overcomes the limitation in drm_gem_mmap's assignment of a
> * drm_gem_object as the vma->vm_private_data. Since we need to
> @@ -762,18 +784,18 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> case I915_MMAP_TYPE_WC:
> vma->vm_page_prot =
> pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
> - vma->vm_ops = &vm_ops_cpu;
> + vma->vm_ops = get_vm_cpu_ops(to_intel_bo(obj));
> break;
>
> case I915_MMAP_TYPE_WB:
> vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
> - vma->vm_ops = &vm_ops_cpu;
> + vma->vm_ops = get_vm_cpu_ops(to_intel_bo(obj));
> break;
>
> case I915_MMAP_TYPE_UC:
> vma->vm_page_prot =
> pgprot_noncached(vm_get_page_prot(vma->vm_flags));
> - vma->vm_ops = &vm_ops_cpu;
> + vma->vm_ops = get_vm_cpu_ops(to_intel_bo(obj));
> break;
>
> case I915_MMAP_TYPE_GTT:
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> index 862e01b7cb69..aded4d0564c9 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> @@ -23,6 +23,7 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma);
> int i915_gem_dumb_mmap_offset(struct drm_file *file_priv,
> struct drm_device *dev,
> u32 handle, u64 *offset);
> +vm_fault_t i915_error_to_vmf_fault(int err);
>
> void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
> void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> index 591435c5f368..db78ab33e7cb 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> @@ -9,6 +9,8 @@
> #include "gt/intel_engine_pm.h"
> #include "gt/intel_gt.h"
> #include "gt/intel_gt_pm.h"
> +#include "gem/i915_gem_lmem.h"
> +#include "gem/i915_gem_region.h"
> #include "huge_gem_object.h"
> #include "i915_selftest.h"
> #include "selftests/i915_random.h"
> @@ -726,24 +728,23 @@ static int igt_mmap_offset_exhaustion(void *arg)
> }
>
> #define expand32(x) (((x) << 0) | ((x) << 8) | ((x) << 16) | ((x) << 24))
> -static int igt_mmap(void *arg, enum i915_mmap_type type)
> +static int igt_mmap(struct drm_i915_private *i915, struct drm_i915_gem_object *obj,
> + enum i915_mmap_type type)
> {
> - struct drm_i915_private *i915 = arg;
> - struct drm_i915_gem_object *obj;
> struct i915_mmap_offset *mmo;
> struct vm_area_struct *area;
> unsigned long addr;
> void *vaddr;
> int err = 0, i;
>
> - if (!i915_ggtt_has_aperture(&i915->ggtt))
> - return 0;
> -
> - obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
> - if (IS_ERR(obj))
> - return PTR_ERR(obj);
> + if (!i915_ggtt_has_aperture(&i915->ggtt) &&
> + type == I915_MMAP_TYPE_GTT) {
> + err = 0;
> + goto out;
> + }
>
> - vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB);
> + vaddr = i915_gem_object_pin_map(obj, i915_gem_object_is_lmem(obj) ?
> + I915_MAP_WC : I915_MAP_WB);
> if (IS_ERR(vaddr)) {
> err = PTR_ERR(vaddr);
> goto out;
> @@ -827,12 +828,57 @@ static int igt_mmap(void *arg, enum i915_mmap_type type)
>
> static int igt_mmap_gtt(void *arg)
> {
> - return igt_mmap(arg, I915_MMAP_TYPE_GTT);
> + struct drm_i915_private *i915 = arg;
> + struct drm_i915_gem_object *obj =
> + i915_gem_object_create_internal(i915, PAGE_SIZE);
> + if (IS_ERR(obj))
> + return PTR_ERR(obj);
> +
> + return igt_mmap(i915, obj, I915_MMAP_TYPE_GTT);
> }
>
> static int igt_mmap_cpu(void *arg)
> {
> - return igt_mmap(arg, I915_MMAP_TYPE_WC);
> + struct drm_i915_private *i915 = arg;
> + struct drm_i915_gem_object *obj =
> + i915_gem_object_create_internal(i915, PAGE_SIZE);
> + if (IS_ERR(obj))
> + return PTR_ERR(obj);
> +
> + return igt_mmap(i915, obj, I915_MMAP_TYPE_WC);
> +}
> +
> +static int igt_mmap_lmem(void *arg)
> +{
> + struct drm_i915_private *i915 = arg;
> + int i, err = 0;
> +
> + if (!HAS_LMEM(i915)) {
> + pr_info("device lacks LMEM support, skipping\n");
> + return 0;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(i915->mm.regions); i++) {
> + struct intel_memory_region *mem = i915->mm.regions[i];
> + struct drm_i915_gem_object *obj;
> +
> + if (!mem || mem->type != INTEL_MEMORY_LOCAL)
> + continue;
> +
> + obj = i915_gem_object_create_region(mem, PAGE_SIZE,
> + I915_BO_ALLOC_CONTIGUOUS);
> + if (IS_ERR(obj)) {
> + err = PTR_ERR(obj);
> + break;
> + }
> +
> + pr_info("%s: memory region %u", __func__, mem->instance);
> + err = igt_mmap(i915, obj, I915_MMAP_TYPE_WC);
> + if (err)
> + break;
> + }
> +
> + return err;
> }
>
> static int check_present_pte(pte_t *pte, unsigned long addr, void *data)
> @@ -887,20 +933,18 @@ static int prefault_range(u64 start, u64 len)
> return __get_user(c, end - 1);
> }
>
> -static int igt_mmap_revoke(void *arg, enum i915_mmap_type type)
> +static int igt_mmap_revoke(struct drm_i915_private *i915, struct drm_i915_gem_object *obj,
> + enum i915_mmap_type type)
> {
> - struct drm_i915_private *i915 = arg;
> - struct drm_i915_gem_object *obj;
> struct i915_mmap_offset *mmo;
> unsigned long addr;
> int err;
>
> - if (!i915_ggtt_has_aperture(&i915->ggtt))
> - return 0;
> -
> - obj = i915_gem_object_create_internal(i915, SZ_4M);
> - if (IS_ERR(obj))
> - return PTR_ERR(obj);
> + if (!i915_ggtt_has_aperture(&i915->ggtt) &&
> + type == I915_MMAP_TYPE_GTT) {
> + err = 0;
> + goto out;
> + }
>
> mmo = mmap_offset_attach(obj, type, NULL);
> if (IS_ERR(mmo)) {
> @@ -959,12 +1003,57 @@ static int igt_mmap_revoke(void *arg, enum i915_mmap_type type)
>
> static int igt_mmap_gtt_revoke(void *arg)
> {
> - return igt_mmap_revoke(arg, I915_MMAP_TYPE_GTT);
> + struct drm_i915_private *i915 = arg;
> + struct drm_i915_gem_object *obj =
> + i915_gem_object_create_internal(i915, PAGE_SIZE);
> + if (IS_ERR(obj))
> + return PTR_ERR(obj);
> +
> + return igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_GTT);
> }
>
> static int igt_mmap_cpu_revoke(void *arg)
> {
> - return igt_mmap_revoke(arg, I915_MMAP_TYPE_WC);
> + struct drm_i915_private *i915 = arg;
> + struct drm_i915_gem_object *obj =
> + i915_gem_object_create_internal(i915, PAGE_SIZE);
> + if (IS_ERR(obj))
> + return PTR_ERR(obj);
> +
> + return igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_WC);
Unless you have an exception signed in triplicate, do resource
allocation and cleanup in the same scope.
In this case I would pass down an allocator for igt_mmap_revoke, or aim
to do everything via the object_create_region interface below.
> +}
> +
> +static int igt_mmap_lmem_revoke(void *arg)
> +{
> + struct drm_i915_private *i915 = arg;
> + int i, err = 0;
> +
> + if (!HAS_LMEM(i915)) {
> + pr_info("device lacks LMEM support, skipping\n");
> + return 0;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(i915->mm.regions); i++) {
> + struct intel_memory_region *mem = i915->mm.regions[i];
> + struct drm_i915_gem_object *obj;
> +
> + if (!mem || mem->type != INTEL_MEMORY_LOCAL)
> + continue;
> +
> + obj = i915_gem_object_create_region(mem, PAGE_SIZE,
> + I915_BO_ALLOC_CONTIGUOUS);
> + if (IS_ERR(obj)) {
> + err = PTR_ERR(obj);
> + break;
> + }
I was hoping we would have some more information in the memory region
from which we could derive the expected properties and so test smem,
stolen and lmem from the same test.
Oh well, I can always wish.
> +
> + pr_info("%s: memory region %u", __func__, mem->instance);
> + err = igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_WC);
> + if (err)
> + break;
> + }
> +
> + return err;
> }
>
> int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
> @@ -975,8 +1064,10 @@ int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
> SUBTEST(igt_mmap_offset_exhaustion),
> SUBTEST(igt_mmap_gtt),
> SUBTEST(igt_mmap_cpu),
> + SUBTEST(igt_mmap_lmem),
> SUBTEST(igt_mmap_gtt_revoke),
> SUBTEST(igt_mmap_cpu_revoke),
> + SUBTEST(igt_mmap_lmem_revoke),
> };
>
> return i915_subtests(tests, i915);
> --
> 2.23.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler
2019-12-12 11:34 [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler Abdiel Janulgue
2019-12-12 11:45 ` Chris Wilson
@ 2019-12-12 14:19 ` Chris Wilson
2019-12-12 15:11 ` Matthew Auld
2019-12-12 15:56 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Add lmem fault handler (rev3) Patchwork
2019-12-12 16:22 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
3 siblings, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2019-12-12 14:19 UTC (permalink / raw)
To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld
Quoting Abdiel Janulgue (2019-12-12 11:34:38)
> Fault handler to handle missing pages for lmem objects.
>
> v3: Add get_vm_cpu_ops, iterate over all memory regions in the
> lmem selftest, use remap_io_mapping.
>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 40 +++++
> drivers/gpu/drm/i915/gem/i915_gem_lmem.h | 6 +
> drivers/gpu/drm/i915/gem/i915_gem_mman.c | 32 +++-
> drivers/gpu/drm/i915/gem/i915_gem_mman.h | 1 +
> .../drm/i915/gem/selftests/i915_gem_mman.c | 137 +++++++++++++++---
> 5 files changed, 188 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> index 0e2bf6b7e143..bbe625935005 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> @@ -6,8 +6,36 @@
> #include "intel_memory_region.h"
> #include "gem/i915_gem_region.h"
> #include "gem/i915_gem_lmem.h"
> +#include "gem/i915_gem_mman.h"
> #include "i915_drv.h"
>
> +vm_fault_t vm_fault_iomem(struct vm_fault *vmf)
> +{
> + struct vm_area_struct *area = vmf->vma;
> + struct i915_mmap_offset *priv = area->vm_private_data;
> + struct drm_i915_gem_object *obj = priv->obj;
> + struct intel_memory_region *mem = obj->mm.region;
> + unsigned long size = area->vm_end - area->vm_start;
> + bool write = area->vm_flags & VM_WRITE;
> + int ret;
> +
> + /* Sanity check that we allow writing into this object */
> + if (i915_gem_object_is_readonly(obj) && write)
> + return VM_FAULT_SIGBUS;
> +
> + ret = i915_gem_object_pin_pages(obj);
> + if (ret)
> + return i915_error_to_vmf_fault(ret);
> +
> + ret = remap_io_mapping(area, area->vm_start,
> + i915_gem_object_lmem_io_pfn(obj, 0), size,
> + &mem->iomap);
So this implementation only works with contiguous objects, right?
if (GEM_WARN_ON(obj_noncontiguous(obj)))
return VM_FAULT_SIGBUS;
I think for future awareness.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler
2019-12-12 14:19 ` Chris Wilson
@ 2019-12-12 15:11 ` Matthew Auld
2019-12-12 15:19 ` Chris Wilson
0 siblings, 1 reply; 17+ messages in thread
From: Matthew Auld @ 2019-12-12 15:11 UTC (permalink / raw)
To: Chris Wilson; +Cc: Intel Graphics Development, Matthew Auld
On Thu, 12 Dec 2019 at 14:20, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> Quoting Abdiel Janulgue (2019-12-12 11:34:38)
> > Fault handler to handle missing pages for lmem objects.
> >
> > v3: Add get_vm_cpu_ops, iterate over all memory regions in the
> > lmem selftest, use remap_io_mapping.
> >
> > Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 40 +++++
> > drivers/gpu/drm/i915/gem/i915_gem_lmem.h | 6 +
> > drivers/gpu/drm/i915/gem/i915_gem_mman.c | 32 +++-
> > drivers/gpu/drm/i915/gem/i915_gem_mman.h | 1 +
> > .../drm/i915/gem/selftests/i915_gem_mman.c | 137 +++++++++++++++---
> > 5 files changed, 188 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> > index 0e2bf6b7e143..bbe625935005 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> > @@ -6,8 +6,36 @@
> > #include "intel_memory_region.h"
> > #include "gem/i915_gem_region.h"
> > #include "gem/i915_gem_lmem.h"
> > +#include "gem/i915_gem_mman.h"
> > #include "i915_drv.h"
> >
> > +vm_fault_t vm_fault_iomem(struct vm_fault *vmf)
> > +{
> > + struct vm_area_struct *area = vmf->vma;
> > + struct i915_mmap_offset *priv = area->vm_private_data;
> > + struct drm_i915_gem_object *obj = priv->obj;
> > + struct intel_memory_region *mem = obj->mm.region;
> > + unsigned long size = area->vm_end - area->vm_start;
> > + bool write = area->vm_flags & VM_WRITE;
> > + int ret;
> > +
> > + /* Sanity check that we allow writing into this object */
> > + if (i915_gem_object_is_readonly(obj) && write)
> > + return VM_FAULT_SIGBUS;
> > +
> > + ret = i915_gem_object_pin_pages(obj);
> > + if (ret)
> > + return i915_error_to_vmf_fault(ret);
> > +
> > + ret = remap_io_mapping(area, area->vm_start,
> > + i915_gem_object_lmem_io_pfn(obj, 0), size,
> > + &mem->iomap);
>
> So this implementation only works with contiguous objects, right?
Hmm can't we go back to what we had before, so support !contiguous also?
Also do we need to reject !(WC | UC) somewhere for local-memory?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler
2019-12-12 15:11 ` Matthew Auld
@ 2019-12-12 15:19 ` Chris Wilson
2019-12-13 5:39 ` Abdiel Janulgue
0 siblings, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2019-12-12 15:19 UTC (permalink / raw)
To: Matthew Auld; +Cc: Intel Graphics Development, Matthew Auld
Quoting Matthew Auld (2019-12-12 15:11:02)
> On Thu, 12 Dec 2019 at 14:20, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >
> > Quoting Abdiel Janulgue (2019-12-12 11:34:38)
> > > Fault handler to handle missing pages for lmem objects.
> > >
> > > v3: Add get_vm_cpu_ops, iterate over all memory regions in the
> > > lmem selftest, use remap_io_mapping.
> > >
> > > Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> > > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > ---
> > > drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 40 +++++
> > > drivers/gpu/drm/i915/gem/i915_gem_lmem.h | 6 +
> > > drivers/gpu/drm/i915/gem/i915_gem_mman.c | 32 +++-
> > > drivers/gpu/drm/i915/gem/i915_gem_mman.h | 1 +
> > > .../drm/i915/gem/selftests/i915_gem_mman.c | 137 +++++++++++++++---
> > > 5 files changed, 188 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> > > index 0e2bf6b7e143..bbe625935005 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> > > @@ -6,8 +6,36 @@
> > > #include "intel_memory_region.h"
> > > #include "gem/i915_gem_region.h"
> > > #include "gem/i915_gem_lmem.h"
> > > +#include "gem/i915_gem_mman.h"
> > > #include "i915_drv.h"
> > >
> > > +vm_fault_t vm_fault_iomem(struct vm_fault *vmf)
> > > +{
> > > + struct vm_area_struct *area = vmf->vma;
> > > + struct i915_mmap_offset *priv = area->vm_private_data;
> > > + struct drm_i915_gem_object *obj = priv->obj;
> > > + struct intel_memory_region *mem = obj->mm.region;
> > > + unsigned long size = area->vm_end - area->vm_start;
> > > + bool write = area->vm_flags & VM_WRITE;
> > > + int ret;
> > > +
> > > + /* Sanity check that we allow writing into this object */
> > > + if (i915_gem_object_is_readonly(obj) && write)
> > > + return VM_FAULT_SIGBUS;
> > > +
> > > + ret = i915_gem_object_pin_pages(obj);
> > > + if (ret)
> > > + return i915_error_to_vmf_fault(ret);
> > > +
> > > + ret = remap_io_mapping(area, area->vm_start,
> > > + i915_gem_object_lmem_io_pfn(obj, 0), size,
> > > + &mem->iomap);
> >
> > So this implementation only works with contiguous objects, right?
>
> Hmm can't we go back to what we had before, so support !contiguous also?
The fun part is that you can do both :) Do a discontiguous
remap_io_mapping() The queue part is that remap_io_mapping() avoids the
O(N^2), and should give us O(N) instead.
> Also do we need to reject !(WC | UC) somewhere for local-memory?
We don't strictly need to (caveat emptor), same rules apply for
incoherent device memory, you have clflush around your access. It may
not be advised, but sometimes it is better.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler
2019-12-12 15:19 ` Chris Wilson
@ 2019-12-13 5:39 ` Abdiel Janulgue
0 siblings, 0 replies; 17+ messages in thread
From: Abdiel Janulgue @ 2019-12-13 5:39 UTC (permalink / raw)
To: Chris Wilson, Matthew Auld; +Cc: Intel Graphics Development, Matthew Auld
On 12/12/2019 17.19, Chris Wilson wrote:
> Quoting Matthew Auld (2019-12-12 15:11:02)
>> On Thu, 12 Dec 2019 at 14:20, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>>>
>>> Quoting Abdiel Janulgue (2019-12-12 11:34:38)
>>>> Fault handler to handle missing pages for lmem objects.
>>>>
>>>> v3: Add get_vm_cpu_ops, iterate over all memory regions in the
>>>> lmem selftest, use remap_io_mapping.
>>>>
>>>> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
>>>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>>> ---
>>>> drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 40 +++++
>>>> drivers/gpu/drm/i915/gem/i915_gem_lmem.h | 6 +
>>>> drivers/gpu/drm/i915/gem/i915_gem_mman.c | 32 +++-
>>>> drivers/gpu/drm/i915/gem/i915_gem_mman.h | 1 +
>>>> .../drm/i915/gem/selftests/i915_gem_mman.c | 137 +++++++++++++++---
>>>> 5 files changed, 188 insertions(+), 28 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
>>>> index 0e2bf6b7e143..bbe625935005 100644
>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
>>>> @@ -6,8 +6,36 @@
>>>> #include "intel_memory_region.h"
>>>> #include "gem/i915_gem_region.h"
>>>> #include "gem/i915_gem_lmem.h"
>>>> +#include "gem/i915_gem_mman.h"
>>>> #include "i915_drv.h"
>>>>
>>>> +vm_fault_t vm_fault_iomem(struct vm_fault *vmf)
>>>> +{
>>>> + struct vm_area_struct *area = vmf->vma;
>>>> + struct i915_mmap_offset *priv = area->vm_private_data;
>>>> + struct drm_i915_gem_object *obj = priv->obj;
>>>> + struct intel_memory_region *mem = obj->mm.region;
>>>> + unsigned long size = area->vm_end - area->vm_start;
>>>> + bool write = area->vm_flags & VM_WRITE;
>>>> + int ret;
>>>> +
>>>> + /* Sanity check that we allow writing into this object */
>>>> + if (i915_gem_object_is_readonly(obj) && write)
>>>> + return VM_FAULT_SIGBUS;
>>>> +
>>>> + ret = i915_gem_object_pin_pages(obj);
>>>> + if (ret)
>>>> + return i915_error_to_vmf_fault(ret);
>>>> +
>>>> + ret = remap_io_mapping(area, area->vm_start,
>>>> + i915_gem_object_lmem_io_pfn(obj, 0), size,
>>>> + &mem->iomap);
>>>
>>> So this implementation only works with contiguous objects, right?
>>
>> Hmm can't we go back to what we had before, so support !contiguous also?
>
> The fun part is that you can do both :) Do a discontiguous
> remap_io_mapping() The queue part is that remap_io_mapping() avoids the
> O(N^2), and should give us O(N) instead.
So just to be clear,
if (obj->flags & I915_BO_ALLOC_CONTIGUOUS) {
remap_io_mapping(...)
} else {
for(...)
vmf_insert_pfn()
}
??
- Abdiel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Add lmem fault handler (rev3)
2019-12-12 11:34 [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler Abdiel Janulgue
2019-12-12 11:45 ` Chris Wilson
2019-12-12 14:19 ` Chris Wilson
@ 2019-12-12 15:56 ` Patchwork
2019-12-12 16:22 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
3 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2019-12-12 15:56 UTC (permalink / raw)
To: Abdiel Janulgue; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Add lmem fault handler (rev3)
URL : https://patchwork.freedesktop.org/series/70485/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
4dfb09a5684c drm/i915: Add lmem fault handler
-:146: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#146: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.c:701:
+ .fault = vm_fault_iomem,$
-:147: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#147: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.c:702:
+ .open = vm_open,$
-:148: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#148: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.c:703:
+ .close = vm_close,$
total: 0 errors, 3 warnings, 0 checks, 354 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Add lmem fault handler (rev3)
2019-12-12 11:34 [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler Abdiel Janulgue
` (2 preceding siblings ...)
2019-12-12 15:56 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Add lmem fault handler (rev3) Patchwork
@ 2019-12-12 16:22 ` Patchwork
3 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2019-12-12 16:22 UTC (permalink / raw)
To: Abdiel Janulgue; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Add lmem fault handler (rev3)
URL : https://patchwork.freedesktop.org/series/70485/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_7551 -> Patchwork_15717
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_15717 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_15717, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15717/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_15717:
### IGT changes ###
#### Possible regressions ####
* igt@gem_close_race@basic-threads:
- fi-byt-j1900: [PASS][1] -> [TIMEOUT][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7551/fi-byt-j1900/igt@gem_close_race@basic-threads.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15717/fi-byt-j1900/igt@gem_close_race@basic-threads.html
* igt@runner@aborted:
- fi-byt-j1900: NOTRUN -> [FAIL][3]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15717/fi-byt-j1900/igt@runner@aborted.html
Known issues
------------
Here are the changes found in Patchwork_15717 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live_blt:
- fi-ivb-3770: [PASS][4] -> [DMESG-FAIL][5] ([i915#725])
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7551/fi-ivb-3770/igt@i915_selftest@live_blt.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15717/fi-ivb-3770/igt@i915_selftest@live_blt.html
#### Possible fixes ####
* igt@i915_selftest@live_gem_contexts:
- fi-hsw-peppy: [DMESG-FAIL][6] ([i915#722]) -> [PASS][7]
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7551/fi-hsw-peppy/igt@i915_selftest@live_gem_contexts.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15717/fi-hsw-peppy/igt@i915_selftest@live_gem_contexts.html
* igt@kms_chamelium@hdmi-edid-read:
- fi-kbl-7500u: [FAIL][8] ([i915#217]) -> [PASS][9]
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7551/fi-kbl-7500u/igt@kms_chamelium@hdmi-edid-read.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15717/fi-kbl-7500u/igt@kms_chamelium@hdmi-edid-read.html
#### Warnings ####
* igt@gem_exec_suspend@basic-s4-devices:
- fi-kbl-x1275: [DMESG-WARN][10] ([fdo#107139] / [i915#62] / [i915#92]) -> [DMESG-WARN][11] ([fdo#107139] / [i915#62] / [i915#92] / [i915#95])
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7551/fi-kbl-x1275/igt@gem_exec_suspend@basic-s4-devices.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15717/fi-kbl-x1275/igt@gem_exec_suspend@basic-s4-devices.html
* igt@i915_selftest@live_blt:
- fi-hsw-4770: [DMESG-FAIL][12] ([i915#553] / [i915#725]) -> [DMESG-FAIL][13] ([i915#563])
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7551/fi-hsw-4770/igt@i915_selftest@live_blt.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15717/fi-hsw-4770/igt@i915_selftest@live_blt.html
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7500u: [FAIL][14] ([fdo#111096] / [i915#323]) -> [FAIL][15] ([fdo#111407])
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7551/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15717/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
- fi-kbl-x1275: [DMESG-WARN][16] ([i915#62] / [i915#92]) -> [DMESG-WARN][17] ([i915#62] / [i915#92] / [i915#95]) +4 similar issues
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7551/fi-kbl-x1275/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15717/fi-kbl-x1275/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- fi-kbl-x1275: [DMESG-WARN][18] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][19] ([i915#62] / [i915#92]) +5 similar issues
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7551/fi-kbl-x1275/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15717/fi-kbl-x1275/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#107139]: https://bugs.freedesktop.org/show_bug.cgi?id=107139
[fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
[fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407
[i915#217]: https://gitlab.freedesktop.org/drm/intel/issues/217
[i915#323]: https://gitlab.freedesktop.org/drm/intel/issues/323
[i915#476]: https://gitlab.freedesktop.org/drm/intel/issues/476
[i915#553]: https://gitlab.freedesktop.org/drm/intel/issues/553
[i915#563]: https://gitlab.freedesktop.org/drm/intel/issues/563
[i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
[i915#722]: https://gitlab.freedesktop.org/drm/intel/issues/722
[i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725
[i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
[i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
Participating hosts (52 -> 45)
------------------------------
Additional (1): fi-gdg-551
Missing (8): fi-hsw-4770r fi-icl-1065g7 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_7551 -> Patchwork_15717
CI-20190529: 20190529
CI_DRM_7551: e60aa4ffc106f910452d28f2ea49ae2ff44d85d5 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5346: 466b0e6cbcbaccff012b484d1fd7676364b37b93 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_15717: 4dfb09a5684c8639e9b96a591a61452b0238eef4 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
4dfb09a5684c drm/i915: Add lmem fault handler
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15717/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler
@ 2019-12-11 5:59 Abdiel Janulgue
2019-12-11 9:39 ` Chris Wilson
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Abdiel Janulgue @ 2019-12-11 5:59 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
Fault handler to handle missing pages for lmem objects.
v2: Handle ENXIO in fault error, account for offset in region start
for fake lmem (Matt).
Add selftest (Chris).
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 44 ++++++++
drivers/gpu/drm/i915/gem/i915_gem_lmem.h | 6 +
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 16 ++-
drivers/gpu/drm/i915/gem/i915_gem_mman.h | 1 +
.../drm/i915/gem/selftests/i915_gem_mman.c | 105 ++++++++++++++----
5 files changed, 147 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
index 0e2bf6b7e143..7e6d8d1546e3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
@@ -6,8 +6,40 @@
#include "intel_memory_region.h"
#include "gem/i915_gem_region.h"
#include "gem/i915_gem_lmem.h"
+#include "gem/i915_gem_mman.h"
#include "i915_drv.h"
+vm_fault_t vm_fault_lmem(struct vm_fault *vmf)
+{
+ struct vm_area_struct *area = vmf->vma;
+ struct i915_mmap_offset *priv = area->vm_private_data;
+ struct drm_i915_gem_object *obj = priv->obj;
+ unsigned long size = area->vm_end - area->vm_start;
+ bool write = area->vm_flags & VM_WRITE;
+ vm_fault_t vmf_ret;
+ int i, ret;
+
+ /* Sanity check that we allow writing into this object */
+ if (i915_gem_object_is_readonly(obj) && write)
+ return VM_FAULT_SIGBUS;
+
+ ret = i915_gem_object_pin_pages(obj);
+ if (ret)
+ return i915_error_to_vmf_fault(ret);
+
+ for (i = 0; i < size >> PAGE_SHIFT; i++) {
+ vmf_ret = vmf_insert_pfn(area,
+ (unsigned long)area->vm_start + i * PAGE_SIZE,
+ i915_gem_object_lmem_io_pfn(obj, i));
+ if (vmf_ret != VM_FAULT_NOPAGE)
+ break;
+ }
+
+ i915_gem_object_unpin_pages(obj);
+
+ return vmf_ret;
+}
+
const struct drm_i915_gem_object_ops i915_gem_lmem_obj_ops = {
.flags = I915_GEM_OBJECT_HAS_IOMEM,
@@ -56,6 +88,18 @@ i915_gem_object_lmem_io_map(struct drm_i915_gem_object *obj,
return io_mapping_map_wc(&obj->mm.region->iomap, offset, size);
}
+unsigned long i915_gem_object_lmem_io_pfn(struct drm_i915_gem_object *obj,
+ unsigned long n)
+{
+ struct intel_memory_region *mem = obj->mm.region;
+ resource_size_t offset;
+
+ offset = i915_gem_object_get_dma_address(obj, n);
+ offset -= mem->region.start;
+
+ return (mem->io_start + offset) >> PAGE_SHIFT;
+}
+
bool i915_gem_object_is_lmem(struct drm_i915_gem_object *obj)
{
return obj->ops == &i915_gem_lmem_obj_ops;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.h b/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
index 7c176b8b7d2f..917ebef1529f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
@@ -7,6 +7,7 @@
#define __I915_GEM_LMEM_H
#include <linux/types.h>
+#include <linux/mman.h>
struct drm_i915_private;
struct drm_i915_gem_object;
@@ -22,8 +23,13 @@ void __iomem *
i915_gem_object_lmem_io_map_page_atomic(struct drm_i915_gem_object *obj,
unsigned long n);
+unsigned long i915_gem_object_lmem_io_pfn(struct drm_i915_gem_object *obj,
+ unsigned long n);
+
bool i915_gem_object_is_lmem(struct drm_i915_gem_object *obj);
+vm_fault_t vm_fault_lmem(struct vm_fault *vmf);
+
struct drm_i915_gem_object *
i915_gem_object_create_lmem(struct drm_i915_private *i915,
resource_size_t size,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 879fff8adc48..c67c07905df5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -11,6 +11,7 @@
#include "gt/intel_gt.h"
#include "gt/intel_gt_requests.h"
+#include "i915_gem_lmem.h"
#include "i915_drv.h"
#include "i915_gem_gtt.h"
#include "i915_gem_ioctls.h"
@@ -203,7 +204,7 @@ compute_partial_view(const struct drm_i915_gem_object *obj,
return view;
}
-static vm_fault_t i915_error_to_vmf_fault(int err)
+vm_fault_t i915_error_to_vmf_fault(int err)
{
switch (err) {
default:
@@ -216,6 +217,7 @@ static vm_fault_t i915_error_to_vmf_fault(int err)
case -ENOSPC: /* shmemfs allocation failure */
case -ENOMEM: /* our allocation failure */
+ case -ENXIO:
return VM_FAULT_OOM;
case 0:
@@ -560,7 +562,8 @@ __assign_mmap_offset(struct drm_file *file,
}
if (mmap_type != I915_MMAP_TYPE_GTT &&
- !i915_gem_object_has_struct_page(obj)) {
+ !i915_gem_object_has_struct_page(obj) &&
+ !i915_gem_object_is_lmem(obj)) {
err = -ENODEV;
goto out;
}
@@ -694,6 +697,12 @@ static const struct vm_operations_struct vm_ops_cpu = {
.close = vm_close,
};
+static const struct vm_operations_struct vm_ops_lmem = {
+ .fault = vm_fault_lmem,
+ .open = vm_open,
+ .close = vm_close,
+};
+
/*
* This overcomes the limitation in drm_gem_mmap's assignment of a
* drm_gem_object as the vma->vm_private_data. Since we need to
@@ -784,6 +793,9 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
}
vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
+ if (i915_gem_object_is_lmem(mmo->obj))
+ vma->vm_ops = &vm_ops_lmem;
+
return 0;
}
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
index 862e01b7cb69..aded4d0564c9 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
@@ -23,6 +23,7 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma);
int i915_gem_dumb_mmap_offset(struct drm_file *file_priv,
struct drm_device *dev,
u32 handle, u64 *offset);
+vm_fault_t i915_error_to_vmf_fault(int err);
void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 591435c5f368..d1335a586b7e 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -9,6 +9,8 @@
#include "gt/intel_engine_pm.h"
#include "gt/intel_gt.h"
#include "gt/intel_gt_pm.h"
+#include "gem/i915_gem_lmem.h"
+#include "gem/i915_gem_region.h"
#include "huge_gem_object.h"
#include "i915_selftest.h"
#include "selftests/i915_random.h"
@@ -726,24 +728,23 @@ static int igt_mmap_offset_exhaustion(void *arg)
}
#define expand32(x) (((x) << 0) | ((x) << 8) | ((x) << 16) | ((x) << 24))
-static int igt_mmap(void *arg, enum i915_mmap_type type)
+static int igt_mmap(struct drm_i915_private *i915, struct drm_i915_gem_object *obj,
+ enum i915_mmap_type type)
{
- struct drm_i915_private *i915 = arg;
- struct drm_i915_gem_object *obj;
struct i915_mmap_offset *mmo;
struct vm_area_struct *area;
unsigned long addr;
void *vaddr;
int err = 0, i;
- if (!i915_ggtt_has_aperture(&i915->ggtt))
- return 0;
-
- obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
- if (IS_ERR(obj))
- return PTR_ERR(obj);
+ if (!i915_ggtt_has_aperture(&i915->ggtt) &&
+ type == I915_MMAP_TYPE_GTT) {
+ err = 0;
+ goto out;
+ }
- vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB);
+ vaddr = i915_gem_object_pin_map(obj, i915_gem_object_is_lmem(obj) ?
+ I915_MAP_WC : I915_MAP_WB);
if (IS_ERR(vaddr)) {
err = PTR_ERR(vaddr);
goto out;
@@ -827,12 +828,41 @@ static int igt_mmap(void *arg, enum i915_mmap_type type)
static int igt_mmap_gtt(void *arg)
{
- return igt_mmap(arg, I915_MMAP_TYPE_GTT);
+ struct drm_i915_private *i915 = arg;
+ struct drm_i915_gem_object *obj =
+ i915_gem_object_create_internal(i915, PAGE_SIZE);
+ if (IS_ERR(obj))
+ return PTR_ERR(obj);
+
+ return igt_mmap(i915, obj, I915_MMAP_TYPE_GTT);
}
static int igt_mmap_cpu(void *arg)
{
- return igt_mmap(arg, I915_MMAP_TYPE_WC);
+ struct drm_i915_private *i915 = arg;
+ struct drm_i915_gem_object *obj =
+ i915_gem_object_create_internal(i915, PAGE_SIZE);
+ if (IS_ERR(obj))
+ return PTR_ERR(obj);
+
+ return igt_mmap(i915, obj, I915_MMAP_TYPE_WC);
+}
+
+static int igt_mmap_lmem(void *arg)
+{
+ struct drm_i915_private *i915 = arg;
+ struct drm_i915_gem_object *obj;
+
+ if (!HAS_LMEM(i915)) {
+ pr_info("device lacks LMEM support, skipping\n");
+ return 0;
+ }
+
+ obj = i915_gem_object_create_lmem(i915, PAGE_SIZE, I915_BO_ALLOC_CONTIGUOUS);
+ if (IS_ERR(obj))
+ return PTR_ERR(obj);
+
+ return igt_mmap(i915, obj, I915_MMAP_TYPE_WC);
}
static int check_present_pte(pte_t *pte, unsigned long addr, void *data)
@@ -887,20 +917,18 @@ static int prefault_range(u64 start, u64 len)
return __get_user(c, end - 1);
}
-static int igt_mmap_revoke(void *arg, enum i915_mmap_type type)
+static int igt_mmap_revoke(struct drm_i915_private *i915, struct drm_i915_gem_object *obj,
+ enum i915_mmap_type type)
{
- struct drm_i915_private *i915 = arg;
- struct drm_i915_gem_object *obj;
struct i915_mmap_offset *mmo;
unsigned long addr;
int err;
- if (!i915_ggtt_has_aperture(&i915->ggtt))
- return 0;
-
- obj = i915_gem_object_create_internal(i915, SZ_4M);
- if (IS_ERR(obj))
- return PTR_ERR(obj);
+ if (!i915_ggtt_has_aperture(&i915->ggtt) &&
+ type == I915_MMAP_TYPE_GTT) {
+ err = 0;
+ goto out;
+ }
mmo = mmap_offset_attach(obj, type, NULL);
if (IS_ERR(mmo)) {
@@ -959,12 +987,41 @@ static int igt_mmap_revoke(void *arg, enum i915_mmap_type type)
static int igt_mmap_gtt_revoke(void *arg)
{
- return igt_mmap_revoke(arg, I915_MMAP_TYPE_GTT);
+ struct drm_i915_private *i915 = arg;
+ struct drm_i915_gem_object *obj =
+ i915_gem_object_create_internal(i915, PAGE_SIZE);
+ if (IS_ERR(obj))
+ return PTR_ERR(obj);
+
+ return igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_GTT);
}
static int igt_mmap_cpu_revoke(void *arg)
{
- return igt_mmap_revoke(arg, I915_MMAP_TYPE_WC);
+ struct drm_i915_private *i915 = arg;
+ struct drm_i915_gem_object *obj =
+ i915_gem_object_create_internal(i915, PAGE_SIZE);
+ if (IS_ERR(obj))
+ return PTR_ERR(obj);
+
+ return igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_WC);
+}
+
+static int igt_mmap_lmem_revoke(void *arg)
+{
+ struct drm_i915_private *i915 = arg;
+ struct drm_i915_gem_object *obj;
+
+ if (!HAS_LMEM(i915)) {
+ pr_info("device lacks LMEM support, skipping\n");
+ return 0;
+ }
+
+ obj = i915_gem_object_create_lmem(i915, PAGE_SIZE, I915_BO_ALLOC_CONTIGUOUS);
+ if (IS_ERR(obj))
+ return PTR_ERR(obj);
+
+ return igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_WC);
}
int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
@@ -975,8 +1032,10 @@ int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
SUBTEST(igt_mmap_offset_exhaustion),
SUBTEST(igt_mmap_gtt),
SUBTEST(igt_mmap_cpu),
+ SUBTEST(igt_mmap_lmem),
SUBTEST(igt_mmap_gtt_revoke),
SUBTEST(igt_mmap_cpu_revoke),
+ SUBTEST(igt_mmap_lmem_revoke),
};
return i915_subtests(tests, i915);
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler
2019-12-11 5:59 [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler Abdiel Janulgue
@ 2019-12-11 9:39 ` Chris Wilson
2019-12-11 15:29 ` Chris Wilson
2019-12-13 9:49 ` Dan Carpenter
2 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2019-12-11 9:39 UTC (permalink / raw)
To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld
Quoting Abdiel Janulgue (2019-12-11 05:59:07)
> Fault handler to handle missing pages for lmem objects.
>
> v2: Handle ENXIO in fault error, account for offset in region start
> for fake lmem (Matt).
> Add selftest (Chris).
>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 44 ++++++++
> drivers/gpu/drm/i915/gem/i915_gem_lmem.h | 6 +
> drivers/gpu/drm/i915/gem/i915_gem_mman.c | 16 ++-
> drivers/gpu/drm/i915/gem/i915_gem_mman.h | 1 +
> .../drm/i915/gem/selftests/i915_gem_mman.c | 105 ++++++++++++++----
> 5 files changed, 147 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> index 0e2bf6b7e143..7e6d8d1546e3 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> @@ -6,8 +6,40 @@
> #include "intel_memory_region.h"
> #include "gem/i915_gem_region.h"
> #include "gem/i915_gem_lmem.h"
> +#include "gem/i915_gem_mman.h"
> #include "i915_drv.h"
>
> +vm_fault_t vm_fault_lmem(struct vm_fault *vmf)
> +{
> + struct vm_area_struct *area = vmf->vma;
> + struct i915_mmap_offset *priv = area->vm_private_data;
> + struct drm_i915_gem_object *obj = priv->obj;
> + unsigned long size = area->vm_end - area->vm_start;
> + bool write = area->vm_flags & VM_WRITE;
> + vm_fault_t vmf_ret;
> + int i, ret;
> +
> + /* Sanity check that we allow writing into this object */
> + if (i915_gem_object_is_readonly(obj) && write)
> + return VM_FAULT_SIGBUS;
> +
> + ret = i915_gem_object_pin_pages(obj);
> + if (ret)
> + return i915_error_to_vmf_fault(ret);
> +
> + for (i = 0; i < size >> PAGE_SHIFT; i++) {
> + vmf_ret = vmf_insert_pfn(area,
> + (unsigned long)area->vm_start + i * PAGE_SIZE,
> + i915_gem_object_lmem_io_pfn(obj, i));
> + if (vmf_ret != VM_FAULT_NOPAGE)
> + break;
> + }
> +
> + i915_gem_object_unpin_pages(obj);
> +
> + return vmf_ret;
> +}
> +
> const struct drm_i915_gem_object_ops i915_gem_lmem_obj_ops = {
> .flags = I915_GEM_OBJECT_HAS_IOMEM,
>
> @@ -56,6 +88,18 @@ i915_gem_object_lmem_io_map(struct drm_i915_gem_object *obj,
> return io_mapping_map_wc(&obj->mm.region->iomap, offset, size);
> }
>
> +unsigned long i915_gem_object_lmem_io_pfn(struct drm_i915_gem_object *obj,
> + unsigned long n)
> +{
> + struct intel_memory_region *mem = obj->mm.region;
> + resource_size_t offset;
> +
> + offset = i915_gem_object_get_dma_address(obj, n);
> + offset -= mem->region.start;
> +
> + return (mem->io_start + offset) >> PAGE_SHIFT;
> +}
> +
> bool i915_gem_object_is_lmem(struct drm_i915_gem_object *obj)
> {
> return obj->ops == &i915_gem_lmem_obj_ops;
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.h b/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
> index 7c176b8b7d2f..917ebef1529f 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
> @@ -7,6 +7,7 @@
> #define __I915_GEM_LMEM_H
>
> #include <linux/types.h>
> +#include <linux/mman.h>
>
> struct drm_i915_private;
> struct drm_i915_gem_object;
> @@ -22,8 +23,13 @@ void __iomem *
> i915_gem_object_lmem_io_map_page_atomic(struct drm_i915_gem_object *obj,
> unsigned long n);
>
> +unsigned long i915_gem_object_lmem_io_pfn(struct drm_i915_gem_object *obj,
> + unsigned long n);
> +
> bool i915_gem_object_is_lmem(struct drm_i915_gem_object *obj);
>
> +vm_fault_t vm_fault_lmem(struct vm_fault *vmf);
> +
> struct drm_i915_gem_object *
> i915_gem_object_create_lmem(struct drm_i915_private *i915,
> resource_size_t size,
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> index 879fff8adc48..c67c07905df5 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> @@ -11,6 +11,7 @@
> #include "gt/intel_gt.h"
> #include "gt/intel_gt_requests.h"
>
> +#include "i915_gem_lmem.h"
> #include "i915_drv.h"
> #include "i915_gem_gtt.h"
> #include "i915_gem_ioctls.h"
> @@ -203,7 +204,7 @@ compute_partial_view(const struct drm_i915_gem_object *obj,
> return view;
> }
>
> -static vm_fault_t i915_error_to_vmf_fault(int err)
> +vm_fault_t i915_error_to_vmf_fault(int err)
> {
> switch (err) {
> default:
> @@ -216,6 +217,7 @@ static vm_fault_t i915_error_to_vmf_fault(int err)
>
> case -ENOSPC: /* shmemfs allocation failure */
> case -ENOMEM: /* our allocation failure */
> + case -ENXIO:
> return VM_FAULT_OOM;
>
> case 0:
> @@ -560,7 +562,8 @@ __assign_mmap_offset(struct drm_file *file,
> }
>
> if (mmap_type != I915_MMAP_TYPE_GTT &&
> - !i915_gem_object_has_struct_page(obj)) {
> + !i915_gem_object_has_struct_page(obj) &&
> + !i915_gem_object_is_lmem(obj)) {
!i915_gem_object_type_has(STRUCT_PAGE | IOMEM)
> err = -ENODEV;
> goto out;
> }
> @@ -694,6 +697,12 @@ static const struct vm_operations_struct vm_ops_cpu = {
> .close = vm_close,
> };
>
> +static const struct vm_operations_struct vm_ops_lmem = {
> + .fault = vm_fault_lmem,
> + .open = vm_open,
> + .close = vm_close,
> +};
> +
> /*
> * This overcomes the limitation in drm_gem_mmap's assignment of a
> * drm_gem_object as the vma->vm_private_data. Since we need to
> @@ -784,6 +793,9 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> }
> vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
>
> + if (i915_gem_object_is_lmem(mmo->obj))
> + vma->vm_ops = &vm_ops_lmem;
lmem or iomem?
vm_ops = get_vm_cpu_ops(obj);
if (i915_gem_object_type_has(STRUCT_PAGE))
return &vm_ops_cpu;
if (i915_gem_object_type_has(IOMEM))
return &vm_ops_iomem;
GEM_BUG_ON(1);
return NULL;
?
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> index 862e01b7cb69..aded4d0564c9 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> @@ -23,6 +23,7 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma);
> int i915_gem_dumb_mmap_offset(struct drm_file *file_priv,
> struct drm_device *dev,
> u32 handle, u64 *offset);
> +vm_fault_t i915_error_to_vmf_fault(int err);
>
> void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
> void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> index 591435c5f368..d1335a586b7e 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> @@ -9,6 +9,8 @@
> #include "gt/intel_engine_pm.h"
> #include "gt/intel_gt.h"
> #include "gt/intel_gt_pm.h"
> +#include "gem/i915_gem_lmem.h"
> +#include "gem/i915_gem_region.h"
> #include "huge_gem_object.h"
> #include "i915_selftest.h"
> #include "selftests/i915_random.h"
> @@ -726,24 +728,23 @@ static int igt_mmap_offset_exhaustion(void *arg)
> }
>
> #define expand32(x) (((x) << 0) | ((x) << 8) | ((x) << 16) | ((x) << 24))
> -static int igt_mmap(void *arg, enum i915_mmap_type type)
> +static int igt_mmap(struct drm_i915_private *i915, struct drm_i915_gem_object *obj,
> + enum i915_mmap_type type)
> {
> - struct drm_i915_private *i915 = arg;
> - struct drm_i915_gem_object *obj;
> struct i915_mmap_offset *mmo;
> struct vm_area_struct *area;
> unsigned long addr;
> void *vaddr;
> int err = 0, i;
>
> - if (!i915_ggtt_has_aperture(&i915->ggtt))
> - return 0;
> -
> - obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
> - if (IS_ERR(obj))
> - return PTR_ERR(obj);
> + if (!i915_ggtt_has_aperture(&i915->ggtt) &&
> + type == I915_MMAP_TYPE_GTT) {
> + err = 0;
> + goto out;
> + }
>
> - vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB);
> + vaddr = i915_gem_object_pin_map(obj, i915_gem_object_is_lmem(obj) ?
> + I915_MAP_WC : I915_MAP_WB);
> if (IS_ERR(vaddr)) {
> err = PTR_ERR(vaddr);
> goto out;
> @@ -827,12 +828,41 @@ static int igt_mmap(void *arg, enum i915_mmap_type type)
>
> static int igt_mmap_gtt(void *arg)
> {
> - return igt_mmap(arg, I915_MMAP_TYPE_GTT);
> + struct drm_i915_private *i915 = arg;
> + struct drm_i915_gem_object *obj =
> + i915_gem_object_create_internal(i915, PAGE_SIZE);
> + if (IS_ERR(obj))
> + return PTR_ERR(obj);
> +
> + return igt_mmap(i915, obj, I915_MMAP_TYPE_GTT);
> }
>
> static int igt_mmap_cpu(void *arg)
> {
> - return igt_mmap(arg, I915_MMAP_TYPE_WC);
> + struct drm_i915_private *i915 = arg;
> + struct drm_i915_gem_object *obj =
> + i915_gem_object_create_internal(i915, PAGE_SIZE);
> + if (IS_ERR(obj))
> + return PTR_ERR(obj);
> +
> + return igt_mmap(i915, obj, I915_MMAP_TYPE_WC);
> +}
> +
> +static int igt_mmap_lmem(void *arg)
> +{
> + struct drm_i915_private *i915 = arg;
> + struct drm_i915_gem_object *obj;
> +
> + if (!HAS_LMEM(i915)) {
> + pr_info("device lacks LMEM support, skipping\n");
> + return 0;
> + }
> +
> + obj = i915_gem_object_create_lmem(i915, PAGE_SIZE, I915_BO_ALLOC_CONTIGUOUS);
> + if (IS_ERR(obj))
> + return PTR_ERR(obj);
> +
> + return igt_mmap(i915, obj, I915_MMAP_TYPE_WC);
Could we not generalise these to iterate over all memory regions?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler
2019-12-11 5:59 [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler Abdiel Janulgue
2019-12-11 9:39 ` Chris Wilson
@ 2019-12-11 15:29 ` Chris Wilson
2019-12-13 9:49 ` Dan Carpenter
2 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2019-12-11 15:29 UTC (permalink / raw)
To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld
Quoting Abdiel Janulgue (2019-12-11 05:59:07)
> Fault handler to handle missing pages for lmem objects.
>
> v2: Handle ENXIO in fault error, account for offset in region start
> for fake lmem (Matt).
> Add selftest (Chris).
>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 44 ++++++++
> drivers/gpu/drm/i915/gem/i915_gem_lmem.h | 6 +
> drivers/gpu/drm/i915/gem/i915_gem_mman.c | 16 ++-
> drivers/gpu/drm/i915/gem/i915_gem_mman.h | 1 +
> .../drm/i915/gem/selftests/i915_gem_mman.c | 105 ++++++++++++++----
> 5 files changed, 147 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> index 0e2bf6b7e143..7e6d8d1546e3 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> @@ -6,8 +6,40 @@
> #include "intel_memory_region.h"
> #include "gem/i915_gem_region.h"
> #include "gem/i915_gem_lmem.h"
> +#include "gem/i915_gem_mman.h"
> #include "i915_drv.h"
>
> +vm_fault_t vm_fault_lmem(struct vm_fault *vmf)
> +{
> + struct vm_area_struct *area = vmf->vma;
> + struct i915_mmap_offset *priv = area->vm_private_data;
> + struct drm_i915_gem_object *obj = priv->obj;
> + unsigned long size = area->vm_end - area->vm_start;
> + bool write = area->vm_flags & VM_WRITE;
> + vm_fault_t vmf_ret;
> + int i, ret;
> +
> + /* Sanity check that we allow writing into this object */
> + if (i915_gem_object_is_readonly(obj) && write)
> + return VM_FAULT_SIGBUS;
> +
> + ret = i915_gem_object_pin_pages(obj);
> + if (ret)
> + return i915_error_to_vmf_fault(ret);
> +
> + for (i = 0; i < size >> PAGE_SHIFT; i++) {
> + vmf_ret = vmf_insert_pfn(area,
> + (unsigned long)area->vm_start + i * PAGE_SIZE,
> + i915_gem_object_lmem_io_pfn(obj, i));
> + if (vmf_ret != VM_FAULT_NOPAGE)
> + break;
> + }
So why aren't we using remap_io_mapping() ?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler
2019-12-11 5:59 [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler Abdiel Janulgue
2019-12-11 9:39 ` Chris Wilson
@ 2019-12-13 9:49 ` Dan Carpenter
2019-12-13 9:49 ` Dan Carpenter
2 siblings, 0 replies; 17+ messages in thread
From: Dan Carpenter @ 2019-12-13 9:49 UTC (permalink / raw)
To: kbuild, Abdiel Janulgue; +Cc: intel-gfx, kbuild-all, Matthew Auld
Hi Abdiel,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-tip/drm-tip next-20191210]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Abdiel-Janulgue/drm-i915-Add-lmem-fault-handler/20191212-031235
base: git://anongit.freedesktop.org/drm-intel for-linux-next
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/gpu/drm/i915/gem/i915_gem_lmem.c:40 vm_fault_lmem() error: uninitialized symbol 'vmf_ret'.
# https://github.com/0day-ci/linux/commit/527bcb2414222221b5b3cea4909756095ae07d6a
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 527bcb2414222221b5b3cea4909756095ae07d6a
vim +/vmf_ret +40 drivers/gpu/drm/i915/gem/i915_gem_lmem.c
527bcb24142222 Abdiel Janulgue 2019-12-11 12 vm_fault_t vm_fault_lmem(struct vm_fault *vmf)
527bcb24142222 Abdiel Janulgue 2019-12-11 13 {
527bcb24142222 Abdiel Janulgue 2019-12-11 14 struct vm_area_struct *area = vmf->vma;
527bcb24142222 Abdiel Janulgue 2019-12-11 15 struct i915_mmap_offset *priv = area->vm_private_data;
527bcb24142222 Abdiel Janulgue 2019-12-11 16 struct drm_i915_gem_object *obj = priv->obj;
527bcb24142222 Abdiel Janulgue 2019-12-11 17 unsigned long size = area->vm_end - area->vm_start;
527bcb24142222 Abdiel Janulgue 2019-12-11 18 bool write = area->vm_flags & VM_WRITE;
527bcb24142222 Abdiel Janulgue 2019-12-11 19 vm_fault_t vmf_ret;
^^^^^^^^^^^^^^^^^^^
527bcb24142222 Abdiel Janulgue 2019-12-11 20 int i, ret;
527bcb24142222 Abdiel Janulgue 2019-12-11 21
527bcb24142222 Abdiel Janulgue 2019-12-11 22 /* Sanity check that we allow writing into this object */
527bcb24142222 Abdiel Janulgue 2019-12-11 23 if (i915_gem_object_is_readonly(obj) && write)
527bcb24142222 Abdiel Janulgue 2019-12-11 24 return VM_FAULT_SIGBUS;
527bcb24142222 Abdiel Janulgue 2019-12-11 25
527bcb24142222 Abdiel Janulgue 2019-12-11 26 ret = i915_gem_object_pin_pages(obj);
527bcb24142222 Abdiel Janulgue 2019-12-11 27 if (ret)
527bcb24142222 Abdiel Janulgue 2019-12-11 28 return i915_error_to_vmf_fault(ret);
527bcb24142222 Abdiel Janulgue 2019-12-11 29
527bcb24142222 Abdiel Janulgue 2019-12-11 30 for (i = 0; i < size >> PAGE_SHIFT; i++) {
Can size be less than a page?
527bcb24142222 Abdiel Janulgue 2019-12-11 31 vmf_ret = vmf_insert_pfn(area,
527bcb24142222 Abdiel Janulgue 2019-12-11 32 (unsigned long)area->vm_start + i * PAGE_SIZE,
527bcb24142222 Abdiel Janulgue 2019-12-11 33 i915_gem_object_lmem_io_pfn(obj, i));
527bcb24142222 Abdiel Janulgue 2019-12-11 34 if (vmf_ret != VM_FAULT_NOPAGE)
527bcb24142222 Abdiel Janulgue 2019-12-11 35 break;
527bcb24142222 Abdiel Janulgue 2019-12-11 36 }
527bcb24142222 Abdiel Janulgue 2019-12-11 37
527bcb24142222 Abdiel Janulgue 2019-12-11 38 i915_gem_object_unpin_pages(obj);
527bcb24142222 Abdiel Janulgue 2019-12-11 39
527bcb24142222 Abdiel Janulgue 2019-12-11 @40 return vmf_ret;
527bcb24142222 Abdiel Janulgue 2019-12-11 41 }
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler
@ 2019-12-13 9:49 ` Dan Carpenter
0 siblings, 0 replies; 17+ messages in thread
From: Dan Carpenter @ 2019-12-13 9:49 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3712 bytes --]
Hi Abdiel,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-tip/drm-tip next-20191210]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Abdiel-Janulgue/drm-i915-Add-lmem-fault-handler/20191212-031235
base: git://anongit.freedesktop.org/drm-intel for-linux-next
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/gpu/drm/i915/gem/i915_gem_lmem.c:40 vm_fault_lmem() error: uninitialized symbol 'vmf_ret'.
# https://github.com/0day-ci/linux/commit/527bcb2414222221b5b3cea4909756095ae07d6a
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 527bcb2414222221b5b3cea4909756095ae07d6a
vim +/vmf_ret +40 drivers/gpu/drm/i915/gem/i915_gem_lmem.c
527bcb24142222 Abdiel Janulgue 2019-12-11 12 vm_fault_t vm_fault_lmem(struct vm_fault *vmf)
527bcb24142222 Abdiel Janulgue 2019-12-11 13 {
527bcb24142222 Abdiel Janulgue 2019-12-11 14 struct vm_area_struct *area = vmf->vma;
527bcb24142222 Abdiel Janulgue 2019-12-11 15 struct i915_mmap_offset *priv = area->vm_private_data;
527bcb24142222 Abdiel Janulgue 2019-12-11 16 struct drm_i915_gem_object *obj = priv->obj;
527bcb24142222 Abdiel Janulgue 2019-12-11 17 unsigned long size = area->vm_end - area->vm_start;
527bcb24142222 Abdiel Janulgue 2019-12-11 18 bool write = area->vm_flags & VM_WRITE;
527bcb24142222 Abdiel Janulgue 2019-12-11 19 vm_fault_t vmf_ret;
^^^^^^^^^^^^^^^^^^^
527bcb24142222 Abdiel Janulgue 2019-12-11 20 int i, ret;
527bcb24142222 Abdiel Janulgue 2019-12-11 21
527bcb24142222 Abdiel Janulgue 2019-12-11 22 /* Sanity check that we allow writing into this object */
527bcb24142222 Abdiel Janulgue 2019-12-11 23 if (i915_gem_object_is_readonly(obj) && write)
527bcb24142222 Abdiel Janulgue 2019-12-11 24 return VM_FAULT_SIGBUS;
527bcb24142222 Abdiel Janulgue 2019-12-11 25
527bcb24142222 Abdiel Janulgue 2019-12-11 26 ret = i915_gem_object_pin_pages(obj);
527bcb24142222 Abdiel Janulgue 2019-12-11 27 if (ret)
527bcb24142222 Abdiel Janulgue 2019-12-11 28 return i915_error_to_vmf_fault(ret);
527bcb24142222 Abdiel Janulgue 2019-12-11 29
527bcb24142222 Abdiel Janulgue 2019-12-11 30 for (i = 0; i < size >> PAGE_SHIFT; i++) {
Can size be less than a page?
527bcb24142222 Abdiel Janulgue 2019-12-11 31 vmf_ret = vmf_insert_pfn(area,
527bcb24142222 Abdiel Janulgue 2019-12-11 32 (unsigned long)area->vm_start + i * PAGE_SIZE,
527bcb24142222 Abdiel Janulgue 2019-12-11 33 i915_gem_object_lmem_io_pfn(obj, i));
527bcb24142222 Abdiel Janulgue 2019-12-11 34 if (vmf_ret != VM_FAULT_NOPAGE)
527bcb24142222 Abdiel Janulgue 2019-12-11 35 break;
527bcb24142222 Abdiel Janulgue 2019-12-11 36 }
527bcb24142222 Abdiel Janulgue 2019-12-11 37
527bcb24142222 Abdiel Janulgue 2019-12-11 38 i915_gem_object_unpin_pages(obj);
527bcb24142222 Abdiel Janulgue 2019-12-11 39
527bcb24142222 Abdiel Janulgue 2019-12-11 @40 return vmf_ret;
527bcb24142222 Abdiel Janulgue 2019-12-11 41 }
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org Intel Corporation
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler
@ 2019-12-13 9:49 ` Dan Carpenter
0 siblings, 0 replies; 17+ messages in thread
From: Dan Carpenter @ 2019-12-13 9:49 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 3712 bytes --]
Hi Abdiel,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-tip/drm-tip next-20191210]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Abdiel-Janulgue/drm-i915-Add-lmem-fault-handler/20191212-031235
base: git://anongit.freedesktop.org/drm-intel for-linux-next
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/gpu/drm/i915/gem/i915_gem_lmem.c:40 vm_fault_lmem() error: uninitialized symbol 'vmf_ret'.
# https://github.com/0day-ci/linux/commit/527bcb2414222221b5b3cea4909756095ae07d6a
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 527bcb2414222221b5b3cea4909756095ae07d6a
vim +/vmf_ret +40 drivers/gpu/drm/i915/gem/i915_gem_lmem.c
527bcb24142222 Abdiel Janulgue 2019-12-11 12 vm_fault_t vm_fault_lmem(struct vm_fault *vmf)
527bcb24142222 Abdiel Janulgue 2019-12-11 13 {
527bcb24142222 Abdiel Janulgue 2019-12-11 14 struct vm_area_struct *area = vmf->vma;
527bcb24142222 Abdiel Janulgue 2019-12-11 15 struct i915_mmap_offset *priv = area->vm_private_data;
527bcb24142222 Abdiel Janulgue 2019-12-11 16 struct drm_i915_gem_object *obj = priv->obj;
527bcb24142222 Abdiel Janulgue 2019-12-11 17 unsigned long size = area->vm_end - area->vm_start;
527bcb24142222 Abdiel Janulgue 2019-12-11 18 bool write = area->vm_flags & VM_WRITE;
527bcb24142222 Abdiel Janulgue 2019-12-11 19 vm_fault_t vmf_ret;
^^^^^^^^^^^^^^^^^^^
527bcb24142222 Abdiel Janulgue 2019-12-11 20 int i, ret;
527bcb24142222 Abdiel Janulgue 2019-12-11 21
527bcb24142222 Abdiel Janulgue 2019-12-11 22 /* Sanity check that we allow writing into this object */
527bcb24142222 Abdiel Janulgue 2019-12-11 23 if (i915_gem_object_is_readonly(obj) && write)
527bcb24142222 Abdiel Janulgue 2019-12-11 24 return VM_FAULT_SIGBUS;
527bcb24142222 Abdiel Janulgue 2019-12-11 25
527bcb24142222 Abdiel Janulgue 2019-12-11 26 ret = i915_gem_object_pin_pages(obj);
527bcb24142222 Abdiel Janulgue 2019-12-11 27 if (ret)
527bcb24142222 Abdiel Janulgue 2019-12-11 28 return i915_error_to_vmf_fault(ret);
527bcb24142222 Abdiel Janulgue 2019-12-11 29
527bcb24142222 Abdiel Janulgue 2019-12-11 30 for (i = 0; i < size >> PAGE_SHIFT; i++) {
Can size be less than a page?
527bcb24142222 Abdiel Janulgue 2019-12-11 31 vmf_ret = vmf_insert_pfn(area,
527bcb24142222 Abdiel Janulgue 2019-12-11 32 (unsigned long)area->vm_start + i * PAGE_SIZE,
527bcb24142222 Abdiel Janulgue 2019-12-11 33 i915_gem_object_lmem_io_pfn(obj, i));
527bcb24142222 Abdiel Janulgue 2019-12-11 34 if (vmf_ret != VM_FAULT_NOPAGE)
527bcb24142222 Abdiel Janulgue 2019-12-11 35 break;
527bcb24142222 Abdiel Janulgue 2019-12-11 36 }
527bcb24142222 Abdiel Janulgue 2019-12-11 37
527bcb24142222 Abdiel Janulgue 2019-12-11 38 i915_gem_object_unpin_pages(obj);
527bcb24142222 Abdiel Janulgue 2019-12-11 39
527bcb24142222 Abdiel Janulgue 2019-12-11 @40 return vmf_ret;
527bcb24142222 Abdiel Janulgue 2019-12-11 41 }
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org Intel Corporation
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler
@ 2019-12-05 10:14 Abdiel Janulgue
2019-12-05 10:20 ` Matthew Auld
0 siblings, 1 reply; 17+ messages in thread
From: Abdiel Janulgue @ 2019-12-05 10:14 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
Fault handler to handle missing pages for lmem objects.
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 43 ++++++++++++++++++++++++
drivers/gpu/drm/i915/gem/i915_gem_lmem.h | 6 ++++
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 15 +++++++--
drivers/gpu/drm/i915/gem/i915_gem_mman.h | 1 +
4 files changed, 63 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
index 0e2bf6b7e143..78ac8d160cd7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
@@ -6,8 +6,40 @@
#include "intel_memory_region.h"
#include "gem/i915_gem_region.h"
#include "gem/i915_gem_lmem.h"
+#include "gem/i915_gem_mman.h"
#include "i915_drv.h"
+vm_fault_t vm_fault_lmem(struct vm_fault *vmf)
+{
+ struct vm_area_struct *area = vmf->vma;
+ struct i915_mmap_offset *priv = area->vm_private_data;
+ struct drm_i915_gem_object *obj = priv->obj;
+ unsigned long size = area->vm_end - area->vm_start;
+ bool write = area->vm_flags & VM_WRITE;
+ vm_fault_t vmf_ret;
+ int i, ret;
+
+ /* Sanity check that we allow writing into this object */
+ if (i915_gem_object_is_readonly(obj) && write)
+ return VM_FAULT_SIGBUS;
+
+ ret = i915_gem_object_pin_pages(obj);
+ if (ret)
+ return i915_error_to_vmf_fault(ret);
+
+ for (i = 0; i < size >> PAGE_SHIFT; i++) {
+ vmf_ret = vmf_insert_pfn(area,
+ (unsigned long)area->vm_start + i * PAGE_SIZE,
+ i915_gem_object_lmem_io_pfn(obj, i));
+ if (vmf_ret != VM_FAULT_NOPAGE)
+ break;
+ }
+
+ i915_gem_object_unpin_pages(obj);
+
+ return vmf_ret;
+}
+
const struct drm_i915_gem_object_ops i915_gem_lmem_obj_ops = {
.flags = I915_GEM_OBJECT_HAS_IOMEM,
@@ -56,6 +88,17 @@ i915_gem_object_lmem_io_map(struct drm_i915_gem_object *obj,
return io_mapping_map_wc(&obj->mm.region->iomap, offset, size);
}
+unsigned long i915_gem_object_lmem_io_pfn(struct drm_i915_gem_object *obj,
+ unsigned long n)
+{
+ struct intel_memory_region *mem = obj->mm.region;
+ resource_size_t offset;
+
+ offset = i915_gem_object_get_dma_address(obj, n);
+
+ return (mem->io_start + offset) >> PAGE_SHIFT;
+}
+
bool i915_gem_object_is_lmem(struct drm_i915_gem_object *obj)
{
return obj->ops == &i915_gem_lmem_obj_ops;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.h b/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
index 7c176b8b7d2f..917ebef1529f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
@@ -7,6 +7,7 @@
#define __I915_GEM_LMEM_H
#include <linux/types.h>
+#include <linux/mman.h>
struct drm_i915_private;
struct drm_i915_gem_object;
@@ -22,8 +23,13 @@ void __iomem *
i915_gem_object_lmem_io_map_page_atomic(struct drm_i915_gem_object *obj,
unsigned long n);
+unsigned long i915_gem_object_lmem_io_pfn(struct drm_i915_gem_object *obj,
+ unsigned long n);
+
bool i915_gem_object_is_lmem(struct drm_i915_gem_object *obj);
+vm_fault_t vm_fault_lmem(struct vm_fault *vmf);
+
struct drm_i915_gem_object *
i915_gem_object_create_lmem(struct drm_i915_private *i915,
resource_size_t size,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 3a3f30bc8ac7..5f6451ede53d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -11,6 +11,7 @@
#include "gt/intel_gt.h"
#include "gt/intel_gt_requests.h"
+#include "i915_gem_lmem.h"
#include "i915_drv.h"
#include "i915_gem_gtt.h"
#include "i915_gem_ioctls.h"
@@ -203,7 +204,7 @@ compute_partial_view(const struct drm_i915_gem_object *obj,
return view;
}
-static vm_fault_t i915_error_to_vmf_fault(int err)
+vm_fault_t i915_error_to_vmf_fault(int err)
{
switch (err) {
default:
@@ -560,7 +561,8 @@ __assign_mmap_offset(struct drm_file *file,
}
if (mmap_type != I915_MMAP_TYPE_GTT &&
- !i915_gem_object_has_struct_page(obj)) {
+ !i915_gem_object_has_struct_page(obj) &&
+ !i915_gem_object_is_lmem(obj)) {
err = -ENODEV;
goto out;
}
@@ -685,6 +687,12 @@ static const struct vm_operations_struct vm_ops_cpu = {
.close = vm_close,
};
+static const struct vm_operations_struct vm_ops_lmem = {
+ .fault = vm_fault_lmem,
+ .open = vm_open,
+ .close = vm_close,
+};
+
/*
* This overcomes the limitation in drm_gem_mmap's assignment of a
* drm_gem_object as the vma->vm_private_data. Since we need to
@@ -775,6 +783,9 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
}
vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
+ if (i915_gem_object_is_lmem(mmo->obj))
+ vma->vm_ops = &vm_ops_lmem;
+
return 0;
}
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
index 862e01b7cb69..aded4d0564c9 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
@@ -23,6 +23,7 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma);
int i915_gem_dumb_mmap_offset(struct drm_file *file_priv,
struct drm_device *dev,
u32 handle, u64 *offset);
+vm_fault_t i915_error_to_vmf_fault(int err);
void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler
2019-12-05 10:14 Abdiel Janulgue
@ 2019-12-05 10:20 ` Matthew Auld
2019-12-05 10:31 ` Chris Wilson
0 siblings, 1 reply; 17+ messages in thread
From: Matthew Auld @ 2019-12-05 10:20 UTC (permalink / raw)
To: Abdiel Janulgue; +Cc: Intel Graphics Development, Matthew Auld
On Thu, 5 Dec 2019 at 10:14, Abdiel Janulgue
<abdiel.janulgue@linux.intel.com> wrote:
>
> Fault handler to handle missing pages for lmem objects.
>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 43 ++++++++++++++++++++++++
> drivers/gpu/drm/i915/gem/i915_gem_lmem.h | 6 ++++
> drivers/gpu/drm/i915/gem/i915_gem_mman.c | 15 +++++++--
> drivers/gpu/drm/i915/gem/i915_gem_mman.h | 1 +
> 4 files changed, 63 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> index 0e2bf6b7e143..78ac8d160cd7 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> @@ -6,8 +6,40 @@
> #include "intel_memory_region.h"
> #include "gem/i915_gem_region.h"
> #include "gem/i915_gem_lmem.h"
> +#include "gem/i915_gem_mman.h"
> #include "i915_drv.h"
>
> +vm_fault_t vm_fault_lmem(struct vm_fault *vmf)
> +{
> + struct vm_area_struct *area = vmf->vma;
> + struct i915_mmap_offset *priv = area->vm_private_data;
> + struct drm_i915_gem_object *obj = priv->obj;
> + unsigned long size = area->vm_end - area->vm_start;
> + bool write = area->vm_flags & VM_WRITE;
> + vm_fault_t vmf_ret;
> + int i, ret;
> +
> + /* Sanity check that we allow writing into this object */
> + if (i915_gem_object_is_readonly(obj) && write)
> + return VM_FAULT_SIGBUS;
> +
> + ret = i915_gem_object_pin_pages(obj);
> + if (ret)
> + return i915_error_to_vmf_fault(ret);
Don't we need to handle -ENXIO in i915_error_to_vmf_fault?
> +
> + for (i = 0; i < size >> PAGE_SHIFT; i++) {
> + vmf_ret = vmf_insert_pfn(area,
> + (unsigned long)area->vm_start + i * PAGE_SIZE,
> + i915_gem_object_lmem_io_pfn(obj, i));
> + if (vmf_ret != VM_FAULT_NOPAGE)
> + break;
> + }
> +
> + i915_gem_object_unpin_pages(obj);
> +
> + return vmf_ret;
> +}
> +
> const struct drm_i915_gem_object_ops i915_gem_lmem_obj_ops = {
> .flags = I915_GEM_OBJECT_HAS_IOMEM,
>
> @@ -56,6 +88,17 @@ i915_gem_object_lmem_io_map(struct drm_i915_gem_object *obj,
> return io_mapping_map_wc(&obj->mm.region->iomap, offset, size);
> }
>
> +unsigned long i915_gem_object_lmem_io_pfn(struct drm_i915_gem_object *obj,
> + unsigned long n)
> +{
> + struct intel_memory_region *mem = obj->mm.region;
> + resource_size_t offset;
> +
> + offset = i915_gem_object_get_dma_address(obj, n);
offset -= mem->region.start; for poor old fake local-memory.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler
2019-12-05 10:20 ` Matthew Auld
@ 2019-12-05 10:31 ` Chris Wilson
0 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2019-12-05 10:31 UTC (permalink / raw)
To: Abdiel Janulgue, Matthew Auld; +Cc: Intel Graphics Development, Matthew Auld
Quoting Matthew Auld (2019-12-05 10:20:55)
> On Thu, 5 Dec 2019 at 10:14, Abdiel Janulgue
> <abdiel.janulgue@linux.intel.com> wrote:
> >
> > Fault handler to handle missing pages for lmem objects.
> >
> > Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 43 ++++++++++++++++++++++++
> > drivers/gpu/drm/i915/gem/i915_gem_lmem.h | 6 ++++
> > drivers/gpu/drm/i915/gem/i915_gem_mman.c | 15 +++++++--
> > drivers/gpu/drm/i915/gem/i915_gem_mman.h | 1 +
> > 4 files changed, 63 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> > index 0e2bf6b7e143..78ac8d160cd7 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> > @@ -6,8 +6,40 @@
> > #include "intel_memory_region.h"
> > #include "gem/i915_gem_region.h"
> > #include "gem/i915_gem_lmem.h"
> > +#include "gem/i915_gem_mman.h"
> > #include "i915_drv.h"
> >
> > +vm_fault_t vm_fault_lmem(struct vm_fault *vmf)
> > +{
> > + struct vm_area_struct *area = vmf->vma;
> > + struct i915_mmap_offset *priv = area->vm_private_data;
> > + struct drm_i915_gem_object *obj = priv->obj;
> > + unsigned long size = area->vm_end - area->vm_start;
> > + bool write = area->vm_flags & VM_WRITE;
> > + vm_fault_t vmf_ret;
> > + int i, ret;
> > +
> > + /* Sanity check that we allow writing into this object */
> > + if (i915_gem_object_is_readonly(obj) && write)
> > + return VM_FAULT_SIGBUS;
> > +
> > + ret = i915_gem_object_pin_pages(obj);
> > + if (ret)
> > + return i915_error_to_vmf_fault(ret);
>
> Don't we need to handle -ENXIO in i915_error_to_vmf_fault?
>
> > +
> > + for (i = 0; i < size >> PAGE_SHIFT; i++) {
> > + vmf_ret = vmf_insert_pfn(area,
> > + (unsigned long)area->vm_start + i * PAGE_SIZE,
> > + i915_gem_object_lmem_io_pfn(obj, i));
> > + if (vmf_ret != VM_FAULT_NOPAGE)
> > + break;
> > + }
> > +
> > + i915_gem_object_unpin_pages(obj);
> > +
> > + return vmf_ret;
> > +}
> > +
> > const struct drm_i915_gem_object_ops i915_gem_lmem_obj_ops = {
> > .flags = I915_GEM_OBJECT_HAS_IOMEM,
> >
> > @@ -56,6 +88,17 @@ i915_gem_object_lmem_io_map(struct drm_i915_gem_object *obj,
> > return io_mapping_map_wc(&obj->mm.region->iomap, offset, size);
> > }
> >
> > +unsigned long i915_gem_object_lmem_io_pfn(struct drm_i915_gem_object *obj,
> > + unsigned long n)
> > +{
> > + struct intel_memory_region *mem = obj->mm.region;
> > + resource_size_t offset;
> > +
> > + offset = i915_gem_object_get_dma_address(obj, n);
>
> offset -= mem->region.start; for poor old fake local-memory.
And so one asks for the selftests...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2019-12-13 20:44 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-12 11:34 [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler Abdiel Janulgue
2019-12-12 11:45 ` Chris Wilson
2019-12-12 14:19 ` Chris Wilson
2019-12-12 15:11 ` Matthew Auld
2019-12-12 15:19 ` Chris Wilson
2019-12-13 5:39 ` Abdiel Janulgue
2019-12-12 15:56 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Add lmem fault handler (rev3) Patchwork
2019-12-12 16:22 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
-- strict thread matches above, loose matches on Subject: below --
2019-12-11 5:59 [Intel-gfx] [PATCH] drm/i915: Add lmem fault handler Abdiel Janulgue
2019-12-11 9:39 ` Chris Wilson
2019-12-11 15:29 ` Chris Wilson
2019-12-13 9:49 ` Dan Carpenter
2019-12-13 9:49 ` Dan Carpenter
2019-12-13 9:49 ` Dan Carpenter
2019-12-05 10:14 Abdiel Janulgue
2019-12-05 10:20 ` Matthew Auld
2019-12-05 10:31 ` Chris Wilson
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.