| Message ID | 20240313193907.95205-1-andi.shyti@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/gt: Report full vm address range | expand |
Commit 9bb66c179f50 ("drm/i915: Reserve some kernel space per
vm") has reserved an object for kernel space usage.
Userspace, though, needs to know the full address range.
Fixes: 9bb66c179f50 ("drm/i915: Reserve some kernel space per vm")
Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: Chris Wilson <chris.p.wilson@linux.intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Michal Mrozek <michal.mrozek@intel.com>
Cc: Nirmoy Das <nirmoy.das@intel.com>
Cc: <stable@vger.kernel.org> # v6.2+
---
drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
index fa46d2308b0e..d76831f50106 100644
--- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
@@ -982,8 +982,9 @@ static int gen8_init_rsvd(struct i915_address_space *vm)
vm->rsvd.vma = i915_vma_make_unshrinkable(vma);
vm->rsvd.obj = obj;
- vm->total -= vma->node.size;
+
return 0;
+
unref:
i915_gem_object_put(obj);
return ret;
Hi Andi, In Mesa we've been relying on I915_CONTEXT_PARAM_GTT_SIZE so as long as that is adjusted by the kernel, we should be able to continue working without issues. Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Thanks, -Lionel On 13/03/2024 21:39, Andi Shyti wrote: > Commit 9bb66c179f50 ("drm/i915: Reserve some kernel space per > vm") has reserved an object for kernel space usage. > > Userspace, though, needs to know the full address range. > > Fixes: 9bb66c179f50 ("drm/i915: Reserve some kernel space per vm") > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com> > Cc: Andrzej Hajda <andrzej.hajda@intel.com> > Cc: Chris Wilson <chris.p.wilson@linux.intel.com> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Cc: Michal Mrozek <michal.mrozek@intel.com> > Cc: Nirmoy Das <nirmoy.das@intel.com> > Cc: <stable@vger.kernel.org> # v6.2+ > --- > drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c > index fa46d2308b0e..d76831f50106 100644 > --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c > +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c > @@ -982,8 +982,9 @@ static int gen8_init_rsvd(struct i915_address_space *vm) > > vm->rsvd.vma = i915_vma_make_unshrinkable(vma); > vm->rsvd.obj = obj; > - vm->total -= vma->node.size; > + > return 0; > + > unref: > i915_gem_object_put(obj); > return ret;
On 3/14/2024 3:04 PM, Lionel Landwerlin wrote: > Hi Andi, > > In Mesa we've been relying on I915_CONTEXT_PARAM_GTT_SIZE so as long > as that is adjusted by the kernel What do you mean by adjusted by, should it be a aligned size? I915_CONTEXT_PARAM_GTT_SIZE ioctl is returning vm->total which is adjusted(reduced by a page). This patch might cause silent error as it is not removing WABB which is using the reserved page to add dummy blt and if userspace is using that page then it will be overwritten. Regards, Nirmoy > , we should be able to continue working without issues. > > Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > > Thanks, > > -Lionel > > On 13/03/2024 21:39, Andi Shyti wrote: >> Commit 9bb66c179f50 ("drm/i915: Reserve some kernel space per >> vm") has reserved an object for kernel space usage. >> >> Userspace, though, needs to know the full address range. >> >> Fixes: 9bb66c179f50 ("drm/i915: Reserve some kernel space per vm") >> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com> >> Cc: Andrzej Hajda <andrzej.hajda@intel.com> >> Cc: Chris Wilson <chris.p.wilson@linux.intel.com> >> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >> Cc: Michal Mrozek <michal.mrozek@intel.com> >> Cc: Nirmoy Das <nirmoy.das@intel.com> >> Cc: <stable@vger.kernel.org> # v6.2+ >> --- >> drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c >> b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c >> index fa46d2308b0e..d76831f50106 100644 >> --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c >> +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c >> @@ -982,8 +982,9 @@ static int gen8_init_rsvd(struct >> i915_address_space *vm) >> vm->rsvd.vma = i915_vma_make_unshrinkable(vma); >> vm->rsvd.obj = obj; >> - vm->total -= vma->node.size; >> + >> return 0; >> + >> unref: >> i915_gem_object_put(obj); >> return ret; > >
Hi Nirmoy, > > In Mesa we've been relying on I915_CONTEXT_PARAM_GTT_SIZE so as long as > > that is adjusted by the kernel > > What do you mean by adjusted by, should it be a aligned size? > > I915_CONTEXT_PARAM_GTT_SIZE ioctl is returning vm->total which is > adjusted(reduced by a page). > > This patch might cause silent error as it is not removing WABB which is > using the reserved page to add dummy blt and if userspace is using that > > page then it will be overwritten. yes, I think this could happen, but there is no solution, unfortunately. We need to fail at some point. On the other hand, I think mesa is miscalculating the vm size. In userspace the total size is derived by the bit size (maxNBitValue()). By doing so, I guess there will always be cases of miscalculation. There are two solutions here: 1. we track two sizes, one the true available size and one the total size. But this looks like a dirty hack to me. 2. UMD fixes the size calculation by taking for granted what the driver provides and we don't have anything to do in KMD. Lionel, Michal, thoughts? Andi
> > Lionel, Michal, thoughts?
Compute UMD needs to know exact GTT total size.
Hi Michal, On Mon, Mar 18, 2024 at 05:21:54AM +0000, Mrozek, Michal wrote: > > > Lionel, Michal, thoughts? > Compute UMD needs to know exact GTT total size. the problem is that we cannot apply the workaround without reserving one page from the GTT total size and we need to apply the workaround. If we provide the total GTT size we will have one page that will be contended between kernel and userspace and, if userspace is unaware that the page belongs to the kernel, we might step on each other toe. The ask here from kernel side is to relax the check on the maxNBitValue() in userspace and take what the kernel provides. Thanks, Andi
> If we provide the total GTT size we will have one page that will be contended between kernel and userspace and, if userspace is unaware that the page belongs to the > kernel, we might step on each other toe.
That's fine, Compute needs to know total GTT size.
Not available GTT size.
diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c index fa46d2308b0e..d76831f50106 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c @@ -982,8 +982,9 @@ static int gen8_init_rsvd(struct i915_address_space *vm) vm->rsvd.vma = i915_vma_make_unshrinkable(vma); vm->rsvd.obj = obj; - vm->total -= vma->node.size; + return 0; + unref: i915_gem_object_put(obj); return ret;
Commit 9bb66c179f50 ("drm/i915: Reserve some kernel space per vm") has reserved an object for kernel space usage. Userspace, though, needs to know the full address range. Fixes: 9bb66c179f50 ("drm/i915: Reserve some kernel space per vm") Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com> Cc: Andrzej Hajda <andrzej.hajda@intel.com> Cc: Chris Wilson <chris.p.wilson@linux.intel.com> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Cc: Michal Mrozek <michal.mrozek@intel.com> Cc: Nirmoy Das <nirmoy.das@intel.com> Cc: <stable@vger.kernel.org> # v6.2+ --- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)