dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] drm/i915: Sanity Check for device memory region
@ 2021-12-08 15:34 Ramalingam C
  2021-12-08 15:34 ` [PATCH 1/3] drm/i915: Exclude reserved stolen from driver use Ramalingam C
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Ramalingam C @ 2021-12-08 15:34 UTC (permalink / raw)
  To: intel-gfx, dri-devel

Changes for introducing the quick test on the device memory range and
also a test of detailed validation for each addr of the range with read
and write.

Detailed testing is optionally enabled with a modparam i915.memtest=1

And third patch fixes the driver accessible stolen memory.

v2: Adding a wrapper for the memtest [Chris]
v3: Handling a bisecting issue.

Chris Wilson (3):
  drm/i915: Exclude reserved stolen from driver use
  drm/i915: Sanitycheck device iomem on probe
  drm/i915: Test all device memory on probing

 drivers/gpu/drm/i915/gem/i915_gem_stolen.c |   3 +
 drivers/gpu/drm/i915/i915_params.c         |   3 +
 drivers/gpu/drm/i915/i915_params.h         |   1 +
 drivers/gpu/drm/i915/intel_memory_region.c | 128 +++++++++++++++++++++
 4 files changed, 135 insertions(+)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] drm/i915: Exclude reserved stolen from driver use
  2021-12-08 15:34 [PATCH 0/3] drm/i915: Sanity Check for device memory region Ramalingam C
@ 2021-12-08 15:34 ` Ramalingam C
  2021-12-08 15:34 ` [PATCH 2/3] drm/i915: Sanitycheck device iomem on probe Ramalingam C
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Ramalingam C @ 2021-12-08 15:34 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Matthew Auld, Andi Shyti, Chris Wilson

From: Chris Wilson <chris@chris-wilson.co.uk>

Remove the portion of stolen memory reserved for private use from driver
access.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
cc: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index bce03d74a0b4..6ea3ca21cdf3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -488,6 +488,9 @@ static int i915_gem_init_stolen(struct intel_memory_region *mem)
 		return 0;
 	}
 
+	/* Exclude the reserved region from driver use */
+	mem->region.end = reserved_base - 1;
+
 	/* It is possible for the reserved area to end before the end of stolen
 	 * memory, so just consider the start. */
 	reserved_total = stolen_top - reserved_base;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] drm/i915: Sanitycheck device iomem on probe
  2021-12-08 15:34 [PATCH 0/3] drm/i915: Sanity Check for device memory region Ramalingam C
  2021-12-08 15:34 ` [PATCH 1/3] drm/i915: Exclude reserved stolen from driver use Ramalingam C
@ 2021-12-08 15:34 ` Ramalingam C
  2021-12-09 16:26   ` [PATCH v4 " Ramalingam C
  2021-12-08 15:34 ` [PATCH 3/3] drm/i915: Test all device memory on probing Ramalingam C
  2021-12-14  9:45 ` [PATCH 0/3] drm/i915: Sanity Check for device memory region Ramalingam C
  3 siblings, 1 reply; 7+ messages in thread
From: Ramalingam C @ 2021-12-08 15:34 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Matthew Auld, Andi Shyti, Chris Wilson

From: Chris Wilson <chris@chris-wilson.co.uk>

As we setup the memory regions for the device, give each a quick test to
verify that we can read and write to the full iomem range. This ensures
that our physical addressing for the device's memory is correct, and
some reassurance that the memory is functional.

v2: wrapper for memtest [Chris]

v3: Removed the unused ptr i915 [Chris]

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_memory_region.c | 116 +++++++++++++++++++++
 1 file changed, 116 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_memory_region.c b/drivers/gpu/drm/i915/intel_memory_region.c
index b43121609e25..458afc648772 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/intel_memory_region.c
@@ -3,6 +3,8 @@
  * Copyright © 2019 Intel Corporation
  */
 
+#include <linux/prandom.h>
+
 #include "intel_memory_region.h"
 #include "i915_drv.h"
 #include "i915_ttm_buddy_manager.h"
@@ -29,6 +31,99 @@ static const struct {
 	},
 };
 
