lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 14 Mar 2024 11:28:11 +0530
From: "Khatri, Sunil" <sukhatri@....com>
To: Alex Deucher <alexdeucher@...il.com>, Sunil Khatri <sunil.khatri@....com>
Cc: Alex Deucher <alexander.deucher@....com>,
 Christian König <christian.koenig@....com>,
 Shashank Sharma <shashank.sharma@....com>, amd-gfx@...ts.freedesktop.org,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] drm:amdgpu: add firmware information of all IP's


On 3/14/2024 2:06 AM, Alex Deucher wrote:
> On Tue, Mar 12, 2024 at 8:42 AM Sunil Khatri <sunil.khatri@....com> wrote:
>> Add firmware version information of each
>> IP and each instance where applicable.
>>
> Is there a way we can share some common code with devcoredump,
> debugfs, and the info IOCTL?  All three places need to query this
> information and the same logic is repeated in each case.

Hello Alex,

Yes you re absolutely right the same information is being retrieved 
again as done in debugfs. I can reorganize the code so same function 
could be used by debugfs and devcoredump but this is exactly what i 
tried to avoid here. I did try to use minimum functionality in 
devcoredump without shuffling a lot of code here and there.

Also our devcoredump is implemented in amdgpu_reset.c and not all the 
information is available here and there we might have to include lot of 
header and cross functions in amdgpu_reset until we want a dedicated 
file for devcoredump.

Info IOCTL does have a lot of information which also is in pipeline to 
be dumped but this if we want to reuse the functionality of IOCTL we 
need to reorganize a lot of code.

If that is the need of the hour i could work on that. Please let me know.

This is my suggestion if it makes sense:

1. If we want to reuse a lot of functionality then we need to modularize 
some of the functions further so they could be consumed directly by 
devcoredump.
2. We should also have a dedicated file for devcoredump.c/.h so its easy 
to include headers of needed functionality cleanly and easy to expand 
devcoredump.
3. based on the priority and importance of this task we can add 
information else some repetition is a real possibility.

