[Intel-gfx] Patch "drm/i915/gt: Fix reservation address in ggtt_reserve_guc_top" has been added to the 6.5-stable tree
This is a note to let you know that I've just added the patch titled
drm/i915/gt: Fix reservation address in ggtt_reserve_guc_top
to the 6.5-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
drm-i915-gt-fix-reservation-address-in-ggtt_reserve_guc_top.patch
and it can be found in the queue-6.5 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let know about it.
>From b7599d241778d0b10cdf7a5c755aa7db9b83250c Mon Sep 17 00:00:00 2001
From: Javier Pello
Date: Sat, 2 Sep 2023 17:10:39 +0200
Subject: drm/i915/gt: Fix reservation address in ggtt_reserve_guc_top
From: Javier Pello
commit b7599d241778d0b10cdf7a5c755aa7db9b83250c upstream.
There is an assertion in ggtt_reserve_guc_top that the global GTT
is of size at least GUC_GGTT_TOP, which is not the case on a 32-bit
platform; see commit 562d55d991b39ce376c492df2f7890fd6a541ffc
("drm/i915/bdw: Only use 2g GGTT for 32b platforms"). If GEM_BUG_ON
is enabled, this triggers a BUG(); if GEM_BUG_ON is disabled, the
subsequent reservation fails and the driver fails to initialise
the device:
i915 :00:02.0: [drm:i915_init_ggtt [i915]] Failed to reserve top of GGTT
for GuC
i915 :00:02.0: Device initialization failed (-28)
i915 :00:02.0: Please file a bug on drm/i915; see
https://gitlab.freedesktop.org/drm/intel/-/wikis/How-to-file-i915-bugs for
details.
i915: probe of :00:02.0 failed with error -28
Make the reservation at the top of the available space, whatever
that is, instead of assuming that the top will be GUC_GGTT_TOP.
Fixes: 911800765ef6 ("drm/i915/uc: Reserve upper range of GGTT")
Link: https://gitlab.freedesktop.org/drm/intel/-/issues/9080
Signed-off-by: Javier Pello
Reviewed-by: Daniele Ceraolo Spurio
Cc: Fernando Pacheco
Cc: Chris Wilson
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-gfx@lists.freedesktop.org
Cc: sta...@vger.kernel.org # v5.3+
Signed-off-by: John Harrison
Link:
https://patchwork.freedesktop.org/patch/msgid/20230902171039.2229126186d697dbcf62d...@otheo.eu
(cherry picked from commit 0f3fa942d91165c2702577e9274d2ee1c7212afc)
Signed-off-by: Rodrigo Vivi
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/gt/intel_ggtt.c | 23 +--
1 file changed, 17 insertions(+), 6 deletions(-)
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -511,20 +511,31 @@ void intel_ggtt_unbind_vma(struct i915_a
vm->clear_range(vm, vma_res->start, vma_res->vma_size);
}
+/*
+ * Reserve the top of the GuC address space for firmware images. Addresses
+ * beyond GUC_GGTT_TOP in the GuC address space are inaccessible by GuC,
+ * which makes for a suitable range to hold GuC/HuC firmware images if the
+ * size of the GGTT is 4G. However, on a 32-bit platform the size of the GGTT
+ * is limited to 2G, which is less than GUC_GGTT_TOP, but we reserve a chunk
+ * of the same size anyway, which is far more than needed, to keep the logic
+ * in uc_fw_ggtt_offset() simple.
+ */
+#define GUC_TOP_RESERVE_SIZE (SZ_4G - GUC_GGTT_TOP)
+
static int ggtt_reserve_guc_top(struct i915_ggtt *ggtt)
{
- u64 size;
+ u64 offset;
int ret;
if (!intel_uc_uses_guc(>vm.gt->uc))
return 0;
- GEM_BUG_ON(ggtt->vm.total <= GUC_GGTT_TOP);
- size = ggtt->vm.total - GUC_GGTT_TOP;
+ GEM_BUG_ON(ggtt->vm.total <= GUC_TOP_RESERVE_SIZE);
+ offset = ggtt->vm.total - GUC_TOP_RESERVE_SIZE;
- ret = i915_gem_gtt_reserve(>vm, NULL, >uc_fw, size,
- GUC_GGTT_TOP, I915_COLOR_UNEVICTABLE,
- PIN_NOEVICT);
+ ret = i915_gem_gtt_reserve(>vm, NULL, >uc_fw,
+ GUC_TOP_RESERVE_SIZE, offset,
+ I915_COLOR_UNEVICTABLE, PIN_NOEVICT);
if (ret)
drm_dbg(>vm.i915->drm,
"Failed to reserve top of GGTT for GuC\n");
Patches currently in stable-queue which might be from de...@otheo.eu are
queue-6.5/drm-i915-gt-fix-reservation-address-in-ggtt_reserve_guc_top.patch
[Intel-gfx] Patch "drm/i915/gt: Fix reservation address in ggtt_reserve_guc_top" has been added to the 6.1-stable tree
This is a note to let you know that I've just added the patch titled
drm/i915/gt: Fix reservation address in ggtt_reserve_guc_top
to the 6.1-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
drm-i915-gt-fix-reservation-address-in-ggtt_reserve_guc_top.patch
and it can be found in the queue-6.1 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let know about it.
>From b7599d241778d0b10cdf7a5c755aa7db9b83250c Mon Sep 17 00:00:00 2001
From: Javier Pello
Date: Sat, 2 Sep 2023 17:10:39 +0200
Subject: drm/i915/gt: Fix reservation address in ggtt_reserve_guc_top
From: Javier Pello
commit b7599d241778d0b10cdf7a5c755aa7db9b83250c upstream.
There is an assertion in ggtt_reserve_guc_top that the global GTT
is of size at least GUC_GGTT_TOP, which is not the case on a 32-bit
platform; see commit 562d55d991b39ce376c492df2f7890fd6a541ffc
("drm/i915/bdw: Only use 2g GGTT for 32b platforms"). If GEM_BUG_ON
is enabled, this triggers a BUG(); if GEM_BUG_ON is disabled, the
subsequent reservation fails and the driver fails to initialise
the device:
i915 :00:02.0: [drm:i915_init_ggtt [i915]] Failed to reserve top of GGTT
for GuC
i915 :00:02.0: Device initialization failed (-28)
i915 :00:02.0: Please file a bug on drm/i915; see
https://gitlab.freedesktop.org/drm/intel/-/wikis/How-to-file-i915-bugs for
details.
i915: probe of :00:02.0 failed with error -28
Make the reservation at the top of the available space, whatever
that is, instead of assuming that the top will be GUC_GGTT_TOP.
Fixes: 911800765ef6 ("drm/i915/uc: Reserve upper range of GGTT")
Link: https://gitlab.freedesktop.org/drm/intel/-/issues/9080
Signed-off-by: Javier Pello
Reviewed-by: Daniele Ceraolo Spurio
Cc: Fernando Pacheco
Cc: Chris Wilson
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: intel-gfx@lists.freedesktop.org
Cc: sta...@vger.kernel.org # v5.3+
Signed-off-by: John Harrison
Link:
https://patchwork.freedesktop.org/patch/msgid/20230902171039.2229126186d697dbcf62d...@otheo.eu
(cherry picked from commit 0f3fa942d91165c2702577e9274d2ee1c7212afc)
Signed-off-by: Rodrigo Vivi
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/i915/gt/intel_ggtt.c | 23 +--
1 file changed, 17 insertions(+), 6 deletions(-)
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -500,20 +500,31 @@ void intel_ggtt_unbind_vma(struct i915_a
vm->clear_range(vm, vma_res->start, vma_res->vma_size);
}
+/*
+ * Reserve the top of the GuC address space for firmware images. Addresses
+ * beyond GUC_GGTT_TOP in the GuC address space are inaccessible by GuC,
+ * which makes for a suitable range to hold GuC/HuC firmware images if the
+ * size of the GGTT is 4G. However, on a 32-bit platform the size of the GGTT
+ * is limited to 2G, which is less than GUC_GGTT_TOP, but we reserve a chunk
+ * of the same size anyway, which is far more than needed, to keep the logic
+ * in uc_fw_ggtt_offset() simple.
+ */
+#define GUC_TOP_RESERVE_SIZE (SZ_4G - GUC_GGTT_TOP)
+
static int ggtt_reserve_guc_top(struct i915_ggtt *ggtt)
{
- u64 size;
+ u64 offset;
int ret;
if (!intel_uc_uses_guc(>vm.gt->uc))
return 0;
- GEM_BUG_ON(ggtt->vm.total <= GUC_GGTT_TOP);
- size = ggtt->vm.total - GUC_GGTT_TOP;
+ GEM_BUG_ON(ggtt->vm.total <= GUC_TOP_RESERVE_SIZE);
+ offset = ggtt->vm.total - GUC_TOP_RESERVE_SIZE;
- ret = i915_gem_gtt_reserve(>vm, NULL, >uc_fw, size,
- GUC_GGTT_TOP, I915_COLOR_UNEVICTABLE,
- PIN_NOEVICT);
+ ret = i915_gem_gtt_reserve(>vm, NULL, >uc_fw,
+ GUC_TOP_RESERVE_SIZE, offset,
+ I915_COLOR_UNEVICTABLE, PIN_NOEVICT);
if (ret)
drm_dbg(>vm.i915->drm,
"Failed to reserve top of GGTT for GuC\n");
Patches currently in stable-queue which might be from de...@otheo.eu are
queue-6.1/drm-i915-gt-fix-reservation-address-in-ggtt_reserve_guc_top.patch
Re: [Intel-gfx] [PATCH] drm/i915/gt: Fix reservation address in ggtt_reserve_guc_top
On Thu, 31 Aug 2023 15:49:28 -0700
"Ceraolo Spurio, Daniele" wrote:
> On 8/25/2023 7:33 AM, Javier Pello wrote:
> > There is an assertion in ggtt_reserve_guc_top that the global GTT
> > is of size at least GUC_GGTT_TOP, which is not the case on a 32-bit
> > platform; see commit 562d55d991b39ce376c492df2f7890fd6a541ffc
> > ("drm/i915/bdw: Only use 2g GGTT for 32b platforms"). If GEM_BUG_ON
> > is enabled, this triggers a BUG(); if GEM_BUG_ON is disabled, the
> > subsequent reservation fails and the driver fails to initialise
> > the device:
> >
> > i915 :00:02.0: [drm:i915_init_ggtt [i915]] Failed to reserve top of
> > GGTT for GuC
> > i915 :00:02.0: Device initialization failed (-28)
> > i915 :00:02.0: Please file a bug on drm/i915; see
> > https://gitlab.freedesktop.org/drm/intel/-/wikis/How-to-file-i915-bugs for
> > details.
> > i915: probe of :00:02.0 failed with error -28
> >
> > Make the reservation at the top of the available space, whatever
> > that is, instead of assuming that the top will be GUC_GGTT_TOP.
> >
> > Fixes: 911800765ef6 ("drm/i915/uc: Reserve upper range of GGTT")
>
> For tracking, it might be good to also add:
>
> Link: https://gitlab.freedesktop.org/drm/intel/-/issues/9080
Sure.
> > Signed-off-by: Javier Pello
> > Cc: intel-gfx@lists.freedesktop.org
> > Cc: sta...@vger.kernel.org # v5.3+
>
> Need the full CC list here, so that when the patch gets back-ported the
> relevant developers get automatically added.
I was unsure if I should add them from the start or wait for a review.
> > ---
> > drivers/gpu/drm/i915/gt/intel_ggtt.c | 21 +++--
> > 1 file changed, 15 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > index e9328e1a..0157bebb 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > @@ -511,20 +511,29 @@ void intel_ggtt_unbind_vma(struct i915_address_space
> > *vm,
> > vm->clear_range(vm, vma_res->start, vma_res->vma_size);
> > }
> >
> > +/* Reserve the top of the GuC address space for firmware images. Addresses
> > + * beyond GUC_GGTT_TOP in the GuC address space are inaccessible by GuC,
> > + * which makes for a suitable range to hold GuC/HuC firmware images if the
> > + * size of the GGTT is 4G. However, on a 32-bit platform the size of the
> > GGTT
> > + * is limited to 2G, which is less than GUC_GGTT_TOP, but we reserve a
> > chunk
> > + * of the same size anyway, which is far more than needed, to keep the
> > logic
> > + * in uc_fw_ggtt_offset() simple. */
>
> Style: multi-line comment should be formatted as:
>
> /*
> * Text
> * more text
> */
Yes, I realised that after I sent the patch, but I thought I would
wait for replies rather than resending with only a style change.
> > +#define GUC_TOP_RESERVE_SIZE (SZ_4G - GUC_GGTT_TOP)
> > +
> > static int ggtt_reserve_guc_top(struct i915_ggtt *ggtt)
> > {
> > - u64 size;
> > + u64 offset;
> > int ret;
> >
> > if (!intel_uc_uses_guc(>vm.gt->uc))
> > return 0;
> >
> > - GEM_BUG_ON(ggtt->vm.total <= GUC_GGTT_TOP);
> > - size = ggtt->vm.total - GUC_GGTT_TOP;
> > + GEM_BUG_ON(ggtt->vm.total <= GUC_TOP_RESERVE_SIZE);
> > + offset = ggtt->vm.total - GUC_TOP_RESERVE_SIZE;
> >
> > - ret = i915_gem_gtt_reserve(>vm, NULL, >uc_fw, size,
> > - GUC_GGTT_TOP, I915_COLOR_UNEVICTABLE,
> > - PIN_NOEVICT);
> > + ret = i915_gem_gtt_reserve(>vm, NULL, >uc_fw,
> > + GUC_TOP_RESERVE_SIZE, offset,
> > + I915_COLOR_UNEVICTABLE, PIN_NOEVICT);
>
> The code change looks good to me, so with the style fix and the
> additions to the commit message this is:
>
> Reviewed-by: Daniele Ceraolo Spurio
Thank you.
Javier
Re: [Intel-gfx] [PATCH] drm/i915/gt: Fix reservation address in ggtt_reserve_guc_top
On 8/25/2023 7:33 AM, Javier Pello wrote:
There is an assertion in ggtt_reserve_guc_top that the global GTT
is of size at least GUC_GGTT_TOP, which is not the case on a 32-bit
platform; see commit 562d55d991b39ce376c492df2f7890fd6a541ffc
("drm/i915/bdw: Only use 2g GGTT for 32b platforms"). If GEM_BUG_ON
is enabled, this triggers a BUG(); if GEM_BUG_ON is disabled, the
subsequent reservation fails and the driver fails to initialise
the device:
i915 :00:02.0: [drm:i915_init_ggtt [i915]] Failed to reserve top of GGTT
for GuC
i915 :00:02.0: Device initialization failed (-28)
i915 :00:02.0: Please file a bug on drm/i915; see
https://gitlab.freedesktop.org/drm/intel/-/wikis/How-to-file-i915-bugs for
details.
i915: probe of :00:02.0 failed with error -28
Make the reservation at the top of the available space, whatever
that is, instead of assuming that the top will be GUC_GGTT_TOP.
Fixes: 911800765ef6 ("drm/i915/uc: Reserve upper range of GGTT")
For tracking, it might be good to also add:
Link: https://gitlab.freedesktop.org/drm/intel/-/issues/9080
Signed-off-by: Javier Pello
Cc: intel-gfx@lists.freedesktop.org
Cc: sta...@vger.kernel.org # v5.3+
Need the full CC list here, so that when the patch gets back-ported the
relevant developers get automatically added.
---
drivers/gpu/drm/i915/gt/intel_ggtt.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index e9328e1a..0157bebb 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -511,20 +511,29 @@ void intel_ggtt_unbind_vma(struct i915_address_space *vm,
vm->clear_range(vm, vma_res->start, vma_res->vma_size);
}
+/* Reserve the top of the GuC address space for firmware images. Addresses
+ * beyond GUC_GGTT_TOP in the GuC address space are inaccessible by GuC,
+ * which makes for a suitable range to hold GuC/HuC firmware images if the
+ * size of the GGTT is 4G. However, on a 32-bit platform the size of the GGTT
+ * is limited to 2G, which is less than GUC_GGTT_TOP, but we reserve a chunk
+ * of the same size anyway, which is far more than needed, to keep the logic
+ * in uc_fw_ggtt_offset() simple. */
Style: multi-line comment should be formatted as:
/*
* Text
* more text
*/
+#define GUC_TOP_RESERVE_SIZE (SZ_4G - GUC_GGTT_TOP)
+
static int ggtt_reserve_guc_top(struct i915_ggtt *ggtt)
{
- u64 size;
+ u64 offset;
int ret;
if (!intel_uc_uses_guc(>vm.gt->uc))
return 0;
- GEM_BUG_ON(ggtt->vm.total <= GUC_GGTT_TOP);
- size = ggtt->vm.total - GUC_GGTT_TOP;
+ GEM_BUG_ON(ggtt->vm.total <= GUC_TOP_RESERVE_SIZE);
+ offset = ggtt->vm.total - GUC_TOP_RESERVE_SIZE;
- ret = i915_gem_gtt_reserve(>vm, NULL, >uc_fw, size,
- GUC_GGTT_TOP, I915_COLOR_UNEVICTABLE,
- PIN_NOEVICT);
+ ret = i915_gem_gtt_reserve(>vm, NULL, >uc_fw,
+ GUC_TOP_RESERVE_SIZE, offset,
+ I915_COLOR_UNEVICTABLE, PIN_NOEVICT);
The code change looks good to me, so with the style fix and the
additions to the commit message this is:
Reviewed-by: Daniele Ceraolo Spurio
Daniele
if (ret)
drm_dbg(>vm.i915->drm,
"Failed to reserve top of GGTT for GuC\n");
[Intel-gfx] [PATCH] drm/i915/gt: Fix reservation address in ggtt_reserve_guc_top
There is an assertion in ggtt_reserve_guc_top that the global GTT
is of size at least GUC_GGTT_TOP, which is not the case on a 32-bit
platform; see commit 562d55d991b39ce376c492df2f7890fd6a541ffc
("drm/i915/bdw: Only use 2g GGTT for 32b platforms"). If GEM_BUG_ON
is enabled, this triggers a BUG(); if GEM_BUG_ON is disabled, the
subsequent reservation fails and the driver fails to initialise
the device:
i915 :00:02.0: [drm:i915_init_ggtt [i915]] Failed to reserve top of GGTT
for GuC
i915 :00:02.0: Device initialization failed (-28)
i915 :00:02.0: Please file a bug on drm/i915; see
https://gitlab.freedesktop.org/drm/intel/-/wikis/How-to-file-i915-bugs for
details.
i915: probe of :00:02.0 failed with error -28
Make the reservation at the top of the available space, whatever
that is, instead of assuming that the top will be GUC_GGTT_TOP.
Fixes: 911800765ef6 ("drm/i915/uc: Reserve upper range of GGTT")
Signed-off-by: Javier Pello
Cc: intel-gfx@lists.freedesktop.org
Cc: sta...@vger.kernel.org # v5.3+
---
drivers/gpu/drm/i915/gt/intel_ggtt.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index e9328e1a..0157bebb 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -511,20 +511,29 @@ void intel_ggtt_unbind_vma(struct i915_address_space *vm,
vm->clear_range(vm, vma_res->start, vma_res->vma_size);
}
+/* Reserve the top of the GuC address space for firmware images. Addresses
+ * beyond GUC_GGTT_TOP in the GuC address space are inaccessible by GuC,
+ * which makes for a suitable range to hold GuC/HuC firmware images if the
+ * size of the GGTT is 4G. However, on a 32-bit platform the size of the GGTT
+ * is limited to 2G, which is less than GUC_GGTT_TOP, but we reserve a chunk
+ * of the same size anyway, which is far more than needed, to keep the logic
+ * in uc_fw_ggtt_offset() simple. */
+#define GUC_TOP_RESERVE_SIZE (SZ_4G - GUC_GGTT_TOP)
+
static int ggtt_reserve_guc_top(struct i915_ggtt *ggtt)
{
- u64 size;
+ u64 offset;
int ret;
if (!intel_uc_uses_guc(>vm.gt->uc))
return 0;
- GEM_BUG_ON(ggtt->vm.total <= GUC_GGTT_TOP);
- size = ggtt->vm.total - GUC_GGTT_TOP;
+ GEM_BUG_ON(ggtt->vm.total <= GUC_TOP_RESERVE_SIZE);
+ offset = ggtt->vm.total - GUC_TOP_RESERVE_SIZE;
- ret = i915_gem_gtt_reserve(>vm, NULL, >uc_fw, size,
- GUC_GGTT_TOP, I915_COLOR_UNEVICTABLE,
- PIN_NOEVICT);
+ ret = i915_gem_gtt_reserve(>vm, NULL, >uc_fw,
+ GUC_TOP_RESERVE_SIZE, offset,
+ I915_COLOR_UNEVICTABLE, PIN_NOEVICT);
if (ret)
drm_dbg(>vm.i915->drm,
"Failed to reserve top of GGTT for GuC\n");
--
2.41.0
