AMD General Thank you for the test and comment, will update the patch description while submitting
Thanks Asad -----Original Message----- From: John Olender <[email protected]> Sent: Tuesday, July 14, 2026 5:53 AM To: Kamal, Asad <[email protected]>; [email protected] Cc: Lazar, Lijo <[email protected]>; Zhang, Hawking <[email protected]>; Ma, Le <[email protected]>; Zhang, Morris <[email protected]>; Deucher, Alexander <[email protected]>; Wang, Yang(Kevin) <[email protected]> Subject: Re: [PATCH] drm/amd/pm: Fix pp_entries_max() bios check On 7/13/26 12:29 AM, Asad Kamal wrote: > Commit 3cfe43363050 ("drm/amd/pm: Use uploaded size for legacy custom > PPTable") changed pp_dpm_set_pp_table() to kmemdup the uploaded buffer > directly and set soft_pp_table_size to the uploaded size. As a result > soft_pp_table now points to an allocation completely outside > adev->bios, making the unconditional pp_end > bios_end check in > pp_entries_max() always true for custom PP tables — silently returning > 0 and breaking PP table overrides via sysfs. > When I hit this issue, the pp_end > bios_end check didn't always return true. The order of pp_table overrides mattered for triggering the failure on a system with multiple cards. I also wouldn't call this failure path silent. Callers of pp_entries_max() print appropriate warnings, painting a nice picture of what's happening during the resulting cascade: $ sudo cp custom_pp_table.bin /sys/class/drm/cardN/device/pp_table Killed [ 71.320109] kernel: amdgpu: amdgpu: MM dependency table: clamping ucNumEntries 8 -> 0 [ 71.320137] kernel: amdgpu: amdgpu: Polaris SCLK dependency table: clamping ucNumEntries 8 -> 0 [ 71.320149] kernel: amdgpu: amdgpu: MCLK dependency table: clamping ucNumEntries 4 -> 0 [ 71.320166] kernel: amdgpu: Number of Pcie Entries exceed the number of SCLK Dpm Levels! Disregarding the excess entries... [ 71.320179] kernel: amdgpu: SCLK DPM index for VRHot cannot exceed the total sclk level count! [ 71.320190] kernel: amdgpu: amdgpu: VddcLookup table: clamping ucNumEntries 15 -> 0 [ 71.320200] kernel: amdgpu: amdgpu: VddcLookup table: clamping ucNumEntries 8 -> 0 [ 71.320211] kernel: amdgpu: [powerplay] Lookup table is empty [ 71.320222] kernel: amdgpu: [powerplay] Lookup table is empty ... [ 71.678679] kernel: note: cp[2243] exited with irqs disabled > Fix this by conditioning the BIOS containment check on > hardcode_pp_table being NULL. hardcode_pp_table is zero-initialised > (kzalloc) and only set when a custom table is uploaded via sysfs, so: > > - hardcode_pp_table == NULL: VBIOS path — enforce pp_end <= bios_end > to reject a malicious VBIOS inflating usStructureSize past the BIOS > image. > > - hardcode_pp_table != NULL: custom upload path — skip the bios_end > check, soft_pp_table_size is the kernel-supplied upload size and is > already trusted. > > Fixes: e30b3e3ab51a ("drm/amdgpu/pm: add pp_entries_max() helper") > Reported-by: John Olender <[email protected]> > Signed-off-by: Asad Kamal <[email protected]> Tested-by: John Olender <[email protected]> Thanks, John > --- > drivers/gpu/drm/amd/pm/powerplay/inc/hwmgr.h | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/amd/pm/powerplay/inc/hwmgr.h > b/drivers/gpu/drm/amd/pm/powerplay/inc/hwmgr.h > index 7ebc1344023f..a23e01921842 100644 > --- a/drivers/gpu/drm/amd/pm/powerplay/inc/hwmgr.h > +++ b/drivers/gpu/drm/amd/pm/powerplay/inc/hwmgr.h > @@ -833,14 +833,17 @@ static inline uint32_t pp_entries_max(const struct > pp_hwmgr *hwmgr, > const void *sub_table, > size_t hdr_size, size_t rec_size) { > - struct amdgpu_device *adev = (struct amdgpu_device *)hwmgr->adev; > - const char *bios_end = (const char *)adev->bios + adev->bios_size; > - const char *pp_end = (const char *)hwmgr->soft_pp_table > - + hwmgr->soft_pp_table_size; > + const char *pp_start = (const char *)hwmgr->soft_pp_table; > + const char *pp_end = pp_start + hwmgr->soft_pp_table_size; > const char *entries = (const char *)sub_table + hdr_size; > > - if (pp_end > bios_end) > - return 0; > + if (!hwmgr->hardcode_pp_table) { > + struct amdgpu_device *adev = (struct amdgpu_device > *)hwmgr->adev; > + const char *bios_end = (const char *)adev->bios + > adev->bios_size; > + > + if (pp_end > bios_end) > + return 0; > + } > if (!rec_size || entries >= pp_end) > return 0; > return (uint32_t)((pp_end - entries) / rec_size);
