[PATCH 05/12] tests/amdgpu: expand write linear helper for security (v3)

[Luben Tuikov]

On 2019-11-14 10:34 p.m., Aaron Liu wrote:
> From: Huang Rui <ray.huang at 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.huang at amd.com>
> Signed-off-by: Aaron Liu <aaron.liu at amd.com>
> Reviewed-by: Alex Deucher <alexander.deucher at 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);
>