On 2019-11-14 10:34 p.m., Aaron Liu wrote:
> From: Huang Rui <ray.hu...@amd.com>
> 
> This patch expand write linear helper for security to submit the command
> with secure context.
> 
> v2: refine the function implementation.
> v3: remove amdgpu_cs_ctx_create3.
> 
> Signed-off-by: Huang Rui <ray.hu...@amd.com>
> Signed-off-by: Aaron Liu <aaron....@amd.com>
> Reviewed-by: Alex Deucher <alexander.deuc...@amd.com>
> ---
>  tests/amdgpu/amdgpu_test.h |  4 ++++
>  tests/amdgpu/basic_tests.c | 15 +++++++++++++--
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/amdgpu/amdgpu_test.h b/tests/amdgpu/amdgpu_test.h
> index b7f8de2..67be437 100644
> --- a/tests/amdgpu/amdgpu_test.h
> +++ b/tests/amdgpu/amdgpu_test.h
> @@ -262,6 +262,9 @@ CU_BOOL suite_security_tests_enable(void);
>   */
>  extern CU_TestInfo security_tests[];
>  
> +extern void
> +amdgpu_command_submission_write_linear_helper_with_secure(unsigned ip_type,
> +                                                       bool secure);
>  
>  /**
>   * Helper functions
> @@ -452,4 +455,5 @@ static inline bool asic_is_arcturus(uint32_t asic_id)
>       }
>  }
>  
> +
>  #endif  /* #ifdef _AMDGPU_TEST_H_ */
> diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c
> index a57dcbb..31c9a54 100644
> --- a/tests/amdgpu/basic_tests.c
> +++ b/tests/amdgpu/basic_tests.c
> @@ -71,7 +71,7 @@ static void 
> amdgpu_test_exec_cs_helper(amdgpu_context_handle context_handle,
>                                      int res_cnt, amdgpu_bo_handle *resources,
>                                      struct amdgpu_cs_ib_info *ib_info,
>                                      struct amdgpu_cs_request *ibs_request);
> - 
> +
>  CU_TestInfo basic_tests[] = {
>       { "Query Info Test",  amdgpu_query_info_test },
>       { "Userptr Test",  amdgpu_userptr_test },
> @@ -1361,7 +1361,8 @@ static void 
> amdgpu_test_exec_cs_helper(amdgpu_context_handle context_handle,
>       CU_ASSERT_EQUAL(r, 0);
>  }
>  
> -static void amdgpu_command_submission_write_linear_helper(unsigned ip_type)
> +void amdgpu_command_submission_write_linear_helper_with_secure(unsigned 
> ip_type,
> +                                                            bool secure)
>  {

This is an example of bad naming of a function. And it is also very long. Too 
long.

Why does the name need to end with "_secure("? Does it mean that the write is 
always
secure? No! No, it doesn't! If the parameter, in this case the security state, 
is
parameterized, as it is via a function argument, then you don't need to add this
also to the name of the function, as you did.

amdgpu_command_submission_write_linear_helper(unsigned ip_type, bool secure)

is fine for a name. Leave it at that.

Regards,
Luben

>       const int sdma_write_length = 128;
>       const int pm4_dw = 256;
> @@ -1390,7 +1391,11 @@ static void 
> amdgpu_command_submission_write_linear_helper(unsigned ip_type)
>       r = amdgpu_query_hw_ip_info(device_handle, ip_type, 0, &hw_ip_info);
>       CU_ASSERT_EQUAL(r, 0);
>  
> +     for (i = 0; secure && (i < 2); i++)
> +             gtt_flags[i] |= AMDGPU_GEM_CREATE_ENCRYPTED;
> +
>       r = amdgpu_cs_ctx_create(device_handle, &context_handle);
> +
>       CU_ASSERT_EQUAL(r, 0);
>  
>       /* prepare resource */
> @@ -1469,6 +1474,12 @@ static void 
> amdgpu_command_submission_write_linear_helper(unsigned ip_type)
>       CU_ASSERT_EQUAL(r, 0);
>  }
>  
> +static void amdgpu_command_submission_write_linear_helper(unsigned ip_type)
> +{
> +     amdgpu_command_submission_write_linear_helper_with_secure(ip_type,
> +                                                               false);
> +}
> +
>  static void amdgpu_command_submission_sdma_write_linear(void)
>  {
>       amdgpu_command_submission_write_linear_helper(AMDGPU_HW_IP_DMA);
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to