Re: [PATCH 17/32] drm/amdkfd: add raise exception event function

[Felix Kuehling <felix.kuehling@amd.com>]

On 2023-01-25 14:53, Jonathan Kim wrote:
> Exception events can be generated from interrupts or queue activitity.
>
> The raise event function will save exception status of a queue, device
> or process then notify the debugger of the status change by writing to
> a debugger polled file descriptor that the debugger provides during
> debug attach.
>
> For memory violation exceptions, extra exception data will be saved.
>
> The debugger will be able to query the saved exception states by query
> operation that will be provided by follow up patches.
>
> Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_debug.c | 91 +++++++++++++++++++++++++-
>   drivers/gpu/drm/amd/amdkfd/kfd_debug.h |  5 ++
>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h  |  7 ++
>   3 files changed, 102 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> index 659dfc7411fe..fcd064b13f6a 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> @@ -38,6 +38,93 @@ void debug_event_write_work_handler(struct work_struct *work)
>   	kernel_write(process->dbg_ev_file, &write_data, 1, &pos);
>   }
>   
> +/* update process/device/queue exception status, write to descriptor
> + * only if exception_status is enabled.
> + */
> +bool kfd_dbg_ev_raise(uint64_t event_mask,
> +			struct kfd_process *process, struct kfd_dev *dev,
> +			unsigned int source_id, bool use_worker,
> +			void *exception_data, size_t exception_data_size)
> +{
> +	struct process_queue_manager *pqm;
> +	struct process_queue_node *pqn;
> +	int i;
> +	static const char write_data = '.';
> +	loff_t pos = 0;
> +	bool is_subscribed = true;
> +
> +	if (!(process && process->debug_trap_enabled))
> +		return false;
> +
> +	mutex_lock(&process->event_mutex);
> +
> +	if (event_mask & KFD_EC_MASK_DEVICE) {
> +		for (i = 0; i < process->n_pdds; i++) {
> +			struct kfd_process_device *pdd = process->pdds[i];
> +
> +			if (pdd->dev != dev)
> +				continue;
> +
> +			pdd->exception_status |= event_mask & KFD_EC_MASK_DEVICE;
> +
> +			if (event_mask & KFD_EC_MASK(EC_DEVICE_MEMORY_VIOLATION)) {
> +				if (!pdd->vm_fault_exc_data) {
> +					pdd->vm_fault_exc_data = kmemdup(
> +							exception_data,
> +							exception_data_size,
> +							GFP_KERNEL);
> +					if (!pdd->vm_fault_exc_data)
> +						pr_debug("Failed to allocate exception data memory");
> +				} else {
> +					pr_debug("Debugger exception data not saved\n");
> +					print_hex_dump_bytes("exception data: ",
> +							DUMP_PREFIX_OFFSET,
> +							exception_data,
> +							exception_data_size);
> +				}
> +			}
> +			break;
> +		}
> +	} else if (event_mask & KFD_EC_MASK_PROCESS) {
> +		process->exception_status |= event_mask & KFD_EC_MASK_PROCESS;
> +	} else {
> +		pqm = &process->pqm;
> +		list_for_each_entry(pqn, &pqm->queues,
> +				process_queue_list) {
> +			int target_id;
> +
> +			if (!pqn->q)
> +				continue;
> +
> +			target_id = event_mask & KFD_EC_MASK(EC_QUEUE_NEW) ?
> +					pqn->q->properties.queue_id :
> +							pqn->q->doorbell_id;
> +
> +			if (pqn->q->device != dev || target_id != source_id)
> +				continue;
> +
> +			pqn->q->properties.exception_status |= event_mask;
> +			break;
> +		}
> +	}
> +
> +	if (process->exception_enable_mask & event_mask) {
> +		if (use_worker)
> +			schedule_work(&process->debug_event_workarea);

The worker definition should be in the same patch.


> +		else
> +			kernel_write(process->dbg_ev_file,
> +					&write_data,
> +					1,
> +					&pos);
> +	} else {
> +		is_subscribed = false;
> +	}
> +
> +	mutex_unlock(&process->event_mutex);
> +
> +	return is_subscribed;
> +}
> +
>   static int kfd_dbg_set_queue_workaround(struct queue *q, bool enable)
>   {
>   	struct mqd_update_info minfo = {0};
> @@ -88,7 +175,6 @@ static int kfd_dbg_set_workaround(struct kfd_process *target, bool enable)
>   	}
>   
>   	return r;
> -}

This looks wrong.

Regards,
   Felix


>   
>   static int kfd_dbg_set_mes_debug_mode(struct kfd_process_device *pdd)
>   {
> @@ -114,6 +200,9 @@ static void kfd_dbg_trap_deactivate(struct kfd_process *target, bool unwind, int
>   {
>   	int i, count = 0;
>   
> +	if (!unwind)
> +		cancel_work_sync(&target->debug_event_workarea);
> +
>   	for (i = 0; i < target->n_pdds; i++) {
>   		struct kfd_process_device *pdd = target->pdds[i];
>   
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.h b/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
> index f199698d8d60..2d5bc102f6b4 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
> @@ -28,6 +28,11 @@
>   void kgd_gfx_v9_set_wave_launch_stall(struct amdgpu_device *adev,
>   					uint32_t vmid,
>   					bool stall);
> +bool kfd_dbg_ev_raise(uint64_t event_mask,
> +			struct kfd_process *process, struct kfd_dev *dev,
> +			unsigned int source_id, bool use_worker,
> +			void *exception_data,
> +			size_t exception_data_size);
>   int kfd_dbg_trap_disable(struct kfd_process *target);
>   int kfd_dbg_trap_enable(struct kfd_process *target, uint32_t fd,
>   			void __user *runtime_info,
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 75521d96e937..e503bd94dda6 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -495,6 +495,7 @@ struct queue_properties {
>   	uint32_t ctl_stack_size;
>   	uint64_t tba_addr;
>   	uint64_t tma_addr;
> +	uint64_t exception_status;
>   };
>   
>   #define QUEUE_IS_ACTIVE(q) ((q).queue_size > 0 &&	\
> @@ -786,6 +787,11 @@ struct kfd_process_device {
>   	uint64_t page_in;
>   	uint64_t page_out;
>   
> +	/* Exception code status*/
> +	uint64_t exception_status;
> +	void *vm_fault_exc_data;
> +	size_t vm_fault_exc_data_size;
> +
>   	/* Tracks debug per-vmid request settings */
>   	uint32_t spi_dbg_override;
>   	uint32_t spi_dbg_launch_mode;
> @@ -921,6 +927,7 @@ struct kfd_process {
>   
>   	/* Exception code enable mask and status */
>   	uint64_t exception_enable_mask;
> +	uint64_t exception_status;
>   
>   	/* shared virtual memory registered by this process */
>   	struct svm_range_list svms;