All of lore.kernel.org
 help / color / mirror / Atom feed
From: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
To: <intel-gfx@lists.freedesktop.org>, <dri-devel@lists.freedesktop.org>
Cc: "Matthew Brost" <matthew.brost@intel.com>,
	"Michał Winiarski" <michal.winiarski@intel.com>,
	"Balasubramani Vivekanandan"
	<balasubramani.vivekanandan@intel.com>,
	lucas.demarchi@intel.com, siva.mullati@intel.com,
	"Nirmoy Das" <nirmoy.das@intel.com>
Subject: [PATCH v3 6/7] drm/i915/gt: Avoid direct dereferencing of io memory
Date: Tue, 26 Apr 2022 22:21:47 +0530	[thread overview]
Message-ID: <20220426165148.1784-7-balasubramani.vivekanandan@intel.com> (raw)
In-Reply-To: <20220426165148.1784-1-balasubramani.vivekanandan@intel.com>

io mapped memory should not be directly dereferenced to ensure
portability. io memory should be read/written/copied using helper
functions.
i915_memcpy_from_wc() function was used to copy the data from io memory to
a temporary buffer and pointer to the temporary buffer was passed to CRC
calculation function.
But i915_memcpy_from_wc() only does a copy if the platform supports fast
copy using non-temporal instructions. Otherwise the pointer to io memory
was passed for CRC calculation. CRC function will directly dereference
io memory and would not work properly on non-x86 platforms.
To make it portable, it should be ensured always temporary buffer is
used for CRC and not io memory.
drm_memcpy_from_wc_vaddr() is now used for copying instead of
i915_memcpy_from_wc() for 2 reasons.
- i915_memcpy_from_wc() will be deprecated.
- drm_memcpy_from_wc_vaddr() will not fail if the fast copy is not
supported but uses memcpy_fromio as fallback for copying.

Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>

Signed-off-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
Acked-by: Nirmoy Das <nirmoy.das@intel.com>
---
 drivers/gpu/drm/i915/gt/selftest_reset.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_reset.c b/drivers/gpu/drm/i915/gt/selftest_reset.c
index 37c38bdd5f47..7a455583c687 100644
--- a/drivers/gpu/drm/i915/gt/selftest_reset.c
+++ b/drivers/gpu/drm/i915/gt/selftest_reset.c
@@ -3,6 +3,7 @@
  * Copyright © 2018 Intel Corporation
  */
 
+#include <drm/drm_cache.h>
 #include <linux/crc32.h>
 
 #include "gem/i915_gem_stolen.h"
