Re: [Intel-gfx] [PATCH 18/20] drm/i915/uapi: forbid ALLOC_TOPDOWN for error capture
On 03/02/2022 09:43, Thomas Hellström wrote:
On 1/26/22 16:21, Matthew Auld wrote:
On platforms where there might be non-mappable LMEM, force userspace to
mark the buffers with the correct hint. When dumping the BO contents
during capture we need CPU access. Note this only applies to buffers
that can be placed in LMEM, and also doesn't impact DG1.
Oddly enough this seems to break DG1. We probably need to understand why.
I think that's just because of the last HAX patch.
/Thomas
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
---
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 498b458fd784..3c8083852620 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1965,7 +1965,7 @@ eb_find_first_request_added(struct
i915_execbuffer *eb)
#if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR)
/* Stage with GFP_KERNEL allocations before we enter the signaling
critical path */
-static void eb_capture_stage(struct i915_execbuffer *eb)
+static int eb_capture_stage(struct i915_execbuffer *eb)
{
const unsigned int count = eb->buffer_count;
unsigned int i = count, j;
@@ -1978,6 +1978,9 @@ static void eb_capture_stage(struct
i915_execbuffer *eb)
if (!(flags & EXEC_OBJECT_CAPTURE))
continue;
+ if (vma->obj->flags & I915_BO_ALLOC_TOPDOWN)
+ return -EINVAL;
+
for_each_batch_create_order(eb, j) {
struct i915_capture_list *capture;
@@ -1990,6 +1993,8 @@ static void eb_capture_stage(struct
i915_execbuffer *eb)
eb->capture_lists[j] = capture;
}
}
+
+ return 0;
}
/* Commit once we're in the critical path */
@@ -3418,7 +3423,9 @@ i915_gem_do_execbuffer(struct drm_device *dev,
}
ww_acquire_done();
- eb_capture_stage();
+ err = eb_capture_stage();
+ if (err)
+ goto err_vma;
out_fence = eb_requests_create(, in_fence, out_fence_fd);
if (IS_ERR(out_fence)) {
Re: [Intel-gfx] [PATCH 18/20] drm/i915/uapi: forbid ALLOC_TOPDOWN for error capture
On 1/26/22 16:21, Matthew Auld wrote:
On platforms where there might be non-mappable LMEM, force userspace to
mark the buffers with the correct hint. When dumping the BO contents
during capture we need CPU access. Note this only applies to buffers
that can be placed in LMEM, and also doesn't impact DG1.
Oddly enough this seems to break DG1. We probably need to understand why.
/Thomas
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
---
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 498b458fd784..3c8083852620 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1965,7 +1965,7 @@ eb_find_first_request_added(struct i915_execbuffer *eb)
#if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR)
/* Stage with GFP_KERNEL allocations before we enter the signaling critical path */
-static void eb_capture_stage(struct i915_execbuffer *eb)
+static int eb_capture_stage(struct i915_execbuffer *eb)
{
const unsigned int count = eb->buffer_count;
unsigned int i = count, j;
@@ -1978,6 +1978,9 @@ static void eb_capture_stage(struct i915_execbuffer *eb)
if (!(flags & EXEC_OBJECT_CAPTURE))
continue;
+ if (vma->obj->flags & I915_BO_ALLOC_TOPDOWN)
+ return -EINVAL;
+
for_each_batch_create_order(eb, j) {
struct i915_capture_list *capture;
@@ -1990,6 +1993,8 @@ static void eb_capture_stage(struct i915_execbuffer *eb)
eb->capture_lists[j] = capture;
}
}
+
+ return 0;
}
/* Commit once we're in the critical path */
@@ -3418,7 +3423,9 @@ i915_gem_do_execbuffer(struct drm_device *dev,
}
ww_acquire_done();
- eb_capture_stage();
+ err = eb_capture_stage();
+ if (err)
+ goto err_vma;
out_fence = eb_requests_create(, in_fence, out_fence_fd);
if (IS_ERR(out_fence)) {
Re: [Intel-gfx] [PATCH 18/20] drm/i915/uapi: forbid ALLOC_TOPDOWN for error capture
Hi Matthew,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-tip/drm-tip]
[cannot apply to drm-intel/for-linux-next drm/drm-next v5.17-rc1 next-20220125]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Matthew-Auld/Initial-support-for-small-BAR-recovery/20220126-232640
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: x86_64-randconfig-a013-20220124
(https://download.01.org/0day-ci/archive/20220127/202201270346.fzrpmvzl-...@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project
2a1b7aa016c0f4b5598806205bdfbab1ea2d92c4)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/33b0a9f1f9810bd16cef89ce1e5787751583661e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Matthew-Auld/Initial-support-for-small-BAR-recovery/20220126-232640
git checkout 33b0a9f1f9810bd16cef89ce1e5787751583661e
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1
O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/i915/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:3426:6: error: assigning to
>> 'int' from incompatible type 'void'
err = eb_capture_stage();
^ ~
1 error generated.
vim +3426 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
3381
3382 if (args->flags & I915_EXEC_FENCE_OUT) {
3383 out_fence_fd = get_unused_fd_flags(O_CLOEXEC);
3384 if (out_fence_fd < 0) {
3385 err = out_fence_fd;
3386 goto err_in_fence;
3387 }
3388 }
3389
3390 err = eb_create();
3391 if (err)
3392 goto err_out_fence;
3393
3394 GEM_BUG_ON(!eb.lut_size);
3395
3396 err = eb_select_context();
3397 if (unlikely(err))
3398 goto err_destroy;
3399
3400 err = eb_select_engine();
3401 if (unlikely(err))
3402 goto err_context;
3403
3404 err = eb_lookup_vmas();
3405 if (err) {
3406 eb_release_vmas(, true);
3407 goto err_engine;
3408 }
3409
3410 i915_gem_ww_ctx_init(, true);
3411
3412 err = eb_relocate_parse();
3413 if (err) {
3414 /*
3415 * If the user expects the execobject.offset and
3416 * reloc.presumed_offset to be an exact match,
3417 * as for using NO_RELOC, then we cannot update
3418 * the execobject.offset until we have completed
3419 * relocation.
3420 */
3421 args->flags &= ~__EXEC_HAS_RELOC;
3422 goto err_vma;
3423 }
3424
3425 ww_acquire_done();
> 3426 err = eb_capture_stage();
3427 if (err)
3428 goto err_vma;
3429
3430 out_fence = eb_requests_create(, in_fence, out_fence_fd);
3431 if (IS_ERR(out_fence)) {
3432 err = PTR_ERR(out_fence);
3433 out_fence = NULL;
3434 if (eb.requests[0])
3435 goto err_request;
3436 else
3437 goto err_vma;
3438 }
3439
3440 err = eb_submit();
3441
3442 err_request:
3443 eb_requests_get();
3444 err = eb_requests_add(, err);
3445
3446 if (eb.fences)
3447 signal_fence_array(, eb.composite_fence ?
3448 eb.composite_fence :
3449 [0]->fence);
3450
3451 if (out_fence) {
3452 if (err == 0) {
3453 fd_install(out_fence_fd, out_fence->file);
3454 args->rsvd2 &= GENMASK_ULL(31, 0); /* keep
in-fence */
3455 args->rsvd2 |= (u64)out_fence_fd << 32;
3456 out_fence_fd = -1;
3457 } else {
3458 fput(out_fence->file);
3459 }
3460 }
3461
3462 if (unlikely(eb.gem_context->syncobj)) {
Re: [Intel-gfx] [PATCH 18/20] drm/i915/uapi: forbid ALLOC_TOPDOWN for error capture
Hi Matthew,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-tip/drm-tip]
[cannot apply to drm-intel/for-linux-next drm/drm-next v5.17-rc1 next-20220125]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Matthew-Auld/Initial-support-for-small-BAR-recovery/20220126-232640
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: i386-randconfig-a002-20220124
(https://download.01.org/0day-ci/archive/20220127/202201270314.twkiundm-...@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
#
https://github.com/0day-ci/linux/commit/33b0a9f1f9810bd16cef89ce1e5787751583661e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Matthew-Auld/Initial-support-for-small-BAR-recovery/20220126-232640
git checkout 33b0a9f1f9810bd16cef89ce1e5787751583661e
# save the config file to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/i915/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c: In function
'i915_gem_do_execbuffer':
>> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:3426:6: error: void value not
>> ignored as it ought to be
3426 | err = eb_capture_stage();
| ^
vim +3426 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
3381
3382 if (args->flags & I915_EXEC_FENCE_OUT) {
3383 out_fence_fd = get_unused_fd_flags(O_CLOEXEC);
3384 if (out_fence_fd < 0) {
3385 err = out_fence_fd;
3386 goto err_in_fence;
3387 }
3388 }
3389
3390 err = eb_create();
3391 if (err)
3392 goto err_out_fence;
3393
3394 GEM_BUG_ON(!eb.lut_size);
3395
3396 err = eb_select_context();
3397 if (unlikely(err))
3398 goto err_destroy;
3399
3400 err = eb_select_engine();
3401 if (unlikely(err))
3402 goto err_context;
3403
3404 err = eb_lookup_vmas();
3405 if (err) {
3406 eb_release_vmas(, true);
3407 goto err_engine;
3408 }
3409
3410 i915_gem_ww_ctx_init(, true);
3411
3412 err = eb_relocate_parse();
3413 if (err) {
3414 /*
3415 * If the user expects the execobject.offset and
3416 * reloc.presumed_offset to be an exact match,
3417 * as for using NO_RELOC, then we cannot update
3418 * the execobject.offset until we have completed
3419 * relocation.
3420 */
3421 args->flags &= ~__EXEC_HAS_RELOC;
3422 goto err_vma;
3423 }
3424
3425 ww_acquire_done();
> 3426 err = eb_capture_stage();
3427 if (err)
3428 goto err_vma;
3429
3430 out_fence = eb_requests_create(, in_fence, out_fence_fd);
3431 if (IS_ERR(out_fence)) {
3432 err = PTR_ERR(out_fence);
3433 out_fence = NULL;
3434 if (eb.requests[0])
3435 goto err_request;
3436 else
3437 goto err_vma;
3438 }
3439
3440 err = eb_submit();
3441
3442 err_request:
3443 eb_requests_get();
3444 err = eb_requests_add(, err);
3445
3446 if (eb.fences)
3447 signal_fence_array(, eb.composite_fence ?
3448 eb.composite_fence :
3449 [0]->fence);
3450
3451 if (out_fence) {
3452 if (err == 0) {
3453 fd_install(out_fence_fd, out_fence->file);
3454 args->rsvd2 &= GENMASK_ULL(31, 0); /* keep
in-fence */
3455 args->rsvd2 |= (u64)out_fence_fd << 32;
3456 out_fence_fd = -1;
3457 } else {
3458 fput(out_fence->file);
3459 }
3460 }
3461
3462 if (unlikely(eb.gem_context->syncobj)) {
3463 drm_syncobj_replace_fence(eb.gem_context->syncobj,
3464eb.composite_fence ?
3465eb.composite_fence :
3466
[Intel-gfx] [PATCH 18/20] drm/i915/uapi: forbid ALLOC_TOPDOWN for error capture
On platforms where there might be non-mappable LMEM, force userspace to
mark the buffers with the correct hint. When dumping the BO contents
during capture we need CPU access. Note this only applies to buffers
that can be placed in LMEM, and also doesn't impact DG1.
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
---
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 498b458fd784..3c8083852620 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1965,7 +1965,7 @@ eb_find_first_request_added(struct i915_execbuffer *eb)
#if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR)
/* Stage with GFP_KERNEL allocations before we enter the signaling critical
path */
-static void eb_capture_stage(struct i915_execbuffer *eb)
+static int eb_capture_stage(struct i915_execbuffer *eb)
{
const unsigned int count = eb->buffer_count;
unsigned int i = count, j;
@@ -1978,6 +1978,9 @@ static void eb_capture_stage(struct i915_execbuffer *eb)
if (!(flags & EXEC_OBJECT_CAPTURE))
continue;
+ if (vma->obj->flags & I915_BO_ALLOC_TOPDOWN)
+ return -EINVAL;
+
for_each_batch_create_order(eb, j) {
struct i915_capture_list *capture;
@@ -1990,6 +1993,8 @@ static void eb_capture_stage(struct i915_execbuffer *eb)
eb->capture_lists[j] = capture;
}
}
+
+ return 0;
}
/* Commit once we're in the critical path */
@@ -3418,7 +3423,9 @@ i915_gem_do_execbuffer(struct drm_device *dev,
}
ww_acquire_done();
- eb_capture_stage();
+ err = eb_capture_stage();
+ if (err)
+ goto err_vma;
out_fence = eb_requests_create(, in_fence, out_fence_fd);
if (IS_ERR(out_fence)) {
--
2.34.1
