All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH Review 1/1] drm/amdgpu: handle denied inject error into critical regions
@ 2022-01-12  1:42 Stanley.Yang
  2022-01-12  8:51 ` Zhou1, Tao
  2022-01-12 15:20 ` Lazar, Lijo
  0 siblings, 2 replies; 3+ messages in thread
From: Stanley.Yang @ 2022-01-12  1:42 UTC (permalink / raw)
  To: amd-gfx; +Cc: Stanley.Yang, Tao.Zhou1, John.Clements, Hawking.Zhang

Signed-off-by: Stanley.Yang <Stanley.Yang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 10 +++++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/ta_ras_if.h  |  3 ++-
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index c742d1aacf5a..8e0ea582b9c7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -1309,6 +1309,12 @@ static void psp_ras_ta_check_status(struct psp_context *psp)
 		break;
 	case TA_RAS_STATUS__SUCCESS:
 		break;
+	case TA_RAS_STATUS__TEE_ERROR_ACCESS_DENIED:
+		if (ras_cmd->cmd_id == TA_RAS_COMMAND__TRIGGER_ERROR) {
+			dev_info(psp->adev->dev,
+					"RAS INFO: Inject error to critical region is not allowed\n");
+		}
+		break;
 	default:
 		dev_warn(psp->adev->dev,
 				"RAS WARNING: ras status = 0x%X\n", ras_cmd->ras_status);
