[PATCH] drm/amd/pm: Prevent divide by zero

[PATCH] drm/amd/pm: Prevent divide by zero

[Yefim Barashkin]

divide error: 0000 [#1] SMP PTI
CPU: 3 PID: 78925 Comm: tee Not tainted 5.15.50-1-lts #1
Hardware name: MSI MS-7A59/Z270 SLI PLUS (MS-7A59), BIOS 1.90 01/30/2018
RIP: 0010:smu_v11_0_set_fan_speed_rpm+0x11/0x110 [amdgpu]

Speed is user-configurable through a file.
I accidentally set it to zero, and the driver crashed.

Signed-off-by: Yefim Barashkin <mr.b34r@kolabnow.com>
---
 drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
index 5f8809f6990d..69cebdb58c04 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
@@ -1236,6 +1236,10 @@ int smu_v11_0_set_fan_speed_rpm(struct smu_context *smu,
 	 * - For some Sienna Cichlid SKU, the fan speed cannot be set
 	 *   lower than 500 RPM.
 	 */
+
+	if (speed == 0)
+		return -EINVAL;
+
 	tach_period = 60 * crystal_clock_freq * 10000 / (8 * speed);
 	WREG32_SOC15(THM, 0, mmCG_TACH_CTRL,
 		     REG_SET_FIELD(RREG32_SOC15(THM, 0, mmCG_TACH_CTRL),
--
2.36.1

Sorry about the mess with my name and formatting. It should be fixed now, hopefully.

RE: [PATCH] drm/amd/pm: Prevent divide by zero

[Quan, Evan]

[AMD Official Use Only - General]

Thanks for the fix! Patch is reviewed-by: Evan Quan <evan.quan@amd.com>

BR
Evan
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Yefim Barashkin
> Sent: Tuesday, July 12, 2022 10:01 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Yefim Barashkin <mr.b34r@kolabnow.com>
> Subject: [PATCH] drm/amd/pm: Prevent divide by zero
> 
> divide error: 0000 [#1] SMP PTI
> CPU: 3 PID: 78925 Comm: tee Not tainted 5.15.50-1-lts #1 Hardware name:
> MSI MS-7A59/Z270 SLI PLUS (MS-7A59), BIOS 1.90 01/30/2018
> RIP: 0010:smu_v11_0_set_fan_speed_rpm+0x11/0x110 [amdgpu]
> 
> Speed is user-configurable through a file.
> I accidentally set it to zero, and the driver crashed.
> 
> Signed-off-by: Yefim Barashkin <mr.b34r@kolabnow.com>
> ---
>  drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> index 5f8809f6990d..69cebdb58c04 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> @@ -1236,6 +1236,10 @@ int smu_v11_0_set_fan_speed_rpm(struct
> smu_context *smu,
>  	 * - For some Sienna Cichlid SKU, the fan speed cannot be set
>  	 *   lower than 500 RPM.
>  	 */
> +
> +	if (speed == 0)
> +		return -EINVAL;
> +
>  	tach_period = 60 * crystal_clock_freq * 10000 / (8 * speed);
>  	WREG32_SOC15(THM, 0, mmCG_TACH_CTRL,
>  		     REG_SET_FIELD(RREG32_SOC15(THM, 0,
> mmCG_TACH_CTRL),
> --
> 2.36.1
> 
> Sorry about the mess with my name and formatting. It should be fixed now,
> hopefully.

Re: [PATCH] drm/amd/pm: Prevent divide by zero

[André Almeida]

Às 23:01 de 11/07/22, Yefim Barashkin escreveu:
> divide error: 0000 [#1] SMP PTI
> CPU: 3 PID: 78925 Comm: tee Not tainted 5.15.50-1-lts #1
> Hardware name: MSI MS-7A59/Z270 SLI PLUS (MS-7A59), BIOS 1.90 01/30/2018
> RIP: 0010:smu_v11_0_set_fan_speed_rpm+0x11/0x110 [amdgpu]
> 
> Speed is user-configurable through a file.
> I accidentally set it to zero, and the driver crashed.
> 
> Signed-off-by: Yefim Barashkin <mr.b34r@kolabnow.com>
> ---


Reviewed-by: André Almeida <andrealmeid@igalia.com>



Minor nit, I would write this commit message like this:

Speed is user-configurable through a file. I accidentally set it to
zero, and the driver crashed:

divide error: 0000 [#1] SMP PTI
CPU: 3 PID: 78925 Comm: tee Not tainted 5.15.50-1-lts #1
Hardware name: MSI MS-7A59/Z270 SLI PLUS (MS-7A59), BIOS 1.90 01/30/2018
RIP: 0010:smu_v11_0_set_fan_speed_rpm+0x11/0x110 [amdgpu]

Fix that by checking user input value.

Re: [PATCH] drm/amd/pm: Prevent divide by zero

[Alex Deucher]

Applied.  Thanks!

On Mon, Jul 11, 2022 at 9:06 PM Mr. B34r <mr.b34r@kolabnow.com> wrote:
>
>  From: Yefim Barashkin <mr.b34r@kolabnow.com>
>
> divide error: 0000 [#1] SMP PTI
> CPU: 3 PID: 78925 Comm: tee Not tainted 5.15.50-1-lts #1
> Hardware name: MSI MS-7A59/Z270 SLI PLUS (MS-7A59), BIOS 1.90 01/30/2018
> RIP: 0010:smu_v11_0_set_fan_speed_rpm+0x11/0x110 [amdgpu]
>
> Speed is user-configurable through a file.
> I accidentally set it to zero, and the driver crashed.
>
> Signed-off-by: Yefim Barashkin <mr.b34r@kolabnow.com>
> ---
>   drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> index 5f8809f6990d..69cebdb58c04 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> @@ -1236,6 +1236,10 @@ int smu_v11_0_set_fan_speed_rpm(struct
> smu_context *smu,
>       * - For some Sienna Cichlid SKU, the fan speed cannot be set
>       *   lower than 500 RPM.
>       */
> +
> +   if (speed == 0)
> +       return -EINVAL;
> +
>      tach_period = 60 * crystal_clock_freq * 10000 / (8 * speed);
>      WREG32_SOC15(THM, 0, mmCG_TACH_CTRL,
>               REG_SET_FIELD(RREG32_SOC15(THM, 0, mmCG_TACH_CTRL),
> --
> 2.36.1

Re: [PATCH] drm/amd/pm: Prevent divide by zero

[Mr. B34r]

 From: Yefim Barashkin <mr.b34r@kolabnow.com>

divide error: 0000 [#1] SMP PTI
CPU: 3 PID: 78925 Comm: tee Not tainted 5.15.50-1-lts #1
Hardware name: MSI MS-7A59/Z270 SLI PLUS (MS-7A59), BIOS 1.90 01/30/2018
RIP: 0010:smu_v11_0_set_fan_speed_rpm+0x11/0x110 [amdgpu]

Speed is user-configurable through a file.
I accidentally set it to zero, and the driver crashed.

Signed-off-by: Yefim Barashkin <mr.b34r@kolabnow.com>
---
  drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 4 ++++
  1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
index 5f8809f6990d..69cebdb58c04 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
@@ -1236,6 +1236,10 @@ int smu_v11_0_set_fan_speed_rpm(struct 
smu_context *smu,
      * - For some Sienna Cichlid SKU, the fan speed cannot be set
      *   lower than 500 RPM.
      */
+
+   if (speed == 0)
+       return -EINVAL;
+
     tach_period = 60 * crystal_clock_freq * 10000 / (8 * speed);
     WREG32_SOC15(THM, 0, mmCG_TACH_CTRL,
              REG_SET_FIELD(RREG32_SOC15(THM, 0, mmCG_TACH_CTRL),
--
2.36.1

Re: [PATCH] drm/amd/pm: Prevent divide by zero

[Christian König]

Am 09.07.22 um 04:31 schrieb Mr. B34r:
> From: Vannoonnoo <mr.b34r@kolabnow.com>
>
> divide error: 0000 [#1] SMP PTI
> CPU: 3 PID: 78925 Comm: tee Not tainted 5.15.50-1-lts #1 5c320a339f0e397222fdfc9449eff04c0b12ea83
> Hardware name: MSI MS-7A59/Z270 SLI PLUS (MS-7A59), BIOS 1.90 01/30/2018
> RIP: 0010:smu_v11_0_set_fan_speed_rpm+0x11/0x110 [amdgpu]
>
> Speed is user-configurable through a file. I accidentally set it to zero, and the driver crashed.

Good catch, just some minor style problems. Your Signed-of-by line is 
missing for example.

There is a checkpath.pl script in the Linux kernel source which can 
point out such trivial mistakes.

Please run it on your patch and resend, apart from that looks good to me.

Christian.

>
> ---
>   drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> index 5f8809f6990d..69cebdb58c04 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> @@ -1236,6 +1236,10 @@ int smu_v11_0_set_fan_speed_rpm(struct smu_context *smu,
>   	 * - For some Sienna Cichlid SKU, the fan speed cannot be set
>   	 *   lower than 500 RPM.
>   	 */
> +
> +	if (speed == 0)
> +		return -EINVAL;
> +
>   	tach_period = 60 * crystal_clock_freq * 10000 / (8 * speed);
>   	WREG32_SOC15(THM, 0, mmCG_TACH_CTRL,
>   		     REG_SET_FIELD(RREG32_SOC15(THM, 0, mmCG_TACH_CTRL),

RE: [PATCH] drm/amd/pm: Prevent divide by zero

[Quan, Evan]

[AMD Official Use Only - General]

Reviewed-by: Evan Quan <evan.quan@amd.com>

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Mr.
> B34r
> Sent: Saturday, July 9, 2022 10:31 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Vannoonnoo <mr.b34r@kolabnow.com>
> Subject: [PATCH] drm/amd/pm: Prevent divide by zero
> 
> From: Vannoonnoo <mr.b34r@kolabnow.com>
> 
> divide error: 0000 [#1] SMP PTI
> CPU: 3 PID: 78925 Comm: tee Not tainted 5.15.50-1-lts #1
> 5c320a339f0e397222fdfc9449eff04c0b12ea83
> Hardware name: MSI MS-7A59/Z270 SLI PLUS (MS-7A59), BIOS 1.90
> 01/30/2018
> RIP: 0010:smu_v11_0_set_fan_speed_rpm+0x11/0x110 [amdgpu]
> 
> Speed is user-configurable through a file. I accidentally set it to zero, and the
> driver crashed.
> 
> ---
>  drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> index 5f8809f6990d..69cebdb58c04 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> @@ -1236,6 +1236,10 @@ int smu_v11_0_set_fan_speed_rpm(struct
> smu_context *smu,
>  	 * - For some Sienna Cichlid SKU, the fan speed cannot be set
>  	 *   lower than 500 RPM.
>  	 */
> +
> +	if (speed == 0)
> +		return -EINVAL;
> +
>  	tach_period = 60 * crystal_clock_freq * 10000 / (8 * speed);
>  	WREG32_SOC15(THM, 0, mmCG_TACH_CTRL,
>  		     REG_SET_FIELD(RREG32_SOC15(THM, 0,
> mmCG_TACH_CTRL),
> --
> 2.36.1

[PATCH] drm/amd/pm: Prevent divide by zero

[Mr. B34r]

From: Vannoonnoo <mr.b34r@kolabnow.com>

divide error: 0000 [#1] SMP PTI
CPU: 3 PID: 78925 Comm: tee Not tainted 5.15.50-1-lts #1 5c320a339f0e397222fdfc9449eff04c0b12ea83
Hardware name: MSI MS-7A59/Z270 SLI PLUS (MS-7A59), BIOS 1.90 01/30/2018
RIP: 0010:smu_v11_0_set_fan_speed_rpm+0x11/0x110 [amdgpu]

Speed is user-configurable through a file. I accidentally set it to zero, and the driver crashed.

---
 drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
index 5f8809f6990d..69cebdb58c04 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
@@ -1236,6 +1236,10 @@ int smu_v11_0_set_fan_speed_rpm(struct smu_context *smu,
 	 * - For some Sienna Cichlid SKU, the fan speed cannot be set
 	 *   lower than 500 RPM.
 	 */
+
+	if (speed == 0)
+		return -EINVAL;
+
 	tach_period = 60 * crystal_clock_freq * 10000 / (8 * speed);
 	WREG32_SOC15(THM, 0, mmCG_TACH_CTRL,
 		     REG_SET_FIELD(RREG32_SOC15(THM, 0, mmCG_TACH_CTRL),
-- 
2.36.1