Re: [PATCH i-g-t] tests/amd_security: fix secure bounce test issue

[vitaly prosyak <vprosyak@amd.com>]

On 2024-01-10 05:15, Christian König wrote:
>  From coding style etc it looks good and of hand it seems to make sense.
>
> But I don't understand the background of the test and why that direction 
> needs to be reverted.
>
> So Vitaly can you take a look?
Hi Christian,

The issue was real.
The typo was hidden in the fact that the same pattern was used for
both buffers and even when the second copy did not occur due to an
incorrect address, the problem did not pop up. To address the issue

i use the second pattern for the second copy.

I reworked and posted Jesse's finding with 2 patches. There is some improvement like using ASIC-dependent linear copy function vs hard coded earlier.

Thanks, Vitaly

>
> Thanks,
> Christian.
>
> Am 10.01.24 um 10:13 schrieb Zhang, Jesse(Jie):
>> [AMD Official Use Only - General]
>>
>> Ping ...
>> Can anyone help review this.
>> -----Original Message-----
>> From: Jesse Zhang <jesse.zhang@amd.com>
>> Sent: Monday, December 4, 2023 11:00 AM
>> To: igt-dev@lists.freedesktop.org
>> Cc: Prosyak, Vitaly <Vitaly.Prosyak@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Tuikov, Luben <Luben.Tuikov@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Zhang, Jesse(Jie) <Jesse.Zhang@amd.com>; Kamil Konieczny <kamil.konieczny@linux.intel.com>; Zhang, Jesse(Jie) <Jesse.Zhang@amd.com>
>> Subject: [PATCH i-g-t] tests/amd_security: fix secure bounce test issue
>>
>> When it copy from scr (bo) to destination (bo2), and move bo2 from vram to gtt.
>> Then it need to copy from bo2(src)
>> to bo (destination) for verification.
>>
>> Cc: Vitaly Prosyak <vitaly.prosyak@amd.com>
>> Cc: Luben Tuikov <luben.tuikov@amd.com>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: Christian Koenig <christian.koenig@amd.com>
>> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
>>
>> Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
>> ---
>>   tests/amdgpu/amd_security.c | 17 ++++++++++++-----
>>   1 file changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/tests/amdgpu/amd_security.c b/tests/amdgpu/amd_security.c index d1146a7ce..7fb6df3b6 100644
>> --- a/tests/amdgpu/amd_security.c
>> +++ b/tests/amdgpu/amd_security.c
>> @@ -99,7 +99,7 @@ static void
>>   amdgpu_bo_lcopy(amdgpu_device_handle device,
>>                  struct amdgpu_ring_context *ring_context,
>>                  const struct amdgpu_ip_block_version *ip_block, uint32_t size,
>> -               uint32_t secure)
>> +               uint32_t secure, int direction)
>>   {
>>          ring_context->pm4 = calloc(PACKET_LCOPY_SIZE, sizeof(*ring_context->pm4));
>>          ring_context->secure = secure;
>> @@ -108,8 +108,15 @@ amdgpu_bo_lcopy(amdgpu_device_handle device,
>>          ring_context->res_cnt = 2;
>>          igt_assert(ring_context->pm4);
>>
>> -       amdgpu_sdma_lcopy(ring_context->pm4, ring_context->bo_mc2,
>> -                       ring_context->bo_mc, size, secure);
>> +       if (direction) {
>> +               /* copy from bo2 to bo */
>> +               amdgpu_sdma_lcopy(ring_context->pm4, ring_context->bo_mc,
>> +                               ring_context->bo_mc2, size, secure);
>> +       } else {
>> +               /* copy from bo to bo bo2 */
>> +               amdgpu_sdma_lcopy(ring_context->pm4, ring_context->bo_mc2,
>> +                               ring_context->bo_mc, size, secure);
>> +       }
>>          amdgpu_test_exec_cs_helper(device, ip_block->type, ring_context, 0);
>>          free(ring_context->pm4);
>>   }
>> @@ -222,7 +229,7 @@ amdgpu_secure_bounce(amdgpu_device_handle device_handle, int fd,
>>          ring_context->resources[1] = ring_context->bo;  // Alice
>>
>>          amdgpu_bo_lcopy(device_handle, ring_context, ip_block, SECURE_BUFFER_SIZE,
>> -                       secure == true ? 1 : 0);
>> +                       secure == true ? 1 : 0, 0);
>>
>>          /* Verify the contents of Bob. */
>>          for (pp = (__typeof__(pp))ring_context->bo2_cpu;
>> @@ -249,7 +256,7 @@ amdgpu_secure_bounce(amdgpu_device_handle device_handle, int fd,
>>
>>          /* sDMA TMZ copy from Bob to Alice. */
>>          amdgpu_bo_lcopy(device_handle, ring_context, ip_block, SECURE_BUFFER_SIZE,
>> -                       secure == true ? 1 : 0);
>> +                       secure == true ? 1 : 0, 1);
>>
>>          /* Verify the contents of Alice */
>>          for (pp = (__typeof__(pp))ring_context->bo_cpu;
>> --
>> 2.25.1
>>