+static int __iopagetest(struct intel_memory_region *mem,
+			u8 __iomem *va, int pagesize,
+			u8 value, resource_size_t offset,
+			const void *caller)
+{
+	int byte = prandom_u32_max(pagesize);
+	u8 result[3];
+
+	memset_io(va, value, pagesize); /* or GPF! */
+	wmb();
+
+	result[0] = ioread8(va);
+	result[1] = ioread8(va + byte);
+	result[2] = ioread8(va + pagesize - 1);
+	if (memchr_inv(result, value, sizeof(result))) {
+		dev_err(mem->i915->drm.dev,
+			"Failed to read back from memory region:%pR at [%pa + %pa] for %ps; wrote %x, read (%x, %x, %x)\n",
+			&mem->region, &mem->io_start, &offset, caller,
+			value, result[0], result[1], result[2]);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int iopagetest(struct intel_memory_region *mem,
+		      resource_size_t offset,
+		      const void *caller)
+{
+	const u8 val[] = { 0x0, 0xa5, 0xc3, 0xf0 };
+	void __iomem *va;
+	int err;
+	int i;
+
+	va = ioremap_wc(mem->io_start + offset, PAGE_SIZE);
+	if (!va) {
+		dev_err(mem->i915->drm.dev,
+			"Failed to ioremap memory region [%pa + %px] for %ps\n",
+			&mem->io_start, &offset, caller);
+		return -EFAULT;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(val); i++) {
+		err = __iopagetest(mem, va, PAGE_SIZE, val[i], offset, caller);
+		if (err)
+			break;
+
+		err = __iopagetest(mem, va, PAGE_SIZE, ~val[i], offset, caller);
+		if (err)
+			break;
+	}
+
+	iounmap(va);
+	return err;
+}
+
+static resource_size_t random_page(resource_size_t last)
+{
+	/* Limited to low 44b (16TiB), but should suffice for a spot check */
+	return prandom_u32_max(last >> PAGE_SHIFT) << PAGE_SHIFT;
+}
+
+static int iomemtest(struct intel_memory_region *mem, const void *caller)
+{
+	resource_size_t last = resource_size(&mem->region) - PAGE_SIZE;
+	int err;
+
+	/*
+	 * Quick test to check read/write access to the iomap (backing store).
+	 *
+	 * Write a byte, read it back. If the iomapping fails, we expect
+	 * a GPF preventing further execution. If the backing store does not
+	 * exist, the read back will return garbage. We check a couple of pages,
+	 * the first and last of the specified region to confirm the backing
+	 * store + iomap does cover the entire memory region; and we check
+	 * a random offset within as a quick spot check for bad memory.
+	 */
+
+	err = iopagetest(mem, 0, caller);
+	if (err)
+		return err;
+
+	err = iopagetest(mem, last, caller);
+	if (err)
+		return err;
+
+	err = iopagetest(mem, random_page(last), caller);
+	if (err)
+		return err;
+
+	return 0;
+}
+
 struct intel_memory_region *
 intel_memory_region_lookup(struct drm_i915_private *i915,
 			   u16 class, u16 instance)
@@ -90,6 +185,20 @@ void intel_memory_region_debug(struct intel_memory_region *mr,
 			   &mr->total, &mr->avail);
 }
 
+static int intel_memory_region_memtest(struct intel_memory_region *mem,
+				       void *caller)
+{
+	int err = 0;
+
+	if (!mem->io_start)
+		return 0;
+
+	if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
+		err = iomemtest(mem, caller);
+
+	return err;
+}
+
 struct intel_memory_region *
 intel_memory_region_create(struct drm_i915_private *i915,
 			   resource_size_t start,
@@ -126,8 +235,15 @@ intel_memory_region_create(struct drm_i915_private *i915,
 			goto err_free;
 	}
 
+	err = intel_memory_region_memtest(mem, (void *)_RET_IP_);
+	if (err)
+		goto err_release;
+
 	return mem;
 
+err_release:
+	if (mem->ops->release)
+		mem->ops->release(mem);
 err_free:
 	kfree(mem);
 	return ERR_PTR(err);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] drm/i915: Test all device memory on probing
  2021-12-08 15:34 [PATCH 0/3] drm/i915: Sanity Check for device memory region Ramalingam C
  2021-12-08 15:34 ` [PATCH 1/3] drm/i915: Exclude reserved stolen from driver use Ramalingam C
  2021-12-08 15:34 ` [PATCH 2/3] drm/i915: Sanitycheck device iomem on probe Ramalingam C
@ 2021-12-08 15:34 ` Ramalingam C
  2021-12-14  9:45 ` [PATCH 0/3] drm/i915: Sanity Check for device memory region Ramalingam C
  3 siblings, 0 replies; 7+ messages in thread
From: Ramalingam C @ 2021-12-08 15:34 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Matthew Auld, Andi Shyti, Chris Wilson

From: Chris Wilson <chris@chris-wilson.co.uk>

This extends the previous sanitychecking of device memory to read/write
all the memory on the device during the device probe, ala memtest86,
as an optional module parameter: i915.memtest=1. This is not expected to
be fast, but a reasonably thorough verfification that the device memory
is accessible and doesn't return bit errors.

v2: Rebased.

Suggested-by: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_params.c         |  3 ++
 drivers/gpu/drm/i915/i915_params.h         |  1 +
 drivers/gpu/drm/i915/intel_memory_region.c | 36 ++++++++++++++--------
 3 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index e07f4cfea63a..525ae832aa9a 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -140,6 +140,9 @@ i915_param_named_unsafe(invert_brightness, int, 0400,
 i915_param_named(disable_display, bool, 0400,
 	"Disable display (default: false)");
 
+i915_param_named(memtest, bool, 0400,
+	"Perform a read/write test of all device memory on module load (default: off)");
+
 i915_param_named(mmio_debug, int, 0400,
 	"Enable the MMIO debug code for the first N failures (default: off). "
 	"This may negatively affect performance.");
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 8d725b64592d..c9d53ff910a0 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -64,6 +64,7 @@ struct drm_printer;
 	param(char *, guc_firmware_path, NULL, 0400) \
 	param(char *, huc_firmware_path, NULL, 0400) \
 	param(char *, dmc_firmware_path, NULL, 0400) \
+	param(bool, memtest, false, 0400) \
 	param(int, mmio_debug, -IS_ENABLED(CONFIG_DRM_I915_DEBUG_MMIO), 0600) \
 	param(int, edp_vswing, 0, 0400) \
 	param(unsigned int, reset, 3, 0600) \
diff --git a/drivers/gpu/drm/i915/intel_memory_region.c b/drivers/gpu/drm/i915/intel_memory_region.c
index 458afc648772..bab5b5caa6d8 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/intel_memory_region.c
@@ -93,9 +93,12 @@ static resource_size_t random_page(resource_size_t last)
 	return prandom_u32_max(last >> PAGE_SHIFT) << PAGE_SHIFT;
 }
 
-static int iomemtest(struct intel_memory_region *mem, const void *caller)
+static int iomemtest(struct intel_memory_region *mem,
+		     bool test_all,
+		     const void *caller)
 {
 	resource_size_t last = resource_size(&mem->region) - PAGE_SIZE;
+	resource_size_t page;
 	int err;
 
 	/*
@@ -109,17 +112,25 @@ static int iomemtest(struct intel_memory_region *mem, const void *caller)
 	 * a random offset within as a quick spot check for bad memory.
 	 */
 
-	err = iopagetest(mem, 0, caller);
-	if (err)
-		return err;
+	if (test_all) {
+		for (page = 0; page <= last; page += PAGE_SIZE) {
+			err = iopagetest(mem, page, caller);
+			if (err)
+				return err;
+		}
+	} else {
+		err = iopagetest(mem, 0, caller);
+		if (err)
+			return err;
 
-	err = iopagetest(mem, last, caller);
-	if (err)
-		return err;
+		err = iopagetest(mem, last, caller);
+		if (err)
+			return err;
 
-	err = iopagetest(mem, random_page(last), caller);
-	if (err)
-		return err;
+		err = iopagetest(mem, random_page(last), caller);
+		if (err)
+			return err;
+	}
 
 	return 0;
 }
@@ -188,13 +199,14 @@ void intel_memory_region_debug(struct intel_memory_region *mr,
 static int intel_memory_region_memtest(struct intel_memory_region *mem,
 				       void *caller)
 {
+	struct drm_i915_private *i915 = mem->i915;
 	int err = 0;
 
 	if (!mem->io_start)
 		return 0;
 
-	if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
-		err = iomemtest(mem, caller);
+	if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) || i915->params.memtest)
+		err = iomemtest(mem, i915->params.memtest, caller);
 
 	return err;
 }
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v4 2/3] drm/i915: Sanitycheck device iomem on probe
  2021-12-08 15:34 ` [PATCH 2/3] drm/i915: Sanitycheck device iomem on probe Ramalingam C
@ 2021-12-09 16:26   ` Ramalingam C
  0 siblings, 0 replies; 7+ messages in thread
