Re: [PATCH] drm/amd/display: Guard calls to hdcp_ta and dtm_ta

2020-04-01 Thread Kazlauskas, Nicholas

On 2020-03-31 2:08 p.m., Lakha, Bhawanpreet wrote:

[AMD Official Use Only - Internal Distribution Only]


mod_hdcp_hdcp2_get_link_encryption_status() isn't being used, should 
probably remove it in a followup patch


I still think it's better if you add it to the function for consistency.

Feel free to fix that bit up before you merge if you want, either way 
this patch is:


Reiviewed-by: Nicholas Kazlauskas 

Regards,
Nicholas Kazlauskas



*From:* Kazlauskas, Nicholas 
*Sent:* March 31, 2020 2:03 PM
*To:* Alex Deucher ; Lakha, Bhawanpreet 

*Cc:* Deucher, Alexander ; amd-gfx list 
; Zhang, Hawking 

*Subject:* Re: [PATCH] drm/amd/display: Guard calls to hdcp_ta and dtm_ta
On 2020-03-31 1:37 p.m., Alex Deucher wrote:

On Mon, Mar 30, 2020 at 6:36 PM Bhawanpreet Lakha
 wrote:


[Why]
The buffer used when calling psp is a shared buffer. If we have multiple calls
at the same time we can overwrite the buffer.

[How]
Add mutex to guard the shared buffer.

Signed-off-by: Bhawanpreet Lakha 


Acked-by: Alex Deucher 


One comment inline:




---
   drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c   |   2 +
   drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h   |   2 +
   .../drm/amd/display/modules/hdcp/hdcp_psp.c   | 420 +++---
   3 files changed, 257 insertions(+), 167 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index dbaeffc4431e..9d587bc27663 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -888,6 +888,7 @@ static int psp_hdcp_load(struct psp_context *psp)
  if (!ret) {
  psp->hdcp_context.hdcp_initialized = true;
  psp->hdcp_context.session_id = cmd->resp.session_id;
+   mutex_init(>hdcp_context.mutex);
  }

  kfree(cmd);
