On 5/14/2025 12:55 PM, Lazar, Lijo wrote:
>
>
> On 5/14/2025 12:53 PM, Lazar, Lijo wrote:
>>
>>
>> On 5/14/2025 11:51 AM, Shiwu Zhang wrote:
>>> 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.
>>>
>>> v2: move the vbios gfl macros to the common header and rename the
>>> bo triplet struct to spirom_bo for this specfic usage (Hawking)
>>>
>>> v3: return directly the result of last command execution (Lijo)
>>>
>>> Signed-off-by: Shiwu Zhang <shiwu.zh...@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 | 29 ++++++
>>> drivers/gpu/drm/amd/amdgpu/psp_v13_0.c | 44 +++++++--
>>> 4 files changed, 168 insertions(+), 10 deletions(-)
>>>
>>> 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..3a27ce75f80c 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 spirom_bo *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;
>>> + }
>>> +
>>
>> Didn't notice this before. Once rom buffer is allocated/read, you could
>> allow multiple reads at the same time. FS backend will handle that.
>> Release is called only when all references to the file is closed.
>>
>> For ex: in open, you could do like
>>
>> if (file_count(filp) > 1)
> Correction -
> if (file_count(filp))
>
After further check, it won't work for this usecase. The patch as it is
good enough.
Reviewed-by: Lijo Lazar <lijo.la...@amd.com>
Thanks,
Lijo
> Thanks,
> Lijo
>
>> return 0;
>>
>> If the file is successfully opened by someone else we have a valid rom
>> buffer available. Rest of the ops like read/seek etc. are handled
>> properly by FS. Once all users do close(fd), release() is called and
>> that time we are also releasing the rom buffer.
>>
>> Thanks,
>> Lijo
>>
>>> + bo_triplet = kzalloc(sizeof(struct spirom_bo), 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 spirom_bo *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 spirom_bo *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..089b6ae48329 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
>>> @@ -39,6 +39,18 @@
>>> #define PSP_TMR_ALIGNMENT 0x100000
>>> #define PSP_FW_NAME_LEN 0x24
>>>
>>> +/* VBIOS gfl defines */
>>> +#define MBOX_READY_MASK 0x80000000
>>> +#define MBOX_STATUS_MASK 0x0000FFFF
>>> +#define MBOX_COMMAND_MASK 0x00FF0000
>>> +#define MBOX_READY_FLAG 0x80000000
>>> +#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
>>> +
>>> extern const struct attribute_group amdgpu_flash_attr_group;
>>>
>>> enum psp_shared_mem_size {
>>> @@ -138,6 +150,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 +335,14 @@ struct psp_runtime_scpm_entry {
>>> enum psp_runtime_scpm_authentication scpm_status;
>>> };
>>>
>>> +#if defined(CONFIG_DEBUG_FS)
>>> +struct spirom_bo {
>>> + 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 +430,9 @@ struct psp_context {
>>> char *vbflash_tmp_buf;
>>> size_t vbflash_image_size;
>>> bool vbflash_done;
>>> +#if defined(CONFIG_DEBUG_FS)
>>> + struct spirom_bo *spirom_dump_trip;
>>> +#endif
>>> };
>>>
>>> struct amdgpu_psp_funcs {
>>> @@ -467,6 +491,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 +606,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..ec9462336bcf 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
>>> @@ -71,15 +71,6 @@ MODULE_FIRMWARE("amdgpu/psp_14_0_4_ta.bin");
>>> /* Retry times for vmbx ready wait */
>>> #define PSP_VMBX_POLLING_LIMIT 3000
>>>
>>> -/* VBIOS gfl defines */
>>> -#define MBOX_READY_MASK 0x80000000
>>> -#define MBOX_STATUS_MASK 0x0000FFFF
>>> -#define MBOX_COMMAND_MASK 0x00FF0000
>>> -#define MBOX_READY_FLAG 0x80000000
>>> -#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
>>> -
>>> /* memory training timeout define */
>>> #define MEM_TRAIN_SEND_MSG_TIMEOUT_US 3000000
>>>
>>> @@ -710,7 +701,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 +758,37 @@ 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);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> static int psp_v13_0_vbflash_status(struct psp_context *psp)
>>> {
>>> struct amdgpu_device *adev = psp->adev;
>>> @@ -898,6 +921,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,
>>
>