@@ -1521,7 +1527,9 @@ int psp_ras_trigger_error(struct psp_context *psp,
 	if (amdgpu_ras_intr_triggered())
 		return 0;
 
-	if (ras_cmd->ras_status)
+	if (ras_cmd->ras_status == TA_RAS_STATUS__TEE_ERROR_ACCESS_DENIED)
+		return -EACCES;
+	else if (ras_cmd->ras_status)
 		return -EINVAL;
 
 	return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index e674dbed3615..8bdc2e85cb20 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -449,7 +449,7 @@ static ssize_t amdgpu_ras_debugfs_ctrl_write(struct file *f,
 	}
 
 	if (ret)
-		return -EINVAL;
+		return ret;
 
 	return size;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/ta_ras_if.h b/drivers/gpu/drm/amd/amdgpu/ta_ras_if.h
index 5093826a43d1..509d8a1945eb 100644
--- a/drivers/gpu/drm/amd/amdgpu/ta_ras_if.h
+++ b/drivers/gpu/drm/amd/amdgpu/ta_ras_if.h
@@ -64,7 +64,8 @@ enum ta_ras_status {
 	TA_RAS_STATUS__ERROR_PCS_STATE_ERROR            = 0xA016,
 	TA_RAS_STATUS__ERROR_PCS_STATE_HANG             = 0xA017,
 	TA_RAS_STATUS__ERROR_PCS_STATE_UNKNOWN          = 0xA018,
-	TA_RAS_STATUS__ERROR_UNSUPPORTED_ERROR_INJ      = 0xA019
+	TA_RAS_STATUS__ERROR_UNSUPPORTED_ERROR_INJ      = 0xA019,
+	TA_RAS_STATUS__TEE_ERROR_ACCESS_DENIED          = 0xA01A
 };
 
 enum ta_ras_block {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* RE: [PATCH Review 1/1] drm/amdgpu: handle denied inject error into critical regions
  2022-01-12  1:42 [PATCH Review 1/1] drm/amdgpu: handle denied inject error into critical regions Stanley.Yang
@ 2022-01-12  8:51 ` Zhou1, Tao
  2022-01-12 15:20 ` Lazar, Lijo
  1 sibling, 0 replies; 3+ messages in thread
From: Zhou1, Tao @ 2022-01-12  8:51 UTC (permalink / raw)
  To: Yang, Stanley, amd-gfx; +Cc: Yang, Stanley, Clements, John, Zhang, Hawking

[AMD Official Use Only]



> -----Original Message-----
> From: Stanley.Yang <Stanley.Yang@amd.com>
> Sent: Wednesday, January 12, 2022 9:43 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Clements, John
> <John.Clements@amd.com>; Zhou1, Tao <Tao.Zhou1@amd.com>; Yang,
> Stanley <Stanley.Yang@amd.com>
> Subject: [PATCH Review 1/1] drm/amdgpu: handle denied inject error into
> critical regions
> 
> Signed-off-by: Stanley.Yang <Stanley.Yang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 10 +++++++++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c |  2 +-
> drivers/gpu/drm/amd/amdgpu/ta_ras_if.h  |  3 ++-
>  3 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index c742d1aacf5a..8e0ea582b9c7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -1309,6 +1309,12 @@ static void psp_ras_ta_check_status(struct
> psp_context *psp)
>  		break;
>  	case TA_RAS_STATUS__SUCCESS:
>  		break;
> +	case TA_RAS_STATUS__TEE_ERROR_ACCESS_DENIED:
> +		if (ras_cmd->cmd_id == TA_RAS_COMMAND__TRIGGER_ERROR)
> {
> +			dev_info(psp->adev->dev,
[Tao] Is dev_warn better? But either way is OK for me.

> +					"RAS INFO: Inject error to critical
> region is not allowed\n");
> +		}
[Tao] The {} can be removed.

> +		break;
>  	default:
>  		dev_warn(psp->adev->dev,
>  				"RAS WARNING: ras status = 0x%X\n",
> ras_cmd->ras_status); @@ -1521,7 +1527,9 @@ int
> psp_ras_trigger_error(struct psp_context *psp,
>  	if (amdgpu_ras_intr_triggered())
>  		return 0;
> 
> -	if (ras_cmd->ras_status)
> +	if (ras_cmd->ras_status ==
> TA_RAS_STATUS__TEE_ERROR_ACCESS_DENIED)
> +		return -EACCES;
> +	else if (ras_cmd->ras_status)
>  		return -EINVAL;
> 
>  	return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> index e674dbed3615..8bdc2e85cb20 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -449,7 +449,7 @@ static ssize_t amdgpu_ras_debugfs_ctrl_write(struct file
> *f,
>  	}
> 
>  	if (ret)
> -		return -EINVAL;
> +		return ret;
> 
>  	return size;
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/ta_ras_if.h
> b/drivers/gpu/drm/amd/amdgpu/ta_ras_if.h
> index 5093826a43d1..509d8a1945eb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/ta_ras_if.h
> +++ b/drivers/gpu/drm/amd/amdgpu/ta_ras_if.h
> @@ -64,7 +64,8 @@ enum ta_ras_status {
>  	TA_RAS_STATUS__ERROR_PCS_STATE_ERROR            = 0xA016,
>  	TA_RAS_STATUS__ERROR_PCS_STATE_HANG             = 0xA017,
>  	TA_RAS_STATUS__ERROR_PCS_STATE_UNKNOWN          = 0xA018,
> -	TA_RAS_STATUS__ERROR_UNSUPPORTED_ERROR_INJ      = 0xA019
> +	TA_RAS_STATUS__ERROR_UNSUPPORTED_ERROR_INJ      = 0xA019,
> +	TA_RAS_STATUS__TEE_ERROR_ACCESS_DENIED          = 0xA01A
>  };
> 
>  enum ta_ras_block {
> --
> 2.17.1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH Review 1/1] drm/amdgpu: handle denied inject error into critical regions
  2022-01-12  1:42 [PATCH Review 1/1] drm/amdgpu: handle denied inject error into critical regions Stanley.Yang
  2022-01-12  8:51 ` Zhou1, Tao
@ 2022-01-12 15:20 ` Lazar, Lijo
  1 sibling, 0 replies; 3+ messages in thread
From: Lazar, Lijo @ 2022-01-12 15:20 UTC (permalink / raw)
  To: Stanley.Yang, amd-gfx; +Cc: Tao.Zhou1, John.Clements, Hawking.Zhang



On 1/12/2022 7:12 AM, Stanley.Yang wrote:
> Signed-off-by: Stanley.Yang <Stanley.Yang@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 10 +++++++++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c |  2 +-
>   drivers/gpu/drm/amd/amdgpu/ta_ras_if.h  |  3 ++-
>   3 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index c742d1aacf5a..8e0ea582b9c7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -1309,6 +1309,12 @@ static void psp_ras_ta_check_status(struct psp_context *psp)
>   		break;
>   	case TA_RAS_STATUS__SUCCESS:
>   		break;
> +	case TA_RAS_STATUS__TEE_ERROR_ACCESS_DENIED:
> +		if (ras_cmd->cmd_id == TA_RAS_COMMAND__TRIGGER_ERROR) {
> +			dev_info(psp->adev->dev,
> +					"RAS INFO: Inject error to critical region is not allowed\n");
> +		}

Instead of doing this, why not print this in psp_ras_trigger_error(). 
i.e. caller interprets the error code and prints the appropriate 
message. I guess that is the single entry point to send TRIGGER_ERROR 
command.

Thanks,
Lijo

> +		break;
>   	default:
>   		dev_warn(psp->adev->dev,
>   				"RAS WARNING: ras status = 0x%X\n", ras_cmd->ras_status);
> @@ -1521,7 +1527,9 @@ int psp_ras_trigger_error(struct psp_context *psp,
>   	if (amdgpu_ras_intr_triggered())
>   		return 0;
>   
> -	if (ras_cmd->ras_status)
> +	if (ras_cmd->ras_status == TA_RAS_STATUS__TEE_ERROR_ACCESS_DENIED)
> +		return -EACCES;
> +	else if (ras_cmd->ras_status)
>   		return -EINVAL;
>   
>   	return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> index e674dbed3615..8bdc2e85cb20 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -449,7 +449,7 @@ static ssize_t amdgpu_ras_debugfs_ctrl_write(struct file *f,
>   	}
>   
>   	if (ret)
> -		return -EINVAL;
> +		return ret;
>   
>   	return size;
>   }
> diff --git a/drivers/gpu/drm/amd/amdgpu/ta_ras_if.h b/drivers/gpu/drm/amd/amdgpu/ta_ras_if.h
> index 5093826a43d1..509d8a1945eb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/ta_ras_if.h
> +++ b/drivers/gpu/drm/amd/amdgpu/ta_ras_if.h
> @@ -64,7 +64,8 @@ enum ta_ras_status {
>   	TA_RAS_STATUS__ERROR_PCS_STATE_ERROR            = 0xA016,
>   	TA_RAS_STATUS__ERROR_PCS_STATE_HANG             = 0xA017,
>   	TA_RAS_STATUS__ERROR_PCS_STATE_UNKNOWN          = 0xA018,
> -	TA_RAS_STATUS__ERROR_UNSUPPORTED_ERROR_INJ      = 0xA019
> +	TA_RAS_STATUS__ERROR_UNSUPPORTED_ERROR_INJ      = 0xA019,
> +	TA_RAS_STATUS__TEE_ERROR_ACCESS_DENIED          = 0xA01A
>   };
>   
>   enum ta_ras_block {
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-01-12 15:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-12  1:42 [PATCH Review 1/1] drm/amdgpu: handle denied inject error into critical regions Stanley.Yang
2022-01-12  8:51 ` Zhou1, Tao
2022-01-12 15:20 ` Lazar, Lijo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.