[PATCH v2 6/6] drm/xe: Add VM bind IOCTL error injection
Cavitt, Jonathan
jonathan.cavitt at intel.com
Wed May 15 21:29:18 UTC 2024
-----Original Message-----
From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of Matthew Brost
Sent: Tuesday, May 14, 2024 5:40 PM
To: intel-xe at lists.freedesktop.org
Cc: Brost, Matthew <matthew.brost at intel.com>
Subject: [PATCH v2 6/6] drm/xe: Add VM bind IOCTL error injection
>
> Add VM bind IOCTL error injection which steals MSB of the bind flags
> field which if set injects errors at various points in the VM bind
> IOCTL. Intended to validate error paths. Enabled by CONFIG_DRM_XE_DEBUG.
>
> Signed-off-by: Matthew Brost <matthew.brost at intel.com>
I have a minor nit lower down, but nothing worth blocking over:
Reviewed-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
> ---
> drivers/gpu/drm/xe/xe_device_types.h | 12 ++++++++++++
> drivers/gpu/drm/xe/xe_pt.c | 12 ++++++++++++
> drivers/gpu/drm/xe/xe_vm.c | 24 ++++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_vm_types.h | 14 ++++++++++++++
> 4 files changed, 62 insertions(+)
>
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index 0af739981ebf..6c8c55e9addd 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -22,6 +22,10 @@
> #include "xe_sriov_types.h"
> #include "xe_step_types.h"
>
> +#if IS_ENABLED(CONFIG_DRM_XE_DEBUG)
> +#define TEST_VM_OPS_ERROR
> +#endif
> +
> #if IS_ENABLED(CONFIG_DRM_XE_DISPLAY)
> #include "soc/intel_pch.h"
> #include "intel_display_core.h"
> @@ -471,6 +475,14 @@ struct xe_device {
> int mode;
> } wedged;
>
> +#ifdef TEST_VM_OPS_ERROR
> + /**
> + * @vm_inject_error_position: inject errors at different places in VM
> + * bind IOCTL based on this value
> + */
> + u8 vm_inject_error_position;
> +#endif
> +
> /* private: */
>
> #if IS_ENABLED(CONFIG_DRM_XE_DISPLAY)
> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> index 7f792b7feb06..0907aeaa25ed 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
> @@ -1865,6 +1865,12 @@ int xe_pt_update_ops_prepare(struct xe_tile *tile, struct xe_vma_ops *vops)
> xe_tile_assert(tile, pt_update_ops->current_op <=
> pt_update_ops->num_ops);
>
> +#ifdef TEST_VM_OPS_ERROR
> + if (vops->inject_error &&
> + vops->vm->xe->vm_inject_error_position == FORCE_OP_ERROR_PREPARE)
> + return -ENOSPC;
> +#endif
> +
> return 0;
> }
>
> @@ -2005,6 +2011,12 @@ xe_pt_update_ops_run(struct xe_tile *tile, struct xe_vma_ops *vops)
> return dma_fence_get_stub();
> }
>
> +#ifdef TEST_VM_OPS_ERROR
> + if (vops->inject_error &&
> + vm->xe->vm_inject_error_position == FORCE_OP_ERROR_RUN)
> + return ERR_PTR(-ENOSPC);
> +#endif
> +
> if (pt_update_ops->needs_invalidation) {
> ifence = kzalloc(sizeof(*ifence), GFP_KERNEL);
> if (!ifence) {
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 7fa533902bda..2bac8539d024 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -2467,6 +2467,12 @@ static int vm_bind_ioctl_ops_lock_and_prep(struct drm_exec *exec,
> return err;
> }
>
> +#ifdef TEST_VM_OPS_ERROR
> + if (vops->inject_error &&
> + vm->xe->vm_inject_error_position == FORCE_OP_ERROR_LOCK)
> + return -ENOSPC;
> +#endif
> +
> return 0;
> }
>
> @@ -2703,11 +2709,20 @@ static int vm_bind_ioctl_ops_execute(struct xe_vm *vm,
> return err;
> }
>
> +#ifdef TEST_VM_OPS_ERROR
> +#define SUPPORTED_FLAGS \
> + (FORCE_OP_ERROR | \
> + DRM_XE_VM_BIND_FLAG_READONLY | \
> + DRM_XE_VM_BIND_FLAG_IMMEDIATE | \
> + DRM_XE_VM_BIND_FLAG_NULL | \
> + DRM_XE_VM_BIND_FLAG_DUMPABLE)
> +#else
> #define SUPPORTED_FLAGS \
> (DRM_XE_VM_BIND_FLAG_READONLY | \
> DRM_XE_VM_BIND_FLAG_IMMEDIATE | \
> DRM_XE_VM_BIND_FLAG_NULL | \
> DRM_XE_VM_BIND_FLAG_DUMPABLE)
> +#endif
I'm aware this is basically the only correct way of doing this,
but the fact that both instances of SUPPORTED_FLAGS are
the same save for the presence or absence of
FORCE_OP_ERROR rubs me the wrong way.
I know we can't just "append" to SUPPORTED_FLAGS, but
maybe we can use a stub? Something like:
#define SUPPORTED_FLAGS_STUB \
(DRM_XE_VM_BIND_FLAG_READONLY | \
DRM_XE_VM_BIND_FLAG_IMMEDIATE | \
DRM_XE_VM_BIND_FLAG_NULL | \
DRM_XE_VM_BIND_FLAG_DUMPABLE)
#ifdef TEST_VM_OPS_ERROR
#define SUPPORTED_FLAGS \
(SUPPORTED_FLAGS_STUB)
#else
#define SUPPORTED_FLAGS \
(SUPPORTED_FLAGS_STUB |\
FORCE_OP_ERROR)
#endif
Feel free to disregard this, it's just a suggestion.
-Jonathan Cavitt
> #define XE_64K_PAGE_MASK 0xffffull
> #define ALL_DRM_XE_SYNCS_FLAGS (DRM_XE_SYNCS_FLAG_WAIT_FOR_OP)
>
> @@ -3055,6 +3070,15 @@ int xe_vm_bind_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> err = vm_bind_ioctl_ops_parse(vm, ops[i], &vops);
> if (err)
> goto unwind_ops;
> +
> +#ifdef TEST_VM_OPS_ERROR
> + if (flags & FORCE_OP_ERROR) {
> + vops.inject_error = true;
> + vm->xe->vm_inject_error_position =
> + (vm->xe->vm_inject_error_position + 1) %
> + FORCE_OP_ERROR_COUNT;
> + }
> +#endif
> }
>
> /* Nothing to do */
> diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h
> index 39bfa259c7be..2ba7d9ad5ba6 100644
> --- a/drivers/gpu/drm/xe/xe_vm_types.h
> +++ b/drivers/gpu/drm/xe/xe_vm_types.h
> @@ -23,6 +23,16 @@ struct xe_user_fence;
> struct xe_vm;
> struct xe_vm_pgtable_update_op;
>
> +#if IS_ENABLED(CONFIG_DRM_XE_DEBUG)
> +#define TEST_VM_OPS_ERROR
> +#define FORCE_OP_ERROR BIT(31)
> +
> +#define FORCE_OP_ERROR_LOCK 0
> +#define FORCE_OP_ERROR_PREPARE 1
> +#define FORCE_OP_ERROR_RUN 2
> +#define FORCE_OP_ERROR_COUNT 3
> +#endif
> +
> #define XE_VMA_READ_ONLY DRM_GPUVA_USERBITS
> #define XE_VMA_DESTROYED (DRM_GPUVA_USERBITS << 1)
> #define XE_VMA_ATOMIC_PTE_BIT (DRM_GPUVA_USERBITS << 2)
> @@ -361,6 +371,10 @@ struct xe_vma_ops {
> u32 num_syncs;
> /** @pt_update_ops: page table update operations */
> struct xe_vm_pgtable_update_ops pt_update_ops[XE_MAX_TILES_PER_DEVICE];
> +#ifdef TEST_VM_OPS_ERROR
> + /** @inject_error: inject error to test error handling */
> + bool inject_error;
> +#endif
> };
>
> #endif
> --
> 2.34.1
>
>
More information about the Intel-xe
mailing list