* [PATCH] drm/i915: Avoid writing relocs with addresses in non-canonical form
@ 2015-12-04 11:33 Michał Winiarski
2015-12-04 11:56 ` Ville Syrjälä
` (7 more replies)
0 siblings, 8 replies; 30+ messages in thread
From: Michał Winiarski @ 2015-12-04 11:33 UTC (permalink / raw)
To: intel-gfx
According to bspec, some parts of HW expect the addresses to be in
a canonical form, where bits [63:48] == [47]. Let's convert addresses to
canonical form prior to relocating and return converted offsets to
userspace.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 9 ++++++---
drivers/gpu/drm/i915/i915_gem_gtt.h | 5 +++++
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index a4c243c..9f27be9 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -395,7 +395,7 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
target_i915_obj = target_vma->obj;
target_obj = &target_vma->obj->base;
- target_offset = target_vma->node.start;
+ target_offset = gen8_canonical_addr(target_vma->node.start);
/* Sandybridge PPGTT errata: We need a global gtt mapping for MI and
* pipe_control writes because the gpu doesn't properly redirect them
@@ -583,6 +583,7 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
struct drm_i915_gem_object *obj = vma->obj;
struct drm_i915_gem_exec_object2 *entry = vma->exec_entry;
uint64_t flags;
+ uint64_t offset;
int ret;
flags = PIN_USER;
@@ -623,8 +624,10 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
entry->flags |= __EXEC_OBJECT_HAS_FENCE;
}
- if (entry->offset != vma->node.start) {
- entry->offset = vma->node.start;
+ offset = gen8_canonical_addr(vma->node.start);
+
+ if (entry->offset != offset) {
+ entry->offset = offset;
*need_reloc = true;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 877c32c..65e8f88 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -507,6 +507,11 @@ static inline size_t gen8_pte_count(uint64_t address, uint64_t length)
return i915_pte_count(address, length, GEN8_PDE_SHIFT);
}
+static inline uint64_t gen8_canonical_addr(uint64_t address)
+{
+ return ((int64_t)address << 16) >> 16;
+}
+
static inline dma_addr_t
i915_page_dir_dma_addr(const struct i915_hw_ppgtt *ppgtt, const unsigned n)
{
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-04 11:33 [PATCH] drm/i915: Avoid writing relocs with addresses in non-canonical form Michał Winiarski
@ 2015-12-04 11:56 ` Ville Syrjälä
2015-12-04 12:05 ` Chris Wilson
` (6 subsequent siblings)
7 siblings, 0 replies; 30+ messages in thread
From: Ville Syrjälä @ 2015-12-04 11:56 UTC (permalink / raw)
To: Michał Winiarski; +Cc: intel-gfx
On Fri, Dec 04, 2015 at 12:33:44PM +0100, Michał Winiarski wrote:
> According to bspec, some parts of HW expect the addresses to be in
> a canonical form, where bits [63:48] == [47]. Let's convert addresses to
> canonical form prior to relocating and return converted offsets to
> userspace.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 9 ++++++---
> drivers/gpu/drm/i915/i915_gem_gtt.h | 5 +++++
> 2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index a4c243c..9f27be9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -395,7 +395,7 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
> target_i915_obj = target_vma->obj;
> target_obj = &target_vma->obj->base;
>
> - target_offset = target_vma->node.start;
> + target_offset = gen8_canonical_addr(target_vma->node.start);
>
> /* Sandybridge PPGTT errata: We need a global gtt mapping for MI and
> * pipe_control writes because the gpu doesn't properly redirect them
> @@ -583,6 +583,7 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
> struct drm_i915_gem_object *obj = vma->obj;
> struct drm_i915_gem_exec_object2 *entry = vma->exec_entry;
> uint64_t flags;
> + uint64_t offset;
> int ret;
>
> flags = PIN_USER;
> @@ -623,8 +624,10 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
> entry->flags |= __EXEC_OBJECT_HAS_FENCE;
> }
>
> - if (entry->offset != vma->node.start) {
> - entry->offset = vma->node.start;
> + offset = gen8_canonical_addr(vma->node.start);
> +
> + if (entry->offset != offset) {
> + entry->offset = offset;
> *need_reloc = true;
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 877c32c..65e8f88 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -507,6 +507,11 @@ static inline size_t gen8_pte_count(uint64_t address, uint64_t length)
> return i915_pte_count(address, length, GEN8_PDE_SHIFT);
> }
>
> +static inline uint64_t gen8_canonical_addr(uint64_t address)
> +{
> + return ((int64_t)address << 16) >> 16;
> +}
This could really use a comment explaining what it's doing and why.
Just lifting the first sentence of your commit message into a comment
would seem to be enough.
> +
> static inline dma_addr_t
> i915_page_dir_dma_addr(const struct i915_hw_ppgtt *ppgtt, const unsigned n)
> {
> --
> 2.5.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-04 11:33 [PATCH] drm/i915: Avoid writing relocs with addresses in non-canonical form Michał Winiarski
2015-12-04 11:56 ` Ville Syrjälä
@ 2015-12-04 12:05 ` Chris Wilson
2015-12-04 15:20 ` [PATCH v2] " Michał Winiarski
` (5 subsequent siblings)
7 siblings, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2015-12-04 12:05 UTC (permalink / raw)
To: Michał Winiarski; +Cc: intel-gfx
On Fri, Dec 04, 2015 at 12:33:44PM +0100, Michał Winiarski wrote:
> According to bspec, some parts of HW expect the addresses to be in
> a canonical form, where bits [63:48] == [47]. Let's convert addresses to
> canonical form prior to relocating and return converted offsets to
> userspace.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 9 ++++++---
> drivers/gpu/drm/i915/i915_gem_gtt.h | 5 +++++
> 2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index a4c243c..9f27be9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -395,7 +395,7 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
> target_i915_obj = target_vma->obj;
> target_obj = &target_vma->obj->base;
>
> - target_offset = target_vma->node.start;
> + target_offset = gen8_canonical_addr(target_vma->node.start);
Ok, this keeps userspace in sync as well as writing the cannonical
addreses to hardware.
> /* Sandybridge PPGTT errata: We need a global gtt mapping for MI and
> * pipe_control writes because the gpu doesn't properly redirect them
> @@ -583,6 +583,7 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
> struct drm_i915_gem_object *obj = vma->obj;
> struct drm_i915_gem_exec_object2 *entry = vma->exec_entry;
> uint64_t flags;
> + uint64_t offset;
> int ret;
>
> flags = PIN_USER;
> @@ -623,8 +624,10 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
> entry->flags |= __EXEC_OBJECT_HAS_FENCE;
> }
>
> - if (entry->offset != vma->node.start) {
> - entry->offset = vma->node.start;
> + offset = gen8_canonical_addr(vma->node.start);
> +
Kill the extra line.
> + if (entry->offset != offset) {
> + entry->offset = offset;
> *need_reloc = true;
And this copies back the cannonical address to userspace. Ok, the
userspace side looks correct (the presumed_offset and execobject.offsets
will be updated to store the cannonical address).
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 877c32c..65e8f88 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -507,6 +507,11 @@ static inline size_t gen8_pte_count(uint64_t address, uint64_t length)
> return i915_pte_count(address, length, GEN8_PDE_SHIFT);
> }
>
> +static inline uint64_t gen8_canonical_addr(uint64_t address)
> +{
You need the bspec reference here as well.
Looks good to me, with just a quick explanation here as to what we are
doing and why.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-04 11:33 [PATCH] drm/i915: Avoid writing relocs with addresses in non-canonical form Michał Winiarski
2015-12-04 11:56 ` Ville Syrjälä
2015-12-04 12:05 ` Chris Wilson
@ 2015-12-04 15:20 ` Michał Winiarski
2015-12-04 15:39 ` Chris Wilson
` (3 more replies)
2015-12-19 8:20 ` ✗ warning: Fi.CI.BAT Patchwork
` (4 subsequent siblings)
7 siblings, 4 replies; 30+ messages in thread
From: Michał Winiarski @ 2015-12-04 15:20 UTC (permalink / raw)
To: intel-gfx
According to bspec, some parts of HW expect the addresses to be in
a canonical form, where bits [63:48] == [47]. Let's convert addresses to
canonical form prior to relocating and return converted offsets to
userspace.
v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville)
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 8 +++++---
drivers/gpu/drm/i915/i915_gem_gtt.h | 12 ++++++++++++
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index a4c243c..ceffef9 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -395,7 +395,7 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
target_i915_obj = target_vma->obj;
target_obj = &target_vma->obj->base;
- target_offset = target_vma->node.start;
+ target_offset = gen8_canonical_addr(target_vma->node.start);
/* Sandybridge PPGTT errata: We need a global gtt mapping for MI and
* pipe_control writes because the gpu doesn't properly redirect them
@@ -583,6 +583,7 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
struct drm_i915_gem_object *obj = vma->obj;
struct drm_i915_gem_exec_object2 *entry = vma->exec_entry;
uint64_t flags;
+ uint64_t offset;
int ret;
flags = PIN_USER;
@@ -623,8 +624,9 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
entry->flags |= __EXEC_OBJECT_HAS_FENCE;
}
- if (entry->offset != vma->node.start) {
- entry->offset = vma->node.start;
+ offset = gen8_canonical_addr(vma->node.start);
+ if (entry->offset != offset) {
+ entry->offset = offset;
*need_reloc = true;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 877c32c..4ea9dab 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -507,6 +507,18 @@ static inline size_t gen8_pte_count(uint64_t address, uint64_t length)
return i915_pte_count(address, length, GEN8_PDE_SHIFT);
}
+/* Used to convert any address to canonical form.
+ * Starting from gen8, some commands (e.g. STATE_BASE_ADDRESS,
+ * MI_LOAD_REGISTER_MEM and others, see Broadwell PRM Vol2a) expect the
+ * addresses to be in a canonical form:
+ * "GraphicsAddress[63:48] are ignored by the HW and assumed to be in correct
+ * canonical form [63:48] == [47]."
+ */
+static inline uint64_t gen8_canonical_addr(uint64_t address)
+{
+ return ((int64_t)address << 16) >> 16;
+}
+
static inline dma_addr_t
i915_page_dir_dma_addr(const struct i915_hw_ppgtt *ppgtt, const unsigned n)
{
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v2] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-04 15:20 ` [PATCH v2] " Michał Winiarski
@ 2015-12-04 15:39 ` Chris Wilson
2015-12-04 16:18 ` Daniel Vetter
` (2 subsequent siblings)
3 siblings, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2015-12-04 15:39 UTC (permalink / raw)
To: Michał Winiarski; +Cc: intel-gfx
On Fri, Dec 04, 2015 at 04:20:43PM +0100, Michał Winiarski wrote:
> According to bspec, some parts of HW expect the addresses to be in
> a canonical form, where bits [63:48] == [47]. Let's convert addresses to
> canonical form prior to relocating and return converted offsets to
> userspace.
>
> v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville)
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
There is a hole! Inside the actual relocation fixup you need
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index a4c243cec4aa..6d1a2d20337b 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -249,6 +249,12 @@ static inline int use_cpu_reloc(struct drm_i915_gem_object *obj)
obj->cache_level != I915_CACHE_NONE);
}
+static uint64_t relocation_target(struct drm_i915_gem_relocation_entry *reloc,
+ uint64_t target_offset)
+{
+ return gen8_cannonical_address((int)reloc->delta + target_offset);
+}
+
static int
relocate_entry_cpu(struct drm_i915_gem_object *obj,
struct drm_i915_gem_relocation_entry *reloc,
@@ -256,7 +262,7 @@ relocate_entry_cpu(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
uint32_t page_offset = offset_in_page(reloc->offset);
- uint64_t delta = reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_get);
char *vaddr;
int ret;
@@ -292,7 +298,7 @@ relocate_entry_gtt(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- uint64_t delta = reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_get);
uint64_t offset;
void __iomem *reloc_page;
int ret;
@@ -347,7 +353,7 @@ relocate_entry_clflush(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
uint32_t page_offset = offset_in_page(reloc->offset);
- uint64_t delta = (int)reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_get);
char *vaddr;
int ret;
Otherwise we may cross the 1<<47 boundary without swapping into cannonical
form.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v2] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-04 15:20 ` [PATCH v2] " Michał Winiarski
2015-12-04 15:39 ` Chris Wilson
@ 2015-12-04 16:18 ` Daniel Vetter
2015-12-04 17:41 ` Winiarski, Michal
2015-12-10 12:50 ` Chris Wilson
2015-12-11 14:13 ` [PATCH v3] " Michał Winiarski
3 siblings, 1 reply; 30+ messages in thread
From: Daniel Vetter @ 2015-12-04 16:18 UTC (permalink / raw)
To: Michał Winiarski; +Cc: intel-gfx
On Fri, Dec 04, 2015 at 04:20:43PM +0100, Michał Winiarski wrote:
> According to bspec, some parts of HW expect the addresses to be in
> a canonical form, where bits [63:48] == [47]. Let's convert addresses to
> canonical form prior to relocating and return converted offsets to
> userspace.
>
> v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville)
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Can we igt this? Maybe with softpin or whatever ... For cpu address space
negative addresses are for the kernel, but I think on the gpu we can do
them.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 8 +++++---
> drivers/gpu/drm/i915/i915_gem_gtt.h | 12 ++++++++++++
> 2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index a4c243c..ceffef9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -395,7 +395,7 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
> target_i915_obj = target_vma->obj;
> target_obj = &target_vma->obj->base;
>
> - target_offset = target_vma->node.start;
> + target_offset = gen8_canonical_addr(target_vma->node.start);
>
> /* Sandybridge PPGTT errata: We need a global gtt mapping for MI and
> * pipe_control writes because the gpu doesn't properly redirect them
> @@ -583,6 +583,7 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
> struct drm_i915_gem_object *obj = vma->obj;
> struct drm_i915_gem_exec_object2 *entry = vma->exec_entry;
> uint64_t flags;
> + uint64_t offset;
> int ret;
>
> flags = PIN_USER;
> @@ -623,8 +624,9 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
> entry->flags |= __EXEC_OBJECT_HAS_FENCE;
> }
>
> - if (entry->offset != vma->node.start) {
> - entry->offset = vma->node.start;
> + offset = gen8_canonical_addr(vma->node.start);
> + if (entry->offset != offset) {
> + entry->offset = offset;
> *need_reloc = true;
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 877c32c..4ea9dab 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -507,6 +507,18 @@ static inline size_t gen8_pte_count(uint64_t address, uint64_t length)
> return i915_pte_count(address, length, GEN8_PDE_SHIFT);
> }
>
> +/* Used to convert any address to canonical form.
> + * Starting from gen8, some commands (e.g. STATE_BASE_ADDRESS,
> + * MI_LOAD_REGISTER_MEM and others, see Broadwell PRM Vol2a) expect the
> + * addresses to be in a canonical form:
> + * "GraphicsAddress[63:48] are ignored by the HW and assumed to be in correct
> + * canonical form [63:48] == [47]."
> + */
> +static inline uint64_t gen8_canonical_addr(uint64_t address)
> +{
> + return ((int64_t)address << 16) >> 16;
> +}
> +
> static inline dma_addr_t
> i915_page_dir_dma_addr(const struct i915_hw_ppgtt *ppgtt, const unsigned n)
> {
> --
> 2.5.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-04 16:18 ` Daniel Vetter
@ 2015-12-04 17:41 ` Winiarski, Michal
2015-12-07 8:31 ` Daniel Vetter
0 siblings, 1 reply; 30+ messages in thread
From: Winiarski, Michal @ 2015-12-04 17:41 UTC (permalink / raw)
To: daniel; +Cc: intel-gfx
On Fri, 2015-12-04 at 17:18 +0100, Daniel Vetter wrote:
> On Fri, Dec 04, 2015 at 04:20:43PM +0100, Michał Winiarski wrote:
> > According to bspec, some parts of HW expect the addresses to be in
> > a canonical form, where bits [63:48] == [47]. Let's convert
> > addresses to
> > canonical form prior to relocating and return converted offsets to
> > userspace.
> >
> > v2: Whitespace fixup, gen8_canonical_addr description (Chris,
> > Ville)
> >
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Michel Thierry <michel.thierry@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
>
> Can we igt this? Maybe with softpin or whatever ... For cpu address
> space
> negative addresses are for the kernel, but I think on the gpu we can
> do
> them.
Yup - would definitely be useful, I can add something to one of the
existing reloc tests, but without softpin it's tricky to cover this in
a reliable way (not to mention corner cases).
-Michał
> -Daniel
>
> > ---
> > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 8 +++++---
> > drivers/gpu/drm/i915/i915_gem_gtt.h | 12 ++++++++++++
> > 2 files changed, 17 insertions(+), 3 deletions(-)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-04 17:41 ` Winiarski, Michal
@ 2015-12-07 8:31 ` Daniel Vetter
0 siblings, 0 replies; 30+ messages in thread
From: Daniel Vetter @ 2015-12-07 8:31 UTC (permalink / raw)
To: Winiarski, Michal; +Cc: intel-gfx
On Fri, Dec 04, 2015 at 05:41:11PM +0000, Winiarski, Michal wrote:
> On Fri, 2015-12-04 at 17:18 +0100, Daniel Vetter wrote:
> > On Fri, Dec 04, 2015 at 04:20:43PM +0100, Michał Winiarski wrote:
> > > According to bspec, some parts of HW expect the addresses to be in
> > > a canonical form, where bits [63:48] == [47]. Let's convert
> > > addresses to
> > > canonical form prior to relocating and return converted offsets to
> > > userspace.
> > >
> > > v2: Whitespace fixup, gen8_canonical_addr description (Chris,
> > > Ville)
> > >
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Michel Thierry <michel.thierry@intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> >
> > Can we igt this? Maybe with softpin or whatever ... For cpu address
> > space
> > negative addresses are for the kernel, but I think on the gpu we can
> > do
> > them.
>
> Yup - would definitely be useful, I can add something to one of the
> existing reloc tests, but without softpin it's tricky to cover this in
> a reliable way (not to mention corner cases).
Oh without softpin it's going to be impossible I think ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-04 15:20 ` [PATCH v2] " Michał Winiarski
2015-12-04 15:39 ` Chris Wilson
2015-12-04 16:18 ` Daniel Vetter
@ 2015-12-10 12:50 ` Chris Wilson
2015-12-11 14:13 ` [PATCH v3] " Michał Winiarski
3 siblings, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2015-12-10 12:50 UTC (permalink / raw)
To: Michał Winiarski; +Cc: intel-gfx
On Fri, Dec 04, 2015 at 04:20:43PM +0100, Michał Winiarski wrote:
> According to bspec, some parts of HW expect the addresses to be in
> a canonical form, where bits [63:48] == [47]. Let's convert addresses to
> canonical form prior to relocating and return converted offsets to
> userspace.
>
> v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville)
What's the impact? The bspec says "assumes" but not required? Does
anything truly break otherwise?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-04 15:20 ` [PATCH v2] " Michał Winiarski
` (2 preceding siblings ...)
2015-12-10 12:50 ` Chris Wilson
@ 2015-12-11 14:13 ` Michał Winiarski
2015-12-11 14:43 ` Michel Thierry
` (2 more replies)
3 siblings, 3 replies; 30+ messages in thread
From: Michał Winiarski @ 2015-12-11 14:13 UTC (permalink / raw)
To: intel-gfx
According to bspec, some parts of HW require the addresses to be in
a canonical form, where bits [63:48] == [47]. Let's convert addresses to
canonical form prior to relocating and return converted offsets to
userspace. We also need to make sure that userspace is using addresses
in canonical form in case of softpin.
v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville)
v3: Rebase on top of softpin, fix a hole in relocate_entry,
s/expect/require (Chris)
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 9 +++++++--
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 21 +++++++++++++++------
drivers/gpu/drm/i915/i915_gem_gtt.h | 12 ++++++++++++
3 files changed, 34 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8e2acde..b83207b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3482,12 +3482,17 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj,
if (flags & PIN_OFFSET_FIXED) {
uint64_t offset = flags & PIN_OFFSET_MASK;
+ uint64_t noncanonical_offset = offset & ((1ULL << 48) - 1);
- if (offset & (alignment - 1) || offset + size > end) {
+ if (offset & (alignment - 1) ||
+ noncanonical_offset + size > end ||
+ offset != gen8_canonical_addr(offset)) {
ret = -EINVAL;
goto err_free_vma;
}
- vma->node.start = offset;
+ /* While userspace is using addresses in canonical form, our
+ * allocator is unaware of this */
+ vma->node.start = noncanonical_offset;
vma->node.size = size;
vma->node.color = obj->cache_level;
ret = drm_mm_reserve_node(&vm->mm, &vma->node);
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 48ec484..445ccc7 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -249,6 +249,13 @@ static inline int use_cpu_reloc(struct drm_i915_gem_object *obj)
obj->cache_level != I915_CACHE_NONE);
}
+static inline uint64_t
+relocation_target(struct drm_i915_gem_relocation_entry *reloc,
+ uint64_t target_offset)
+{
+ return gen8_canonical_addr((int)reloc->delta + target_offset);
+}
+
static int
relocate_entry_cpu(struct drm_i915_gem_object *obj,
struct drm_i915_gem_relocation_entry *reloc,
@@ -256,7 +263,7 @@ relocate_entry_cpu(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
uint32_t page_offset = offset_in_page(reloc->offset);
- uint64_t delta = reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_offset);
char *vaddr;
int ret;
@@ -292,7 +299,7 @@ relocate_entry_gtt(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- uint64_t delta = reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_offset);
uint64_t offset;
void __iomem *reloc_page;
int ret;
@@ -347,7 +354,7 @@ relocate_entry_clflush(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
uint32_t page_offset = offset_in_page(reloc->offset);
- uint64_t delta = (int)reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_offset);
char *vaddr;
int ret;
@@ -395,7 +402,7 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
target_i915_obj = target_vma->obj;
target_obj = &target_vma->obj->base;
- target_offset = target_vma->node.start;
+ target_offset = gen8_canonical_addr(target_vma->node.start);
/* Sandybridge PPGTT errata: We need a global gtt mapping for MI and
* pipe_control writes because the gpu doesn't properly redirect them
@@ -583,6 +590,7 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
struct drm_i915_gem_object *obj = vma->obj;
struct drm_i915_gem_exec_object2 *entry = vma->exec_entry;
uint64_t flags;
+ uint64_t offset;
int ret;
flags = PIN_USER;
@@ -625,8 +633,9 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
entry->flags |= __EXEC_OBJECT_HAS_FENCE;
}
- if (entry->offset != vma->node.start) {
- entry->offset = vma->node.start;
+ offset = gen8_canonical_addr(vma->node.start);
+ if (entry->offset != offset) {
+ entry->offset = offset;
*need_reloc = true;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index b448ad8..fa1f3ab 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -504,6 +504,18 @@ static inline size_t gen8_pte_count(uint64_t address, uint64_t length)
return i915_pte_count(address, length, GEN8_PDE_SHIFT);
}
+/* Used to convert any address to canonical form.
+ * Starting from gen8, some commands (e.g. STATE_BASE_ADDRESS,
+ * MI_LOAD_REGISTER_MEM and others, see Broadwell PRM Vol2a) require the
+ * addresses to be in a canonical form:
+ * "GraphicsAddress[63:48] are ignored by the HW and assumed to be in correct
+ * canonical form [63:48] == [47]."
+ */
+static inline uint64_t gen8_canonical_addr(uint64_t address)
+{
+ return ((int64_t)address << 16) >> 16;
+}
+
static inline dma_addr_t
i915_page_dir_dma_addr(const struct i915_hw_ppgtt *ppgtt, const unsigned n)
{
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v3] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-11 14:13 ` [PATCH v3] " Michał Winiarski
@ 2015-12-11 14:43 ` Michel Thierry
2015-12-18 16:31 ` Chris Wilson
2015-12-18 20:01 ` [PATCH v4] " Michał Winiarski
2 siblings, 0 replies; 30+ messages in thread
From: Michel Thierry @ 2015-12-11 14:43 UTC (permalink / raw)
To: Michał Winiarski, intel-gfx
On 12/11/2015 2:13 PM, Michał Winiarski wrote:
> According to bspec, some parts of HW require the addresses to be in
> a canonical form, where bits [63:48] == [47]. Let's convert addresses to
> canonical form prior to relocating and return converted offsets to
> userspace. We also need to make sure that userspace is using addresses
> in canonical form in case of softpin.
>
> v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville)
> v3: Rebase on top of softpin, fix a hole in relocate_entry,
> s/expect/require (Chris)
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
With updated gem_softpin
[http://patchwork.freedesktop.org/patch/msgid/1449843255-32640-1-git-send-email-michel.thierry@intel.com]
Tested-by: Michel Thierry <michel.thierry@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 9 +++++++--
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 21 +++++++++++++++------
> drivers/gpu/drm/i915/i915_gem_gtt.h | 12 ++++++++++++
> 3 files changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 8e2acde..b83207b 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3482,12 +3482,17 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj,
>
> if (flags & PIN_OFFSET_FIXED) {
> uint64_t offset = flags & PIN_OFFSET_MASK;
> + uint64_t noncanonical_offset = offset & ((1ULL << 48) - 1);
>
> - if (offset & (alignment - 1) || offset + size > end) {
> + if (offset & (alignment - 1) ||
> + noncanonical_offset + size > end ||
> + offset != gen8_canonical_addr(offset)) {
> ret = -EINVAL;
> goto err_free_vma;
> }
> - vma->node.start = offset;
> + /* While userspace is using addresses in canonical form, our
> + * allocator is unaware of this */
> + vma->node.start = noncanonical_offset;
> vma->node.size = size;
> vma->node.color = obj->cache_level;
> ret = drm_mm_reserve_node(&vm->mm, &vma->node);
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 48ec484..445ccc7 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -249,6 +249,13 @@ static inline int use_cpu_reloc(struct drm_i915_gem_object *obj)
> obj->cache_level != I915_CACHE_NONE);
> }
>
> +static inline uint64_t
> +relocation_target(struct drm_i915_gem_relocation_entry *reloc,
> + uint64_t target_offset)
> +{
> + return gen8_canonical_addr((int)reloc->delta + target_offset);
> +}
> +
> static int
> relocate_entry_cpu(struct drm_i915_gem_object *obj,
> struct drm_i915_gem_relocation_entry *reloc,
> @@ -256,7 +263,7 @@ relocate_entry_cpu(struct drm_i915_gem_object *obj,
> {
> struct drm_device *dev = obj->base.dev;
> uint32_t page_offset = offset_in_page(reloc->offset);
> - uint64_t delta = reloc->delta + target_offset;
> + uint64_t delta = relocation_target(reloc, target_offset);
> char *vaddr;
> int ret;
>
> @@ -292,7 +299,7 @@ relocate_entry_gtt(struct drm_i915_gem_object *obj,
> {
> struct drm_device *dev = obj->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - uint64_t delta = reloc->delta + target_offset;
> + uint64_t delta = relocation_target(reloc, target_offset);
> uint64_t offset;
> void __iomem *reloc_page;
> int ret;
> @@ -347,7 +354,7 @@ relocate_entry_clflush(struct drm_i915_gem_object *obj,
> {
> struct drm_device *dev = obj->base.dev;
> uint32_t page_offset = offset_in_page(reloc->offset);
> - uint64_t delta = (int)reloc->delta + target_offset;
> + uint64_t delta = relocation_target(reloc, target_offset);
> char *vaddr;
> int ret;
>
> @@ -395,7 +402,7 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
> target_i915_obj = target_vma->obj;
> target_obj = &target_vma->obj->base;
>
> - target_offset = target_vma->node.start;
> + target_offset = gen8_canonical_addr(target_vma->node.start);
>
> /* Sandybridge PPGTT errata: We need a global gtt mapping for MI and
> * pipe_control writes because the gpu doesn't properly redirect them
> @@ -583,6 +590,7 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
> struct drm_i915_gem_object *obj = vma->obj;
> struct drm_i915_gem_exec_object2 *entry = vma->exec_entry;
> uint64_t flags;
> + uint64_t offset;
> int ret;
>
> flags = PIN_USER;
> @@ -625,8 +633,9 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
> entry->flags |= __EXEC_OBJECT_HAS_FENCE;
> }
>
> - if (entry->offset != vma->node.start) {
> - entry->offset = vma->node.start;
> + offset = gen8_canonical_addr(vma->node.start);
> + if (entry->offset != offset) {
> + entry->offset = offset;
> *need_reloc = true;
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index b448ad8..fa1f3ab 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -504,6 +504,18 @@ static inline size_t gen8_pte_count(uint64_t address, uint64_t length)
> return i915_pte_count(address, length, GEN8_PDE_SHIFT);
> }
>
> +/* Used to convert any address to canonical form.
> + * Starting from gen8, some commands (e.g. STATE_BASE_ADDRESS,
> + * MI_LOAD_REGISTER_MEM and others, see Broadwell PRM Vol2a) require the
> + * addresses to be in a canonical form:
> + * "GraphicsAddress[63:48] are ignored by the HW and assumed to be in correct
> + * canonical form [63:48] == [47]."
> + */
> +static inline uint64_t gen8_canonical_addr(uint64_t address)
> +{
> + return ((int64_t)address << 16) >> 16;
> +}
> +
> static inline dma_addr_t
> i915_page_dir_dma_addr(const struct i915_hw_ppgtt *ppgtt, const unsigned n)
> {
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-11 14:13 ` [PATCH v3] " Michał Winiarski
2015-12-11 14:43 ` Michel Thierry
@ 2015-12-18 16:31 ` Chris Wilson
2015-12-18 20:01 ` [PATCH v4] " Michał Winiarski
2 siblings, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2015-12-18 16:31 UTC (permalink / raw)
To: Michał Winiarski; +Cc: intel-gfx
On Fri, Dec 11, 2015 at 03:13:37PM +0100, Michał Winiarski wrote:
> According to bspec, some parts of HW require the addresses to be in
> a canonical form, where bits [63:48] == [47]. Let's convert addresses to
> canonical form prior to relocating and return converted offsets to
> userspace. We also need to make sure that userspace is using addresses
> in canonical form in case of softpin.
>
> v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville)
> v3: Rebase on top of softpin, fix a hole in relocate_entry,
> s/expect/require (Chris)
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 9 +++++++--
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 21 +++++++++++++++------
> drivers/gpu/drm/i915/i915_gem_gtt.h | 12 ++++++++++++
> 3 files changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 8e2acde..b83207b 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3482,12 +3482,17 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj,
>
> if (flags & PIN_OFFSET_FIXED) {
> uint64_t offset = flags & PIN_OFFSET_MASK;
> + uint64_t noncanonical_offset = offset & ((1ULL << 48) - 1);
No. Core GEM doesn't care about canonical addresses, it just operates
with respect to the drm_mm range manager.
More importantly you missed that the only chance we have to validate the
user parameters is during validate_exec_list().
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-11 14:13 ` [PATCH v3] " Michał Winiarski
2015-12-11 14:43 ` Michel Thierry
2015-12-18 16:31 ` Chris Wilson
@ 2015-12-18 20:01 ` Michał Winiarski
2015-12-18 20:26 ` Chris Wilson
2015-12-22 11:00 ` [PATCH v5] " Michał Winiarski
2 siblings, 2 replies; 30+ messages in thread
From: Michał Winiarski @ 2015-12-18 20:01 UTC (permalink / raw)
To: intel-gfx
According to bspec, some parts of HW require the addresses to be in
a canonical form, where bits [63:48] == [47]. Let's convert addresses to
canonical form prior to relocating and return converted offsets to
userspace. We also need to make sure that userspace is using addresses
in canonical form in case of softpin.
v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville)
v3: Rebase on top of softpin, fix a hole in relocate_entry,
s/expect/require (Chris)
v4: Handle softpin in validate_exec_list (Chris)
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 45 ++++++++++++++++++++++++++----
1 file changed, 39 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 5d01ea6..87d9887 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -249,6 +249,25 @@ static inline int use_cpu_reloc(struct drm_i915_gem_object *obj)
obj->cache_level != I915_CACHE_NONE);
}
+/* Used to convert any address to canonical form.
+ * Starting from gen8, some commands (e.g. STATE_BASE_ADDRESS,
+ * MI_LOAD_REGISTER_MEM and others, see Broadwell PRM Vol2a) require the
+ * addresses to be in a canonical form:
+ * "GraphicsAddress[63:48] are ignored by the HW and assumed to be in correct
+ * canonical form [63:48] == [47]."
+ */
+static inline uint64_t gen8_canonical_addr(uint64_t address)
+{
+ return ((int64_t)address << 16) >> 16;
+}
+
+static inline uint64_t
+relocation_target(struct drm_i915_gem_relocation_entry *reloc,
+ uint64_t target_offset)
+{
+ return gen8_canonical_addr((int)reloc->delta + target_offset);
+}
+
static int
relocate_entry_cpu(struct drm_i915_gem_object *obj,
struct drm_i915_gem_relocation_entry *reloc,
@@ -256,7 +275,7 @@ relocate_entry_cpu(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
uint32_t page_offset = offset_in_page(reloc->offset);
- uint64_t delta = reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_offset);
char *vaddr;
int ret;
@@ -292,7 +311,7 @@ relocate_entry_gtt(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- uint64_t delta = reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_offset);
uint64_t offset;
void __iomem *reloc_page;
int ret;
@@ -347,7 +366,7 @@ relocate_entry_clflush(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
uint32_t page_offset = offset_in_page(reloc->offset);
- uint64_t delta = (int)reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_offset);
char *vaddr;
int ret;
@@ -395,7 +414,7 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
target_i915_obj = target_vma->obj;
target_obj = &target_vma->obj->base;
- target_offset = target_vma->node.start;
+ target_offset = gen8_canonical_addr(target_vma->node.start);
/* Sandybridge PPGTT errata: We need a global gtt mapping for MI and
* pipe_control writes because the gpu doesn't properly redirect them
@@ -583,6 +602,7 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
struct drm_i915_gem_object *obj = vma->obj;
struct drm_i915_gem_exec_object2 *entry = vma->exec_entry;
uint64_t flags;
+ uint64_t offset;
int ret;
flags = PIN_USER;
@@ -625,8 +645,9 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
entry->flags |= __EXEC_OBJECT_HAS_FENCE;
}
- if (entry->offset != vma->node.start) {
- entry->offset = vma->node.start;
+ offset = gen8_canonical_addr(vma->node.start);
+ if (entry->offset != offset) {
+ entry->offset = offset;
*need_reloc = true;
}
@@ -994,6 +1015,18 @@ validate_exec_list(struct drm_device *dev,
if (exec[i].flags & invalid_flags)
return -EINVAL;
+ /* Offset can be used as input (EXEC_OBJECT_PINNED), since
+ * userspace has to use canonical format, we need to reject all
+ * non-canonical addresses.
+ */
+ if (exec[i].offset != gen8_canonical_addr(exec[i].offset))
+ return -EINVAL;
+
+ /* On the other hand, from drm_mm perspective address space is
+ * continuous, so we're converting to non-canonical form
+ */
+ exec[i].offset &= (1ULL << 48) - 1;
+
if (exec[i].alignment && !is_power_of_2(exec[i].alignment))
return -EINVAL;
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v4] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-18 20:01 ` [PATCH v4] " Michał Winiarski
@ 2015-12-18 20:26 ` Chris Wilson
2015-12-22 11:00 ` [PATCH v5] " Michał Winiarski
1 sibling, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2015-12-18 20:26 UTC (permalink / raw)
To: Michał Winiarski; +Cc: intel-gfx
On Fri, Dec 18, 2015 at 09:01:03PM +0100, Michał Winiarski wrote:
> According to bspec, some parts of HW require the addresses to be in
> a canonical form, where bits [63:48] == [47]. Let's convert addresses to
> canonical form prior to relocating and return converted offsets to
> userspace. We also need to make sure that userspace is using addresses
> in canonical form in case of softpin.
>
> v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville)
> v3: Rebase on top of softpin, fix a hole in relocate_entry,
> s/expect/require (Chris)
> v4: Handle softpin in validate_exec_list (Chris)
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> @@ -625,8 +645,9 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
> entry->flags |= __EXEC_OBJECT_HAS_FENCE;
> }
>
> - if (entry->offset != vma->node.start) {
> - entry->offset = vma->node.start;
> + offset = gen8_canonical_addr(vma->node.start);
> + if (entry->offset != offset) {
entry->offset is now in normal form.
> + entry->offset = offset;
> *need_reloc = true;
> }
But we need to convert back to canonical form again on the userspace
boundary (copy_to_user).
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 30+ messages in thread
* ✗ warning: Fi.CI.BAT
2015-12-04 11:33 [PATCH] drm/i915: Avoid writing relocs with addresses in non-canonical form Michał Winiarski
` (2 preceding siblings ...)
2015-12-04 15:20 ` [PATCH v2] " Michał Winiarski
@ 2015-12-19 8:20 ` Patchwork
2015-12-22 11:13 ` Patchwork
` (3 subsequent siblings)
7 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2015-12-19 8:20 UTC (permalink / raw)
To: Michał Winiarski; +Cc: intel-gfx
== Summary ==
Built on 7cdc548e77f503593b83a1c5d58f4dcc862c17e2 drm-intel-nightly: 2015y-12m-18d-19h-26m-21s UTC integration manifest
Test gem_storedw_loop:
Subgroup basic-render:
pass -> DMESG-WARN (skl-i5k-2)
pass -> DMESG-WARN (skl-i7k-2)
Test kms_flip:
Subgroup basic-flip-vs-dpms:
pass -> DMESG-WARN (ilk-hp8440p)
Subgroup basic-flip-vs-modeset:
dmesg-warn -> PASS (hsw-brixbox)
dmesg-warn -> PASS (byt-nuc)
Subgroup basic-flip-vs-wf_vblank:
pass -> DMESG-WARN (byt-nuc)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-a:
dmesg-warn -> PASS (snb-x220t)
Subgroup read-crc-pipe-a-frame-sequence:
dmesg-warn -> PASS (byt-nuc)
Subgroup read-crc-pipe-b:
pass -> DMESG-WARN (snb-dellxps)
Subgroup read-crc-pipe-c:
pass -> DMESG-WARN (bdw-ultra)
dmesg-warn -> PASS (skl-i7k-2)
Test kms_setmode:
Subgroup basic-clone-single-crtc:
dmesg-warn -> PASS (snb-dellxps)
Test pm_rpm:
Subgroup basic-pci-d3-state:
fail -> DMESG-FAIL (snb-x220t)
dmesg-warn -> PASS (bdw-ultra)
bdw-ultra total:132 pass:124 dwarn:2 dfail:0 fail:0 skip:6
byt-nuc total:135 pass:120 dwarn:2 dfail:0 fail:0 skip:13
hsw-brixbox total:135 pass:127 dwarn:1 dfail:0 fail:0 skip:7
hsw-gt2 total:135 pass:130 dwarn:1 dfail:0 fail:0 skip:4
ilk-hp8440p total:135 pass:99 dwarn:1 dfail:0 fail:0 skip:35
ivb-t430s total:135 pass:127 dwarn:2 dfail:0 fail:0 skip:6
skl-i5k-2 total:135 pass:121 dwarn:6 dfail:0 fail:0 skip:8
skl-i7k-2 total:135 pass:122 dwarn:5 dfail:0 fail:0 skip:8
snb-dellxps total:135 pass:121 dwarn:2 dfail:0 fail:0 skip:12
snb-x220t total:135 pass:122 dwarn:1 dfail:1 fail:0 skip:11
Results at /archive/results/CI_IGT_test/Patchwork_725/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v5] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-18 20:01 ` [PATCH v4] " Michał Winiarski
2015-12-18 20:26 ` Chris Wilson
@ 2015-12-22 11:00 ` Michał Winiarski
2015-12-22 11:41 ` Winiarski, Michal
2015-12-22 12:37 ` [PATCH v6] " Michał Winiarski
1 sibling, 2 replies; 30+ messages in thread
From: Michał Winiarski @ 2015-12-22 11:00 UTC (permalink / raw)
To: intel-gfx
According to bspec, some parts of HW require the addresses to be in
a canonical form, where bits [63:48] == [47]. Let's convert addresses to
canonical form prior to relocating and return converted offsets to
userspace. We also need to make sure that userspace is using addresses
in canonical form in case of softpin.
v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville)
v3: Rebase on top of softpin, fix a hole in relocate_entry,
s/expect/require (Chris)
v4: Handle softpin in validate_exec_list (Chris)
v5: Convert back to canonical form at copy_to_user time (Chris)
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 107 ++++++++++++++++++-----------
1 file changed, 66 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 5d01ea6..c906232 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -249,6 +249,25 @@ static inline int use_cpu_reloc(struct drm_i915_gem_object *obj)
obj->cache_level != I915_CACHE_NONE);
}
+/* Used to convert any address to canonical form.
+ * Starting from gen8, some commands (e.g. STATE_BASE_ADDRESS,
+ * MI_LOAD_REGISTER_MEM and others, see Broadwell PRM Vol2a) require the
+ * addresses to be in a canonical form:
+ * "GraphicsAddress[63:48] are ignored by the HW and assumed to be in correct
+ * canonical form [63:48] == [47]."
+ */
+static inline uint64_t gen8_canonical_addr(uint64_t address)
+{
+ return ((int64_t)address << 16) >> 16;
+}
+
+static inline uint64_t
+relocation_target(struct drm_i915_gem_relocation_entry *reloc,
+ uint64_t target_offset)
+{
+ return gen8_canonical_addr((int)reloc->delta + target_offset);
+}
+
static int
relocate_entry_cpu(struct drm_i915_gem_object *obj,
struct drm_i915_gem_relocation_entry *reloc,
@@ -256,7 +275,7 @@ relocate_entry_cpu(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
uint32_t page_offset = offset_in_page(reloc->offset);
- uint64_t delta = reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_offset);
char *vaddr;
int ret;
@@ -292,7 +311,7 @@ relocate_entry_gtt(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- uint64_t delta = reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_offset);
uint64_t offset;
void __iomem *reloc_page;
int ret;
@@ -347,7 +366,7 @@ relocate_entry_clflush(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
uint32_t page_offset = offset_in_page(reloc->offset);
- uint64_t delta = (int)reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_offset);
char *vaddr;
int ret;
@@ -395,7 +414,7 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
target_i915_obj = target_vma->obj;
target_obj = &target_vma->obj->base;
- target_offset = target_vma->node.start;
+ target_offset = gen8_canonical_addr(target_vma->node.start);
/* Sandybridge PPGTT errata: We need a global gtt mapping for MI and
* pipe_control writes because the gpu doesn't properly redirect them
@@ -994,6 +1013,18 @@ validate_exec_list(struct drm_device *dev,
if (exec[i].flags & invalid_flags)
return -EINVAL;
+ /* Offset can be used as input (EXEC_OBJECT_PINNED), since
+ * userspace has to use canonical format, we need to reject all
+ * non-canonical addresses.
+ */
+ if (exec[i].offset != gen8_canonical_addr(exec[i].offset))
+ return -EINVAL;
+
+ /* On the other hand, from drm_mm perspective address space is
+ * continuous, so we're converting to non-canonical form
+ */
+ exec[i].offset &= (1ULL << 48) - 1;
+
if (exec[i].alignment && !is_power_of_2(exec[i].alignment))
return -EINVAL;
@@ -1617,6 +1648,29 @@ pre_mutex_err:
return ret;
}
+static inline int
+__i915_gem_execlist_copy_to_user(struct drm_i915_gem_execbuffer2 *args,
+ struct drm_i915_gem_exec_object2 *exec2_list)
+{
+ struct drm_i915_gem_exec_object2 __user *user_exec_list =
+ to_user_ptr(args->buffers_ptr);
+ int i;
+
+ for (i = 0; i < args->buffer_count; i++) {
+ /* Userspace uses addresses in canonical form */
+ exec2_list[i].offset = gen8_canonical_addr(exec2_list[i].offset);
+ if (__copy_to_user(&user_exec_list[i].offset,
+ &exec2_list[i].offset,
+ sizeof(user_exec_list[i].offset))) {
+ DRM_DEBUG("failed to copy %d exec entries back to user\n",
+ args->buffer_count);
+ return -EFAULT;
+ }
+ }
+
+ return 0;
+}
+
/*
* Legacy execbuffer just creates an exec2 list from the original exec object
* list array and passes it to the real function.
@@ -1681,24 +1735,10 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
i915_execbuffer2_set_context_id(exec2, 0);
ret = i915_gem_do_execbuffer(dev, data, file, &exec2, exec2_list);
- if (!ret) {
- struct drm_i915_gem_exec_object __user *user_exec_list =
- to_user_ptr(args->buffers_ptr);
-
- /* Copy the new buffer offsets back to the user's exec list. */
- for (i = 0; i < args->buffer_count; i++) {
- ret = __copy_to_user(&user_exec_list[i].offset,
- &exec2_list[i].offset,
- sizeof(user_exec_list[i].offset));
- if (ret) {
- ret = -EFAULT;
- DRM_DEBUG("failed to copy %d exec entries "
- "back to user (%d)\n",
- args->buffer_count, ret);
- break;
- }
- }
- }
+
+ /* Copy the new buffer offsets back to the user's exec list. */
+ if (!ret)
+ ret = __i915_gem_execlist_copy_to_user(&exec2, exec2_list);
drm_free_large(exec_list);
drm_free_large(exec2_list);
@@ -1745,25 +1785,10 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data,
}
ret = i915_gem_do_execbuffer(dev, data, file, args, exec2_list);
- if (!ret) {
- /* Copy the new buffer offsets back to the user's exec list. */
- struct drm_i915_gem_exec_object2 __user *user_exec_list =
- to_user_ptr(args->buffers_ptr);
- int i;
-
- for (i = 0; i < args->buffer_count; i++) {
- ret = __copy_to_user(&user_exec_list[i].offset,
- &exec2_list[i].offset,
- sizeof(user_exec_list[i].offset));
- if (ret) {
- ret = -EFAULT;
- DRM_DEBUG("failed to copy %d exec entries "
- "back to user\n",
- args->buffer_count);
- break;
- }
- }
- }
+
+ /* Copy the new buffer offsets back to the user's exec list. */
+ if (!ret)
+ ret = __i915_gem_execlist_copy_to_user(args, exec2_list);
drm_free_large(exec2_list);
return ret;
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 30+ messages in thread
* ✗ warning: Fi.CI.BAT
2015-12-04 11:33 [PATCH] drm/i915: Avoid writing relocs with addresses in non-canonical form Michał Winiarski
` (3 preceding siblings ...)
2015-12-19 8:20 ` ✗ warning: Fi.CI.BAT Patchwork
@ 2015-12-22 11:13 ` Patchwork
2015-12-22 14:49 ` Patchwork
` (2 subsequent siblings)
7 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2015-12-22 11:13 UTC (permalink / raw)
To: Michał Winiarski; +Cc: intel-gfx
== Summary ==
Built on 9d34680058127d4d628a78dab44cd6ecb62e97c6 drm-intel-nightly: 2015y-12m-22d-09h-40m-43s UTC integration manifest
Test gem_storedw_loop:
Subgroup basic-render:
pass -> DMESG-WARN (skl-i7k-2)
Test kms_flip:
Subgroup basic-flip-vs-modeset:
dmesg-warn -> PASS (hsw-xps12)
dmesg-warn -> PASS (hsw-brixbox)
dmesg-warn -> PASS (bdw-nuci7)
Subgroup basic-plain-flip:
dmesg-warn -> PASS (bdw-ultra)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-a:
pass -> DMESG-WARN (snb-x220t)
pass -> DMESG-WARN (skl-i7k-2)
Subgroup read-crc-pipe-b-frame-sequence:
pass -> DMESG-WARN (bdw-ultra)
bdw-nuci7 total:132 pass:122 dwarn:1 dfail:0 fail:0 skip:9
bdw-ultra total:132 pass:124 dwarn:2 dfail:0 fail:0 skip:6
bsw-nuc-2 total:135 pass:114 dwarn:1 dfail:0 fail:0 skip:20
byt-nuc total:135 pass:120 dwarn:2 dfail:0 fail:0 skip:13
hsw-brixbox total:135 pass:127 dwarn:1 dfail:0 fail:0 skip:7
hsw-gt2 total:135 pass:130 dwarn:1 dfail:0 fail:0 skip:4
hsw-xps12 total:132 pass:126 dwarn:2 dfail:0 fail:0 skip:4
ilk-hp8440p total:135 pass:100 dwarn:0 dfail:0 fail:0 skip:35
ivb-t430s total:135 pass:127 dwarn:2 dfail:0 fail:0 skip:6
skl-i5k-2 total:135 pass:121 dwarn:6 dfail:0 fail:0 skip:8
skl-i7k-2 total:135 pass:121 dwarn:6 dfail:0 fail:0 skip:8
snb-dellxps total:135 pass:121 dwarn:2 dfail:0 fail:0 skip:12
snb-x220t total:135 pass:121 dwarn:2 dfail:0 fail:1 skip:11
Results at /archive/results/CI_IGT_test/Patchwork_790/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-22 11:00 ` [PATCH v5] " Michał Winiarski
@ 2015-12-22 11:41 ` Winiarski, Michal
2015-12-22 12:37 ` [PATCH v6] " Michał Winiarski
1 sibling, 0 replies; 30+ messages in thread
From: Winiarski, Michal @ 2015-12-22 11:41 UTC (permalink / raw)
To: intel-gfx
On Tue, 2015-12-22 at 12:00 +0100, Michał Winiarski wrote:
> According to bspec, some parts of HW require the addresses to be in
> a canonical form, where bits [63:48] == [47]. Let's convert addresses
> to
> canonical form prior to relocating and return converted offsets to
> userspace. We also need to make sure that userspace is using
> addresses
> in canonical form in case of softpin.
>
> v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville)
> v3: Rebase on top of softpin, fix a hole in relocate_entry,
> s/expect/require (Chris)
> v4: Handle softpin in validate_exec_list (Chris)
> v5: Convert back to canonical form at copy_to_user time (Chris)
Please ignore this one, I'm breaking old execbuffer here.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 107 ++++++++++++++++++-
> ----------
> 1 file changed, 66 insertions(+), 41 deletions(-)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v6] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-22 11:00 ` [PATCH v5] " Michał Winiarski
2015-12-22 11:41 ` Winiarski, Michal
@ 2015-12-22 12:37 ` Michał Winiarski
2015-12-22 15:00 ` Chris Wilson
2015-12-29 15:49 ` [PATCH v7] " Michał Winiarski
1 sibling, 2 replies; 30+ messages in thread
From: Michał Winiarski @ 2015-12-22 12:37 UTC (permalink / raw)
To: intel-gfx
According to bspec, some parts of HW require the addresses to be in
a canonical form, where bits [63:48] == [47]. Let's convert addresses to
canonical form prior to relocating and return converted offsets to
userspace. We also need to make sure that userspace is using addresses
in canonical form in case of softpin.
v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville)
v3: Rebase on top of softpin, fix a hole in relocate_entry,
s/expect/require (Chris)
v4: Handle softpin in validate_exec_list (Chris)
v5: Convert back to canonical form at copy_to_user time (Chris)
v6: Don't use struct exec_object2 in place of exec_object
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 43 +++++++++++++++++++++++++++---
1 file changed, 39 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 5d01ea6..4e13174 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -249,6 +249,25 @@ static inline int use_cpu_reloc(struct drm_i915_gem_object *obj)
obj->cache_level != I915_CACHE_NONE);
}
+/* Used to convert any address to canonical form.
+ * Starting from gen8, some commands (e.g. STATE_BASE_ADDRESS,
+ * MI_LOAD_REGISTER_MEM and others, see Broadwell PRM Vol2a) require the
+ * addresses to be in a canonical form:
+ * "GraphicsAddress[63:48] are ignored by the HW and assumed to be in correct
+ * canonical form [63:48] == [47]."
+ */
+static inline uint64_t gen8_canonical_addr(uint64_t address)
+{
+ return ((int64_t)address << 16) >> 16;
+}
+
+static inline uint64_t
+relocation_target(struct drm_i915_gem_relocation_entry *reloc,
+ uint64_t target_offset)
+{
+ return gen8_canonical_addr((int)reloc->delta + target_offset);
+}
+
static int
relocate_entry_cpu(struct drm_i915_gem_object *obj,
struct drm_i915_gem_relocation_entry *reloc,
@@ -256,7 +275,7 @@ relocate_entry_cpu(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
uint32_t page_offset = offset_in_page(reloc->offset);
- uint64_t delta = reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_offset);
char *vaddr;
int ret;
@@ -292,7 +311,7 @@ relocate_entry_gtt(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- uint64_t delta = reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_offset);
uint64_t offset;
void __iomem *reloc_page;
int ret;
@@ -347,7 +366,7 @@ relocate_entry_clflush(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
uint32_t page_offset = offset_in_page(reloc->offset);
- uint64_t delta = (int)reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_offset);
char *vaddr;
int ret;
@@ -395,7 +414,7 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
target_i915_obj = target_vma->obj;
target_obj = &target_vma->obj->base;
- target_offset = target_vma->node.start;
+ target_offset = gen8_canonical_addr(target_vma->node.start);
/* Sandybridge PPGTT errata: We need a global gtt mapping for MI and
* pipe_control writes because the gpu doesn't properly redirect them
@@ -994,6 +1013,18 @@ validate_exec_list(struct drm_device *dev,
if (exec[i].flags & invalid_flags)
return -EINVAL;
+ /* Offset can be used as input (EXEC_OBJECT_PINNED), since
+ * userspace has to use canonical format, we need to reject all
+ * non-canonical addresses.
+ */
+ if (exec[i].offset != gen8_canonical_addr(exec[i].offset))
+ return -EINVAL;
+
+ /* On the other hand, from drm_mm perspective address space is
+ * continuous, so we're converting to non-canonical form
+ */
+ exec[i].offset &= (1ULL << 48) - 1;
+
if (exec[i].alignment && !is_power_of_2(exec[i].alignment))
return -EINVAL;
@@ -1687,6 +1718,8 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
/* Copy the new buffer offsets back to the user's exec list. */
for (i = 0; i < args->buffer_count; i++) {
+ exec2_list[i].offset =
+ gen8_canonical_addr(exec2_list[i].offset);
ret = __copy_to_user(&user_exec_list[i].offset,
&exec2_list[i].offset,
sizeof(user_exec_list[i].offset));
@@ -1752,6 +1785,8 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data,
int i;
for (i = 0; i < args->buffer_count; i++) {
+ exec2_list[i].offset =
+ gen8_canonical_addr(exec2_list[i].offset);
ret = __copy_to_user(&user_exec_list[i].offset,
&exec2_list[i].offset,
sizeof(user_exec_list[i].offset));
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 30+ messages in thread
* ✗ warning: Fi.CI.BAT
2015-12-04 11:33 [PATCH] drm/i915: Avoid writing relocs with addresses in non-canonical form Michał Winiarski
` (4 preceding siblings ...)
2015-12-22 11:13 ` Patchwork
@ 2015-12-22 14:49 ` Patchwork
2015-12-29 16:20 ` ✗ failure: Fi.CI.BAT Patchwork
2015-12-29 17:49 ` ✗ warning: Fi.CI.BAT Patchwork
7 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2015-12-22 14:49 UTC (permalink / raw)
To: Michał Winiarski; +Cc: intel-gfx
== Summary ==
Built on 7e671e69deffb88d60687dacffe6e34a5d046500 drm-intel-nightly: 2015y-12m-22d-13h-28m-34s UTC integration manifest
Test kms_flip:
Subgroup basic-flip-vs-modeset:
pass -> DMESG-WARN (skl-i5k-2)
dmesg-warn -> PASS (bsw-nuc-2)
dmesg-warn -> PASS (hsw-xps12)
pass -> DMESG-WARN (hsw-brixbox)
dmesg-warn -> PASS (ilk-hp8440p)
Subgroup basic-plain-flip:
pass -> DMESG-WARN (snb-x220t)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-a-frame-sequence:
dmesg-warn -> PASS (byt-nuc)
Subgroup read-crc-pipe-b:
dmesg-warn -> PASS (skl-i5k-2)
Subgroup read-crc-pipe-b-frame-sequence:
pass -> DMESG-WARN (bdw-ultra)
Test kms_psr_sink_crc:
Subgroup psr_basic:
dmesg-warn -> PASS (bdw-ultra)
Test pm_rpm:
Subgroup basic-rte:
pass -> DMESG-WARN (hsw-xps12)
bdw-nuci7 total:132 pass:122 dwarn:1 dfail:0 fail:0 skip:9
bdw-ultra total:132 pass:124 dwarn:2 dfail:0 fail:0 skip:6
bsw-nuc-2 total:135 pass:114 dwarn:1 dfail:0 fail:0 skip:20
byt-nuc total:135 pass:119 dwarn:3 dfail:0 fail:0 skip:13
hsw-brixbox total:135 pass:126 dwarn:2 dfail:0 fail:0 skip:7
hsw-gt2 total:135 pass:130 dwarn:1 dfail:0 fail:0 skip:4
hsw-xps12 total:132 pass:125 dwarn:3 dfail:0 fail:0 skip:4
ilk-hp8440p total:135 pass:99 dwarn:1 dfail:0 fail:0 skip:35
ivb-t430s total:135 pass:127 dwarn:2 dfail:0 fail:0 skip:6
skl-i5k-2 total:135 pass:124 dwarn:3 dfail:0 fail:0 skip:8
skl-i7k-2 total:135 pass:124 dwarn:3 dfail:0 fail:0 skip:8
snb-dellxps total:135 pass:121 dwarn:2 dfail:0 fail:0 skip:12
snb-x220t total:135 pass:121 dwarn:2 dfail:0 fail:1 skip:11
Results at /archive/results/CI_IGT_test/Patchwork_791/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v6] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-22 12:37 ` [PATCH v6] " Michał Winiarski
@ 2015-12-22 15:00 ` Chris Wilson
2015-12-29 15:49 ` [PATCH v7] " Michał Winiarski
1 sibling, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2015-12-22 15:00 UTC (permalink / raw)
To: Michał Winiarski; +Cc: intel-gfx
On Tue, Dec 22, 2015 at 01:37:10PM +0100, Michał Winiarski wrote:
> + /* Offset can be used as input (EXEC_OBJECT_PINNED), since
> + * userspace has to use canonical format, we need to reject all
> + * non-canonical addresses.
> + */
> + if (exec[i].offset != gen8_canonical_addr(exec[i].offset))
> + return -EINVAL;
> +
> + /* On the other hand, from drm_mm perspective address space is
> + * continuous, so we're converting to non-canonical form
> + */
> + exec[i].offset &= (1ULL << 48) - 1;
I'm still dubious about making this ABI change backwards. Only
EXEC_OBJECT_PINNED uses this as an input and in the past we have stuffed
bad values in here to force relocations etc.
Also note that EXEC_OBJECT_PINNED doesn't do if (exec[i].offset & 4095)
return -EINVAL;
I would address both at once.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v7] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-22 12:37 ` [PATCH v6] " Michał Winiarski
2015-12-22 15:00 ` Chris Wilson
@ 2015-12-29 15:49 ` Michał Winiarski
2015-12-29 16:25 ` Chris Wilson
2015-12-29 17:14 ` [PATCH v8] " Michał Winiarski
1 sibling, 2 replies; 30+ messages in thread
From: Michał Winiarski @ 2015-12-29 15:49 UTC (permalink / raw)
To: intel-gfx
According to PRM, some parts of HW require the addresses to be in
a canonical form, where bits [63:48] == [47]. Let's convert addresses to
canonical form prior to relocating and return converted offsets to
userspace. We also need to make sure that userspace is using addresses
in canonical form in case of softpin.
v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville)
v3: Rebase on top of softpin, fix a hole in relocate_entry,
s/expect/require (Chris)
v4: Handle softpin in validate_exec_list (Chris)
v5: Convert back to canonical form at copy_to_user time (Chris)
v6: Don't use struct exec_object2 in place of exec_object
v7: Use sign_extend64 for converting to canonical form (Joonas),
reject non-canonical and non-page-aligned offset for softpin (Chris)
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 45 +++++++++++++++++++++++++++---
1 file changed, 41 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 5d01ea6..dd081fd 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -249,6 +249,25 @@ static inline int use_cpu_reloc(struct drm_i915_gem_object *obj)
obj->cache_level != I915_CACHE_NONE);
}
+/* Used to convert any address to canonical form.
+ * Starting from gen8, some commands (e.g. STATE_BASE_ADDRESS,
+ * MI_LOAD_REGISTER_MEM and others, see Broadwell PRM Vol2a) require the
+ * addresses to be in a canonical form:
+ * "GraphicsAddress[63:48] are ignored by the HW and assumed to be in correct
+ * canonical form [63:48] == [47]."
+ */
+static inline uint64_t gen8_canonical_addr(uint64_t address)
+{
+ return sign_extend64(address, 47);
+}
+
+static inline uint64_t
+relocation_target(struct drm_i915_gem_relocation_entry *reloc,
+ uint64_t target_offset)
+{
+ return gen8_canonical_addr((int)reloc->delta + target_offset);
+}
+
static int
relocate_entry_cpu(struct drm_i915_gem_object *obj,
struct drm_i915_gem_relocation_entry *reloc,
@@ -256,7 +275,7 @@ relocate_entry_cpu(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
uint32_t page_offset = offset_in_page(reloc->offset);
- uint64_t delta = reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_offset);
char *vaddr;
int ret;
@@ -292,7 +311,7 @@ relocate_entry_gtt(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- uint64_t delta = reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_offset);
uint64_t offset;
void __iomem *reloc_page;
int ret;
@@ -347,7 +366,7 @@ relocate_entry_clflush(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
uint32_t page_offset = offset_in_page(reloc->offset);
- uint64_t delta = (int)reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_offset);
char *vaddr;
int ret;
@@ -395,7 +414,7 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
target_i915_obj = target_vma->obj;
target_obj = &target_vma->obj->base;
- target_offset = target_vma->node.start;
+ target_offset = gen8_canonical_addr(target_vma->node.start);
/* Sandybridge PPGTT errata: We need a global gtt mapping for MI and
* pipe_control writes because the gpu doesn't properly redirect them
@@ -994,6 +1013,20 @@ validate_exec_list(struct drm_device *dev,
if (exec[i].flags & invalid_flags)
return -EINVAL;
+ /* Offset can be used as input (EXEC_OBJECT_PINNED), reject
+ * any non-page-aligned or non-canonial addresses.
+ */
+ if (exec[i].flags & EXEC_OBJECT_PINNED &&
+ (exec[i].offset != gen8_canonical_addr(exec[i].offset) ||
+ offset_in_page(exec[i].offset)))
+ return -EINVAL;
+
+ /* From drm_mm perspective address space is continuous,
+ * so from this point we're always using non-canonical form
+ * internally.
+ */
+ exec[i].offset &= (1ULL << 48) - 1;
+
if (exec[i].alignment && !is_power_of_2(exec[i].alignment))
return -EINVAL;
@@ -1687,6 +1720,8 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
/* Copy the new buffer offsets back to the user's exec list. */
for (i = 0; i < args->buffer_count; i++) {
+ exec2_list[i].offset =
+ gen8_canonical_addr(exec2_list[i].offset);
ret = __copy_to_user(&user_exec_list[i].offset,
&exec2_list[i].offset,
sizeof(user_exec_list[i].offset));
@@ -1752,6 +1787,8 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data,
int i;
for (i = 0; i < args->buffer_count; i++) {
+ exec2_list[i].offset =
+ gen8_canonical_addr(exec2_list[i].offset);
ret = __copy_to_user(&user_exec_list[i].offset,
&exec2_list[i].offset,
sizeof(user_exec_list[i].offset));
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 30+ messages in thread
* ✗ failure: Fi.CI.BAT
2015-12-04 11:33 [PATCH] drm/i915: Avoid writing relocs with addresses in non-canonical form Michał Winiarski
` (5 preceding siblings ...)
2015-12-22 14:49 ` Patchwork
@ 2015-12-29 16:20 ` Patchwork
2015-12-29 17:49 ` ✗ warning: Fi.CI.BAT Patchwork
7 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2015-12-29 16:20 UTC (permalink / raw)
To: Michał Winiarski; +Cc: intel-gfx
== Summary ==
Built on ec0382c73cb1adc972bebdd94afad3f0ea117114 drm-intel-nightly: 2015y-12m-23d-22h-28m-25s UTC integration manifest
Test gem_ctx_param_basic:
Subgroup root-set:
skip -> PASS (ilk-hp8440p)
Test gem_storedw_loop:
Subgroup basic-render:
dmesg-warn -> PASS (skl-i5k-2)
dmesg-warn -> PASS (bdw-nuci7)
Test kms_flip:
Subgroup basic-flip-vs-dpms:
dmesg-warn -> PASS (ilk-hp8440p)
Subgroup basic-flip-vs-modeset:
dmesg-warn -> PASS (skl-i5k-2)
pass -> DMESG-WARN (hsw-xps12)
pass -> DMESG-WARN (hsw-brixbox)
pass -> DMESG-WARN (bdw-nuci7)
Subgroup basic-plain-flip:
pass -> DMESG-WARN (snb-x220t)
pass -> DMESG-WARN (snb-dellxps)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-a:
dmesg-warn -> PASS (snb-x220t)
pass -> DMESG-WARN (skl-i7k-2)
Subgroup read-crc-pipe-b:
pass -> DMESG-WARN (skl-i5k-2)
Subgroup read-crc-pipe-b-frame-sequence:
pass -> DMESG-WARN (byt-nuc)
Test kms_setmode:
Subgroup basic-clone-single-crtc:
dmesg-warn -> PASS (snb-dellxps)
Test pm_rpm:
Subgroup basic-rte:
dmesg-warn -> PASS (hsw-xps12)
pass -> DMESG-WARN (byt-nuc)
bdw-nuci7 total:132 pass:121 dwarn:2 dfail:0 fail:0 skip:9
bdw-ultra total:132 pass:124 dwarn:2 dfail:0 fail:0 skip:6
bsw-nuc-2 total:135 pass:113 dwarn:2 dfail:0 fail:0 skip:20
byt-nuc total:135 pass:119 dwarn:3 dfail:0 fail:0 skip:13
hsw-brixbox total:135 pass:126 dwarn:2 dfail:0 fail:0 skip:7
hsw-xps12 total:132 pass:125 dwarn:3 dfail:0 fail:0 skip:4
ilk-hp8440p total:135 pass:101 dwarn:0 dfail:0 fail:0 skip:34
ivb-t430s total:135 pass:127 dwarn:2 dfail:0 fail:0 skip:6
skl-i5k-2 total:135 pass:124 dwarn:3 dfail:0 fail:0 skip:8
skl-i7k-2 total:135 pass:123 dwarn:4 dfail:0 fail:0 skip:8
snb-dellxps total:135 pass:121 dwarn:2 dfail:0 fail:0 skip:12
snb-x220t total:135 pass:121 dwarn:2 dfail:0 fail:1 skip:11
HANGED hsw-gt2 in igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c
Results at /archive/results/CI_IGT_test/Patchwork_945/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v7] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-29 15:49 ` [PATCH v7] " Michał Winiarski
@ 2015-12-29 16:25 ` Chris Wilson
2015-12-29 17:14 ` [PATCH v8] " Michał Winiarski
1 sibling, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2015-12-29 16:25 UTC (permalink / raw)
To: Michał Winiarski; +Cc: intel-gfx
On Tue, Dec 29, 2015 at 04:49:01PM +0100, Michał Winiarski wrote:
> @@ -994,6 +1013,20 @@ validate_exec_list(struct drm_device *dev,
> if (exec[i].flags & invalid_flags)
> return -EINVAL;
>
> + /* Offset can be used as input (EXEC_OBJECT_PINNED), reject
> + * any non-page-aligned or non-canonial addresses.
> + */
> + if (exec[i].flags & EXEC_OBJECT_PINNED &&
> + (exec[i].offset != gen8_canonical_addr(exec[i].offset) ||
> + offset_in_page(exec[i].offset)))
> + return -EINVAL;
> +
if (exec[i].flags & EXEC_OBJECT_PINNED) {
if (exec[i].offset != gen8_canonical_addr(exec[i].offset & PAGE_MASK))
return -EINVAL;
/* From drm_mm perspective address space is continuous,
* so from this point we're always using non-canonical form
* internally.
*/
exec[i].offset &= (1ULL << 48) - 1;
}
Splitting up the two tests just makes it a bit easier to read (imo, and
I've been told on numerous occasions to do the same :) Whilst not as
obvious atm, it also helps when we have multiple extension checks in the
validate(). As a secondary point, we can then also demonstate that we
can fully restrict manipulating exec[i].offset to the pinned path.
#define GEN8_HIGH_ADDRESS_BIT 47
#define GEN8_ADDRESS_MASK (1ULL << (GEN8_HIGH_ADDRESS_BIT+1)) - 1
GEN8_CANONICAL_HIGH_BIT ?
since we have two places now that know about the address format, or
perhaps
static u64 gen8_undo_canonical_addr(u64);
exec[i].offset = gen8_undo_canonical_addr(exec[i].offset);
so that we can put them next to each other. That seems a better idea.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v8] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-29 15:49 ` [PATCH v7] " Michał Winiarski
2015-12-29 16:25 ` Chris Wilson
@ 2015-12-29 17:14 ` Michał Winiarski
2015-12-29 17:24 ` [PATCH v9] " Michał Winiarski
2015-12-29 17:25 ` [PATCH v8] " Chris Wilson
1 sibling, 2 replies; 30+ messages in thread
From: Michał Winiarski @ 2015-12-29 17:14 UTC (permalink / raw)
To: intel-gfx
According to PRM, some parts of HW require the addresses to be in
a canonical form, where bits [63:48] == [47]. Let's convert addresses to
canonical form prior to relocating and return converted offsets to
userspace. We also need to make sure that userspace is using addresses
in canonical form in case of softpin.
v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville)
v3: Rebase on top of softpin, fix a hole in relocate_entry,
s/expect/require (Chris)
v4: Handle softpin in validate_exec_list (Chris)
v5: Convert back to canonical form at copy_to_user time (Chris)
v6: Don't use struct exec_object2 in place of exec_object
v7: Use sign_extend64 for converting to canonical form (Joonas),
reject non-canonical and non-page-aligned offset for softpin (Chris)
v8: Convert back to non-canonical form in a function,
split the test for EXEC_OBJECT_PINNED (Chris)
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 53 +++++++++++++++++++++++++++---
1 file changed, 49 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 5d01ea6..97fb9af 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -249,6 +249,31 @@ static inline int use_cpu_reloc(struct drm_i915_gem_object *obj)
obj->cache_level != I915_CACHE_NONE);
}
+/* Used to convert any address to canonical form.
+ * Starting from gen8, some commands (e.g. STATE_BASE_ADDRESS,
+ * MI_LOAD_REGISTER_MEM and others, see Broadwell PRM Vol2a) require the
+ * addresses to be in a canonical form:
+ * "GraphicsAddress[63:48] are ignored by the HW and assumed to be in correct
+ * canonical form [63:48] == [47]."
+ */
+#define GEN8_HIGH_ADDRESS_BIT 47
+static inline uint64_t gen8_canonical_addr(uint64_t address)
+{
+ return sign_extend64(address, GEN8_HIGH_ADDRESS_BIT);
+}
+
+static inline uint64_t gen8_noncanonical_addr(uint64_t address)
+{
+ return address & ((1ULL << (GEN8_HIGH_ADDRESS_BIT + 1)) - 1);
+}
+
+static inline uint64_t
+relocation_target(struct drm_i915_gem_relocation_entry *reloc,
+ uint64_t target_offset)
+{
+ return gen8_canonical_addr((int)reloc->delta + target_offset);
+}
+
static int
relocate_entry_cpu(struct drm_i915_gem_object *obj,
struct drm_i915_gem_relocation_entry *reloc,
@@ -256,7 +281,7 @@ relocate_entry_cpu(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
uint32_t page_offset = offset_in_page(reloc->offset);
- uint64_t delta = reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_offset);
char *vaddr;
int ret;
@@ -292,7 +317,7 @@ relocate_entry_gtt(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- uint64_t delta = reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_offset);
uint64_t offset;
void __iomem *reloc_page;
int ret;
@@ -347,7 +372,7 @@ relocate_entry_clflush(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
uint32_t page_offset = offset_in_page(reloc->offset);
- uint64_t delta = (int)reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_offset);
char *vaddr;
int ret;
@@ -395,7 +420,7 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
target_i915_obj = target_vma->obj;
target_obj = &target_vma->obj->base;
- target_offset = target_vma->node.start;
+ target_offset = gen8_canonical_addr(target_vma->node.start);
/* Sandybridge PPGTT errata: We need a global gtt mapping for MI and
* pipe_control writes because the gpu doesn't properly redirect them
@@ -994,6 +1019,22 @@ validate_exec_list(struct drm_device *dev,
if (exec[i].flags & invalid_flags)
return -EINVAL;
+ /* Offset can be used as input (EXEC_OBJECT_PINNED), reject
+ * any non-page-aligned or non-canonial addresses.
+ */
+ if (exec[i].flags & EXEC_OBJECT_PINNED) {
+ if (exec[i].offset !=
+ gen8_canonical_addr(exec[i].offset & PAGE_MASK))
+ return -EINVAL;
+
+ /* From drm_mm perspective address space is continuous,
+ * so from this point we're always using non-canonical
+ * form internally.
+ */
+ exec[i].offset = gen8_noncanonical_addr(exec[i].offset);
+ }
+
+
if (exec[i].alignment && !is_power_of_2(exec[i].alignment))
return -EINVAL;
@@ -1687,6 +1728,8 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
/* Copy the new buffer offsets back to the user's exec list. */
for (i = 0; i < args->buffer_count; i++) {
+ exec2_list[i].offset =
+ gen8_canonical_addr(exec2_list[i].offset);
ret = __copy_to_user(&user_exec_list[i].offset,
&exec2_list[i].offset,
sizeof(user_exec_list[i].offset));
@@ -1752,6 +1795,8 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data,
int i;
for (i = 0; i < args->buffer_count; i++) {
+ exec2_list[i].offset =
+ gen8_canonical_addr(exec2_list[i].offset);
ret = __copy_to_user(&user_exec_list[i].offset,
&exec2_list[i].offset,
sizeof(user_exec_list[i].offset));
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v9] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-29 17:14 ` [PATCH v8] " Michał Winiarski
@ 2015-12-29 17:24 ` Michał Winiarski
2015-12-29 17:28 ` Chris Wilson
2015-12-29 17:25 ` [PATCH v8] " Chris Wilson
1 sibling, 1 reply; 30+ messages in thread
From: Michał Winiarski @ 2015-12-29 17:24 UTC (permalink / raw)
To: intel-gfx
According to PRM, some parts of HW require the addresses to be in
a canonical form, where bits [63:48] == [47]. Let's convert addresses to
canonical form prior to relocating and return converted offsets to
userspace. We also need to make sure that userspace is using addresses
in canonical form in case of softpin.
v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville)
v3: Rebase on top of softpin, fix a hole in relocate_entry,
s/expect/require (Chris)
v4: Handle softpin in validate_exec_list (Chris)
v5: Convert back to canonical form at copy_to_user time (Chris)
v6: Don't use struct exec_object2 in place of exec_object
v7: Use sign_extend64 for converting to canonical form (Joonas),
reject non-canonical and non-page-aligned offset for softpin (Chris)
v8: Convert back to non-canonical form in a function,
split the test for EXEC_OBJECT_PINNED (Chris)
v9: s/canonial/canonical, drop accidental double newline (Chris)
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 52 +++++++++++++++++++++++++++---
1 file changed, 48 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 5d01ea6..dccb517 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -249,6 +249,31 @@ static inline int use_cpu_reloc(struct drm_i915_gem_object *obj)
obj->cache_level != I915_CACHE_NONE);
}
+/* Used to convert any address to canonical form.
+ * Starting from gen8, some commands (e.g. STATE_BASE_ADDRESS,
+ * MI_LOAD_REGISTER_MEM and others, see Broadwell PRM Vol2a) require the
+ * addresses to be in a canonical form:
+ * "GraphicsAddress[63:48] are ignored by the HW and assumed to be in correct
+ * canonical form [63:48] == [47]."
+ */
+#define GEN8_HIGH_ADDRESS_BIT 47
+static inline uint64_t gen8_canonical_addr(uint64_t address)
+{
+ return sign_extend64(address, GEN8_HIGH_ADDRESS_BIT);
+}
+
+static inline uint64_t gen8_noncanonical_addr(uint64_t address)
+{
+ return address & ((1ULL << (GEN8_HIGH_ADDRESS_BIT + 1)) - 1);
+}
+
+static inline uint64_t
+relocation_target(struct drm_i915_gem_relocation_entry *reloc,
+ uint64_t target_offset)
+{
+ return gen8_canonical_addr((int)reloc->delta + target_offset);
+}
+
static int
relocate_entry_cpu(struct drm_i915_gem_object *obj,
struct drm_i915_gem_relocation_entry *reloc,
@@ -256,7 +281,7 @@ relocate_entry_cpu(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
uint32_t page_offset = offset_in_page(reloc->offset);
- uint64_t delta = reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_offset);
char *vaddr;
int ret;
@@ -292,7 +317,7 @@ relocate_entry_gtt(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- uint64_t delta = reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_offset);
uint64_t offset;
void __iomem *reloc_page;
int ret;
@@ -347,7 +372,7 @@ relocate_entry_clflush(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
uint32_t page_offset = offset_in_page(reloc->offset);
- uint64_t delta = (int)reloc->delta + target_offset;
+ uint64_t delta = relocation_target(reloc, target_offset);
char *vaddr;
int ret;
@@ -395,7 +420,7 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
target_i915_obj = target_vma->obj;
target_obj = &target_vma->obj->base;
- target_offset = target_vma->node.start;
+ target_offset = gen8_canonical_addr(target_vma->node.start);
/* Sandybridge PPGTT errata: We need a global gtt mapping for MI and
* pipe_control writes because the gpu doesn't properly redirect them
@@ -994,6 +1019,21 @@ validate_exec_list(struct drm_device *dev,
if (exec[i].flags & invalid_flags)
return -EINVAL;
+ /* Offset can be used as input (EXEC_OBJECT_PINNED), reject
+ * any non-page-aligned or non-canonical addresses.
+ */
+ if (exec[i].flags & EXEC_OBJECT_PINNED) {
+ if (exec[i].offset !=
+ gen8_canonical_addr(exec[i].offset & PAGE_MASK))
+ return -EINVAL;
+
+ /* From drm_mm perspective address space is continuous,
+ * so from this point we're always using non-canonical
+ * form internally.
+ */
+ exec[i].offset = gen8_noncanonical_addr(exec[i].offset);
+ }
+
if (exec[i].alignment && !is_power_of_2(exec[i].alignment))
return -EINVAL;
@@ -1687,6 +1727,8 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
/* Copy the new buffer offsets back to the user's exec list. */
for (i = 0; i < args->buffer_count; i++) {
+ exec2_list[i].offset =
+ gen8_canonical_addr(exec2_list[i].offset);
ret = __copy_to_user(&user_exec_list[i].offset,
&exec2_list[i].offset,
sizeof(user_exec_list[i].offset));
@@ -1752,6 +1794,8 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data,
int i;
for (i = 0; i < args->buffer_count; i++) {
+ exec2_list[i].offset =
+ gen8_canonical_addr(exec2_list[i].offset);
ret = __copy_to_user(&user_exec_list[i].offset,
&exec2_list[i].offset,
sizeof(user_exec_list[i].offset));
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v8] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-29 17:14 ` [PATCH v8] " Michał Winiarski
2015-12-29 17:24 ` [PATCH v9] " Michał Winiarski
@ 2015-12-29 17:25 ` Chris Wilson
1 sibling, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2015-12-29 17:25 UTC (permalink / raw)
To: Michał Winiarski; +Cc: intel-gfx
On Tue, Dec 29, 2015 at 06:14:19PM +0100, Michał Winiarski wrote:
> According to PRM, some parts of HW require the addresses to be in
> a canonical form, where bits [63:48] == [47]. Let's convert addresses to
> canonical form prior to relocating and return converted offsets to
> userspace. We also need to make sure that userspace is using addresses
> in canonical form in case of softpin.
>
> v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville)
> v3: Rebase on top of softpin, fix a hole in relocate_entry,
> s/expect/require (Chris)
> v4: Handle softpin in validate_exec_list (Chris)
> v5: Convert back to canonical form at copy_to_user time (Chris)
> v6: Don't use struct exec_object2 in place of exec_object
> v7: Use sign_extend64 for converting to canonical form (Joonas),
> reject non-canonical and non-page-aligned offset for softpin (Chris)
> v8: Convert back to non-canonical form in a function,
> split the test for EXEC_OBJECT_PINNED (Chris)
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
I double checked that all the roundtrips between kernel<->userspace
should be idempotent, so
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Out of curiousity, you should not see any difference in runtime of
gem_exec_lut_handle.
There should also be a Testcase: and perhaps a
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92699
> + /* Offset can be used as input (EXEC_OBJECT_PINNED), reject
> + * any non-page-aligned or non-canonial addresses.
^non-canonical
> + */
> + if (exec[i].flags & EXEC_OBJECT_PINNED) {
> + if (exec[i].offset !=
> + gen8_canonical_addr(exec[i].offset & PAGE_MASK))
> + return -EINVAL;
> +
> + /* From drm_mm perspective address space is continuous,
> + * so from this point we're always using non-canonical
> + * form internally.
> + */
> + exec[i].offset = gen8_noncanonical_addr(exec[i].offset);
> + }
> +
> +
^ extra newline
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v9] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-29 17:24 ` [PATCH v9] " Michał Winiarski
@ 2015-12-29 17:28 ` Chris Wilson
2016-01-05 10:01 ` Daniel Vetter
0 siblings, 1 reply; 30+ messages in thread
From: Chris Wilson @ 2015-12-29 17:28 UTC (permalink / raw)
To: Michał Winiarski; +Cc: intel-gfx
On Tue, Dec 29, 2015 at 06:24:52PM +0100, Michał Winiarski wrote:
> According to PRM, some parts of HW require the addresses to be in
> a canonical form, where bits [63:48] == [47]. Let's convert addresses to
> canonical form prior to relocating and return converted offsets to
> userspace. We also need to make sure that userspace is using addresses
> in canonical form in case of softpin.
>
> v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville)
> v3: Rebase on top of softpin, fix a hole in relocate_entry,
> s/expect/require (Chris)
> v4: Handle softpin in validate_exec_list (Chris)
> v5: Convert back to canonical form at copy_to_user time (Chris)
> v6: Don't use struct exec_object2 in place of exec_object
> v7: Use sign_extend64 for converting to canonical form (Joonas),
> reject non-canonical and non-page-aligned offset for softpin (Chris)
> v8: Convert back to non-canonical form in a function,
> split the test for EXEC_OBJECT_PINNED (Chris)
> v9: s/canonial/canonical, drop accidental double newline (Chris)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92699
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 30+ messages in thread
* ✗ warning: Fi.CI.BAT
2015-12-04 11:33 [PATCH] drm/i915: Avoid writing relocs with addresses in non-canonical form Michał Winiarski
` (6 preceding siblings ...)
2015-12-29 16:20 ` ✗ failure: Fi.CI.BAT Patchwork
@ 2015-12-29 17:49 ` Patchwork
7 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2015-12-29 17:49 UTC (permalink / raw)
To: Michał Winiarski; +Cc: intel-gfx
== Summary ==
Built on ec0382c73cb1adc972bebdd94afad3f0ea117114 drm-intel-nightly: 2015y-12m-23d-22h-28m-25s UTC integration manifest
Test gem_storedw_loop:
Subgroup basic-render:
dmesg-warn -> PASS (skl-i5k-2)
dmesg-warn -> PASS (bdw-nuci7)
Test kms_flip:
Subgroup basic-flip-vs-dpms:
dmesg-warn -> PASS (ilk-hp8440p)
Subgroup basic-flip-vs-modeset:
pass -> DMESG-WARN (bdw-nuci7)
pass -> DMESG-WARN (byt-nuc)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-b:
pass -> DMESG-WARN (skl-i5k-2)
pass -> DMESG-WARN (snb-dellxps)
Subgroup suspend-read-crc-pipe-b:
dmesg-warn -> PASS (skl-i7k-2)
Test kms_setmode:
Subgroup basic-clone-single-crtc:
dmesg-warn -> PASS (snb-dellxps)
Test pm_rpm:
Subgroup basic-rte:
pass -> DMESG-WARN (byt-nuc)
bdw-nuci7 total:132 pass:121 dwarn:2 dfail:0 fail:0 skip:9
bdw-ultra total:132 pass:124 dwarn:2 dfail:0 fail:0 skip:6
bsw-nuc-2 total:135 pass:113 dwarn:2 dfail:0 fail:0 skip:20
byt-nuc total:135 pass:119 dwarn:3 dfail:0 fail:0 skip:13
hsw-brixbox total:135 pass:127 dwarn:1 dfail:0 fail:0 skip:7
hsw-gt2 total:135 pass:130 dwarn:1 dfail:0 fail:0 skip:4
ilk-hp8440p total:135 pass:100 dwarn:0 dfail:0 fail:0 skip:35
ivb-t430s total:135 pass:127 dwarn:2 dfail:0 fail:0 skip:6
skl-i5k-2 total:135 pass:123 dwarn:4 dfail:0 fail:0 skip:8
skl-i7k-2 total:135 pass:125 dwarn:2 dfail:0 fail:0 skip:8
snb-dellxps total:135 pass:121 dwarn:2 dfail:0 fail:0 skip:12
snb-x220t total:135 pass:121 dwarn:2 dfail:0 fail:1 skip:11
Results at /archive/results/CI_IGT_test/Patchwork_948/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v9] drm/i915: Avoid writing relocs with addresses in non-canonical form
2015-12-29 17:28 ` Chris Wilson
@ 2016-01-05 10:01 ` Daniel Vetter
0 siblings, 0 replies; 30+ messages in thread
From: Daniel Vetter @ 2016-01-05 10:01 UTC (permalink / raw)
To: Chris Wilson, Michał Winiarski, intel-gfx, Michel Thierry,
Ville Syrjälä
On Tue, Dec 29, 2015 at 05:28:04PM +0000, Chris Wilson wrote:
> On Tue, Dec 29, 2015 at 06:24:52PM +0100, Michał Winiarski wrote:
> > According to PRM, some parts of HW require the addresses to be in
> > a canonical form, where bits [63:48] == [47]. Let's convert addresses to
> > canonical form prior to relocating and return converted offsets to
> > userspace. We also need to make sure that userspace is using addresses
> > in canonical form in case of softpin.
> >
> > v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville)
> > v3: Rebase on top of softpin, fix a hole in relocate_entry,
> > s/expect/require (Chris)
> > v4: Handle softpin in validate_exec_list (Chris)
> > v5: Convert back to canonical form at copy_to_user time (Chris)
> > v6: Don't use struct exec_object2 in place of exec_object
> > v7: Use sign_extend64 for converting to canonical form (Joonas),
> > reject non-canonical and non-page-aligned offset for softpin (Chris)
> > v8: Convert back to non-canonical form in a function,
> > split the test for EXEC_OBJECT_PINNED (Chris)
> > v9: s/canonial/canonical, drop accidental double newline (Chris)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92699
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Michel Thierry <michel.thierry@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Queued for -next, with Testcase: and Cc: -fixes added, thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2016-01-05 10:01 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-04 11:33 [PATCH] drm/i915: Avoid writing relocs with addresses in non-canonical form Michał Winiarski
2015-12-04 11:56 ` Ville Syrjälä
2015-12-04 12:05 ` Chris Wilson
2015-12-04 15:20 ` [PATCH v2] " Michał Winiarski
2015-12-04 15:39 ` Chris Wilson
2015-12-04 16:18 ` Daniel Vetter
2015-12-04 17:41 ` Winiarski, Michal
2015-12-07 8:31 ` Daniel Vetter
2015-12-10 12:50 ` Chris Wilson
2015-12-11 14:13 ` [PATCH v3] " Michał Winiarski
2015-12-11 14:43 ` Michel Thierry
2015-12-18 16:31 ` Chris Wilson
2015-12-18 20:01 ` [PATCH v4] " Michał Winiarski
2015-12-18 20:26 ` Chris Wilson
2015-12-22 11:00 ` [PATCH v5] " Michał Winiarski
2015-12-22 11:41 ` Winiarski, Michal
2015-12-22 12:37 ` [PATCH v6] " Michał Winiarski
2015-12-22 15:00 ` Chris Wilson
2015-12-29 15:49 ` [PATCH v7] " Michał Winiarski
2015-12-29 16:25 ` Chris Wilson
2015-12-29 17:14 ` [PATCH v8] " Michał Winiarski
2015-12-29 17:24 ` [PATCH v9] " Michał Winiarski
2015-12-29 17:28 ` Chris Wilson
2016-01-05 10:01 ` Daniel Vetter
2015-12-29 17:25 ` [PATCH v8] " Chris Wilson
2015-12-19 8:20 ` ✗ warning: Fi.CI.BAT Patchwork
2015-12-22 11:13 ` Patchwork
2015-12-22 14:49 ` Patchwork
2015-12-29 16:20 ` ✗ failure: Fi.CI.BAT Patchwork
2015-12-29 17:49 ` ✗ warning: Fi.CI.BAT Patchwork
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.