All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-xe] [PATCH v2 1/2] drm/xe/mmio: don't leak the forcewake ref
@ 2023-02-14 17:32 Matthew Auld
  2023-02-14 17:32 ` [Intel-xe] [PATCH v2 2/2] drm/xe/stolen: don't map stolen on small-bar Matthew Auld
  2023-02-14 19:37 ` [Intel-xe] [PATCH v2 1/2] drm/xe/mmio: don't leak the forcewake ref Lucas De Marchi
  0 siblings, 2 replies; 4+ messages in thread
From: Matthew Auld @ 2023-02-14 17:32 UTC (permalink / raw)
  To: intel-xe; +Cc: Lucas De Marchi

Make sure we properly release the forcewake ref on all error paths.

Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/xe/xe_mmio.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
index 8a953df2b468..497f643271ae 100644
--- a/drivers/gpu/drm/xe/xe_mmio.c
+++ b/drivers/gpu/drm/xe/xe_mmio.c
@@ -426,12 +426,16 @@ int xe_mmio_ioctl(struct drm_device *dev, void *data,
 	if (args->flags & DRM_XE_MMIO_WRITE) {
 		switch (bits_flag) {
 		case DRM_XE_MMIO_8BIT:
-			return -EINVAL; /* TODO */
+			ret = -EINVAL; /* TODO */
+			goto exit;
 		case DRM_XE_MMIO_16BIT:
-			return -EINVAL; /* TODO */
+			ret = -EINVAL; /* TODO */
+			goto exit;
 		case DRM_XE_MMIO_32BIT:
-			if (XE_IOCTL_ERR(xe, args->value > U32_MAX))
-				return -EINVAL;
+			if (XE_IOCTL_ERR(xe, args->value > U32_MAX)) {
+				ret = -EINVAL;
+				goto exit;
+			}
 			xe_mmio_write32(to_gt(xe), args->addr, args->value);
 			break;
 		case DRM_XE_MMIO_64BIT:
@@ -447,9 +451,11 @@ int xe_mmio_ioctl(struct drm_device *dev, void *data,
 	if (args->flags & DRM_XE_MMIO_READ) {
 		switch (bits_flag) {
 		case DRM_XE_MMIO_8BIT:
-			return -EINVAL; /* TODO */
+			ret = -EINVAL; /* TODO */
+			break;
 		case DRM_XE_MMIO_16BIT:
-			return -EINVAL; /* TODO */
+			ret = -EINVAL; /* TODO */
+			break;
 		case DRM_XE_MMIO_32BIT:
 			args->value = xe_mmio_read32(to_gt(xe), args->addr);
 			break;
-- 
2.39.1


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

* [Intel-xe] [PATCH v2 2/2] drm/xe/stolen: don't map stolen on small-bar
  2023-02-14 17:32 [Intel-xe] [PATCH v2 1/2] drm/xe/mmio: don't leak the forcewake ref Matthew Auld
@ 2023-02-14 17:32 ` Matthew Auld
  2023-02-14 19:45   ` Lucas De Marchi
  2023-02-14 19:37 ` [Intel-xe] [PATCH v2 1/2] drm/xe/mmio: don't leak the forcewake ref Lucas De Marchi
  1 sibling, 1 reply; 4+ messages in thread
From: Matthew Auld @ 2023-02-14 17:32 UTC (permalink / raw)
  To: intel-xe; +Cc: Lucas De Marchi

The driver should still be functional with small-bar, just that the vram
size is clamped to the BAR size (until we add proper support for tiered
vram). For stolen vram we shouldn't iomap anything if the BAR size
doesn't also contain the stolen portion, since on discrete the stolen
portion is always at the end of normal vram. Stolen should still be
functional, just that allocating CPU visible io memory will always
return an error.

v2 (Lucas)
  - Mention in the commit message that stolen vram is always as the end
    of normal vram, which is why stolen in not mappable on small-bar
    systems.
  - Just make xe_ttm_stolen_inaccessible() return true for such cases.
    Also rename to xe_ttm_stolen_cpu_inaccessible to better describe
    that we are talking about direct CPU access. Plus add some
    kernel-doc.

Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/209
Reported-by: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
---
 drivers/gpu/drm/xe/xe_bo.c             |  2 +-
 drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c | 47 +++++++++++++++++++-------
 drivers/gpu/drm/xe/xe_ttm_stolen_mgr.h |  2 +-
 3 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
index 9b7b9c8f84be..4173c1418b63 100644
--- a/drivers/gpu/drm/xe/xe_bo.c
+++ b/drivers/gpu/drm/xe/xe_bo.c
@@ -1149,7 +1149,7 @@ struct xe_bo *xe_bo_create_pin_map_at(struct xe_device *xe, struct xe_gt *gt,
 	u64 end = offset == ~0ull ? offset : start + size;
 
 	if (flags & XE_BO_CREATE_STOLEN_BIT &&
-	    xe_ttm_stolen_inaccessible(xe))
+	    xe_ttm_stolen_cpu_inaccessible(xe))
 		flags |= XE_BO_CREATE_GGTT_BIT;
 
 	bo = xe_bo_create_locked_range(xe, gt, vm, size, start, end, type, flags);
diff --git a/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c b/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c
index b4e9c88644e4..e20c567f276f 100644
--- a/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c
+++ b/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c
@@ -21,11 +21,6 @@
 #include "xe_ttm_stolen_mgr.h"
 #include "xe_ttm_vram_mgr.h"
 
-bool xe_ttm_stolen_inaccessible(struct xe_device *xe)
-{
-	return !IS_DGFX(xe) && GRAPHICS_VERx100(xe) < 1270;
-}
-
 struct xe_ttm_stolen_mgr {
 	struct xe_ttm_vram_mgr base;
 
@@ -43,6 +38,34 @@ to_stolen_mgr(struct ttm_resource_manager *man)
 	return container_of(man, struct xe_ttm_stolen_mgr, base.manager);
 }
 
+/**
+ * xe_ttm_stolen_cpu_inaccessible - Can we directly CPU access stolen memory for
+ * this device.
+ *
+ * On some integrated platforms we can't directly access stolen via the CPU
+ * (like some normal system memory).  Also on small-bar systems for discrete,
+ * since stolen is always as the end of normal VRAM, and the BAR likely doesn't
+ * stretch that far. However CPU access of stolen is generally rare, and at
+ * least on discrete should not be needed.
+ *
+ * If this is indeed inaccessible then we fallback to using the GGTT mappable
+ * aperture for CPU access. On discrete platforms we have no such thing, so when
+ * later attempting to CPU map the memory an error is instead thrown.
+ */
+bool xe_ttm_stolen_cpu_inaccessible(struct xe_device *xe)
+{
+	struct ttm_resource_manager *ttm_mgr =
+		ttm_manager_type(&xe->ttm, XE_PL_STOLEN);
+	struct xe_ttm_stolen_mgr *mgr;
+
+	if (!ttm_mgr)
+		return true;
+
+	mgr = to_stolen_mgr(ttm_mgr);
+
+	return !mgr->io_base || GRAPHICS_VERx100(xe) < 1270;
+}
+
 static s64 detect_bar2_dgfx(struct xe_device *xe, struct xe_ttm_stolen_mgr *mgr)
 {
 	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
@@ -126,7 +149,7 @@ void xe_ttm_stolen_mgr_init(struct xe_device *xe)
 
 	if (IS_DGFX(xe))
 		stolen_size = detect_bar2_dgfx(xe, mgr);
-	else if (!xe_ttm_stolen_inaccessible(xe))
+	else if (GRAPHICS_VERx100(xe) >= 1270)
 		stolen_size = detect_bar2_integrated(xe, mgr);
 	else
 		stolen_size = detect_stolen(xe, mgr);
@@ -140,7 +163,6 @@ void xe_ttm_stolen_mgr_init(struct xe_device *xe)
 	if (pgsize < PAGE_SIZE)
 		pgsize = PAGE_SIZE;
 
-
 	err = __xe_ttm_vram_mgr_init(xe, &mgr->base, XE_PL_STOLEN, stolen_size, pgsize);
 	if (err) {
 		drm_dbg_kms(&xe->drm, "Stolen mgr init failed: %i\n", err);
@@ -150,7 +172,7 @@ void xe_ttm_stolen_mgr_init(struct xe_device *xe)
 	drm_dbg_kms(&xe->drm, "Initialized stolen memory support with %llu bytes\n",
 		    stolen_size);
 
-	if (!xe_ttm_stolen_inaccessible(xe))
+	if (!xe_ttm_stolen_cpu_inaccessible(xe))
 		mgr->mapping = devm_ioremap_wc(&pdev->dev, mgr->io_base, stolen_size);
 }
 
@@ -161,10 +183,9 @@ u64 xe_ttm_stolen_io_offset(struct xe_bo *bo, u32 offset)
 	struct xe_ttm_stolen_mgr *mgr = to_stolen_mgr(ttm_mgr);
 	struct xe_res_cursor cur;
 
-	if (!mgr->io_base)
-		return 0;
+	XE_BUG_ON(!mgr->io_base);
 
-	if (!IS_DGFX(xe) && xe_ttm_stolen_inaccessible(xe))
+	if (!IS_DGFX(xe) && xe_ttm_stolen_cpu_inaccessible(xe))
 		return mgr->io_base + xe_bo_ggtt_addr(bo) + offset;
 
 	xe_res_first(bo->ttm.resource, offset, 4096, &cur);
@@ -202,6 +223,8 @@ static int __xe_ttm_stolen_io_mem_reserve_stolen(struct xe_device *xe,
 #ifdef CONFIG_X86
 	struct xe_bo *bo = ttm_to_xe_bo(mem->bo);
 
+	XE_BUG_ON(IS_DGFX(xe));
+
 	/* XXX: Require BO to be mapped to GGTT? */
 	if (drm_WARN_ON(&xe->drm, !(bo->flags & XE_BO_CREATE_GGTT_BIT)))
 		return -EIO;
@@ -228,7 +251,7 @@ int xe_ttm_stolen_io_mem_reserve(struct xe_device *xe, struct ttm_resource *mem)
 	if (!mgr || !mgr->io_base)
 		return -EIO;
 
-	if (!xe_ttm_stolen_inaccessible(xe))
+	if (!xe_ttm_stolen_cpu_inaccessible(xe))
 		return __xe_ttm_stolen_io_mem_reserve_bar2(xe, mgr, mem);
 	else
 		return __xe_ttm_stolen_io_mem_reserve_stolen(xe, mgr, mem);
diff --git a/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.h b/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.h
index ade37abb0623..2fda97b97a05 100644
--- a/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.h
+++ b/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.h
@@ -14,7 +14,7 @@ struct xe_device;
 
 void xe_ttm_stolen_mgr_init(struct xe_device *xe);
 int xe_ttm_stolen_io_mem_reserve(struct xe_device *xe, struct ttm_resource *mem);
-bool xe_ttm_stolen_inaccessible(struct xe_device *xe);
+bool xe_ttm_stolen_cpu_inaccessible(struct xe_device *xe);
 u64 xe_ttm_stolen_io_offset(struct xe_bo *bo, u32 offset);
 u64 xe_ttm_stolen_gpu_offset(struct xe_device *xe);
 
-- 
2.39.1


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

* Re: [Intel-xe] [PATCH v2 1/2] drm/xe/mmio: don't leak the forcewake ref
  2023-02-14 17:32 [Intel-xe] [PATCH v2 1/2] drm/xe/mmio: don't leak the forcewake ref Matthew Auld
  2023-02-14 17:32 ` [Intel-xe] [PATCH v2 2/2] drm/xe/stolen: don't map stolen on small-bar Matthew Auld
@ 2023-02-14 19:37 ` Lucas De Marchi
  1 sibling, 0 replies; 4+ messages in thread
From: Lucas De Marchi @ 2023-02-14 19:37 UTC (permalink / raw)
  To: Matthew Auld; +Cc: intel-xe

On Tue, Feb 14, 2023 at 05:32:23PM +0000, Matthew Auld wrote:
>Make sure we properly release the forcewake ref on all error paths.
>
>Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>---
> drivers/gpu/drm/xe/xe_mmio.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
>index 8a953df2b468..497f643271ae 100644
>--- a/drivers/gpu/drm/xe/xe_mmio.c
>+++ b/drivers/gpu/drm/xe/xe_mmio.c
>@@ -426,12 +426,16 @@ int xe_mmio_ioctl(struct drm_device *dev, void *data,
> 	if (args->flags & DRM_XE_MMIO_WRITE) {
> 		switch (bits_flag) {
> 		case DRM_XE_MMIO_8BIT:
>-			return -EINVAL; /* TODO */
>+			ret = -EINVAL; /* TODO */
>+			goto exit;
> 		case DRM_XE_MMIO_16BIT:
>-			return -EINVAL; /* TODO */
>+			ret = -EINVAL; /* TODO */
>+			goto exit;
> 		case DRM_XE_MMIO_32BIT:
>-			if (XE_IOCTL_ERR(xe, args->value > U32_MAX))
>-				return -EINVAL;
>+			if (XE_IOCTL_ERR(xe, args->value > U32_MAX)) {
>+				ret = -EINVAL;
>+				goto exit;
>+			}
> 			xe_mmio_write32(to_gt(xe), args->addr, args->value);
> 			break;
> 		case DRM_XE_MMIO_64BIT:
>@@ -447,9 +451,11 @@ int xe_mmio_ioctl(struct drm_device *dev, void *data,
> 	if (args->flags & DRM_XE_MMIO_READ) {
> 		switch (bits_flag) {
> 		case DRM_XE_MMIO_8BIT:
>-			return -EINVAL; /* TODO */
>+			ret = -EINVAL; /* TODO */
>+			break;
> 		case DRM_XE_MMIO_16BIT:
>-			return -EINVAL; /* TODO */
>+			ret = -EINVAL; /* TODO */
>+			break;


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>


Suggestion, up to you:  move the read and write to separate functions
and just surround the them with the forcewake. Or just moving the
default and adding a fallthrough would make it less verbose too. Up to
you.

-----------------8<----------------
diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
index ced757ba9ccb..10704d8ed8b8 100644
--- a/drivers/gpu/drm/xe/xe_mmio.c
+++ b/drivers/gpu/drm/xe/xe_mmio.c
@@ -383,6 +383,52 @@ static const i915_reg_t mmio_read_whitelist[] = {
  	RING_TIMESTAMP(RENDER_RING_BASE),
  };
  
+static int mmio_ioctl_read(struct xe_device *xe,
+			   struct drm_xe_mmio *args)
+{
+	switch (args->flags & DRM_XE_MMIO_BITS_MASK) {
+	case DRM_XE_MMIO_32BIT:
+		if (XE_IOCTL_ERR(xe, args->value > U32_MAX))
+			return -EINVAL;
+		xe_mmio_write32(to_gt(xe), args->addr, args->value);
+		break;
+	case DRM_XE_MMIO_64BIT:
+		xe_mmio_write64(to_gt(xe), args->addr, args->value);
+		break;
+
+	default:
+		drm_WARN(&xe->drm, 1, "Invalid MMIO bit size");
+		fallthrough;
+	case DRM_XE_MMIO_8BIT: /* TODO */
+	case DRM_XE_MMIO_16BIT: /* TODO */
+		return -ENOTSUPP;
+	}
+
+	return 0;
+}
+
+static int mmio_ioctl_write(struct xe_device *xe,
+			    struct drm_xe_mmio *args)
+{
+	switch (args->flags & DRM_XE_MMIO_BITS_MASK) {
+	case DRM_XE_MMIO_32BIT:
+		args->value = xe_mmio_read32(to_gt(xe), args->addr);
+		break;
+	case DRM_XE_MMIO_64BIT:
+		args->value = xe_mmio_read64(to_gt(xe), args->addr);
+		break;
+
+	default:
+		drm_WARN(&xe->drm, 1, "Invalid MMIO bit size");
+		fallthrough;
+	case DRM_XE_MMIO_8BIT: /* TODO */
+	case DRM_XE_MMIO_16BIT: /* TODO */
+		return -ENOTSUPP;
+	}
+
+	return 0;
+}
+
  int xe_mmio_ioctl(struct drm_device *dev, void *data,
  		  struct drm_file *file)
  {
@@ -424,43 +470,13 @@ int xe_mmio_ioctl(struct drm_device *dev, void *data,
  	xe_force_wake_get(gt_to_fw(&xe->gt[0]), XE_FORCEWAKE_ALL);
  
  	if (args->flags & DRM_XE_MMIO_WRITE) {
-		switch (bits_flag) {
-		case DRM_XE_MMIO_8BIT:
-			return -EINVAL; /* TODO */
-		case DRM_XE_MMIO_16BIT:
-			return -EINVAL; /* TODO */
-		case DRM_XE_MMIO_32BIT:
-			if (XE_IOCTL_ERR(xe, args->value > U32_MAX))
-				return -EINVAL;
-			xe_mmio_write32(to_gt(xe), args->addr, args->value);
-			break;
-		case DRM_XE_MMIO_64BIT:
-			xe_mmio_write64(to_gt(xe), args->addr, args->value);
-			break;
-		default:
-			drm_WARN(&xe->drm, 1, "Invalid MMIO bit size");
-			ret = -EINVAL;
+		ret = mmio_ioctl_write(xe, args);
+		if (ret)
  			goto exit;
-		}
  	}
  
-	if (args->flags & DRM_XE_MMIO_READ) {
-		switch (bits_flag) {
-		case DRM_XE_MMIO_8BIT:
-			return -EINVAL; /* TODO */
-		case DRM_XE_MMIO_16BIT:
-			return -EINVAL; /* TODO */
-		case DRM_XE_MMIO_32BIT:
-			args->value = xe_mmio_read32(to_gt(xe), args->addr);
-			break;
-		case DRM_XE_MMIO_64BIT:
-			args->value = xe_mmio_read64(to_gt(xe), args->addr);
-			break;
-		default:
-			drm_WARN(&xe->drm, 1, "Invalid MMIO bit size");
-			ret = -EINVAL;
-		}
-	}
+	if (args->flags & DRM_XE_MMIO_READ)
+		ret = mmio_ioctl_read(xe, args);
  
  exit:
  	xe_force_wake_put(gt_to_fw(&xe->gt[0]), XE_FORCEWAKE_ALL);
-----------------8<----------------

Lucas De Marchi

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

* Re: [Intel-xe] [PATCH v2 2/2] drm/xe/stolen: don't map stolen on small-bar
  2023-02-14 17:32 ` [Intel-xe] [PATCH v2 2/2] drm/xe/stolen: don't map stolen on small-bar Matthew Auld
@ 2023-02-14 19:45   ` Lucas De Marchi
  0 siblings, 0 replies; 4+ messages in thread
From: Lucas De Marchi @ 2023-02-14 19:45 UTC (permalink / raw)
  To: Matthew Auld; +Cc: intel-xe

On Tue, Feb 14, 2023 at 05:32:24PM +0000, Matthew Auld wrote:
>The driver should still be functional with small-bar, just that the vram
>size is clamped to the BAR size (until we add proper support for tiered
>vram). For stolen vram we shouldn't iomap anything if the BAR size
>doesn't also contain the stolen portion, since on discrete the stolen
>portion is always at the end of normal vram. Stolen should still be
>functional, just that allocating CPU visible io memory will always
>return an error.
>
>v2 (Lucas)
>  - Mention in the commit message that stolen vram is always as the end
>    of normal vram, which is why stolen in not mappable on small-bar
>    systems.
>  - Just make xe_ttm_stolen_inaccessible() return true for such cases.
>    Also rename to xe_ttm_stolen_cpu_inaccessible to better describe
>    that we are talking about direct CPU access. Plus add some
>    kernel-doc.
>
>Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/209
>Reported-by: Lucas De Marchi <lucas.demarchi@intel.com>
>Signed-off-by: Matthew Auld <matthew.auld@intel.com>


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>


Lucas De Marchi

>---
> drivers/gpu/drm/xe/xe_bo.c             |  2 +-
> drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c | 47 +++++++++++++++++++-------
> drivers/gpu/drm/xe/xe_ttm_stolen_mgr.h |  2 +-
> 3 files changed, 37 insertions(+), 14 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
>index 9b7b9c8f84be..4173c1418b63 100644
>--- a/drivers/gpu/drm/xe/xe_bo.c
>+++ b/drivers/gpu/drm/xe/xe_bo.c
>@@ -1149,7 +1149,7 @@ struct xe_bo *xe_bo_create_pin_map_at(struct xe_device *xe, struct xe_gt *gt,
> 	u64 end = offset == ~0ull ? offset : start + size;
>
> 	if (flags & XE_BO_CREATE_STOLEN_BIT &&
>-	    xe_ttm_stolen_inaccessible(xe))
>+	    xe_ttm_stolen_cpu_inaccessible(xe))
> 		flags |= XE_BO_CREATE_GGTT_BIT;
>
> 	bo = xe_bo_create_locked_range(xe, gt, vm, size, start, end, type, flags);
>diff --git a/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c b/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c
>index b4e9c88644e4..e20c567f276f 100644
>--- a/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c
>+++ b/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c
>@@ -21,11 +21,6 @@
> #include "xe_ttm_stolen_mgr.h"
> #include "xe_ttm_vram_mgr.h"
>
>-bool xe_ttm_stolen_inaccessible(struct xe_device *xe)
>-{
>-	return !IS_DGFX(xe) && GRAPHICS_VERx100(xe) < 1270;
>-}
>-
> struct xe_ttm_stolen_mgr {
> 	struct xe_ttm_vram_mgr base;
>
>@@ -43,6 +38,34 @@ to_stolen_mgr(struct ttm_resource_manager *man)
> 	return container_of(man, struct xe_ttm_stolen_mgr, base.manager);
> }
>
>+/**
>+ * xe_ttm_stolen_cpu_inaccessible - Can we directly CPU access stolen memory for
>+ * this device.
>+ *
>+ * On some integrated platforms we can't directly access stolen via the CPU
>+ * (like some normal system memory).  Also on small-bar systems for discrete,
>+ * since stolen is always as the end of normal VRAM, and the BAR likely doesn't
>+ * stretch that far. However CPU access of stolen is generally rare, and at
>+ * least on discrete should not be needed.
>+ *
>+ * If this is indeed inaccessible then we fallback to using the GGTT mappable
>+ * aperture for CPU access. On discrete platforms we have no such thing, so when
>+ * later attempting to CPU map the memory an error is instead thrown.
>+ */
>+bool xe_ttm_stolen_cpu_inaccessible(struct xe_device *xe)
>+{
>+	struct ttm_resource_manager *ttm_mgr =
>+		ttm_manager_type(&xe->ttm, XE_PL_STOLEN);
>+	struct xe_ttm_stolen_mgr *mgr;
>+
>+	if (!ttm_mgr)
>+		return true;
>+
>+	mgr = to_stolen_mgr(ttm_mgr);
>+
>+	return !mgr->io_base || GRAPHICS_VERx100(xe) < 1270;
>+}
>+
> static s64 detect_bar2_dgfx(struct xe_device *xe, struct xe_ttm_stolen_mgr *mgr)
> {
> 	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>@@ -126,7 +149,7 @@ void xe_ttm_stolen_mgr_init(struct xe_device *xe)
>
> 	if (IS_DGFX(xe))
> 		stolen_size = detect_bar2_dgfx(xe, mgr);
>-	else if (!xe_ttm_stolen_inaccessible(xe))
>+	else if (GRAPHICS_VERx100(xe) >= 1270)
> 		stolen_size = detect_bar2_integrated(xe, mgr);
> 	else
> 		stolen_size = detect_stolen(xe, mgr);
>@@ -140,7 +163,6 @@ void xe_ttm_stolen_mgr_init(struct xe_device *xe)
> 	if (pgsize < PAGE_SIZE)
> 		pgsize = PAGE_SIZE;
>
>-
> 	err = __xe_ttm_vram_mgr_init(xe, &mgr->base, XE_PL_STOLEN, stolen_size, pgsize);
> 	if (err) {
> 		drm_dbg_kms(&xe->drm, "Stolen mgr init failed: %i\n", err);
>@@ -150,7 +172,7 @@ void xe_ttm_stolen_mgr_init(struct xe_device *xe)
> 	drm_dbg_kms(&xe->drm, "Initialized stolen memory support with %llu bytes\n",
> 		    stolen_size);
>
>-	if (!xe_ttm_stolen_inaccessible(xe))
>+	if (!xe_ttm_stolen_cpu_inaccessible(xe))
> 		mgr->mapping = devm_ioremap_wc(&pdev->dev, mgr->io_base, stolen_size);
> }
>
>@@ -161,10 +183,9 @@ u64 xe_ttm_stolen_io_offset(struct xe_bo *bo, u32 offset)
> 	struct xe_ttm_stolen_mgr *mgr = to_stolen_mgr(ttm_mgr);
> 	struct xe_res_cursor cur;
>
>-	if (!mgr->io_base)
>-		return 0;
>+	XE_BUG_ON(!mgr->io_base);
>
>-	if (!IS_DGFX(xe) && xe_ttm_stolen_inaccessible(xe))
>+	if (!IS_DGFX(xe) && xe_ttm_stolen_cpu_inaccessible(xe))
> 		return mgr->io_base + xe_bo_ggtt_addr(bo) + offset;
>
> 	xe_res_first(bo->ttm.resource, offset, 4096, &cur);
>@@ -202,6 +223,8 @@ static int __xe_ttm_stolen_io_mem_reserve_stolen(struct xe_device *xe,
> #ifdef CONFIG_X86
> 	struct xe_bo *bo = ttm_to_xe_bo(mem->bo);
>
>+	XE_BUG_ON(IS_DGFX(xe));
>+
> 	/* XXX: Require BO to be mapped to GGTT? */
> 	if (drm_WARN_ON(&xe->drm, !(bo->flags & XE_BO_CREATE_GGTT_BIT)))
> 		return -EIO;
>@@ -228,7 +251,7 @@ int xe_ttm_stolen_io_mem_reserve(struct xe_device *xe, struct ttm_resource *mem)
> 	if (!mgr || !mgr->io_base)
> 		return -EIO;
>
>-	if (!xe_ttm_stolen_inaccessible(xe))
>+	if (!xe_ttm_stolen_cpu_inaccessible(xe))
> 		return __xe_ttm_stolen_io_mem_reserve_bar2(xe, mgr, mem);
> 	else
> 		return __xe_ttm_stolen_io_mem_reserve_stolen(xe, mgr, mem);
>diff --git a/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.h b/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.h
>index ade37abb0623..2fda97b97a05 100644
>--- a/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.h
>+++ b/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.h
>@@ -14,7 +14,7 @@ struct xe_device;
>
> void xe_ttm_stolen_mgr_init(struct xe_device *xe);
> int xe_ttm_stolen_io_mem_reserve(struct xe_device *xe, struct ttm_resource *mem);
>-bool xe_ttm_stolen_inaccessible(struct xe_device *xe);
>+bool xe_ttm_stolen_cpu_inaccessible(struct xe_device *xe);
> u64 xe_ttm_stolen_io_offset(struct xe_bo *bo, u32 offset);
> u64 xe_ttm_stolen_gpu_offset(struct xe_device *xe);
>
>-- 
>2.39.1
>

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

end of thread, other threads:[~2023-02-14 19:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-14 17:32 [Intel-xe] [PATCH v2 1/2] drm/xe/mmio: don't leak the forcewake ref Matthew Auld
2023-02-14 17:32 ` [Intel-xe] [PATCH v2 2/2] drm/xe/stolen: don't map stolen on small-bar Matthew Auld
2023-02-14 19:45   ` Lucas De Marchi
2023-02-14 19:37 ` [Intel-xe] [PATCH v2 1/2] drm/xe/mmio: don't leak the forcewake ref Lucas De Marchi

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.