[PATCH] drm/amdgpu: add debugfs for spirom IFWI dump
Zhang, Morris
Shiwu.Zhang at amd.com
Mon May 12 06:39:13 UTC 2025
[AMD Official Use Only - AMD Internal Distribution Only]
Hi hawking,
thanks for taking your time reviewing. simple_read_from_buffer() is just a linux wrapper around copy_to_user() and it handles the reading count and buffer position checking well so it can clean up the code a little bit. FYI.
And the one-time handshake with psp is put into the .open() for 1. Normally user will call syscall open() only once but can call the read() multi times and 2. The mutex operations include buffer allocation and handshake can be put into one place so that the .read() and .release() can eliminate the need to request the mutex. Thanks!
> -----Original Message-----
> From: Zhang, Hawking <Hawking.Zhang at amd.com>
> Sent: Friday, May 9, 2025 2:20 PM
> To: Zhang, Morris <Shiwu.Zhang at amd.com>; Gao, Likun <Likun.Gao at amd.com>;
> amd-gfx at lists.freedesktop.org
> Subject: RE: [PATCH] drm/amdgpu: add debugfs for spirom IFWI dump
>
> [AMD Official Use Only - AMD Internal Distribution Only]
>
> Hi Morris,
>
> I will review the change later today. At first glance, it seems that some
> implementations are not included in the patch. For example, I couldn't find the
> implementation of simple_read_from_buffer. Did I miss something?
>
> + return simple_read_from_buffer(buf,
> + size,
> + pos, bo_triplet->cpu_addr,
> + AMD_VBIOS_FILE_MAX_SIZE_B * 2); }
>
> Regards,
> Hawking
> -----Original Message-----
> From: Zhang, Morris <Shiwu.Zhang at amd.com>
> Sent: Thursday, May 8, 2025 17:34
> To: Zhang, Morris <Shiwu.Zhang at amd.com>; Zhang, Hawking
> <Hawking.Zhang at amd.com>; Gao, Likun <Likun.Gao at amd.com>; amd-
> gfx at lists.freedesktop.org
> Subject: RE: [PATCH] drm/amdgpu: add debugfs for spirom IFWI dump
>
> [AMD Official Use Only - AMD Internal Distribution Only]
>
> Ping. Thanks!
>
> --Brs,
> Morris Zhang
> MLSE Linux ML SRDC
> Ext. 25147
>
> > -----Original Message-----
> > From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of
> > Shiwu Zhang
> > Sent: Wednesday, May 7, 2025 12:42 PM
> > To: Zhang, Hawking <Hawking.Zhang at amd.com>; Gao, Likun
> > <Likun.Gao at amd.com>; amd-gfx at lists.freedesktop.org
> > Subject: [PATCH] drm/amdgpu: add debugfs for spirom IFWI dump
> >
> > Expose the debugfs file node for user space to dump the IFWI image on spirom.
> >
> > For one transaction between PSP and host, it will read out the images
> > on both active and inactive partitions so a buffer with two times the
> > size of maximum IFWI image (currently 16MByte) is needed.
> >
> > Signed-off-by: Shiwu Zhang <shiwu.zhang at amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 1 +
> > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 104 ++++++++++++++++++++
> > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h | 17 ++++
> > drivers/gpu/drm/amd/amdgpu/psp_v13_0.c | 40 +++++++-
> > 4 files changed, 161 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > index 4835123c99f3..bfa3b1519d4c 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > @@ -2113,6 +2113,7 @@ int amdgpu_debugfs_init(struct amdgpu_device *adev)
> > amdgpu_rap_debugfs_init(adev);
> > amdgpu_securedisplay_debugfs_init(adev);
> > amdgpu_fw_attestation_debugfs_init(adev);
> > + amdgpu_psp_debugfs_init(adev);
> >
> > debugfs_create_file("amdgpu_evict_vram", 0400, root, adev,
> > &amdgpu_evict_vram_fops); diff --git
> > a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > index 6f9bcffda875..210a7bdda332 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > @@ -4185,6 +4185,110 @@ const struct attribute_group
> > amdgpu_flash_attr_group = {
> > .is_visible = amdgpu_flash_attr_is_visible, };
> >
> > +#if defined(CONFIG_DEBUG_FS)
> > +static int psp_read_spirom_debugfs_open(struct inode *inode, struct
> > +file *filp) {
> > + struct amdgpu_device *adev = filp->f_inode->i_private;
> > + struct bo_address_triplet *bo_triplet;
> > + int ret;
> > +
> > + /* serialize the open() file calling */
> > + if (!mutex_trylock(&adev->psp.mutex))
> > + return -EBUSY;
> > +
> > + /*
> > + * make sure only one userpace process is alive for dumping so that
> > + * only one memory buffer of AMD_VBIOS_FILE_MAX_SIZE * 2 is
> > consumed.
> > + * let's say the case where one process try opening the file while
> > + * another one has proceeded to read or release. In this way, eliminate
> > + * the use of mutex for read() or release() callback as well.
> > + */
> > + if (adev->psp.spirom_dump_trip) {
> > + mutex_unlock(&adev->psp.mutex);
> > + return -EBUSY;
> > + }
> > +
> > + bo_triplet = kzalloc(sizeof(struct bo_address_triplet), GFP_KERNEL);
> > + if (!bo_triplet) {
> > + mutex_unlock(&adev->psp.mutex);
> > + return -ENOMEM;
> > + }
> > +
> > + ret = amdgpu_bo_create_kernel(adev, AMD_VBIOS_FILE_MAX_SIZE_B * 2,
> > + AMDGPU_GPU_PAGE_SIZE,
> > + AMDGPU_GEM_DOMAIN_GTT,
> > + &bo_triplet->bo,
> > + &bo_triplet->mc_addr,
> > + &bo_triplet->cpu_addr);
> > + if (ret)
> > + goto rel_trip;
> > +
> > + ret = psp_dump_spirom(&adev->psp, bo_triplet->mc_addr);
> > + if (ret)
> > + goto rel_bo;
> > +
> > + adev->psp.spirom_dump_trip = bo_triplet;
> > + mutex_unlock(&adev->psp.mutex);
> > + return 0;
> > +rel_bo:
> > + amdgpu_bo_free_kernel(&bo_triplet->bo, &bo_triplet->mc_addr,
> > + &bo_triplet->cpu_addr);
> > +rel_trip:
> > + kfree(bo_triplet);
> > + mutex_unlock(&adev->psp.mutex);
> > + dev_err(adev->dev, "Trying IFWI dump fails, err = %d\n", ret);
> > + return ret;
> > +}
> > +
> > +static ssize_t psp_read_spirom_debugfs_read(struct file *filp, char
> > +__user *buf,
> > size_t size,
> > + loff_t *pos) {
> > + struct amdgpu_device *adev = filp->f_inode->i_private;
> > + struct bo_address_triplet *bo_triplet =
> > +adev->psp.spirom_dump_trip;
> > +
> > + if (!bo_triplet)
> > + return -EINVAL;
> > +
> > + return simple_read_from_buffer(buf,
> > + size,
> > + pos, bo_triplet->cpu_addr,
> > + AMD_VBIOS_FILE_MAX_SIZE_B * 2); }
> > +
> > +static int psp_read_spirom_debugfs_release(struct inode *inode,
> > +struct file *filp) {
> > + struct amdgpu_device *adev = filp->f_inode->i_private;
> > + struct bo_address_triplet *bo_triplet =
> > +adev->psp.spirom_dump_trip;
> > +
> > + if (bo_triplet) {
> > + amdgpu_bo_free_kernel(&bo_triplet->bo, &bo_triplet->mc_addr,
> > + &bo_triplet->cpu_addr);
> > + kfree(bo_triplet);
> > + }
> > +
> > + adev->psp.spirom_dump_trip = NULL;
> > + return 0;
> > +}
> > +
> > +static const struct file_operations psp_dump_spirom_debugfs_ops = {
> > + .owner = THIS_MODULE,
> > + .open = psp_read_spirom_debugfs_open,
> > + .read = psp_read_spirom_debugfs_read,
> > + .release = psp_read_spirom_debugfs_release,
> > + .llseek = default_llseek,
> > +};
> > +#endif
> > +
> > +void amdgpu_psp_debugfs_init(struct amdgpu_device *adev) { #if
> > +defined(CONFIG_DEBUG_FS)
> > + struct drm_minor *minor = adev_to_drm(adev)->primary;
> > +
> > + debugfs_create_file_size("psp_spirom_dump", 0444, minor->debugfs_root,
> > + adev, &psp_dump_spirom_debugfs_ops,
> > AMD_VBIOS_FILE_MAX_SIZE_B * 2);
> > +#endif }
> > +
> > const struct amd_ip_funcs psp_ip_funcs = {
> > .name = "psp",
> > .early_init = psp_early_init,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> > index 3876ac57ce62..8fc4a7bb865e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> > @@ -138,6 +138,7 @@ struct psp_funcs {
> > int (*load_usbc_pd_fw)(struct psp_context *psp, uint64_t fw_pri_mc_addr);
> > int (*read_usbc_pd_fw)(struct psp_context *psp, uint32_t *fw_ver);
> > int (*update_spirom)(struct psp_context *psp, uint64_t
> > fw_pri_mc_addr);
> > + int (*dump_spirom)(struct psp_context *psp, uint64_t
> > + fw_pri_mc_addr);
> > int (*vbflash_stat)(struct psp_context *psp);
> > int (*fatal_error_recovery_quirk)(struct psp_context *psp);
> > bool (*get_ras_capability)(struct psp_context *psp); @@ -322,6
> > +323,14 @@ struct psp_runtime_scpm_entry {
> > enum psp_runtime_scpm_authentication scpm_status; };
> >
> > +#if defined(CONFIG_DEBUG_FS)
> > +struct bo_address_triplet {
> > + struct amdgpu_bo *bo;
> > + uint64_t mc_addr;
> > + void *cpu_addr;
> > +};
> > +#endif
> > +
> > struct psp_context {
> > struct amdgpu_device *adev;
> > struct psp_ring km_ring;
> > @@ -409,6 +418,9 @@ struct psp_context {
> > char *vbflash_tmp_buf;
> > size_t vbflash_image_size;
> > bool vbflash_done;
> > +#if defined(CONFIG_DEBUG_FS)
> > + struct bo_address_triplet *spirom_dump_trip;
> > +#endif
> > };
> >
> > struct amdgpu_psp_funcs {
> > @@ -467,6 +479,10 @@ struct amdgpu_psp_funcs {
> > ((psp)->funcs->update_spirom ? \
> > (psp)->funcs->update_spirom((psp), fw_pri_mc_addr) : -EINVAL)
> >
> > +#define psp_dump_spirom(psp, fw_pri_mc_addr) \
> > + ((psp)->funcs->dump_spirom ? \
> > + (psp)->funcs->dump_spirom((psp), fw_pri_mc_addr) : -EINVAL)
> > +
> > #define psp_vbflash_status(psp) \
> > ((psp)->funcs->vbflash_stat ? \
> > (psp)->funcs->vbflash_stat((psp)) : -EINVAL) @@ -578,6 +594,7 @@
> > int psp_config_sq_perfmon(struct psp_context *psp, uint32_t xcc_id,
> > bool amdgpu_psp_tos_reload_needed(struct amdgpu_device *adev); int
> > amdgpu_psp_reg_program_no_ring(struct psp_context *psp, uint32_t val,
> > enum psp_reg_prog_id id);
> > +void amdgpu_psp_debugfs_init(struct amdgpu_device *adev);
> >
> >
> > #endif
> > diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
> > b/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
> > index 17f1ccd8bd53..78f434f84c22 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
> > @@ -79,6 +79,9 @@ MODULE_FIRMWARE("amdgpu/psp_14_0_4_ta.bin");
> > #define C2PMSG_CMD_SPI_UPDATE_ROM_IMAGE_ADDR_LO 0x2 #define
> > C2PMSG_CMD_SPI_UPDATE_ROM_IMAGE_ADDR_HI 0x3 #define
> > C2PMSG_CMD_SPI_UPDATE_FLASH_IMAGE 0x4
> > +#define C2PMSG_CMD_SPI_GET_ROM_IMAGE_ADDR_LO 0xf #define
> > +C2PMSG_CMD_SPI_GET_ROM_IMAGE_ADDR_HI 0x10 #define
> > +C2PMSG_CMD_SPI_GET_FLASH_IMAGE 0x11
> >
> > /* memory training timeout define */
> > #define MEM_TRAIN_SEND_MSG_TIMEOUT_US 3000000
> > @@ -710,7 +713,8 @@ static int psp_v13_0_exec_spi_cmd(struct
> > psp_context *psp, int cmd)
> > /* Ring the doorbell */
> > WREG32_SOC15(MP0, 0, regMP0_SMN_C2PMSG_73, 1);
> >
> > - if (cmd == C2PMSG_CMD_SPI_UPDATE_FLASH_IMAGE)
> > + if (cmd == C2PMSG_CMD_SPI_UPDATE_FLASH_IMAGE ||
> > + cmd == C2PMSG_CMD_SPI_GET_FLASH_IMAGE)
> > ret = psp_wait_for_spirom_update(psp,
> > SOC15_REG_OFFSET(MP0, 0, regMP0_SMN_C2PMSG_115),
> > MBOX_READY_FLAG,
> > MBOX_READY_MASK, PSP_SPIROM_UPDATE_TIMEOUT);
> > else
> > @@ -766,6 +770,39 @@ static int psp_v13_0_update_spirom(struct
> > psp_context *psp,
> > return 0;
> > }
> >
> > +static int psp_v13_0_dump_spirom(struct psp_context *psp,
> > + uint64_t fw_pri_mc_addr) {
> > + struct amdgpu_device *adev = psp->adev;
> > + int ret;
> > +
> > + /* Confirm PSP is ready to start */
> > + ret = psp_wait_for(psp, SOC15_REG_OFFSET(MP0, 0,
> > regMP0_SMN_C2PMSG_115),
> > + MBOX_READY_FLAG, MBOX_READY_MASK, false);
> > + if (ret) {
> > + dev_err(adev->dev, "PSP Not ready to start processing,
> > + ret = %d",
> > ret);
> > + return ret;
> > + }
> > +
> > + WREG32_SOC15(MP0, 0, regMP0_SMN_C2PMSG_116,
> > +lower_32_bits(fw_pri_mc_addr));
> > +
> > + ret = psp_v13_0_exec_spi_cmd(psp,
> > C2PMSG_CMD_SPI_GET_ROM_IMAGE_ADDR_LO);
> > + if (ret)
> > + return ret;
> > +
> > + WREG32_SOC15(MP0, 0, regMP0_SMN_C2PMSG_116,
> > +upper_32_bits(fw_pri_mc_addr));
> > +
> > + ret = psp_v13_0_exec_spi_cmd(psp,
> > C2PMSG_CMD_SPI_GET_ROM_IMAGE_ADDR_HI);
> > + if (ret)
> > + return ret;
> > +
> > + ret = psp_v13_0_exec_spi_cmd(psp,
> > C2PMSG_CMD_SPI_GET_FLASH_IMAGE);
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +
> > static int psp_v13_0_vbflash_status(struct psp_context *psp) {
> > struct amdgpu_device *adev = psp->adev; @@ -898,6 +935,7 @@
> > static const struct psp_funcs psp_v13_0_funcs = {
> > .load_usbc_pd_fw = psp_v13_0_load_usbc_pd_fw,
> > .read_usbc_pd_fw = psp_v13_0_read_usbc_pd_fw,
> > .update_spirom = psp_v13_0_update_spirom,
> > + .dump_spirom = psp_v13_0_dump_spirom,
> > .vbflash_stat = psp_v13_0_vbflash_status,
> > .fatal_error_recovery_quirk = psp_v13_0_fatal_error_recovery_quirk,
> > .get_ras_capability = psp_v13_0_get_ras_capability,
> > --
> > 2.34.1
>
>
More information about the amd-gfx
mailing list