Re: [PATCH 2/2] drm/amdgpu: Add debugfs TA load/unload/invoke support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[AMD Official Use Only]




From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> on behalf of Candice Li <candice.li@xxxxxxx>
Sent: Monday, April 18, 2022 11:09 AM
To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
Cc: Li, Candice <Candice.Li@xxxxxxx>; Clements, John <John.Clements@xxxxxxx>
Subject: [PATCH 2/2] drm/amdgpu: Add debugfs TA load/unload/invoke support
 
Add debugfs support to load/unload/invoke TA in runtime.

Signed-off-by: John Clements <john.clements@xxxxxxx>
Signed-off-by: Candice Li <candice.li@xxxxxxx>
---
 drivers/gpu/drm/amd/amdgpu/Makefile         |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c |   2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp_ta.c  | 312 ++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp_ta.h  |  30 ++
 4 files changed, 345 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_psp_ta.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_psp_ta.h

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
index 7d7af43a258f83..b525f9be9326f4 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -58,7 +58,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
         amdgpu_vm_sdma.o amdgpu_discovery.o amdgpu_ras_eeprom.o amdgpu_nbio.o \
         amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \
         amdgpu_fw_attestation.o amdgpu_securedisplay.o \
-       amdgpu_eeprom.o amdgpu_mca.o
+       amdgpu_eeprom.o amdgpu_mca.o amdgpu_psp_ta.o
 
 amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index 13e4d8f9b87449..eedb12f6b8a32d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -38,6 +38,7 @@
 #include "amdgpu_umr.h"
 
 #include "amdgpu_reset.h"
+#include "amdgpu_psp_ta.h"
 
 #if defined(CONFIG_DEBUG_FS)
 
@@ -1767,6 +1768,7 @@ int amdgpu_debugfs_init(struct amdgpu_device *adev)
                 DRM_ERROR("registering register debugfs failed (%d).\n", r);
 
         amdgpu_debugfs_firmware_init(adev);
+       amdgpu_ta_if_debugfs_init(adev);
 
 #if defined(CONFIG_DRM_AMD_DC)
         if (amdgpu_device_has_dc_support(adev))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp_ta.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp_ta.c
