<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> <div class="moz-cite-prefix">On 2018-06-25 03:02 PM, Alex Deucher wrote: </div> <blockquote type="cite" cite="mid:CADnq5_O3P68Er_oEf9YybKxFBXk4BFQnJ45wtiKCs9oSxJBcAg@mail.gmail.com"> <pre wrap="">On Mon, Jun 25, 2018 at 2:59 PM, James Zhu <a class="moz-txt-link-rfc2396E" href="mailto:jamesz@amd.com"><jamesz@amd.com></a> wrote: </pre> <blockquote type="cite"> <pre wrap=""> On 2018-06-25 02:53 PM, Alex Deucher wrote: On Mon, Jun 25, 2018 at 2:37 PM, James Zhu <a class="moz-txt-link-rfc2396E" href="mailto:jamesz@amd.com"><jamesz@amd.com></a> wrote: For one UVD instance case,: In function amdgpu_driver_load_kms, all ring->me should be set to zero. adev = kzalloc(sizeof(struct amdgpu_device), GFP_KERNEL); For two UVD instances cases: static void uvd_v7_0_set_ring_funcs(struct amdgpu_device *adev) .. for (i = 0; i < adev->uvd.num_uvd_inst; i++) { adev->uvd.inst[i].ring.me = i; static void uvd_v7_0_set_enc_ring_funcs(struct amdgpu_device *adev) for (j = 0; j < adev->uvd.num_uvd_inst; j++) { adev->uvd.inst[j].ring_enc[i].me = j; uvd_v4_2_early_init in uvd_v4_2.c adev->uvd.num_uvd_inst = 1; uvd_v5_0_early_init in uvd_v5_0.c adev->uvd.num_uvd_inst = 1; uvd_v6_0_early_init in uvd_v6_0.c adev->uvd.num_uvd_inst = 1; uvd_v7_0_early_init in uvd_v7_0.c if (adev->asic_type == CHIP_VEGA20) adev->uvd.num_uvd_inst = UVD7_MAX_HW_INSTANCES_VEGA20;/*2*/ else adev->uvd.num_uvd_inst = 1; I didn't know when ring->me is set to 2. Maybe there is some leakage somewhere. What about older uvd (4.2, 5.0, 6.0) blocks? I think the below code will reset adev->uvd.inst[AMDGPU_MAX_UVD_INSTANCES].ring->me and adev->uvd.inst[AMDGPU_MAX_UVD_INSTANCES].ring_enc[AMDGPU_MAX_UVD_ENC_RINGS]->me to zero. for older uvd IP UVD block. adev = kzalloc(sizeof(struct amdgpu_device), GFP_KERNEL); Do I understand correctly? </pre> </blockquote> <pre wrap=""> Yes, it should. That's why it doesn't make sense that it would be getting another value. Alex </pre> </blockquote> ring point is refer in struct amdgpu_device. Miss using it may cause simple leakage. struct amdgpu_device { .............. struct amdgpu_ring *rings[AMDGPU_MAX_RINGS]; The simple possible leakage could happen at ./gfx_v8_0.c:1995: ring->me = mec + 1; ./amdgpu_gfx.c:190: ring->me = mec + 1; ./gfx_v9_0.c:1406: ring->me = mec + 1; ./gfx_v7_0.c:4471: ring->me = mec + 1; <pre wrap="">Timothy, It is not easy to find root cause based on current information. What asic are you using on this test. IS this UBSAN test open source? Is it easy for you guide me to reproduce it on my bench? James </pre> <blockquote type="cite" cite="mid:CADnq5_O3P68Er_oEf9YybKxFBXk4BFQnJ45wtiKCs9oSxJBcAg@mail.gmail.com"> <pre wrap=""> </pre> <blockquote type="cite"> <pre wrap="">James Alex Best regards! James zhu On 2018-06-25 01:29 PM, Deucher, Alexander wrote: Odd. The structure should be 0 initialized. Does this patch help? Alex ________________________________ From: Timothy Pearson <a class="moz-txt-link-rfc2396E" href="mailto:tpearson@raptorengineering.com"><tpearson@raptorengineering.com></a> Sent: Monday, June 25, 2018 11:53:12 AM To: Zhu, James Cc: <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>; Deucher, Alexander; Zhou, David(ChunMing); Koenig, Christian Subject: Re: [PATCH] Increase AMDGPU_MAX_UVD_INSTANCES to 3 n 06/25/2018 09:46 AM, James Zhu wrote: On 2018-06-23 08:02 PM, Timothy Pearson wrote: amdgpu_fence_driver_start_ring() attempts to access UVD instance 2 during setup, while the existing UVD instance count only allows instances 0 and 1. Increase AMDGPU_MAX_UVD_INSTANCES by one to avoid the invalid array access. Caught by UBSAN. Hi Timothy, >From design of view, it is not right to just change AMDGPU_MAX_UVD_INSTANCES to 3. Could you tell me some detail of UBSAN test and attach the dmesg also? Definitely, was looking for some feedback from anyone knowing more about the internals of the UVD system. What's happening is that "ring->me" in amdgpu_fence_driver_start_ring() (drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c:379) is set to a value of "2". The overall dmesg is otherwise uninteresting, but I can try to grab the UBSAN output if needed. -- Timothy Pearson Raptor Engineering +1 (415) 727-8645 (direct line) +1 (512) 690-0200 (switchboard) <a class="moz-txt-link-freetext" href="https://www.raptorengineering.com">https://www.raptorengineering.com</a> _______________________________________________ amd-gfx mailing list <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> <a class="moz-txt-link-freetext" href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> </pre> </blockquote> </blockquote> </body> </html>