@@ -82,7 +83,7 @@ __igt_reset_stolen(struct intel_gt *gt,
 	for (page = 0; page < num_pages; page++) {
 		dma_addr_t dma = (dma_addr_t)dsm->start + (page << PAGE_SHIFT);
 		void __iomem *s;
-		void *in;
+		struct iosys_map src_map;
 
 		ggtt->vm.insert_page(&ggtt->vm, dma,
 				     ggtt->error_capture.start,
@@ -98,10 +99,9 @@ __igt_reset_stolen(struct intel_gt *gt,
 					     ((page + 1) << PAGE_SHIFT) - 1))
 			memset_io(s, STACK_MAGIC, PAGE_SIZE);
 
-		in = (void __force *)s;
-		if (i915_memcpy_from_wc(tmp, in, PAGE_SIZE))
-			in = tmp;
-		crc[page] = crc32_le(0, in, PAGE_SIZE);
+		iosys_map_set_vaddr_iomem(&src_map, s);
+		drm_memcpy_from_wc_vaddr(tmp, &src_map, 0, PAGE_SIZE);
+		crc[page] = crc32_le(0, tmp, PAGE_SIZE);
 
 		io_mapping_unmap(s);
 	}
@@ -122,7 +122,7 @@ __igt_reset_stolen(struct intel_gt *gt,
 	for (page = 0; page < num_pages; page++) {
 		dma_addr_t dma = (dma_addr_t)dsm->start + (page << PAGE_SHIFT);
 		void __iomem *s;
-		void *in;
+		struct iosys_map src_map;
 		u32 x;
 
 		ggtt->vm.insert_page(&ggtt->vm, dma,
@@ -134,10 +134,9 @@ __igt_reset_stolen(struct intel_gt *gt,
 				      ggtt->error_capture.start,
 				      PAGE_SIZE);
 
-		in = (void __force *)s;
-		if (i915_memcpy_from_wc(tmp, in, PAGE_SIZE))
-			in = tmp;
-		x = crc32_le(0, in, PAGE_SIZE);
+		iosys_map_set_vaddr_iomem(&src_map, s);
+		drm_memcpy_from_wc_vaddr(tmp, &src_map, 0, PAGE_SIZE);
+		x = crc32_le(0, tmp, PAGE_SIZE);
 
 		if (x != crc[page] &&
 		    !__drm_mm_interval_first(&gt->i915->mm.stolen,
@@ -146,7 +145,7 @@ __igt_reset_stolen(struct intel_gt *gt,
 			pr_debug("unused stolen page %pa modified by GPU reset\n",
 				 &page);
 			if (count++ == 0)
-				igt_hexdump(in, PAGE_SIZE);
+				igt_hexdump(tmp, PAGE_SIZE);
 			max = page;
 		}
 
-- 
2.25.1


WARNING: multiple messages have this Message-ID (diff)
From: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
To: <intel-gfx@lists.freedesktop.org>, <dri-devel@lists.freedesktop.org>
Cc: "Michał Winiarski" <michal.winiarski@intel.com>,
	lucas.demarchi@intel.com, siva.mullati@intel.com,
	"Nirmoy Das" <nirmoy.das@intel.com>
Subject: [Intel-gfx] [PATCH v3 6/7] drm/i915/gt: Avoid direct dereferencing of io memory
Date: Tue, 26 Apr 2022 22:21:47 +0530	[thread overview]
Message-ID: <20220426165148.1784-7-balasubramani.vivekanandan@intel.com> (raw)
In-Reply-To: <20220426165148.1784-1-balasubramani.vivekanandan@intel.com>

io mapped memory should not be directly dereferenced to ensure
portability. io memory should be read/written/copied using helper
functions.
i915_memcpy_from_wc() function was used to copy the data from io memory to
a temporary buffer and pointer to the temporary buffer was passed to CRC
calculation function.
But i915_memcpy_from_wc() only does a copy if the platform supports fast
copy using non-temporal instructions. Otherwise the pointer to io memory
was passed for CRC calculation. CRC function will directly dereference
io memory and would not work properly on non-x86 platforms.
To make it portable, it should be ensured always temporary buffer is
used for CRC and not io memory.
drm_memcpy_from_wc_vaddr() is now used for copying instead of
i915_memcpy_from_wc() for 2 reasons.
- i915_memcpy_from_wc() will be deprecated.
- drm_memcpy_from_wc_vaddr() will not fail if the fast copy is not
supported but uses memcpy_fromio as fallback for copying.

Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>

Signed-off-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
Acked-by: Nirmoy Das <nirmoy.das@intel.com>
---
 drivers/gpu/drm/i915/gt/selftest_reset.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_reset.c b/drivers/gpu/drm/i915/gt/selftest_reset.c
index 37c38bdd5f47..7a455583c687 100644
--- a/drivers/gpu/drm/i915/gt/selftest_reset.c
+++ b/drivers/gpu/drm/i915/gt/selftest_reset.c
@@ -3,6 +3,7 @@
  * Copyright © 2018 Intel Corporation
  */
 
+#include <drm/drm_cache.h>
 #include <linux/crc32.h>
 
 #include "gem/i915_gem_stolen.h"
@@ -82,7 +83,7 @@ __igt_reset_stolen(struct intel_gt *gt,
 	for (page = 0; page < num_pages; page++) {
 		dma_addr_t dma = (dma_addr_t)dsm->start + (page << PAGE_SHIFT);
 		void __iomem *s;
-		void *in;
+		struct iosys_map src_map;
 
 		ggtt->vm.insert_page(&ggtt->vm, dma,
 				     ggtt->error_capture.start,
@@ -98,10 +99,9 @@ __igt_reset_stolen(struct intel_gt *gt,
 					     ((page + 1) << PAGE_SHIFT) - 1))
 			memset_io(s, STACK_MAGIC, PAGE_SIZE);
 
-		in = (void __force *)s;
-		if (i915_memcpy_from_wc(tmp, in, PAGE_SIZE))
-			in = tmp;
-		crc[page] = crc32_le(0, in, PAGE_SIZE);
+		iosys_map_set_vaddr_iomem(&src_map, s);
+		drm_memcpy_from_wc_vaddr(tmp, &src_map, 0, PAGE_SIZE);
+		crc[page] = crc32_le(0, tmp, PAGE_SIZE);
 
 		io_mapping_unmap(s);
 	}
@@ -122,7 +122,7 @@ __igt_reset_stolen(struct intel_gt *gt,
 	for (page = 0; page < num_pages; page++) {
 		dma_addr_t dma = (dma_addr_t)dsm->start + (page << PAGE_SHIFT);
 		void __iomem *s;
-		void *in;
+		struct iosys_map src_map;
 		u32 x;
 
 		ggtt->vm.insert_page(&ggtt->vm, dma,
@@ -134,10 +134,9 @@ __igt_reset_stolen(struct intel_gt *gt,
 				      ggtt->error_capture.start,
 				      PAGE_SIZE);
 
-		in = (void __force *)s;
-		if (i915_memcpy_from_wc(tmp, in, PAGE_SIZE))
-			in = tmp;
-		x = crc32_le(0, in, PAGE_SIZE);
+		iosys_map_set_vaddr_iomem(&src_map, s);
+		drm_memcpy_from_wc_vaddr(tmp, &src_map, 0, PAGE_SIZE);
+		x = crc32_le(0, tmp, PAGE_SIZE);
 
 		if (x != crc[page] &&
 		    !__drm_mm_interval_first(&gt->i915->mm.stolen,
@@ -146,7 +145,7 @@ __igt_reset_stolen(struct intel_gt *gt,
 			pr_debug("unused stolen page %pa modified by GPU reset\n",
 				 &page);
 			if (count++ == 0)
-				igt_hexdump(in, PAGE_SIZE);
+				igt_hexdump(tmp, PAGE_SIZE);
 			max = page;
 		}
 
-- 
2.25.1


  parent reply	other threads:[~2022-04-26 16:50 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-26 16:51 [PATCH v3 0/7] drm/i915: Use the memcpy_from_wc function from drm Balasubramani Vivekanandan
2022-04-26 16:51 ` [Intel-gfx] " Balasubramani Vivekanandan
2022-04-26 16:51 ` [PATCH v3 1/7] drm: Relax alignment constraint for destination address Balasubramani Vivekanandan
2022-04-26 16:51   ` [Intel-gfx] " Balasubramani Vivekanandan
2022-04-26 16:51 ` [PATCH v3 2/7] drm: Add drm_memcpy_from_wc() variant which accepts " Balasubramani Vivekanandan
2022-04-26 16:51   ` [Intel-gfx] " Balasubramani Vivekanandan
2022-07-13 15:47   ` Lucas De Marchi
2022-07-13 15:47     ` Lucas De Marchi
2022-07-14 11:20     ` Christian König
2022-07-14 11:20       ` Christian König
2022-04-26 16:51 ` [PATCH v3 3/7] drm/i915: use the memcpy_from_wc call from the drm Balasubramani Vivekanandan
2022-04-26 16:51   ` [Intel-gfx] " Balasubramani Vivekanandan
2022-04-26 16:51 ` [PATCH v3 4/7] drm/i915/guc: use iosys_map abstraction to access GuC log Balasubramani Vivekanandan
2022-04-26 16:51   ` [Intel-gfx] " Balasubramani Vivekanandan
2022-04-26 16:51 ` [PATCH v3 5/7] drm/i915/selftests: use the memcpy_from_wc call from the drm Balasubramani Vivekanandan
2022-04-26 16:51   ` [Intel-gfx] " Balasubramani Vivekanandan
2022-04-26 16:51 ` Balasubramani Vivekanandan [this message]
2022-04-26 16:51   ` [Intel-gfx] [PATCH v3 6/7] drm/i915/gt: Avoid direct dereferencing of io memory Balasubramani Vivekanandan
2022-04-26 16:51 ` [PATCH v3 7/7] drm/i915: Avoid dereferencing io mapped memory Balasubramani Vivekanandan
2022-04-26 16:51   ` [Intel-gfx] " Balasubramani Vivekanandan
2022-04-26 17:57 ` [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Use the memcpy_from_wc function from drm (rev4) Patchwork
2022-04-26 20:16 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220426165148.1784-7-balasubramani.vivekanandan@intel.com \
    --to=balasubramani.vivekanandan@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=lucas.demarchi@intel.com \
    --cc=matthew.brost@intel.com \
    --cc=michal.winiarski@intel.com \
    --cc=nirmoy.das@intel.com \
    --cc=siva.mullati@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.