@@ -1033,6 +1034,7 @@ static int psp_dtm_load(struct psp_context *psp)
  if (!ret) {
  psp->dtm_context.dtm_initialized = true;
  psp->dtm_context.session_id = cmd->resp.session_id;
+   mutex_init(>dtm_context.mutex);
  }

  kfree(cmd);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index 297435c0c7c1..6a717fd5efc7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -161,6 +161,7 @@ struct psp_hdcp_context {
  struct amdgpu_bo    *hdcp_shared_bo;
  uint64_t    hdcp_shared_mc_addr;
  void    *hdcp_shared_buf;
+   struct mutex    mutex;
   };

   struct psp_dtm_context {
@@ -169,6 +170,7 @@ struct psp_dtm_context {
  struct amdgpu_bo    *dtm_shared_bo;
  uint64_t    dtm_shared_mc_addr;
  void    *dtm_shared_buf;
+   struct mutex    mutex;
   };

   #define MEM_TRAIN_SYSTEM_SIGNATURE 0x54534942
diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c 
b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c
index c2929815c3ee..aa147e171557 100644
--- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c
+++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c
@@ -51,12 +51,15 @@ enum mod_hdcp_status mod_hdcp_remove_display_from_topology(
  struct ta_dtm_shared_memory *dtm_cmd;
  struct mod_hdcp_display *display =
  get_active_display_at_index(hdcp, index);
+   enum mod_hdcp_status status = MOD_HDCP_STATUS_SUCCESS;

  dtm_cmd = (struct ta_dtm_shared_memory 
*)psp->dtm_context.dtm_shared_buf;

  if (!display || !is_display_added(display))
  return MOD_HDCP_STATUS_DISPLAY_NOT_FOUND;

+   mutex_lock(>dtm_context.mutex);
+
  memset(dtm_cmd, 0, sizeof(struct ta_dtm_shared_memory));

  dtm_cmd->cmd_id = TA_DTM_COMMAND__TOPOLOGY_UPDATE_V2;
@@ -66,14 +69,15 @@ enum mod_hdcp_status mod_hdcp_remove_display_from_topology(

  psp_dtm_invoke(psp, dtm_cmd->cmd_id);

-   if (dtm_cmd->dtm_status != TA_DTM_STATUS__SUCCESS)
-   return MOD_HDCP_STATUS_UPDATE_TOPOLOGY_FAILURE;
+   if (dtm_cmd->dtm_status != TA_DTM_STATUS__SUCCESS) {
+   status = MOD_HDCP_STATUS_UPDATE_TOPOLOGY_FAILURE;
+   } else {
+   display->state = MOD_HDCP_DISPLAY_ACTIVE;
+   HDCP_TOP_REMOVE_DISPLAY_TRACE(hdcp, display->index);
+   }

-   display->state = MOD_HDCP_DISPLAY_ACTIVE;
-   HDCP_TOP_REMOVE_DISPLAY_TRACE(hdcp, display->index);
-
-   return MOD_HDCP_STATUS_SUCCESS;
-
+   mutex_unlock(>dtm_context.mutex);
+   return status;
   }
   enum mod_hdcp_status mod_hdcp_add_display_to_topology(struct mod_hdcp *hdcp,
    uint8_t index)
@@ -83,6 +87,7 @@ enum mod_hdcp_statu

Re: [PATCH] drm/amd/display: Guard calls to hdcp_ta and dtm_ta

2020-03-31 Thread Lakha, Bhawanpreet
[AMD Official Use Only - Internal Distribution Only]

mod_hdcp_hdcp2_get_link_encryption_status() isn't being used, should probably 
remove it in a followup patch

From: Kazlauskas, Nicholas 
Sent: March 31, 2020 2:03 PM
To: Alex Deucher ; Lakha, Bhawanpreet 

Cc: Deucher, Alexander ; amd-gfx list 
; Zhang, Hawking 
Subject: Re: [PATCH] drm/amd/display: Guard calls to hdcp_ta and dtm_ta

On 2020-03-31 1:37 p.m., Alex Deucher wrote:
> On Mon, Mar 30, 2020 at 6:36 PM Bhawanpreet Lakha
>  wrote:
>>
>> [Why]
>> The buffer used when calling psp is a shared buffer. If we have multiple 
>> calls
>> at the same time we can overwrite the buffer.
>>
>> [How]
>> Add mutex to guard the shared buffer.
>>
>> Signed-off-by: Bhawanpreet Lakha 
>
> Acked-by: Alex Deucher 

One comment inline:

>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c   |   2 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h   |   2 +
>>   .../drm/amd/display/modules/hdcp/hdcp_psp.c   | 420 +++---
>>   3 files changed, 257 insertions(+), 167 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>> index dbaeffc4431e..9d587bc27663 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>> @@ -888,6 +888,7 @@ static int psp_hdcp_load(struct psp_context *psp)
>>  if (!ret) {
>>  psp->hdcp_context.hdcp_initialized = true;
>>  psp->hdcp_context.session_id = cmd->resp.session_id;
>> +   mutex_init(>hdcp_context.mutex);
>>  }
>>
>>  kfree(cmd);
>> @@ -1033,6 +1034,7 @@ static int psp_dtm_load(struct psp_context *psp)
>>  if (!ret) {
>>  psp->dtm_context.dtm_initialized = true;
>>  psp->dtm_context.session_id = cmd->resp.session_id;
>> +   mutex_init(>dtm_context.mutex);
>>  }
>>
>>  kfree(cmd);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
>> index 297435c0c7c1..6a717fd5efc7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
>> @@ -161,6 +161,7 @@ struct psp_hdcp_context {
>>  struct amdgpu_bo*hdcp_shared_bo;
>>  uint64_thdcp_shared_mc_addr;
>>  void*hdcp_shared_buf;
>> +   struct mutexmutex;
>>   };
>>
>>   struct psp_dtm_context {
>> @@ -169,6 +170,7 @@ struct psp_dtm_context {
>>  struct amdgpu_bo*dtm_shared_bo;
>>  uint64_tdtm_shared_mc_addr;
>>  void*dtm_shared_buf;
>> +   struct mutexmutex;
>>   };
>>
>>   #define MEM_TRAIN_SYSTEM_SIGNATURE 0x54534942
>> diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c 
>> b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c
>> index c2929815c3ee..aa147e171557 100644
>> --- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c
>> +++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c
>> @@ -51,12 +51,15 @@ enum mod_hdcp_status 
>> mod_hdcp_remove_display_from_topology(
>>  struct ta_dtm_shared_memory *dtm_cmd;
>>  struct mod_hdcp_display *display =
>>  get_active_display_at_index(hdcp, index);
>> +   enum mod_hdcp_status status = MOD_HDCP_STATUS_SUCCESS;
>>
>>  dtm_cmd = (struct ta_dtm_shared_memory 
>> *)psp->dtm_context.dtm_shared_buf;
>>
>>  if (!display || !is_display_added(display))
>>  return MOD_HDCP_STATUS_DISPLAY_NOT_FOUND;
>>
>> +   mutex_lock(>dtm_context.mutex);
>> +
>>  memset(dtm_cmd, 0, sizeof(struct ta_dtm_shared_memory));
>>
>>  dtm_cmd->cmd_id = TA_DTM_COMMAND__TOPOLOGY_UPDATE_V2;
>> @@ -66,14 +69,15 @@ enum mod_hdcp_status 
>> mod_hdcp_remove_display_from_topology(
>>
>>  psp_dtm_invoke(psp, dtm_cmd->cmd_id);
>>
>> -   if (dtm_cmd->dtm_status != TA_DTM_STATUS__SUCCESS)
>> -   return MOD_HDCP_STATUS_UPDATE_TOPOLOGY_FAILURE;
>> +   if (dtm_cmd->dtm_status != TA_DTM_STATUS__SUCCESS) {
>> +   status = MOD_HDCP_STATUS_UPDATE_TOPOLOGY_FAILURE;
>> +   } else {
>> +   display->state = MOD_HDCP_DISPLAY_ACTIVE;
>> +   HDCP

Re: [PATCH] drm/amd/display: Guard calls to hdcp_ta and dtm_ta

2020-03-31 Thread Kazlauskas, Nicholas

On 2020-03-31 1:37 p.m., Alex Deucher wrote:

On Mon, Mar 30, 2020 at 6:36 PM Bhawanpreet Lakha
 wrote:


[Why]
The buffer used when calling psp is a shared buffer. If we have multiple calls
at the same time we can overwrite the buffer.

[How]
Add mutex to guard the shared buffer.

Signed-off-by: Bhawanpreet Lakha 


Acked-by: Alex Deucher 


One comment inline:




---
  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c   |   2 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h   |   2 +
  .../drm/amd/display/modules/hdcp/hdcp_psp.c   | 420 +++---
  3 files changed, 257 insertions(+), 167 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index dbaeffc4431e..9d587bc27663 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -888,6 +888,7 @@ static int psp_hdcp_load(struct psp_context *psp)
 if (!ret) {
 psp->hdcp_context.hdcp_initialized = true;
 psp->hdcp_context.session_id = cmd->resp.session_id;
+   mutex_init(>hdcp_context.mutex);
 }

 kfree(cmd);
@@ -1033,6 +1034,7 @@ static int psp_dtm_load(struct psp_context *psp)
 if (!ret) {
 psp->dtm_context.dtm_initialized = true;
 psp->dtm_context.session_id = cmd->resp.session_id;
+   mutex_init(>dtm_context.mutex);
 }

 kfree(cmd);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index 297435c0c7c1..6a717fd5efc7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -161,6 +161,7 @@ struct psp_hdcp_context {
 struct amdgpu_bo*hdcp_shared_bo;
 uint64_thdcp_shared_mc_addr;
 void*hdcp_shared_buf;
+   struct mutexmutex;
  };

  struct psp_dtm_context {
@@ -169,6 +170,7 @@ struct psp_dtm_context {
 struct amdgpu_bo*dtm_shared_bo;
 uint64_tdtm_shared_mc_addr;
 void*dtm_shared_buf;
+   struct mutexmutex;
  };

  #define MEM_TRAIN_SYSTEM_SIGNATURE 0x54534942
diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c 
b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c
index c2929815c3ee..aa147e171557 100644
--- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c
+++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c
@@ -51,12 +51,15 @@ enum mod_hdcp_status mod_hdcp_remove_display_from_topology(
 struct ta_dtm_shared_memory *dtm_cmd;
 struct mod_hdcp_display *display =
 get_active_display_at_index(hdcp, index);
+   enum mod_hdcp_status status = MOD_HDCP_STATUS_SUCCESS;

 dtm_cmd = (struct ta_dtm_shared_memory 
*)psp->dtm_context.dtm_shared_buf;

 if (!display || !is_display_added(display))
 return MOD_HDCP_STATUS_DISPLAY_NOT_FOUND;

+   mutex_lock(>dtm_context.mutex);
+
 memset(dtm_cmd, 0, sizeof(struct ta_dtm_shared_memory));

 dtm_cmd->cmd_id = TA_DTM_COMMAND__TOPOLOGY_UPDATE_V2;
@@ -66,14 +69,15 @@ enum mod_hdcp_status mod_hdcp_remove_display_from_topology(

 psp_dtm_invoke(psp, dtm_cmd->cmd_id);

-   if (dtm_cmd->dtm_status != TA_DTM_STATUS__SUCCESS)
-   return MOD_HDCP_STATUS_UPDATE_TOPOLOGY_FAILURE;
+   if (dtm_cmd->dtm_status != TA_DTM_STATUS__SUCCESS) {
+   status = MOD_HDCP_STATUS_UPDATE_TOPOLOGY_FAILURE;
+   } else {
+   display->state = MOD_HDCP_DISPLAY_ACTIVE;
+   HDCP_TOP_REMOVE_DISPLAY_TRACE(hdcp, display->index);
+   }

-   display->state = MOD_HDCP_DISPLAY_ACTIVE;
-   HDCP_TOP_REMOVE_DISPLAY_TRACE(hdcp, display->index);
-
-   return MOD_HDCP_STATUS_SUCCESS;
-
+   mutex_unlock(>dtm_context.mutex);
+   return status;
  }
  enum mod_hdcp_status mod_hdcp_add_display_to_topology(struct mod_hdcp *hdcp,
   uint8_t index)
@@ -83,6 +87,7 @@ enum mod_hdcp_status mod_hdcp_add_display_to_topology(struct 
mod_hdcp *hdcp,
 struct mod_hdcp_display *display =
 get_active_display_at_index(hdcp, index);
 struct mod_hdcp_link *link = >connection.link;
+   enum mod_hdcp_status status = MOD_HDCP_STATUS_SUCCESS;

 if (!psp->dtm_context.dtm_initialized) {
 DRM_ERROR("Failed to add display topology, DTM TA is not 
initialized.");
@@ -94,6 +99,7 @@ enum mod_hdcp_status mod_hdcp_add_display_to_topology(struct 
mod_hdcp *hdcp,

 dtm_cmd = (struct ta_dtm_shared_memory 
*)psp->dtm_context.dtm_shared_buf;

+   mutex_lock(>dtm_context.mutex);
 memset(dtm_cmd, 0, sizeof(struct ta_dtm_shared_memory));

 dtm_cmd->cmd_id = TA_DTM_COMMAND__TOPOLOGY_UPDATE_V2;
@@ -113,13 +119,15 @@ enum mod_hdcp_status 

Re: [PATCH] drm/amd/display: Guard calls to hdcp_ta and dtm_ta

2020-03-31 Thread Alex Deucher
On Mon, Mar 30, 2020 at 6:36 PM Bhawanpreet Lakha
 wrote:
>
> [Why]
> The buffer used when calling psp is a shared buffer. If we have multiple calls
> at the same time we can overwrite the buffer.
>
> [How]
> Add mutex to guard the shared buffer.
>
> Signed-off-by: Bhawanpreet Lakha 

Acked-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c   |   2 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h   |   2 +
>  .../drm/amd/display/modules/hdcp/hdcp_psp.c   | 420 +++---
>  3 files changed, 257 insertions(+), 167 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index dbaeffc4431e..9d587bc27663 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -888,6 +888,7 @@ static int psp_hdcp_load(struct psp_context *psp)
> if (!ret) {
> psp->hdcp_context.hdcp_initialized = true;
> psp->hdcp_context.session_id = cmd->resp.session_id;
> +   mutex_init(>hdcp_context.mutex);
> }
>
> kfree(cmd);
> @@ -1033,6 +1034,7 @@ static int psp_dtm_load(struct psp_context *psp)
> if (!ret) {
> psp->dtm_context.dtm_initialized = true;
> psp->dtm_context.session_id = cmd->resp.session_id;
> +   mutex_init(>dtm_context.mutex);
> }
>
> kfree(cmd);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> index 297435c0c7c1..6a717fd5efc7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> @@ -161,6 +161,7 @@ struct psp_hdcp_context {
> struct amdgpu_bo*hdcp_shared_bo;
> uint64_thdcp_shared_mc_addr;
> void*hdcp_shared_buf;
> +   struct mutexmutex;
>  };
>
>  struct psp_dtm_context {
> @@ -169,6 +170,7 @@ struct psp_dtm_context {
> struct amdgpu_bo*dtm_shared_bo;
> uint64_tdtm_shared_mc_addr;
> void*dtm_shared_buf;
> +   struct mutexmutex;
>  };
>
>  #define MEM_TRAIN_SYSTEM_SIGNATURE 0x54534942
> diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c 
> b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c
> index c2929815c3ee..aa147e171557 100644
> --- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c
> +++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c
> @@ -51,12 +51,15 @@ enum mod_hdcp_status 
> mod_hdcp_remove_display_from_topology(
> struct ta_dtm_shared_memory *dtm_cmd;
> struct mod_hdcp_display *display =
> get_active_display_at_index(hdcp, index);
> +   enum mod_hdcp_status status = MOD_HDCP_STATUS_SUCCESS;
>
> dtm_cmd = (struct ta_dtm_shared_memory 
> *)psp->dtm_context.dtm_shared_buf;
>
> if (!display || !is_display_added(display))
> return MOD_HDCP_STATUS_DISPLAY_NOT_FOUND;
>
> +   mutex_lock(>dtm_context.mutex);
> +
> memset(dtm_cmd, 0, sizeof(struct ta_dtm_shared_memory));
>
> dtm_cmd->cmd_id = TA_DTM_COMMAND__TOPOLOGY_UPDATE_V2;
> @@ -66,14 +69,15 @@ enum mod_hdcp_status 
> mod_hdcp_remove_display_from_topology(
>
> psp_dtm_invoke(psp, dtm_cmd->cmd_id);
>
> -   if (dtm_cmd->dtm_status != TA_DTM_STATUS__SUCCESS)
> -   return MOD_HDCP_STATUS_UPDATE_TOPOLOGY_FAILURE;
> +   if (dtm_cmd->dtm_status != TA_DTM_STATUS__SUCCESS) {
> +   status = MOD_HDCP_STATUS_UPDATE_TOPOLOGY_FAILURE;
> +   } else {
> +   display->state = MOD_HDCP_DISPLAY_ACTIVE;
> +   HDCP_TOP_REMOVE_DISPLAY_TRACE(hdcp, display->index);
> +   }
>
> -   display->state = MOD_HDCP_DISPLAY_ACTIVE;
> -   HDCP_TOP_REMOVE_DISPLAY_TRACE(hdcp, display->index);
> -
> -   return MOD_HDCP_STATUS_SUCCESS;
> -
> +   mutex_unlock(>dtm_context.mutex);
> +   return status;
>  }
>  enum mod_hdcp_status mod_hdcp_add_display_to_topology(struct mod_hdcp *hdcp,
>   uint8_t index)
> @@ -83,6 +87,7 @@ enum mod_hdcp_status 
> mod_hdcp_add_display_to_topology(struct mod_hdcp *hdcp,
> struct mod_hdcp_display *display =
> get_active_display_at_index(hdcp, index);
> struct mod_hdcp_link *link = >connection.link;
> +   enum mod_hdcp_status status = MOD_HDCP_STATUS_SUCCESS;
>
> if (!psp->dtm_context.dtm_initialized) {
> DRM_ERROR("Failed to add display topology, DTM TA is not 
> initialized.");
> @@ -94,6 +99,7 @@ enum mod_hdcp_status 
> mod_hdcp_add_display_to_topology(struct mod_hdcp *hdcp,
>
> dtm_cmd = (struct ta_dtm_shared_memory 
> *)psp->dtm_context.dtm_shared_buf;
>
> +   mutex_lock(>dtm_context.mutex);
> memset(dtm_cmd, 0, sizeof(struct ta_dtm_shared_memory));
>
> dtm_cmd->cmd_id 

[PATCH] drm/amd/display: Guard calls to hdcp_ta and dtm_ta

2020-03-30 Thread Bhawanpreet Lakha
[Why]
The buffer used when calling psp is a shared buffer. If we have multiple calls
at the same time we can overwrite the buffer.

[How]
Add mutex to guard the shared buffer.

Signed-off-by: Bhawanpreet Lakha 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c   |   2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h   |   2 +
 .../drm/amd/display/modules/hdcp/hdcp_psp.c   | 420 +++---
 3 files changed, 257 insertions(+), 167 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index dbaeffc4431e..9d587bc27663 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -888,6 +888,7 @@ static int psp_hdcp_load(struct psp_context *psp)
if (!ret) {
psp->hdcp_context.hdcp_initialized = true;
psp->hdcp_context.session_id = cmd->resp.session_id;
+   mutex_init(>hdcp_context.mutex);
}
 
kfree(cmd);
@@ -1033,6 +1034,7 @@ static int psp_dtm_load(struct psp_context *psp)
if (!ret) {
psp->dtm_context.dtm_initialized = true;
psp->dtm_context.session_id = cmd->resp.session_id;
+   mutex_init(>dtm_context.mutex);
}
 
kfree(cmd);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index 297435c0c7c1..6a717fd5efc7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -161,6 +161,7 @@ struct psp_hdcp_context {
struct amdgpu_bo*hdcp_shared_bo;
uint64_thdcp_shared_mc_addr;
void*hdcp_shared_buf;
+   struct mutexmutex;
 };
 
 struct psp_dtm_context {
@@ -169,6 +170,7 @@ struct psp_dtm_context {
struct amdgpu_bo*dtm_shared_bo;
uint64_tdtm_shared_mc_addr;
void*dtm_shared_buf;
+   struct mutexmutex;
 };
 
 #define MEM_TRAIN_SYSTEM_SIGNATURE 0x54534942
diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c 
b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c
index c2929815c3ee..aa147e171557 100644
--- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c
+++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c
@@ -51,12 +51,15 @@ enum mod_hdcp_status mod_hdcp_remove_display_from_topology(
struct ta_dtm_shared_memory *dtm_cmd;
struct mod_hdcp_display *display =
get_active_display_at_index(hdcp, index);
+   enum mod_hdcp_status status = MOD_HDCP_STATUS_SUCCESS;
 
dtm_cmd = (struct ta_dtm_shared_memory 
*)psp->dtm_context.dtm_shared_buf;
 
if (!display || !is_display_added(display))
return MOD_HDCP_STATUS_DISPLAY_NOT_FOUND;
 
+   mutex_lock(>dtm_context.mutex);
+
memset(dtm_cmd, 0, sizeof(struct ta_dtm_shared_memory));
 
dtm_cmd->cmd_id = TA_DTM_COMMAND__TOPOLOGY_UPDATE_V2;
@@ -66,14 +69,15 @@ enum mod_hdcp_status mod_hdcp_remove_display_from_topology(
 
psp_dtm_invoke(psp, dtm_cmd->cmd_id);
 
-   if (dtm_cmd->dtm_status != TA_DTM_STATUS__SUCCESS)
-   return MOD_HDCP_STATUS_UPDATE_TOPOLOGY_FAILURE;
+   if (dtm_cmd->dtm_status != TA_DTM_STATUS__SUCCESS) {
+   status = MOD_HDCP_STATUS_UPDATE_TOPOLOGY_FAILURE;
+   } else {
+   display->state = MOD_HDCP_DISPLAY_ACTIVE;
+   HDCP_TOP_REMOVE_DISPLAY_TRACE(hdcp, display->index);
+   }
 
-   display->state = MOD_HDCP_DISPLAY_ACTIVE;
-   HDCP_TOP_REMOVE_DISPLAY_TRACE(hdcp, display->index);
- 
-   return MOD_HDCP_STATUS_SUCCESS;
- 
+   mutex_unlock(>dtm_context.mutex);
+   return status;
 }
 enum mod_hdcp_status mod_hdcp_add_display_to_topology(struct mod_hdcp *hdcp,
  uint8_t index)
@@ -83,6 +87,7 @@ enum mod_hdcp_status mod_hdcp_add_display_to_topology(struct 
mod_hdcp *hdcp,
struct mod_hdcp_display *display =
get_active_display_at_index(hdcp, index);
struct mod_hdcp_link *link = >connection.link;
+   enum mod_hdcp_status status = MOD_HDCP_STATUS_SUCCESS;
 
if (!psp->dtm_context.dtm_initialized) {
DRM_ERROR("Failed to add display topology, DTM TA is not 
initialized.");
@@ -94,6 +99,7 @@ enum mod_hdcp_status mod_hdcp_add_display_to_topology(struct 
mod_hdcp *hdcp,
 
dtm_cmd = (struct ta_dtm_shared_memory 
*)psp->dtm_context.dtm_shared_buf;
 
+   mutex_lock(>dtm_context.mutex);
memset(dtm_cmd, 0, sizeof(struct ta_dtm_shared_memory));
 
dtm_cmd->cmd_id = TA_DTM_COMMAND__TOPOLOGY_UPDATE_V2;
@@ -113,13 +119,15 @@ enum mod_hdcp_status 
mod_hdcp_add_display_to_topology(struct mod_hdcp *hdcp,
 
psp_dtm_invoke(psp, dtm_cmd->cmd_id);
 
-   if (dtm_cmd->dtm_status != TA_DTM_STATUS__SUCCESS)
-   return