<div dir="ltr"><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">Looks good to me!<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Den mån 16 maj 2022 kl 20:15 skrev Alex Deucher <<a href="mailto:alexdeucher@gmail.com">alexdeucher@gmail.com</a>>:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, May 16, 2022 at 2:10 PM Christian König<br>
<<a href="mailto:ckoenig.leichtzumerken@gmail.com" target="_blank">ckoenig.leichtzumerken@gmail.com</a>> wrote:<br>
><br>
> Am 16.05.22 um 19:49 schrieb Ernst Sjöstrand:<br>
><br>
> Den mån 16 maj 2022 kl 17:13 skrev Alex Deucher <<a href="mailto:alexdeucher@gmail.com" target="_blank">alexdeucher@gmail.com</a>>:<br>
>><br>
>> On Sun, May 15, 2022 at 11:46 AM Ernst Sjöstrand <<a href="mailto:ernstp@gmail.com" target="_blank">ernstp@gmail.com</a>> wrote:<br>
>> ><br>
>> > smatch found this problem on amd-staging-drm-next:<br>
>> ><br>
>> > drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c:1443 amdgpu_discovery_get_vcn_info() error: buffer overflow 'adev->vcn.vcn_codec_disable_mask' 2 <= 3<br>
>> ><br>
>> > This is caused by:<br>
>> > #define AMDGPU_MAX_VCN_INSTANCES 2<br>
>> > #define VCN_INFO_TABLE_MAX_NUM_INSTANCES 4<br>
>> ><br>
>> > Can we just drop VCN_INFO_TABLE_MAX_NUM_INSTANCES completely and use AMDGPU_MAX_VCN_INSTANCES everywhere instead (and bump it to 4)?<br>
>><br>
>> We should be able to bump AMDGPU_MAX_VCN_INSTANCES to 4 (although it<br>
>> would waste some memory in the places it is used at this point).<br>
>> VCN_INFO_TABLE_MAX_NUM_INSTANCES is part of a firmware structure so we<br>
>> can't change that without breaking the firmware structure.<br>
>><br>
>> Alex<br>
><br>
><br>
> It would be nice to get rid of this pattern and make sure it doesn't happen again when the VCN info table is raised to 5.<br>
> It's very similar to the HWIP_MAX_INSTANCE issue.<br>
><br>
><br>
> No, as Alex explained that distinction is intentional.<br>
><br>
> The firmware definition is 4 for future extensions, that doesn't mean that this is currently used.<br>
><br>
> There is currently simply no need to set AMDGPU_MAX_VCN_INSTANCES to more than 2.<br>
<br>
Right. The attached patch should protect against the scenario you are<br>
envisioning.<br>
<br>
Alex<br>
<br>
><br>
> Regards,<br>
> Christian.<br>
><br>
><br>
> //E<br>
><br>
><br>
</blockquote></div>