On 2023-11-22 16:51, Felix Kuehling wrote:
On 2023-11-03 09:11, James Zhu wrote:
From: David Yat Sin <david.yatsin@xxxxxxx>
Enable pc sampling create.
Co-developed-by: James Zhu <James.Zhu@xxxxxxx>
Signed-off-by: James Zhu <James.Zhu@xxxxxxx>
Signed-off-by: David Yat Sin <david.yatsin@xxxxxxx>
---
drivers/gpu/drm/amd/amdkfd/kfd_pc_sampling.c | 54 +++++++++++++++++++-
drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 10 ++++
2 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_pc_sampling.c
b/drivers/gpu/drm/amd/amdkfd/kfd_pc_sampling.c
index 49fecbc7013e..f0d910ee730c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_pc_sampling.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_pc_sampling.c
@@ -97,7 +97,59 @@ static int kfd_pc_sample_stop(struct
kfd_process_device *pdd)
static int kfd_pc_sample_create(struct kfd_process_device *pdd,
struct kfd_ioctl_pc_sample_args __user *user_args)
{
- return -EINVAL;
+ struct kfd_pc_sample_info *supported_format = NULL;
+ struct kfd_pc_sample_info user_info;
+ int ret;
+ int i;
+
+ if (user_args->num_sample_info != 1)
+ return -EINVAL;
+
+ ret = copy_from_user(&user_info, (void __user *)
user_args->sample_info_ptr,
+ sizeof(struct kfd_pc_sample_info));
+ if (ret) {
+ pr_debug("Failed to copy PC sampling info from user\n");
+ return -EFAULT;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(supported_formats); i++) {
+ if (KFD_GC_VERSION(pdd->dev) == supported_formats[i].ip_version
+ && user_info.method ==
supported_formats[i].sample_info->method
+ && user_info.type == supported_formats[i].sample_info->type
+ && user_info.value <=
supported_formats[i].sample_info->value_max
+ && user_info.value >=
supported_formats[i].sample_info->value_min) {
+ supported_format =
+ (struct kfd_pc_sample_info
*)supported_formats[i].sample_info;
+ break;
+ }
+ }
+
+ if (!supported_format) {
+ pr_debug("Sampling format is not supported!");
+ return -EOPNOTSUPP;
+ }
+
+ mutex_lock(&pdd->dev->pcs_data.mutex);
+ if (pdd->dev->pcs_data.hosttrap_entry.base.use_count &&
+ memcmp(&pdd->dev->pcs_data.hosttrap_entry.base.pc_sample_info,
+ &user_info, sizeof(user_info))) {
I think you can compare structures in C. This would be more readable:
if (pdd->dev->pcs_data.hosttrap_entry.base.use_count &&
pdd->dev->pcs_data.hosttrap_entry.base.pc_sample_info != user_info) {
...
}
[JZ[ Sure
+ ret = copy_to_user((void __user *) user_args->sample_info_ptr,
+ &pdd->dev->pcs_data.hosttrap_entry.base.pc_sample_info,
+ sizeof(struct kfd_pc_sample_info));
+ mutex_unlock(&pdd->dev->pcs_data.mutex);
+ return ret ? ret : -EEXIST;
When copy_to_user fails, it returns the number of bytes not copied.
That's not a useful return value here. This should be
return ret ? -EFAULT : -EEXIST;
Also -EBUSY may be more appropriate than -EEXIST.
[JZ[ Sure
+ }
+
+ /* TODO: add trace_id return */
+
+ if (!pdd->dev->pcs_data.hosttrap_entry.base.use_count)
+ memcpy(&pdd->dev->pcs_data.hosttrap_entry.base.pc_sample_info,
+ &user_info, sizeof(user_info));
I think you can assign structures in C. Just do
pdd->dev->pcs_data.hosttrap_entry.base.pc_sample_info = user_info;
[JZ[ Sure
Regards,
Felix
+
+ pdd->dev->pcs_data.hosttrap_entry.base.use_count++;
+ mutex_unlock(&pdd->dev->pcs_data.mutex);
+
+ return 0;
}
static int kfd_pc_sample_destroy(struct kfd_process_device *pdd,
uint32_t trace_id)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 4a0b66189c67..81c925fb2952 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -256,9 +256,19 @@ struct kfd_vmid_info {
struct kfd_dev;
+struct kfd_dev_pc_sampling_data {
+ uint32_t use_count; /* Num of PC sampling sessions */
+ struct kfd_pc_sample_info pc_sample_info;
+};
+
+struct kfd_dev_pcs_hosttrap {
+ struct kfd_dev_pc_sampling_data base;
+};
+
/* Per device PC Sampling data */
struct kfd_dev_pc_sampling {
struct mutex mutex;
+ struct kfd_dev_pcs_hosttrap hosttrap_entry;
};
struct kfd_node {