new file mode 100644
index 00000000000000..916bf3f6fce0d4
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp_ta.c
@@ -0,0 +1,312 @@
+/*
+ * Copyright 2022 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#include "amdgpu.h"
+#include "amdgpu_psp_ta.h"
+
+static const char *TA_IF_FS_NAME = "ta_if";
+
+struct dentry *dir;
+struct dentry *ta_load_debugfs_dentry;
+struct dentry *ta_unload_debugfs_dentry;
+struct dentry *ta_invoke_debugfs_dentry;

[kevin]:

make above variable as static is better for this case, but it seems it is not used in this patch? 
if so, you will get a "defined but not used" error, when turn on build option " -Werror=unused-variable".

Please don't mind this prompt.

Best Regards,
Kevin
+
+static ssize_t ta_if_load_debugfs_read(struct file *fp, char *buf, size_t len, loff_t *off);
+static ssize_t ta_if_unload_debugfs_read(struct file *fp, char *buf, size_t len, loff_t *off);
+static ssize_t ta_if_invoke_debugfs_read(struct file *fp, char *buf, size_t len, loff_t *off);
+
+
+static uint32_t get_bin_version(const uint8_t *bin)
+{
+       const struct common_firmware_header *hdr =
+                            (const struct common_firmware_header *)bin;
+
+       return hdr->ucode_version;
+}
+
+static uint32_t get_shared_buf_size(uint32_t shared_buf_len)
+{
+       return (shared_buf_len % PAGE_SIZE) ?
+                            (shared_buf_len/PAGE_SIZE + 1) * PAGE_SIZE :
+                            shared_buf_len;
[kevin]:
  1. the above code can be replaced with "ALIGN" macro..
  2.  #define ALIGN(x, a)>---->-------__ALIGN_KERNEL((x), (a)
  1. do you forget to add some lock resource to protect/cover multi process case?
  2. e.g: process#1 exec load_ta() and process#2 exec unload_ta()

and why not use write operation instead of read (load/unload/invoke) this seems more reasonable.

Best Regards,
Kevin

+}
+
+static void prep_ta_mem_context(struct psp_context *psp,
+                                            struct ta_context *context,
+                                            uint8_t *shared_buf,
+                                            uint32_t shared_buf_len)
+{
+       context->mem_context.shared_mem_size = get_shared_buf_size(shared_buf_len);
+       psp_ta_init_shared_buf(psp, &context->mem_context);
+
+       memcpy((void *)context->mem_context.shared_buf, shared_buf, shared_buf_len);
+}
+
+static bool is_ta_type_valid(enum ta_type_id ta_type)
+{
+       bool ret = false;
+
+       switch (ta_type) {
+       case TA_TYPE_RAS:
+               ret = true;
+               break;
+       default:
+               break;
+       }
+
+       return ret;
+}
+
+static const struct file_operations ta_load_debugfs_fops = {
+       .read   = ta_if_load_debugfs_read,
+       .llseek = default_llseek,
+       .owner  = THIS_MODULE
+};
+
+static const struct file_operations ta_unload_debugfs_fops = {
+       .read   = ta_if_unload_debugfs_read,
+       .llseek = default_llseek,
+       .owner  = THIS_MODULE
+};
+
+static const struct file_operations ta_invoke_debugfs_fops = {
+       .read   = ta_if_invoke_debugfs_read,
+       .llseek = default_llseek,
+       .owner  = THIS_MODULE
+};
+
+
+/**
+ * DOC: AMDGPU TA debugfs interfaces
+ *
+ * Three debugfs interfaces can be opened by a program to
+ * load/invoke/unload TA,
+ *
+ * - /sys/kernel/debug/dri/<N>/ta_if/ta_load
+ * - /sys/kernel/debug/dri/<N>/ta_if/ta_invoke
+ * - /sys/kernel/debug/dri/<N>/ta_if/ta_unload
+ *
+ * How to use the interfaces in a program?
+ *
+ * A program needs to provide transmit buffer to the interfaces
+ * and will receive buffer from the interfaces below,
+ *
+ * - For TA load debugfs interface:
+ *   Transmit buffer:
+ *    - TA type (4bytes)
+ *    - TA bin length (4bytes)
+ *    - TA bin
+ *   Receive buffer:
+ *    - TA ID (4bytes)
+ *
+ * - For TA invoke debugfs interface:
+ *   Transmit buffer:
+ *    - TA ID (4bytes)
+ *    - TA CMD ID (4bytes)
+ *    - TA shard buf length (4bytes)
+ *    - TA shared buf
+ *   Receive buffer:
+ *    - TA shared buf
+ *
+ * - For TA unload debugfs interface:
+ *   Transmit buffer:
+ *    - TA ID (4bytes)
+ */
+
+static ssize_t ta_if_load_debugfs_read(struct file *fp, char *buf, size_t len, loff_t *off)
+{
+       uint32_t ta_type    = 0;
+       uint32_t ta_bin_len = 0;
+       uint8_t  *ta_bin    = NULL;
+       uint32_t copy_pos   = 0;
+       int      ret        = 0;
+
+       struct amdgpu_device *adev   = (struct amdgpu_device *)file_inode(fp)->i_private;
+       struct psp_context   *psp    = &adev->psp;
+       struct ta_context    context = {0};
+
+       if ((fp == NULL) || (buf == NULL))
+               return -EINVAL;
+
+       ret = copy_from_user((void *)&ta_type, &buf[copy_pos], sizeof(uint32_t));
+       if (ret || (!is_ta_type_valid(ta_type)))
+               return -EINVAL;
+
+       copy_pos += sizeof(uint32_t);
+
+       ret = copy_from_user((void *)&ta_bin_len, &buf[copy_pos], sizeof(uint32_t));
+       if (ret)
+               return -EINVAL;
+
+       copy_pos += sizeof(uint32_t);
+
+       ta_bin = kzalloc(ta_bin_len, GFP_KERNEL);
+       if (!ta_bin)
+               ret = -ENOMEM;
+       ret = copy_from_user((void *)ta_bin, &buf[copy_pos], ta_bin_len);
+       if (ret)
+               goto err_free_bin;
+
+       ret = psp_ras_terminate(psp);
+       if (ret) {
+               dev_err(adev->dev, "Failed to unload embedded RAS TA\n");
+               goto err_free_bin;
+       }
+
+       context.ta_type             = ta_type;
+       context.ta_load_type        = GFX_CMD_ID_LOAD_TA;
+       context.bin_desc.fw_version = get_bin_version(ta_bin);
+       context.bin_desc.size_bytes = ta_bin_len;
+       context.bin_desc.start_addr = ta_bin;
+
+       ret = psp_ta_load(psp, &context);
+
+       if (ret || context.resp_status) {
+               dev_err(adev->dev, "TA load via debugfs failed (%d) status %d\n",
+                        ret, context.resp_status);
+               goto err_free_bin;
+       }
+
+       context.initialized = true;
+       ret = copy_to_user((char *)buf, (void *)&context.session_id, sizeof(uint32_t));
+
+err_free_bin:
+       kfree(ta_bin);
+
+       return ret;
+}
+
+static ssize_t ta_if_unload_debugfs_read(struct file *fp, char *buf, size_t len, loff_t *off)
+{
+       uint32_t ta_id  = 0;
+       int      ret    = 0;
+
+       struct amdgpu_device *adev   = (struct amdgpu_device *)file_inode(fp)->i_private;
+       struct psp_context   *psp    = &adev->psp;
+       struct ta_context    context = {0};
+
+       if ((fp == NULL) || (buf == NULL))
+               return -1;
+
+       ret = copy_from_user((void *)&ta_id, buf, sizeof(uint32_t));
+       if (ret)
+               return -EINVAL;
+
+       context.session_id = ta_id;
+
+       ret = psp_ta_unload(psp, &context);
+       if (!ret)
+               context.initialized = false;
+
+       return ret;
+}
+
+static ssize_t ta_if_invoke_debugfs_read(struct file *fp, char *buf, size_t len, loff_t *off)
+{
+       uint32_t ta_id          = 0;
+       uint32_t cmd_id         = 0;
+       uint32_t shared_buf_len = 0;
+       uint8_t  *shared_buf    = NULL;
+       uint32_t copy_pos       = 0;
+       int      ret            = 0;
+
+       struct amdgpu_device *adev   = (struct amdgpu_device *)file_inode(fp)->i_private;
+       struct psp_context   *psp    = &adev->psp;
+       struct ta_context    context = {0};
+
+       if ((fp == NULL) || (buf == NULL))
+               return -EINVAL;

[kevin]:
   “if ((fp == NULL) ..."
   The linux kernel will automatically create struct file instance when you open the file, even if you do not provide a callback function (in fops)

[kevin]:

      you should add some check here to avoid that the user buf is overflow.
      e.g: If the length of the user buf is less than that required, an error should be returned directly

+
+       ret = copy_from_user((void *)&ta_id, &buf[copy_pos], sizeof(uint32_t));
+       if (ret)
+               return -EINVAL;
+       copy_pos += sizeof(uint32_t);
+
+       ret = copy_from_user((void *)&cmd_id, &buf[copy_pos], sizeof(uint32_t));
+       if (ret)
+               return -EINVAL;
+       copy_pos += sizeof(uint32_t);
+
+       ret = copy_from_user((void *)&shared_buf_len, &buf[copy_pos], sizeof(uint32_t));
+       if (ret)
+               return -EINVAL;
+       copy_pos += sizeof(uint32_t);
+
+       shared_buf = kzalloc(shared_buf_len, GFP_KERNEL);
+       if (!shared_buf)
+               ret = -ENOMEM;
+       ret = copy_from_user((void *)shared_buf, &buf[copy_pos], shared_buf_len);
+       if (ret)
+               goto err_free_shared_buf;
+
+       context.session_id = ta_id;
+
+       prep_ta_mem_context(psp, &context, shared_buf, shared_buf_len);
+
+       ret = psp_ta_invoke_indirect(psp, cmd_id, &context);
+
+       if (ret || context.resp_status) {
+               dev_err(adev->dev, "TA invoke via debugfs failed (%d) status %d\n",
+                        ret, context.resp_status);
+               goto err_free_ta_shared_buf;
+       }
+
+       ret = copy_to_user((char *)buf, context.mem_context.shared_buf, shared_buf_len);
+
+err_free_ta_shared_buf:
+       psp_ta_free_shared_buf(&context.mem_context);
+
+err_free_shared_buf:
+       kfree(shared_buf);
+
+       return ret;
+}
+
+static struct dentry *amdgpu_ta_if_debugfs_create(struct amdgpu_device *adev)
+{
+       struct drm_minor *minor = adev_to_drm(adev)->primary;
+
+       dir = debugfs_create_dir(TA_IF_FS_NAME, minor->debugfs_root);
+
+       ta_load_debugfs_dentry = debugfs_create_file("ta_load", 0400, dir, adev,
+                                                    &ta_load_debugfs_fops);
+
+       ta_unload_debugfs_dentry = debugfs_create_file("ta_unload", 0400, dir,
+                                                    adev, &ta_unload_debugfs_fops);
+
+       ta_invoke_debugfs_dentry = debugfs_create_file("ta_invoke", 0400, dir,
+                                                    adev, &ta_invoke_debugfs_fops);
+       return dir;
+}
+
+void amdgpu_ta_if_debugfs_init(struct amdgpu_device *adev)
+{
+#if defined(CONFIG_DEBUG_FS)
+       dir = amdgpu_ta_if_debugfs_create(adev);
+#endif
+}
+
+void amdgpu_ta_if_debugfs_remove(void)
+{
+       debugfs_remove_recursive(dir);
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp_ta.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp_ta.h
new file mode 100644
index 00000000000000..883f89d57616d0
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp_ta.h
@@ -0,0 +1,30 @@
+/*
+ * Copyright 2022 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#ifndef __AMDGPU_PSP_TA_H__
+#define __AMDGPU_PSP_TA_H__
+
+void amdgpu_ta_if_debugfs_init(struct amdgpu_device *adev);
+void amdgpu_ta_if_debugfs_remove(void);
+
+#endif
--
2.17.1


[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux