AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [bug report] drm/amdgpu: Add logic for VF ipd and VF bios to init from dynamic crit_region offsets
@ 2025-10-22 11:03 Dan Carpenter
  0 siblings, 0 replies; only message in thread
From: Dan Carpenter @ 2025-10-22 11:03 UTC (permalink / raw)
  To: Ellen Pan; +Cc: amd-gfx, SHANMUGAM, SRINIVASAN

Hello Ellen Pan,

Commit b4a8fcc7826a ("drm/amdgpu: Add logic for VF ipd and VF bios to
init from dynamic crit_region offsets") from Oct 7, 2025
(linux-next), leads to the following Smatch static checker warning:

drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c:128 amdgpu_read_bios_from_vram() warn: corrupting cast 'size'?
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c:312 amdgpu_discovery_read_binary_from_mem() warn: corrupting cast 'adev->discovery.size'?

drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
    271 static int amdgpu_discovery_read_binary_from_mem(struct amdgpu_device *adev,
    272                                                  uint8_t *binary)
    273 {
    274         bool sz_valid = true;
    275         uint64_t vram_size;
    276         int i, ret = 0;
    277         u32 msg;
    278 
    279         if (!amdgpu_sriov_vf(adev)) {
    280                 /* It can take up to two second for IFWI init to complete on some dGPUs,
    281                  * but generally it should be in the 60-100ms range.  Normally this starts
    282                  * as soon as the device gets power so by the time the OS loads this has long
    283                  * completed.  However, when a card is hotplugged via e.g., USB4, we need to
    284                  * wait for this to complete.  Once the C2PMSG is updated, we can
    285                  * continue.
    286                  */
    287 
    288                 for (i = 0; i < 2000; i++) {
    289                         msg = RREG32(mmMP0_SMN_C2PMSG_33);
    290                         if (msg & 0x80000000)
    291                                 break;
    292                         msleep(1);
    293                 }
    294         }
    295 
    296         vram_size = RREG32(mmRCC_CONFIG_MEMSIZE);
    297         if (!vram_size || vram_size == U32_MAX)
    298                 sz_valid = false;
    299         else
    300                 vram_size <<= 20;
    301 
    302         /*
    303          * If in VRAM, discovery TMR is marked for reservation. If it is in system mem,
    304          * then it is not required to be reserved.
    305          */
    306         if (sz_valid) {
    307                 if (amdgpu_sriov_vf(adev) && adev->virt.is_dynamic_crit_regn_enabled) {
    308                         /* For SRIOV VFs with dynamic critical region enabled,
    309                          * we will get the IPD binary via below call.
    310                          * If dynamic critical is disabled, fall through to normal seq.
    311                          */
--> 312                         if (amdgpu_virt_get_dynamic_data_info(adev,
    313                                                 AMD_SRIOV_MSG_IPD_TABLE_ID, binary,
    314                                                 (uint64_t *)&adev->discovery.size)) {

&adev->discovery.size is a u32 but we're writing a 64 bits into it which
will corrupt memory.

In amdgpu_read_bios_from_vram() the issue is similar except that there
size is a resource_size_t so it can be either 32bit or 64bit.  Obviously
no one is really going to run this driver with a 32bit kernel but for
correctness it's better to make the types match.

    315                                 dev_err(adev->dev,
    316                                                 "failed to read discovery info from dynamic critical region.");
    317                                 ret = -EINVAL;
    318                                 goto exit;
    319                         }
    320                 } else {
    321                         uint64_t pos = vram_size - DISCOVERY_TMR_OFFSET;
    322 
    323                         amdgpu_device_vram_access(adev, pos, (uint32_t *)binary,
    324                                         adev->discovery.size, false);
    325                         adev->discovery.reserve_tmr = true;
    326                 }
    327         } else {
    328                 ret = amdgpu_discovery_read_binary_from_sysmem(adev, binary);
    329         }
    330 
    331         if (ret)
    332                 dev_err(adev->dev,
    333                         "failed to read discovery info from memory, vram size read: %llx",
    334                         vram_size);
    335 exit:
    336         return ret;
    337 }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2025-10-23  7:51 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-22 11:03 [bug report] drm/amdgpu: Add logic for VF ipd and VF bios to init from dynamic crit_region offsets Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox