RE: [PATCH] drm/amdgpu: add mutex to protect ras shared memory
[AMD Official Use Only - AMD Internal Distribution Only] BTW, can we consider leveraging psp->mutext to protect both shared memory and command submission? Regards, Hawking -Original Message- From: Chai, Thomas Sent: Friday, June 28, 2024 18:01 To: Zhang, Hawking ; amd-gfx@lists.freedesktop.org Cc: Zhou1, Tao ; Li, Candice ; Wang, Yang(Kevin) ; Yang, Stanley Subject: RE: [PATCH] drm/amdgpu: add mutex to protect ras shared memory [AMD Official Use Only - AMD Internal Distribution Only] OK - Best Regards, Thomas -Original Message- From: Zhang, Hawking Sent: Friday, June 28, 2024 5:42 PM To: Chai, Thomas ; amd-gfx@lists.freedesktop.org Cc: Zhou1, Tao ; Li, Candice ; Wang, Yang(Kevin) ; Yang, Stanley ; Chai, Thomas Subject: RE: [PATCH] drm/amdgpu: add mutex to protect ras shared memory [AMD Official Use Only - AMD Internal Distribution Only] + ret = psp_ras_invoke(psp, ras_cmd->cmd_id); + case TA_RAS_COMMAND__TRIGGER_ERROR: + if (out) { + uint32_t *ras_status = (uint32_t *)out; + + *ras_status = ras_cmd->ras_status; + } + break; + case TA_RAS_COMMAND__QUERY_ADDRESS: + if (ret || ras_cmd->ras_status || psp->cmd_buf_mem->resp.status) + ret = -EINVAL; Shall we make the following check for both TRIGGER_ERROR and QUERY_ADDRESS command? + if (ret || ras_cmd->ras_status || psp->cmd_buf_mem->resp.status) + ret = -EINVAL; Regards, Hawking -Original Message- From: amd-gfx On Behalf Of YiPeng Chai Sent: Wednesday, June 26, 2024 10:20 To: amd-gfx@lists.freedesktop.org Cc: Zhang, Hawking ; Zhou1, Tao ; Li, Candice ; Wang, Yang(Kevin) ; Yang, Stanley ; Chai, Thomas Subject: [PATCH] drm/amdgpu: add mutex to protect ras shared memory Add mutex to protect ras shared memory. Signed-off-by: YiPeng Chai --- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c| 124 ++--- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_psp_ta.c | 2 + 3 files changed, 87 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c index fec968d8340f..323b3c470c5c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c @@ -1573,6 +1573,69 @@ static void psp_ras_ta_check_status(struct psp_context *psp) } } +static int psp_ras_send_cmd(struct psp_context *psp, + enum ras_command cmd_id, void *in, void *out) { + struct ta_ras_shared_memory *ras_cmd; + uint32_t cmd = cmd_id; + int ret = 0; + + if (!in) + return -EINVAL; + + mutex_lock(>ras_context.mutex); + ras_cmd = (struct ta_ras_shared_memory *)psp->ras_context.context.mem_context.shared_buf; + memset(ras_cmd, 0, sizeof(struct ta_ras_shared_memory)); + + switch (cmd) { + case TA_RAS_COMMAND__ENABLE_FEATURES: + case TA_RAS_COMMAND__DISABLE_FEATURES: + memcpy(_cmd->ras_in_message, + in, sizeof(ras_cmd->ras_in_message)); + break; + case TA_RAS_COMMAND__TRIGGER_ERROR: + memcpy(_cmd->ras_in_message.trigger_error, + in, sizeof(ras_cmd->ras_in_message.trigger_error)); + break; + case TA_RAS_COMMAND__QUERY_ADDRESS: + memcpy(_cmd->ras_in_message.address, + in, sizeof(ras_cmd->ras_in_message.address)); + break; + default: + dev_err(psp->adev->dev, "Invalid ras cmd id: %u\n", cmd); + ret = -EINVAL; + goto err_out; + } + + ras_cmd->cmd_id = cmd; + ret = psp_ras_invoke(psp, ras_cmd->cmd_id); + + switch (cmd) { + case TA_RAS_COMMAND__TRIGGER_ERROR: + if (out) { + uint32_t *ras_status = (uint32_t *)out; + + *ras_status = ras_cmd->ras_status; + } + break; + case TA_RAS_COMMAND__QUERY_ADDRESS: + if (ret || ras_cmd->ras_status || psp->cmd_buf_mem->resp.status) + ret = -EINVAL; + else if (out) + memcpy(out, + _cmd->ras_out_message.address, + sizeof(ras_cmd->ras_out_message.address)); + break; + default: + break; + } + +err_out: + mutex_unlock(>ras_context.mutex); + + return ret; +} + int psp_ras_invoke(struct psp_context *psp, uint32_t ta_cmd_id) { struct ta_ras_shared_memory *ras_cmd; @@ -1614,23 +1677,15 @@ int psp_ras_invoke(struct psp_context *psp, uint32_t ta_cmd_id) int psp_ras_enable_features(struct
RE: [PATCH] drm/amdgpu: add mutex to protect ras shared memory
[AMD Official Use Only - AMD Internal Distribution Only] OK - Best Regards, Thomas -Original Message- From: Zhang, Hawking Sent: Friday, June 28, 2024 5:42 PM To: Chai, Thomas ; amd-gfx@lists.freedesktop.org Cc: Zhou1, Tao ; Li, Candice ; Wang, Yang(Kevin) ; Yang, Stanley ; Chai, Thomas Subject: RE: [PATCH] drm/amdgpu: add mutex to protect ras shared memory [AMD Official Use Only - AMD Internal Distribution Only] + ret = psp_ras_invoke(psp, ras_cmd->cmd_id); + case TA_RAS_COMMAND__TRIGGER_ERROR: + if (out) { + uint32_t *ras_status = (uint32_t *)out; + + *ras_status = ras_cmd->ras_status; + } + break; + case TA_RAS_COMMAND__QUERY_ADDRESS: + if (ret || ras_cmd->ras_status || psp->cmd_buf_mem->resp.status) + ret = -EINVAL; Shall we make the following check for both TRIGGER_ERROR and QUERY_ADDRESS command? + if (ret || ras_cmd->ras_status || psp->cmd_buf_mem->resp.status) + ret = -EINVAL; Regards, Hawking -Original Message- From: amd-gfx On Behalf Of YiPeng Chai Sent: Wednesday, June 26, 2024 10:20 To: amd-gfx@lists.freedesktop.org Cc: Zhang, Hawking ; Zhou1, Tao ; Li, Candice ; Wang, Yang(Kevin) ; Yang, Stanley ; Chai, Thomas Subject: [PATCH] drm/amdgpu: add mutex to protect ras shared memory Add mutex to protect ras shared memory. Signed-off-by: YiPeng Chai --- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c| 124 ++--- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_psp_ta.c | 2 + 3 files changed, 87 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c index fec968d8340f..323b3c470c5c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c @@ -1573,6 +1573,69 @@ static void psp_ras_ta_check_status(struct psp_context *psp) } } +static int psp_ras_send_cmd(struct psp_context *psp, + enum ras_command cmd_id, void *in, void *out) { + struct ta_ras_shared_memory *ras_cmd; + uint32_t cmd = cmd_id; + int ret = 0; + + if (!in) + return -EINVAL; + + mutex_lock(>ras_context.mutex); + ras_cmd = (struct ta_ras_shared_memory *)psp->ras_context.context.mem_context.shared_buf; + memset(ras_cmd, 0, sizeof(struct ta_ras_shared_memory)); + + switch (cmd) { + case TA_RAS_COMMAND__ENABLE_FEATURES: + case TA_RAS_COMMAND__DISABLE_FEATURES: + memcpy(_cmd->ras_in_message, + in, sizeof(ras_cmd->ras_in_message)); + break; + case TA_RAS_COMMAND__TRIGGER_ERROR: + memcpy(_cmd->ras_in_message.trigger_error, + in, sizeof(ras_cmd->ras_in_message.trigger_error)); + break; + case TA_RAS_COMMAND__QUERY_ADDRESS: + memcpy(_cmd->ras_in_message.address, + in, sizeof(ras_cmd->ras_in_message.address)); + break; + default: + dev_err(psp->adev->dev, "Invalid ras cmd id: %u\n", cmd); + ret = -EINVAL; + goto err_out; + } + + ras_cmd->cmd_id = cmd; + ret = psp_ras_invoke(psp, ras_cmd->cmd_id); + + switch (cmd) { + case TA_RAS_COMMAND__TRIGGER_ERROR: + if (out) { + uint32_t *ras_status = (uint32_t *)out; + + *ras_status = ras_cmd->ras_status; + } + break; + case TA_RAS_COMMAND__QUERY_ADDRESS: + if (ret || ras_cmd->ras_status || psp->cmd_buf_mem->resp.status) + ret = -EINVAL; + else if (out) + memcpy(out, + _cmd->ras_out_message.address, + sizeof(ras_cmd->ras_out_message.address)); + break; + default: + break; + } + +err_out: + mutex_unlock(>ras_context.mutex); + + return ret; +} + int psp_ras_invoke(struct psp_context *psp, uint32_t ta_cmd_id) { struct ta_ras_shared_memory *ras_cmd; @@ -1614,23 +1677,15 @@ int psp_ras_invoke(struct psp_context *psp, uint32_t ta_cmd_id) int psp_ras_enable_features(struct psp_context *psp, union ta_ras_cmd_input *info, bool enable) { - struct ta_ras_shared_memory *ras_cmd; + enum ras_command cmd_id; int ret; - if (!psp->ras_context.context.initialized) + if (!psp->ras_context.context.initialized || !info) return -EINVAL; - ras_cmd = (struct ta_ras_shared_memory *)psp->ras_context.context.mem_context.shared_buf; - memset(ras
RE: [PATCH] drm/amdgpu: add mutex to protect ras shared memory
[AMD Official Use Only - AMD Internal Distribution Only] + ret = psp_ras_invoke(psp, ras_cmd->cmd_id); + case TA_RAS_COMMAND__TRIGGER_ERROR: + if (out) { + uint32_t *ras_status = (uint32_t *)out; + + *ras_status = ras_cmd->ras_status; + } + break; + case TA_RAS_COMMAND__QUERY_ADDRESS: + if (ret || ras_cmd->ras_status || psp->cmd_buf_mem->resp.status) + ret = -EINVAL; Shall we make the following check for both TRIGGER_ERROR and QUERY_ADDRESS command? + if (ret || ras_cmd->ras_status || psp->cmd_buf_mem->resp.status) + ret = -EINVAL; Regards, Hawking -Original Message- From: amd-gfx On Behalf Of YiPeng Chai Sent: Wednesday, June 26, 2024 10:20 To: amd-gfx@lists.freedesktop.org Cc: Zhang, Hawking ; Zhou1, Tao ; Li, Candice ; Wang, Yang(Kevin) ; Yang, Stanley ; Chai, Thomas Subject: [PATCH] drm/amdgpu: add mutex to protect ras shared memory Add mutex to protect ras shared memory. Signed-off-by: YiPeng Chai --- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c| 124 ++--- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_psp_ta.c | 2 + 3 files changed, 87 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c index fec968d8340f..323b3c470c5c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c @@ -1573,6 +1573,69 @@ static void psp_ras_ta_check_status(struct psp_context *psp) } } +static int psp_ras_send_cmd(struct psp_context *psp, + enum ras_command cmd_id, void *in, void *out) { + struct ta_ras_shared_memory *ras_cmd; + uint32_t cmd = cmd_id; + int ret = 0; + + if (!in) + return -EINVAL; + + mutex_lock(>ras_context.mutex); + ras_cmd = (struct ta_ras_shared_memory *)psp->ras_context.context.mem_context.shared_buf; + memset(ras_cmd, 0, sizeof(struct ta_ras_shared_memory)); + + switch (cmd) { + case TA_RAS_COMMAND__ENABLE_FEATURES: + case TA_RAS_COMMAND__DISABLE_FEATURES: + memcpy(_cmd->ras_in_message, + in, sizeof(ras_cmd->ras_in_message)); + break; + case TA_RAS_COMMAND__TRIGGER_ERROR: + memcpy(_cmd->ras_in_message.trigger_error, + in, sizeof(ras_cmd->ras_in_message.trigger_error)); + break; + case TA_RAS_COMMAND__QUERY_ADDRESS: + memcpy(_cmd->ras_in_message.address, + in, sizeof(ras_cmd->ras_in_message.address)); + break; + default: + dev_err(psp->adev->dev, "Invalid ras cmd id: %u\n", cmd); + ret = -EINVAL; + goto err_out; + } + + ras_cmd->cmd_id = cmd; + ret = psp_ras_invoke(psp, ras_cmd->cmd_id); + + switch (cmd) { + case TA_RAS_COMMAND__TRIGGER_ERROR: + if (out) { + uint32_t *ras_status = (uint32_t *)out; + + *ras_status = ras_cmd->ras_status; + } + break; + case TA_RAS_COMMAND__QUERY_ADDRESS: + if (ret || ras_cmd->ras_status || psp->cmd_buf_mem->resp.status) + ret = -EINVAL; + else if (out) + memcpy(out, + _cmd->ras_out_message.address, + sizeof(ras_cmd->ras_out_message.address)); + break; + default: + break; + } + +err_out: + mutex_unlock(>ras_context.mutex); + + return ret; +} + int psp_ras_invoke(struct psp_context *psp, uint32_t ta_cmd_id) { struct ta_ras_shared_memory *ras_cmd; @@ -1614,23 +1677,15 @@ int psp_ras_invoke(struct psp_context *psp, uint32_t ta_cmd_id) int psp_ras_enable_features(struct psp_context *psp, union ta_ras_cmd_input *info, bool enable) { - struct ta_ras_shared_memory *ras_cmd; + enum ras_command cmd_id; int ret; - if (!psp->ras_context.context.initialized) + if (!psp->ras_context.context.initialized || !info) return -EINVAL; - ras_cmd = (struct ta_ras_shared_memory *)psp->ras_context.context.mem_context.shared_buf; - memset(ras_cmd, 0, sizeof(struct ta_ras_shared_memory)); - - if (enable) - ras_cmd->cmd_id = TA_RAS_COMMAND__ENABLE_FEATURES; - else - ras_cmd->cmd_id = TA_RAS_COMMAND__DISABLE_FEATURES; - - ras_cmd->ras_in_message = *info; - - ret = psp_ras_invoke(psp, ras_cmd->cmd_id); + cmd_id = enable ? + TA_RAS_COMMAND__ENABLE_FEATUR
[PATCH] drm/amdgpu: add mutex to protect ras shared memory
Add mutex to protect ras shared memory. Signed-off-by: YiPeng Chai --- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c| 124 ++--- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_psp_ta.c | 2 + 3 files changed, 87 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c index fec968d8340f..323b3c470c5c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c @@ -1573,6 +1573,69 @@ static void psp_ras_ta_check_status(struct psp_context *psp) } } +static int psp_ras_send_cmd(struct psp_context *psp, + enum ras_command cmd_id, void *in, void *out) +{ + struct ta_ras_shared_memory *ras_cmd; + uint32_t cmd = cmd_id; + int ret = 0; + + if (!in) + return -EINVAL; + + mutex_lock(>ras_context.mutex); + ras_cmd = (struct ta_ras_shared_memory *)psp->ras_context.context.mem_context.shared_buf; + memset(ras_cmd, 0, sizeof(struct ta_ras_shared_memory)); + + switch (cmd) { + case TA_RAS_COMMAND__ENABLE_FEATURES: + case TA_RAS_COMMAND__DISABLE_FEATURES: + memcpy(_cmd->ras_in_message, + in, sizeof(ras_cmd->ras_in_message)); + break; + case TA_RAS_COMMAND__TRIGGER_ERROR: + memcpy(_cmd->ras_in_message.trigger_error, + in, sizeof(ras_cmd->ras_in_message.trigger_error)); + break; + case TA_RAS_COMMAND__QUERY_ADDRESS: + memcpy(_cmd->ras_in_message.address, + in, sizeof(ras_cmd->ras_in_message.address)); + break; + default: + dev_err(psp->adev->dev, "Invalid ras cmd id: %u\n", cmd); + ret = -EINVAL; + goto err_out; + } + + ras_cmd->cmd_id = cmd; + ret = psp_ras_invoke(psp, ras_cmd->cmd_id); + + switch (cmd) { + case TA_RAS_COMMAND__TRIGGER_ERROR: + if (out) { + uint32_t *ras_status = (uint32_t *)out; + + *ras_status = ras_cmd->ras_status; + } + break; + case TA_RAS_COMMAND__QUERY_ADDRESS: + if (ret || ras_cmd->ras_status || psp->cmd_buf_mem->resp.status) + ret = -EINVAL; + else if (out) + memcpy(out, + _cmd->ras_out_message.address, + sizeof(ras_cmd->ras_out_message.address)); + break; + default: + break; + } + +err_out: + mutex_unlock(>ras_context.mutex); + + return ret; +} + int psp_ras_invoke(struct psp_context *psp, uint32_t ta_cmd_id) { struct ta_ras_shared_memory *ras_cmd; @@ -1614,23 +1677,15 @@ int psp_ras_invoke(struct psp_context *psp, uint32_t ta_cmd_id) int psp_ras_enable_features(struct psp_context *psp, union ta_ras_cmd_input *info, bool enable) { - struct ta_ras_shared_memory *ras_cmd; + enum ras_command cmd_id; int ret; - if (!psp->ras_context.context.initialized) + if (!psp->ras_context.context.initialized || !info) return -EINVAL; - ras_cmd = (struct ta_ras_shared_memory *)psp->ras_context.context.mem_context.shared_buf; - memset(ras_cmd, 0, sizeof(struct ta_ras_shared_memory)); - - if (enable) - ras_cmd->cmd_id = TA_RAS_COMMAND__ENABLE_FEATURES; - else - ras_cmd->cmd_id = TA_RAS_COMMAND__DISABLE_FEATURES; - - ras_cmd->ras_in_message = *info; - - ret = psp_ras_invoke(psp, ras_cmd->cmd_id); + cmd_id = enable ? + TA_RAS_COMMAND__ENABLE_FEATURES : TA_RAS_COMMAND__DISABLE_FEATURES; + ret = psp_ras_send_cmd(psp, cmd_id, info, NULL); if (ret) return -EINVAL; @@ -1654,6 +1709,8 @@ int psp_ras_terminate(struct psp_context *psp) psp->ras_context.context.initialized = false; + mutex_destroy(>ras_context.mutex); + return ret; } @@ -1738,9 +1795,10 @@ int psp_ras_initialize(struct psp_context *psp) ret = psp_ta_load(psp, >ras_context.context); - if (!ret && !ras_cmd->ras_status) + if (!ret && !ras_cmd->ras_status) { psp->ras_context.context.initialized = true; - else { + mutex_init(>ras_context.mutex); + } else { if (ras_cmd->ras_status) dev_warn(adev->dev, "RAS Init Status: 0x%X\n", ras_cmd->ras_status); @@ -1754,12 +1812,12 @@ int psp_ras_initialize(struct psp_context *psp) int psp_ras_trigger_error(struct psp_context *psp, struct ta_ras_trigger_error_input *info, uint32_t instance_mask) { - struct ta_ras_shared_memory *ras_cmd; struct amdgpu_device *adev = psp->adev;
Re: [PATCH] drm/amdgpu: add mutex to protect ras shared memory
On 4/28/2024 12:38 PM, YiPeng Chai wrote: > Add mutex to protect ras shared memory. > > Signed-off-by: YiPeng Chai > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c| 121 ++--- > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h| 1 + > drivers/gpu/drm/amd/amdgpu/amdgpu_psp_ta.c | 2 + > 3 files changed, 84 insertions(+), 40 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > index 5583e2d1b12f..fa4fea00f6b4 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > @@ -1564,6 +1564,66 @@ static void psp_ras_ta_check_status(struct psp_context > *psp) > } > } > > +static int psp_ras_send_cmd(struct psp_context *psp, > + enum ras_command cmd_id, void *in, void *out) > +{ > + struct ta_ras_shared_memory *ras_cmd; > + uint32_t cmd = cmd_id; > + int ret = 0; > + > + mutex_lock(>ras_context.mutex); > + ras_cmd = (struct ta_ras_shared_memory > *)psp->ras_context.context.mem_context.shared_buf; > + memset(ras_cmd, 0, sizeof(struct ta_ras_shared_memory)); > + > + switch (cmd) { > + case TA_RAS_COMMAND__ENABLE_FEATURES: > + case TA_RAS_COMMAND__DISABLE_FEATURES: > + memcpy(_cmd->ras_in_message, > + in, sizeof(ras_cmd->ras_in_message)); > + break; > + case TA_RAS_COMMAND__TRIGGER_ERROR: > + memcpy(_cmd->ras_in_message.trigger_error, > + in, sizeof(ras_cmd->ras_in_message.trigger_error)); > + break; > + case TA_RAS_COMMAND__QUERY_ADDRESS: > + memcpy(_cmd->ras_in_message.address, > + in, sizeof(ras_cmd->ras_in_message.address)); > + break; > + default: > + dev_err(psp->adev->dev, "Invalid ras cmd id: %u\n", cmd); > + ret = -EINVAL; > + goto err_out; > + } > + > + ras_cmd->cmd_id = cmd; > + ret = psp_ras_invoke(psp, ras_cmd->cmd_id); > + > + switch (cmd) { > + case TA_RAS_COMMAND__TRIGGER_ERROR: > + if (out) { As NULL check for 'out' is done before copying, better to do the same for 'in' also. > + uint32_t *ras_status = (uint32_t *)out; > + > + *ras_status = ras_cmd->ras_status; > + } > + break; > + case TA_RAS_COMMAND__QUERY_ADDRESS: > + if (ret || ras_cmd->ras_status || psp->cmd_buf_mem->resp.status) > + ret = -EINVAL; > + else if (out) > + memcpy(out, > + _cmd->ras_out_message.address, > + sizeof(ras_cmd->ras_out_message.address)); > + break; > + default: > + break; > + } > + > +err_out: > + mutex_unlock(>ras_context.mutex); > + > + return ret; > +} > + > int psp_ras_invoke(struct psp_context *psp, uint32_t ta_cmd_id) > { > struct ta_ras_shared_memory *ras_cmd; > @@ -1605,23 +1665,15 @@ int psp_ras_invoke(struct psp_context *psp, uint32_t > ta_cmd_id) > int psp_ras_enable_features(struct psp_context *psp, > union ta_ras_cmd_input *info, bool enable) > { > - struct ta_ras_shared_memory *ras_cmd; > + enum ras_command cmd_id; > int ret; > > - if (!psp->ras_context.context.initialized) > + if (!psp->ras_context.context.initialized || !info) > return -EINVAL; > > - ras_cmd = (struct ta_ras_shared_memory > *)psp->ras_context.context.mem_context.shared_buf; > - memset(ras_cmd, 0, sizeof(struct ta_ras_shared_memory)); > - > - if (enable) > - ras_cmd->cmd_id = TA_RAS_COMMAND__ENABLE_FEATURES; > - else > - ras_cmd->cmd_id = TA_RAS_COMMAND__DISABLE_FEATURES; > - > - ras_cmd->ras_in_message = *info; > - > - ret = psp_ras_invoke(psp, ras_cmd->cmd_id); > + cmd_id = enable ? > + TA_RAS_COMMAND__ENABLE_FEATURES : > TA_RAS_COMMAND__DISABLE_FEATURES; > + ret = psp_ras_send_cmd(psp, cmd_id, info, NULL); > if (ret) > return -EINVAL; > > @@ -1645,6 +1697,8 @@ int psp_ras_terminate(struct psp_context *psp) > > psp->ras_context.context.initialized = false; > > + mutex_destroy(>ras_context.mutex); > + > return ret; > } > > @@ -1729,9 +1783,10 @@ int psp_ras_initialize(struct psp_context *psp) > > ret = psp_ta_load(psp, >ras_context.context); > > - if (!ret && !ras_cmd->ras_status) > + if (!ret && !ras_cmd->ras_status) { > psp->ras_context.context.initialized = true; > - else { > + mutex_init(>ras_context.mutex); > + } else { > if (ras_cmd->ras_status) > dev_warn(adev->dev, "RAS Init Status: 0x%X\n", > ras_cmd->ras_status); > > @@ -1745,12 +1800,12 @@ int psp_ras_initialize(struct psp_context *psp) > int psp_ras_trigger_error(struct psp_context *psp, >
RE: [PATCH] drm/amdgpu: add mutex to protect ras shared memory
[AMD Official Use Only - General] OK - Best Regards, Thomas -Original Message- From: Wang, Yang(Kevin) Sent: Sunday, April 28, 2024 3:48 PM To: Chai, Thomas ; amd-gfx@lists.freedesktop.org Cc: Zhang, Hawking ; Zhou1, Tao ; Li, Candice ; Yang, Stanley Subject: RE: [PATCH] drm/amdgpu: add mutex to protect ras shared memory [AMD Official Use Only - General] -Original Message- From: Chai, Thomas Sent: Sunday, April 28, 2024 3:08 PM To: amd-gfx@lists.freedesktop.org Cc: Zhang, Hawking ; Zhou1, Tao ; Li, Candice ; Wang, Yang(Kevin) ; Yang, Stanley ; Chai, Thomas Subject: [PATCH] drm/amdgpu: add mutex to protect ras shared memory Add mutex to protect ras shared memory. Signed-off-by: YiPeng Chai --- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c| 121 ++--- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_psp_ta.c | 2 + 3 files changed, 84 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c index 5583e2d1b12f..fa4fea00f6b4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c @@ -1564,6 +1564,66 @@ static void psp_ras_ta_check_status(struct psp_context *psp) } } +static int psp_ras_send_cmd(struct psp_context *psp, + enum ras_command cmd_id, void *in, void *out) { + struct ta_ras_shared_memory *ras_cmd; + uint32_t cmd = cmd_id; + int ret = 0; + + mutex_lock(>ras_context.mutex); + ras_cmd = (struct ta_ras_shared_memory *)psp->ras_context.context.mem_context.shared_buf; + memset(ras_cmd, 0, sizeof(struct ta_ras_shared_memory)); + + switch (cmd) { + case TA_RAS_COMMAND__ENABLE_FEATURES: + case TA_RAS_COMMAND__DISABLE_FEATURES: + memcpy(_cmd->ras_in_message, + in, sizeof(ras_cmd->ras_in_message)); + break; + case TA_RAS_COMMAND__TRIGGER_ERROR: + memcpy(_cmd->ras_in_message.trigger_error, + in, sizeof(ras_cmd->ras_in_message.trigger_error)); + break; + case TA_RAS_COMMAND__QUERY_ADDRESS: + memcpy(_cmd->ras_in_message.address, + in, sizeof(ras_cmd->ras_in_message.address)); + break; + default: + dev_err(psp->adev->dev, "Invalid ras cmd id: %u\n", cmd); + ret = -EINVAL; + goto err_out; + } + + ras_cmd->cmd_id = cmd; + ret = psp_ras_invoke(psp, ras_cmd->cmd_id); + + switch (cmd) { + case TA_RAS_COMMAND__TRIGGER_ERROR: + if (out) { + uint32_t *ras_status = (uint32_t *)out; [Kevin]: It's better to check 'ret' value first before use this 'out' data. Best Regards, Kevin + + *ras_status = ras_cmd->ras_status; + } + break; + case TA_RAS_COMMAND__QUERY_ADDRESS: + if (ret || ras_cmd->ras_status || psp->cmd_buf_mem->resp.status) + ret = -EINVAL; + else if (out) + memcpy(out, + _cmd->ras_out_message.address, + sizeof(ras_cmd->ras_out_message.address)); + break; + default: + break; + } + +err_out: + mutex_unlock(>ras_context.mutex); + + return ret; +} + int psp_ras_invoke(struct psp_context *psp, uint32_t ta_cmd_id) { struct ta_ras_shared_memory *ras_cmd; @@ -1605,23 +1665,15 @@ int psp_ras_invoke(struct psp_context *psp, uint32_t ta_cmd_id) int psp_ras_enable_features(struct psp_context *psp, union ta_ras_cmd_input *info, bool enable) { - struct ta_ras_shared_memory *ras_cmd; + enum ras_command cmd_id; int ret; - if (!psp->ras_context.context.initialized) + if (!psp->ras_context.context.initialized || !info) return -EINVAL; - ras_cmd = (struct ta_ras_shared_memory *)psp->ras_context.context.mem_context.shared_buf; - memset(ras_cmd, 0, sizeof(struct ta_ras_shared_memory)); - - if (enable) - ras_cmd->cmd_id = TA_RAS_COMMAND__ENABLE_FEATURES; - else - ras_cmd->cmd_id = TA_RAS_COMMAND__DISABLE_FEATURES; - - ras_cmd->ras_in_message = *info; - - ret = psp_ras_invoke(psp, ras_cmd->cmd_id); + cmd_id = enable ? + TA_RAS_COMMAND__ENABLE_FEATURES : TA_RAS_COMMAND__DISABLE_FEATURES; + ret = psp_ras_send_cmd(psp, cmd_id, info, NULL); if (ret) return -EINVAL; @@ -1645,6 +1697,8 @@ int psp_ras_terminate(struct psp_context *psp) psp->ras_context.context.initialized = false; + mutex_destroy(>ras_context.mutex); + return ret; } @@
RE: [PATCH] drm/amdgpu: add mutex to protect ras shared memory
[AMD Official Use Only - General] -Original Message- From: Chai, Thomas Sent: Sunday, April 28, 2024 3:08 PM To: amd-gfx@lists.freedesktop.org Cc: Zhang, Hawking ; Zhou1, Tao ; Li, Candice ; Wang, Yang(Kevin) ; Yang, Stanley ; Chai, Thomas Subject: [PATCH] drm/amdgpu: add mutex to protect ras shared memory Add mutex to protect ras shared memory. Signed-off-by: YiPeng Chai --- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c| 121 ++--- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_psp_ta.c | 2 + 3 files changed, 84 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c index 5583e2d1b12f..fa4fea00f6b4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c @@ -1564,6 +1564,66 @@ static void psp_ras_ta_check_status(struct psp_context *psp) } } +static int psp_ras_send_cmd(struct psp_context *psp, + enum ras_command cmd_id, void *in, void *out) { + struct ta_ras_shared_memory *ras_cmd; + uint32_t cmd = cmd_id; + int ret = 0; + + mutex_lock(>ras_context.mutex); + ras_cmd = (struct ta_ras_shared_memory *)psp->ras_context.context.mem_context.shared_buf; + memset(ras_cmd, 0, sizeof(struct ta_ras_shared_memory)); + + switch (cmd) { + case TA_RAS_COMMAND__ENABLE_FEATURES: + case TA_RAS_COMMAND__DISABLE_FEATURES: + memcpy(_cmd->ras_in_message, + in, sizeof(ras_cmd->ras_in_message)); + break; + case TA_RAS_COMMAND__TRIGGER_ERROR: + memcpy(_cmd->ras_in_message.trigger_error, + in, sizeof(ras_cmd->ras_in_message.trigger_error)); + break; + case TA_RAS_COMMAND__QUERY_ADDRESS: + memcpy(_cmd->ras_in_message.address, + in, sizeof(ras_cmd->ras_in_message.address)); + break; + default: + dev_err(psp->adev->dev, "Invalid ras cmd id: %u\n", cmd); + ret = -EINVAL; + goto err_out; + } + + ras_cmd->cmd_id = cmd; + ret = psp_ras_invoke(psp, ras_cmd->cmd_id); + + switch (cmd) { + case TA_RAS_COMMAND__TRIGGER_ERROR: + if (out) { + uint32_t *ras_status = (uint32_t *)out; [Kevin]: It's better to check 'ret' value first before use this 'out' data. Best Regards, Kevin + + *ras_status = ras_cmd->ras_status; + } + break; + case TA_RAS_COMMAND__QUERY_ADDRESS: + if (ret || ras_cmd->ras_status || psp->cmd_buf_mem->resp.status) + ret = -EINVAL; + else if (out) + memcpy(out, + _cmd->ras_out_message.address, + sizeof(ras_cmd->ras_out_message.address)); + break; + default: + break; + } + +err_out: + mutex_unlock(>ras_context.mutex); + + return ret; +} + int psp_ras_invoke(struct psp_context *psp, uint32_t ta_cmd_id) { struct ta_ras_shared_memory *ras_cmd; @@ -1605,23 +1665,15 @@ int psp_ras_invoke(struct psp_context *psp, uint32_t ta_cmd_id) int psp_ras_enable_features(struct psp_context *psp, union ta_ras_cmd_input *info, bool enable) { - struct ta_ras_shared_memory *ras_cmd; + enum ras_command cmd_id; int ret; - if (!psp->ras_context.context.initialized) + if (!psp->ras_context.context.initialized || !info) return -EINVAL; - ras_cmd = (struct ta_ras_shared_memory *)psp->ras_context.context.mem_context.shared_buf; - memset(ras_cmd, 0, sizeof(struct ta_ras_shared_memory)); - - if (enable) - ras_cmd->cmd_id = TA_RAS_COMMAND__ENABLE_FEATURES; - else - ras_cmd->cmd_id = TA_RAS_COMMAND__DISABLE_FEATURES; - - ras_cmd->ras_in_message = *info; - - ret = psp_ras_invoke(psp, ras_cmd->cmd_id); + cmd_id = enable ? + TA_RAS_COMMAND__ENABLE_FEATURES : TA_RAS_COMMAND__DISABLE_FEATURES; + ret = psp_ras_send_cmd(psp, cmd_id, info, NULL); if (ret) return -EINVAL; @@ -1645,6 +1697,8 @@ int psp_ras_terminate(struct psp_context *psp) psp->ras_context.context.initialized = false; + mutex_destroy(>ras_context.mutex); + return ret; } @@ -1729,9 +1783,10 @@ int psp_ras_initialize(struct psp_context *psp) ret = psp_ta_load(psp, >ras_context.context); - if (!ret && !ras_cmd->ras_status) + if (!ret && !ras_cmd->ras_status) { psp->ras_context.context.initialized = true; - else { + mutex_init(>ras_co
[PATCH] drm/amdgpu: add mutex to protect ras shared memory
Add mutex to protect ras shared memory. Signed-off-by: YiPeng Chai --- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c| 121 ++--- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_psp_ta.c | 2 + 3 files changed, 84 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c index 5583e2d1b12f..fa4fea00f6b4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c @@ -1564,6 +1564,66 @@ static void psp_ras_ta_check_status(struct psp_context *psp) } } +static int psp_ras_send_cmd(struct psp_context *psp, + enum ras_command cmd_id, void *in, void *out) +{ + struct ta_ras_shared_memory *ras_cmd; + uint32_t cmd = cmd_id; + int ret = 0; + + mutex_lock(>ras_context.mutex); + ras_cmd = (struct ta_ras_shared_memory *)psp->ras_context.context.mem_context.shared_buf; + memset(ras_cmd, 0, sizeof(struct ta_ras_shared_memory)); + + switch (cmd) { + case TA_RAS_COMMAND__ENABLE_FEATURES: + case TA_RAS_COMMAND__DISABLE_FEATURES: + memcpy(_cmd->ras_in_message, + in, sizeof(ras_cmd->ras_in_message)); + break; + case TA_RAS_COMMAND__TRIGGER_ERROR: + memcpy(_cmd->ras_in_message.trigger_error, + in, sizeof(ras_cmd->ras_in_message.trigger_error)); + break; + case TA_RAS_COMMAND__QUERY_ADDRESS: + memcpy(_cmd->ras_in_message.address, + in, sizeof(ras_cmd->ras_in_message.address)); + break; + default: + dev_err(psp->adev->dev, "Invalid ras cmd id: %u\n", cmd); + ret = -EINVAL; + goto err_out; + } + + ras_cmd->cmd_id = cmd; + ret = psp_ras_invoke(psp, ras_cmd->cmd_id); + + switch (cmd) { + case TA_RAS_COMMAND__TRIGGER_ERROR: + if (out) { + uint32_t *ras_status = (uint32_t *)out; + + *ras_status = ras_cmd->ras_status; + } + break; + case TA_RAS_COMMAND__QUERY_ADDRESS: + if (ret || ras_cmd->ras_status || psp->cmd_buf_mem->resp.status) + ret = -EINVAL; + else if (out) + memcpy(out, + _cmd->ras_out_message.address, + sizeof(ras_cmd->ras_out_message.address)); + break; + default: + break; + } + +err_out: + mutex_unlock(>ras_context.mutex); + + return ret; +} + int psp_ras_invoke(struct psp_context *psp, uint32_t ta_cmd_id) { struct ta_ras_shared_memory *ras_cmd; @@ -1605,23 +1665,15 @@ int psp_ras_invoke(struct psp_context *psp, uint32_t ta_cmd_id) int psp_ras_enable_features(struct psp_context *psp, union ta_ras_cmd_input *info, bool enable) { - struct ta_ras_shared_memory *ras_cmd; + enum ras_command cmd_id; int ret; - if (!psp->ras_context.context.initialized) + if (!psp->ras_context.context.initialized || !info) return -EINVAL; - ras_cmd = (struct ta_ras_shared_memory *)psp->ras_context.context.mem_context.shared_buf; - memset(ras_cmd, 0, sizeof(struct ta_ras_shared_memory)); - - if (enable) - ras_cmd->cmd_id = TA_RAS_COMMAND__ENABLE_FEATURES; - else - ras_cmd->cmd_id = TA_RAS_COMMAND__DISABLE_FEATURES; - - ras_cmd->ras_in_message = *info; - - ret = psp_ras_invoke(psp, ras_cmd->cmd_id); + cmd_id = enable ? + TA_RAS_COMMAND__ENABLE_FEATURES : TA_RAS_COMMAND__DISABLE_FEATURES; + ret = psp_ras_send_cmd(psp, cmd_id, info, NULL); if (ret) return -EINVAL; @@ -1645,6 +1697,8 @@ int psp_ras_terminate(struct psp_context *psp) psp->ras_context.context.initialized = false; + mutex_destroy(>ras_context.mutex); + return ret; } @@ -1729,9 +1783,10 @@ int psp_ras_initialize(struct psp_context *psp) ret = psp_ta_load(psp, >ras_context.context); - if (!ret && !ras_cmd->ras_status) + if (!ret && !ras_cmd->ras_status) { psp->ras_context.context.initialized = true; - else { + mutex_init(>ras_context.mutex); + } else { if (ras_cmd->ras_status) dev_warn(adev->dev, "RAS Init Status: 0x%X\n", ras_cmd->ras_status); @@ -1745,12 +1800,12 @@ int psp_ras_initialize(struct psp_context *psp) int psp_ras_trigger_error(struct psp_context *psp, struct ta_ras_trigger_error_input *info, uint32_t instance_mask) { - struct ta_ras_shared_memory *ras_cmd; struct amdgpu_device *adev = psp->adev; int ret; uint32_t dev_mask; +