>
> Alex
>
>
>> Signed-off-by: Sunil Khatri <sunil.khatri@....com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c | 122 ++++++++++++++++++++++
>>   1 file changed, 122 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
>> index 611fdb90a1fc..78ddc58aef67 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
>> @@ -168,6 +168,123 @@ void amdgpu_coredump(struct amdgpu_device *adev, bool vram_lost,
>>   {
>>   }
>>   #else
>> +static void amdgpu_devcoredump_fw_info(struct amdgpu_device *adev, struct drm_printer *p)
>> +{
>> +       uint32_t version;
>> +       uint32_t feature;
>> +       uint8_t smu_program, smu_major, smu_minor, smu_debug;
>> +
>> +       drm_printf(p, "VCE feature version: %u, fw version: 0x%08x\n",
>> +                  adev->vce.fb_version, adev->vce.fw_version);
>> +       drm_printf(p, "UVD feature version: %u, fw version: 0x%08x\n",
>> +                  0, adev->uvd.fw_version);
>> +       drm_printf(p, "GMC feature version: %u, fw version: 0x%08x\n",
>> +                  0, adev->gmc.fw_version);
>> +       drm_printf(p, "ME feature version: %u, fw version: 0x%08x\n",
>> +                  adev->gfx.me_feature_version, adev->gfx.me_fw_version);
>> +       drm_printf(p, "PFP feature version: %u, fw version: 0x%08x\n",
>> +                  adev->gfx.pfp_feature_version, adev->gfx.pfp_fw_version);
>> +       drm_printf(p, "CE feature version: %u, fw version: 0x%08x\n",
>> +                  adev->gfx.ce_feature_version, adev->gfx.ce_fw_version);
>> +       drm_printf(p, "RLC feature version: %u, fw version: 0x%08x\n",
>> +                  adev->gfx.rlc_feature_version, adev->gfx.rlc_fw_version);
>> +
>> +       drm_printf(p, "RLC SRLC feature version: %u, fw version: 0x%08x\n",
>> +                  adev->gfx.rlc_srlc_feature_version,
>> +                  adev->gfx.rlc_srlc_fw_version);
>> +       drm_printf(p, "RLC SRLG feature version: %u, fw version: 0x%08x\n",
>> +                  adev->gfx.rlc_srlg_feature_version,
>> +                  adev->gfx.rlc_srlg_fw_version);
>> +       drm_printf(p, "RLC SRLS feature version: %u, fw version: 0x%08x\n",
>> +                  adev->gfx.rlc_srls_feature_version,
>> +                  adev->gfx.rlc_srls_fw_version);
>> +       drm_printf(p, "RLCP feature version: %u, fw version: 0x%08x\n",
>> +                  adev->gfx.rlcp_ucode_feature_version,
>> +                  adev->gfx.rlcp_ucode_version);
>> +       drm_printf(p, "RLCV feature version: %u, fw version: 0x%08x\n",
>> +                  adev->gfx.rlcv_ucode_feature_version,
>> +                  adev->gfx.rlcv_ucode_version);
>> +       drm_printf(p, "MEC feature version: %u, fw version: 0x%08x\n",
>> +                  adev->gfx.mec_feature_version,
>> +                  adev->gfx.mec_fw_version);
>> +
>> +       if (adev->gfx.mec2_fw)
>> +               drm_printf(p,
>> +                          "MEC2 feature version: %u, fw version: 0x%08x\n",
>> +                          adev->gfx.mec2_feature_version,
>> +                          adev->gfx.mec2_fw_version);
>> +
>> +       drm_printf(p, "IMU feature version: %u, fw version: 0x%08x\n",
>> +                  0, adev->gfx.imu_fw_version);
>> +       drm_printf(p, "PSP SOS feature version: %u, fw version: 0x%08x\n",
>> +                  adev->psp.sos.feature_version,
>> +                  adev->psp.sos.fw_version);
>> +       drm_printf(p, "PSP ASD feature version: %u, fw version: 0x%08x\n",
>> +                  adev->psp.asd_context.bin_desc.feature_version,
>> +                  adev->psp.asd_context.bin_desc.fw_version);
>> +
>> +       drm_printf(p, "TA XGMI feature version: 0x%08x, fw version: 0x%08x\n",
>> +                  adev->psp.xgmi_context.context.bin_desc.feature_version,
>> +                  adev->psp.xgmi_context.context.bin_desc.fw_version);
>> +       drm_printf(p, "TA RAS feature version: 0x%08x, fw version: 0x%08x\n",
>> +                  adev->psp.ras_context.context.bin_desc.feature_version,
>> +                  adev->psp.ras_context.context.bin_desc.fw_version);
>> +       drm_printf(p, "TA HDCP feature version: 0x%08x, fw version: 0x%08x\n",
>> +                  adev->psp.hdcp_context.context.bin_desc.feature_version,
>> +                  adev->psp.hdcp_context.context.bin_desc.fw_version);
>> +       drm_printf(p, "TA DTM feature version: 0x%08x, fw version: 0x%08x\n",
>> +                  adev->psp.dtm_context.context.bin_desc.feature_version,
>> +                  adev->psp.dtm_context.context.bin_desc.fw_version);
>> +       drm_printf(p, "TA RAP feature version: 0x%08x, fw version: 0x%08x\n",
>> +                  adev->psp.rap_context.context.bin_desc.feature_version,
>> +                  adev->psp.rap_context.context.bin_desc.fw_version);
>> +       drm_printf(p, "TA SECURE DISPLAY feature version: 0x%08x, fw version: 0x%08x\n",
>> +               adev->psp.securedisplay_context.context.bin_desc.feature_version,
>> +               adev->psp.securedisplay_context.context.bin_desc.fw_version);
>> +
>> +       /* SMC firmware */
>> +       version = adev->pm.fw_version;
>> +
>> +       smu_program = (version >> 24) & 0xff;
>> +       smu_major = (version >> 16) & 0xff;
>> +       smu_minor = (version >> 8) & 0xff;
>> +       smu_debug = (version >> 0) & 0xff;
>> +       drm_printf(p, "SMC feature version: %u, program: %d, fw version: 0x%08x (%d.%d.%d)\n",
>> +                  0, smu_program, version, smu_major, smu_minor, smu_debug);
>> +
>> +       /* SDMA firmware */
>> +       for (int i = 0; i < adev->sdma.num_instances; i++) {
>> +               drm_printf(p, "SDMA%d feature version: %u, firmware version: 0x%08x\n",
>> +                          i, adev->sdma.instance[i].feature_version,
>> +                          adev->sdma.instance[i].fw_version);
>> +       }
>> +
>> +       drm_printf(p, "VCN feature version: %u, fw version: 0x%08x\n",
>> +                  0, adev->vcn.fw_version);
>> +       drm_printf(p, "DMCU feature version: %u, fw version: 0x%08x\n",
>> +                  0, adev->dm.dmcu_fw_version);
>> +       drm_printf(p, "DMCUB feature version: %u, fw version: 0x%08x\n",
>> +                  0, adev->dm.dmcub_fw_version);
>> +       drm_printf(p, "PSP TOC feature version: %u, fw version: 0x%08x\n",
>> +                  adev->psp.toc.feature_version, adev->psp.toc.fw_version);
>> +
>> +       version = adev->mes.kiq_version & AMDGPU_MES_VERSION_MASK;
>> +       feature = (adev->mes.kiq_version & AMDGPU_MES_FEAT_VERSION_MASK)
>> +                                       >> AMDGPU_MES_FEAT_VERSION_SHIFT;
>> +       drm_printf(p, "MES_KIQ feature version: %u, fw version: 0x%08x\n",
>> +                  feature, version);
>> +
>> +       version = adev->mes.sched_version & AMDGPU_MES_VERSION_MASK;
>> +       feature = (adev->mes.sched_version & AMDGPU_MES_FEAT_VERSION_MASK)
>> +                                       >> AMDGPU_MES_FEAT_VERSION_SHIFT;
>> +       drm_printf(p, "MES feature version: %u, fw version: 0x%08x\n",
>> +                  feature, version);
>> +
>> +       drm_printf(p, "VPE feature version: %u, fw version: 0x%08x\n",
>> +                  adev->vpe.feature_version, adev->vpe.fw_version);
>> +
>> +}
>> +
>>   static ssize_t
>>   amdgpu_devcoredump_read(char *buffer, loff_t offset, size_t count,
>>                          void *data, size_t datalen)
>> @@ -215,6 +332,11 @@ amdgpu_devcoredump_read(char *buffer, loff_t offset, size_t count,
>>                  }
>>          }
>>
>> +       if (coredump->adev) {
>> +               drm_printf(&p, "IP Firmwares\n");
>> +               amdgpu_devcoredump_fw_info(coredump->adev, &p);
>> +       }
>> +
>>          if (coredump->ring) {
>>                  drm_printf(&p, "\nRing timed out details\n");
>>                  drm_printf(&p, "IP Type: %d Ring Name: %s\n",
>> --
>> 2.34.1
>>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