From: Ramalingam C @ 2021-12-09 16:26 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Matthew Auld, Andi Shyti, Chris Wilson

From: Chris Wilson <chris@chris-wilson.co.uk>

As we setup the memory regions for the device, give each a quick test to
verify that we can read and write to the full iomem range. This ensures
that our physical addressing for the device's memory is correct, and
some reassurance that the memory is functional.

v2: wrapper for memtest [Chris]

v3: Removed the unused ptr i915 [Chris]

v4: used the %pa for the resource_size_t.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_memory_region.c | 116 +++++++++++++++++++++
 1 file changed, 116 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_memory_region.c b/drivers/gpu/drm/i915/intel_memory_region.c
index b43121609e25..7bfb6df02e72 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/intel_memory_region.c
@@ -3,6 +3,8 @@
  * Copyright © 2019 Intel Corporation
  */
 
+#include <linux/prandom.h>
+
 #include "intel_memory_region.h"
 #include "i915_drv.h"
 #include "i915_ttm_buddy_manager.h"
@@ -29,6 +31,99 @@ static const struct {
 	},
 };
 
+static int __iopagetest(struct intel_memory_region *mem,
+			u8 __iomem *va, int pagesize,
+			u8 value, resource_size_t offset,
+			const void *caller)
+{
+	int byte = prandom_u32_max(pagesize);
+	u8 result[3];
+
+	memset_io(va, value, pagesize); /* or GPF! */
+	wmb();
+
+	result[0] = ioread8(va);
+	result[1] = ioread8(va + byte);
+	result[2] = ioread8(va + pagesize - 1);
+	if (memchr_inv(result, value, sizeof(result))) {
+		dev_err(mem->i915->drm.dev,
+			"Failed to read back from memory region:%pR at [%pa + %pa] for %ps; wrote %x, read (%x, %x, %x)\n",
+			&mem->region, &mem->io_start, &offset, caller,
+			value, result[0], result[1], result[2]);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int iopagetest(struct intel_memory_region *mem,
+		      resource_size_t offset,
+		      const void *caller)
+{
+	const u8 val[] = { 0x0, 0xa5, 0xc3, 0xf0 };
+	void __iomem *va;
+	int err;
+	int i;
+
+	va = ioremap_wc(mem->io_start + offset, PAGE_SIZE);
+	if (!va) {
+		dev_err(mem->i915->drm.dev,
+			"Failed to ioremap memory region [%pa + %pa] for %ps\n",
+			&mem->io_start, &offset, caller);
+		return -EFAULT;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(val); i++) {
+		err = __iopagetest(mem, va, PAGE_SIZE, val[i], offset, caller);
+		if (err)
+			break;
+
+		err = __iopagetest(mem, va, PAGE_SIZE, ~val[i], offset, caller);
+		if (err)
+			break;
+	}
+
+	iounmap(va);
+	return err;
+}
+
+static resource_size_t random_page(resource_size_t last)
+{
+	/* Limited to low 44b (16TiB), but should suffice for a spot check */
+	return prandom_u32_max(last >> PAGE_SHIFT) << PAGE_SHIFT;
+}
+
+static int iomemtest(struct intel_memory_region *mem, const void *caller)
+{
+	resource_size_t last = resource_size(&mem->region) - PAGE_SIZE;
+	int err;
+
+	/*
+	 * Quick test to check read/write access to the iomap (backing store).
+	 *
+	 * Write a byte, read it back. If the iomapping fails, we expect
+	 * a GPF preventing further execution. If the backing store does not
+	 * exist, the read back will return garbage. We check a couple of pages,
+	 * the first and last of the specified region to confirm the backing
+	 * store + iomap does cover the entire memory region; and we check
+	 * a random offset within as a quick spot check for bad memory.
+	 */
+
+	err = iopagetest(mem, 0, caller);
+	if (err)
+		return err;
+
+	err = iopagetest(mem, last, caller);
+	if (err)
+		return err;
+
+	err = iopagetest(mem, random_page(last), caller);
+	if (err)
+		return err;
+
+	return 0;
+}
+
 struct intel_memory_region *
 intel_memory_region_lookup(struct drm_i915_private *i915,
 			   u16 class, u16 instance)
@@ -90,6 +185,20 @@ void intel_memory_region_debug(struct intel_memory_region *mr,
 			   &mr->total, &mr->avail);
 }
 
+static int intel_memory_region_memtest(struct intel_memory_region *mem,
+				       void *caller)
+{
+	int err = 0;
+
+	if (!mem->io_start)
+		return 0;
+
+	if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
+		err = iomemtest(mem, caller);
+
+	return err;
+}
+
 struct intel_memory_region *
 intel_memory_region_create(struct drm_i915_private *i915,
 			   resource_size_t start,
@@ -126,8 +235,15 @@ intel_memory_region_create(struct drm_i915_private *i915,
 			goto err_free;
 	}
 
+	err = intel_memory_region_memtest(mem, (void *)_RET_IP_);
+	if (err)
+		goto err_release;
+
 	return mem;
 
+err_release:
+	if (mem->ops->release)
+		mem->ops->release(mem);
 err_free:
 	kfree(mem);
 	return ERR_PTR(err);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/3] drm/i915: Sanity Check for device memory region
  2021-12-08 15:34 [PATCH 0/3] drm/i915: Sanity Check for device memory region Ramalingam C
                   ` (2 preceding siblings ...)
  2021-12-08 15:34 ` [PATCH 3/3] drm/i915: Test all device memory on probing Ramalingam C
@ 2021-12-14  9:45 ` Ramalingam C
  3 siblings, 0 replies; 7+ messages in thread
From: Ramalingam C @ 2021-12-14  9:45 UTC (permalink / raw)
  To: intel-gfx, dri-devel

On 2021-12-08 at 21:04:01 +0530, Ramalingam C wrote:
> Changes for introducing the quick test on the device memory range and
> also a test of detailed validation for each addr of the range with read
> and write.
> 
> Detailed testing is optionally enabled with a modparam i915.memtest=1
> 
> And third patch fixes the driver accessible stolen memory.
> 
> v2: Adding a wrapper for the memtest [Chris]
> v3: Handling a bisecting issue.

Thanks for the review. The changes are merged.

Ram.
> 
> Chris Wilson (3):
>   drm/i915: Exclude reserved stolen from driver use
>   drm/i915: Sanitycheck device iomem on probe
>   drm/i915: Test all device memory on probing
> 
>  drivers/gpu/drm/i915/gem/i915_gem_stolen.c |   3 +
>  drivers/gpu/drm/i915/i915_params.c         |   3 +
>  drivers/gpu/drm/i915/i915_params.h         |   1 +
>  drivers/gpu/drm/i915/intel_memory_region.c | 128 +++++++++++++++++++++
>  4 files changed, 135 insertions(+)
> 
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/3] drm/i915: Sanitycheck device iomem on probe
  2021-12-08 14:57 Ramalingam C
@ 2021-12-08 14:57 ` Ramalingam C
  0 siblings, 0 replies; 7+ messages in thread
From: Ramalingam C @ 2021-12-08 14:57 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Matthew Auld, Chris Wilson

From: Chris Wilson <chris@chris-wilson.co.uk>

As we setup the memory regions for the device, give each a quick test to
verify that we can read and write to the full iomem range. This ensures
that our physical addressing for the device's memory is correct, and
some reassurance that the memory is functional.

v2: wrapper for memtest [Chris]

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
---
 drivers/gpu/drm/i915/intel_memory_region.c | 117 +++++++++++++++++++++
 1 file changed, 117 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_memory_region.c b/drivers/gpu/drm/i915/intel_memory_region.c
index b43121609e25..ed65b67e2d14 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/intel_memory_region.c
@@ -3,6 +3,8 @@
  * Copyright © 2019 Intel Corporation
  */
 
+#include <linux/prandom.h>
+
 #include "intel_memory_region.h"
 #include "i915_drv.h"
 #include "i915_ttm_buddy_manager.h"
@@ -29,6 +31,99 @@ static const struct {
 	},
 };
 
