All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size
@ 2017-01-12 13:00 Chris Wilson
  2017-01-12 13:04 ` [PATCH v2] " Chris Wilson
  2017-01-12 17:24 ` ✗ Fi.CI.BAT: warning for drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size (rev2) Patchwork
  0 siblings, 2 replies; 5+ messages in thread
From: Chris Wilson @ 2017-01-12 13:00 UTC (permalink / raw)
  To: intel-gfx

The internal object is a collection of struct pages and so is
intrinsically linked to the available physical memory on the machine,
and not an arbitrary type from the uabi. Use phys_addr_t so the link
between size and memory consumption is clear, and then double check that
we don't overflow the maximum object size.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h          | 2 +-
 drivers/gpu/drm/i915/i915_gem_internal.c | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b84c1d1fa12c..d8635d2af7ad 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3506,7 +3506,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_i915_private *dev_priv
 /* i915_gem_internal.c */
 struct drm_i915_gem_object *
 i915_gem_object_create_internal(struct drm_i915_private *dev_priv,
-				unsigned int size);
+				phys_addr_t size);
 
 /* i915_gem_shrinker.c */
 unsigned long i915_gem_shrink(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/i915_gem_internal.c b/drivers/gpu/drm/i915/i915_gem_internal.c
index 2222863e505f..9fd3d94e0e9f 100644
--- a/drivers/gpu/drm/i915/i915_gem_internal.c
+++ b/drivers/gpu/drm/i915/i915_gem_internal.c
@@ -151,10 +151,13 @@ static const struct drm_i915_gem_object_ops i915_gem_object_internal_ops = {
  */
 struct drm_i915_gem_object *
 i915_gem_object_create_internal(struct drm_i915_private *i915,
-				unsigned int size)
+				phys_addr_t size)
 {
 	struct drm_i915_gem_object *obj;
 
+	if (overflows_type(size, obj->base.size))
+		return ERR_PTR(-E2BIG);
+
 	obj = i915_gem_object_alloc(i915);
 	if (!obj)
 		return ERR_PTR(-ENOMEM);
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH v2] drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size
  2017-01-12 13:00 [PATCH] drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size Chris Wilson
@ 2017-01-12 13:04 ` Chris Wilson
  2017-01-12 13:39   ` Joonas Lahtinen
  2017-01-12 17:24 ` ✗ Fi.CI.BAT: warning for drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size (rev2) Patchwork
  1 sibling, 1 reply; 5+ messages in thread
From: Chris Wilson @ 2017-01-12 13:04 UTC (permalink / raw)
  To: intel-gfx

The internal object is a collection of struct pages and so is
intrinsically linked to the available physical memory on the machine,
and not an arbitrary type from the uabi. Use phys_addr_t so the link
between size and memory consumption is clear, and then double check that
we don't overflow the maximum object size.

v2: Also assert that size is not zero - a mistake I made a few times
while writing selftests.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h          | 2 +-
 drivers/gpu/drm/i915/i915_gem_internal.c | 7 ++++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b84c1d1fa12c..d8635d2af7ad 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3506,7 +3506,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_i915_private *dev_priv
 /* i915_gem_internal.c */
 struct drm_i915_gem_object *
 i915_gem_object_create_internal(struct drm_i915_private *dev_priv,
-				unsigned int size);
+				phys_addr_t size);
 
 /* i915_gem_shrinker.c */
 unsigned long i915_gem_shrink(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/i915_gem_internal.c b/drivers/gpu/drm/i915/i915_gem_internal.c
index 2222863e505f..9b39472396ef 100644
--- a/drivers/gpu/drm/i915/i915_gem_internal.c
+++ b/drivers/gpu/drm/i915/i915_gem_internal.c
@@ -151,10 +151,15 @@ static const struct drm_i915_gem_object_ops i915_gem_object_internal_ops = {
  */
 struct drm_i915_gem_object *
 i915_gem_object_create_internal(struct drm_i915_private *i915,
-				unsigned int size)
+				phys_addr_t size)
 {
 	struct drm_i915_gem_object *obj;
 
+	GEM_BUG_ON(!size);
+
+	if (overflows_type(size, obj->base.size))
+		return ERR_PTR(-E2BIG);
+
 	obj = i915_gem_object_alloc(i915);
 	if (!obj)
 		return ERR_PTR(-ENOMEM);
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2] drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size
  2017-01-12 13:04 ` [PATCH v2] " Chris Wilson
@ 2017-01-12 13:39   ` Joonas Lahtinen
  0 siblings, 0 replies; 5+ messages in thread
From: Joonas Lahtinen @ 2017-01-12 13:39 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On to, 2017-01-12 at 13:04 +0000, Chris Wilson wrote:
> The internal object is a collection of struct pages and so is
> intrinsically linked to the available physical memory on the machine,
> and not an arbitrary type from the uabi. Use phys_addr_t so the link
> between size and memory consumption is clear, and then double check that
> we don't overflow the maximum object size.
> 
> v2: Also assert that size is not zero - a mistake I made a few times
> while writing selftests.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✗ Fi.CI.BAT: warning for drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size (rev2)
  2017-01-12 13:00 [PATCH] drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size Chris Wilson
  2017-01-12 13:04 ` [PATCH v2] " Chris Wilson
@ 2017-01-12 17:24 ` Patchwork
  2017-01-12 20:55   ` Chris Wilson
  1 sibling, 1 reply; 5+ messages in thread
From: Patchwork @ 2017-01-12 17:24 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size (rev2)
URL   : https://patchwork.freedesktop.org/series/17905/
State : warning

== Summary ==

Series 17905v2 drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size
https://patchwork.freedesktop.org/api/1.0/series/17905/revisions/2/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                pass       -> DMESG-WARN (fi-bxt-j4205)

fi-bdw-5557u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:246  pass:223  dwarn:1   dfail:0   fail:0   skip:22 
fi-bxt-t5700     total:82   pass:69   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u     total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600      total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 

8f7458237916d6c1e5e1e9ebfc5923a6d1658385 drm-tip: 2017y-01m-12d-15h-39m-40s UTC integration manifest
eb09e09 drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3502/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: ✗ Fi.CI.BAT: warning for drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size (rev2)
  2017-01-12 17:24 ` ✗ Fi.CI.BAT: warning for drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size (rev2) Patchwork
@ 2017-01-12 20:55   ` Chris Wilson
  0 siblings, 0 replies; 5+ messages in thread
From: Chris Wilson @ 2017-01-12 20:55 UTC (permalink / raw)
  To: intel-gfx

On Thu, Jan 12, 2017 at 05:24:12PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size (rev2)
> URL   : https://patchwork.freedesktop.org/series/17905/
> State : warning
> 
> == Summary ==
> 
> Series 17905v2 drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size
> https://patchwork.freedesktop.org/api/1.0/series/17905/revisions/2/mbox/
> 
> Test kms_pipe_crc_basic:
>         Subgroup suspend-read-crc-pipe-c:
>                 pass       -> DMESG-WARN (fi-bxt-j4205)

Nothing to see here, pushed. Thanks for the suggestion and review,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2017-01-12 20:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-12 13:00 [PATCH] drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size Chris Wilson
2017-01-12 13:04 ` [PATCH v2] " Chris Wilson
2017-01-12 13:39   ` Joonas Lahtinen
2017-01-12 17:24 ` ✗ Fi.CI.BAT: warning for drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size (rev2) Patchwork
2017-01-12 20:55   ` 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.