+static int __iopagetest(struct intel_memory_region *mem,
+			u8 __iomem *va, int pagesize,
+			u8 value, resource_size_t offset,
+			const void *caller)
+{
+	int byte = prandom_u32_max(pagesize);
+	u8 result[3];
+
+	memset_io(va, value, pagesize); /* or GPF! */
+	wmb();
+
+	result[0] = ioread8(va);
+	result[1] = ioread8(va + byte);
+	result[2] = ioread8(va + pagesize - 1);
+	if (memchr_inv(result, value, sizeof(result))) {
+		dev_err(mem->i915->drm.dev,
+			"Failed to read back from memory region:%pR at [%pa + %pa] for %ps; wrote %x, read (%x, %x, %x)\n",
+			&mem->region, &mem->io_start, &offset, caller,
+			value, result[0], result[1], result[2]);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int iopagetest(struct intel_memory_region *mem,
+		      resource_size_t offset,
+		      const void *caller)
+{
+	const u8 val[] = { 0x0, 0xa5, 0xc3, 0xf0 };
+	void __iomem *va;
+	int err;
+	int i;
+
+	va = ioremap_wc(mem->io_start + offset, PAGE_SIZE);
+	if (!va) {
+		dev_err(mem->i915->drm.dev,
+			"Failed to ioremap memory region [%pa + %px] for %ps\n",
+			&mem->io_start, &offset, caller);
+		return -EFAULT;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(val); i++) {
+		err = __iopagetest(mem, va, PAGE_SIZE, val[i], offset, caller);
+		if (err)
+			break;
+
+		err = __iopagetest(mem, va, PAGE_SIZE, ~val[i], offset, caller);
+		if (err)
+			break;
+	}
+
+	iounmap(va);
+	return err;
+}
+
+static resource_size_t random_page(resource_size_t last)
+{
+	/* Limited to low 44b (16TiB), but should suffice for a spot check */
+	return prandom_u32_max(last >> PAGE_SHIFT) << PAGE_SHIFT;
+}
+
+static int iomemtest(struct intel_memory_region *mem, const void *caller)
+{
+	resource_size_t last = resource_size(&mem->region) - PAGE_SIZE;
+	int err;
+
+	/*
+	 * Quick test to check read/write access to the iomap (backing store).
+	 *
+	 * Write a byte, read it back. If the iomapping fails, we expect
+	 * a GPF preventing further execution. If the backing store does not
+	 * exist, the read back will return garbage. We check a couple of pages,
+	 * the first and last of the specified region to confirm the backing
+	 * store + iomap does cover the entire memory region; and we check
+	 * a random offset within as a quick spot check for bad memory.
+	 */
+
+	err = iopagetest(mem, 0, caller);
+	if (err)
+		return err;
+
+	err = iopagetest(mem, last, caller);
+	if (err)
+		return err;
+
+	err = iopagetest(mem, random_page(last), caller);
+	if (err)
+		return err;
+
+	return 0;
+}
+
 struct intel_memory_region *
 intel_memory_region_lookup(struct drm_i915_private *i915,
 			   u16 class, u16 instance)
@@ -90,6 +185,21 @@ void intel_memory_region_debug(struct intel_memory_region *mr,
 			   &mr->total, &mr->avail);
 }
 
+static int intel_memory_region_memtest(struct intel_memory_region *mem,
+				       void *caller)
+{
+	struct drm_i915_private *i915 = mem->i915;
+	int err = 0;
+
+	if (!mem->io_start)
+		return 0;
+
+	if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
+		err = iomemtest(mem, caller);
+
+	return err;
+}
+
 struct intel_memory_region *
 intel_memory_region_create(struct drm_i915_private *i915,
 			   resource_size_t start,
@@ -126,8 +236,15 @@ intel_memory_region_create(struct drm_i915_private *i915,
 			goto err_free;
 	}
 
+	err = intel_memory_region_memtest(mem, (void *)_RET_IP_);
+	if (err)
+		goto err_release;
+
 	return mem;
 
+err_release:
+	if (mem->ops->release)
+		mem->ops->release(mem);
 err_free:
 	kfree(mem);
 	return ERR_PTR(err);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-12-14  9:45 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-08 15:34 [PATCH 0/3] drm/i915: Sanity Check for device memory region Ramalingam C
2021-12-08 15:34 ` [PATCH 1/3] drm/i915: Exclude reserved stolen from driver use Ramalingam C
2021-12-08 15:34 ` [PATCH 2/3] drm/i915: Sanitycheck device iomem on probe Ramalingam C
2021-12-09 16:26   ` [PATCH v4 " Ramalingam C
2021-12-08 15:34 ` [PATCH 3/3] drm/i915: Test all device memory on probing Ramalingam C
2021-12-14  9:45 ` [PATCH 0/3] drm/i915: Sanity Check for device memory region Ramalingam C
  -- strict thread matches above, loose matches on Subject: below --
2021-12-08 14:57 Ramalingam C
2021-12-08 14:57 ` [PATCH 2/3] drm/i915: Sanitycheck device iomem on probe Ramalingam C

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).