linux-kernel - Re: [PATCH] drm/radeon: Add error handlings for r420 cp errata initiation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CADnq5_PLNRJarsrJ9i9Q166Yj50CN2sJZSR5uOfokjNUhYDx1g@mail.gmail.com>
Date: Fri, 21 Feb 2025 10:14:08 -0500
From: Alex Deucher <alexdeucher@...il.com>
To: Wentao Liang <vulab@...as.ac.cn>
Cc: alexander.deucher@....com, christian.koenig@....com, Xinhui.Pan@....com, 
	airlied@...il.com, simona@...ll.ch, amd-gfx@...ts.freedesktop.org, 
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org, 
	stable@...r.kernel.org
Subject: Re: [PATCH] drm/radeon: Add error handlings for r420 cp errata initiation

On Thu, Feb 20, 2025 at 1:41 AM Wentao Liang <vulab@...as.ac.cn> wrote:
>
> In r420_cp_errata_init(), the RESYNC information is stored even
> when the Scratch register is not correctly allocated.
>
> Change the return type of r420_cp_errata_init() from void to int
> to propagate errors to the caller. Add error checking after
> radeon_scratch_get() to ensure RESYNC information is stored
> to an allocated address. Log an error message and return the
> error code immediately when radeon_scratch_get() fails.
> Additionally, handle the return value of r420_cp_errata_init() in
> r420_startup() to log an appropriate error message and propagate
> the error if initialization fails.
>
> Fixes: 62cdc0c20663 ("drm/radeon/kms: Workaround RV410/R420 CP errata (V3)")
> Cc: stable@...r.kernel.org # 2.6.33+
> Signed-off-by: Wentao Liang <vulab@...as.ac.cn>
> ---
>  drivers/gpu/drm/radeon/r420.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/r420.c b/drivers/gpu/drm/radeon/r420.c
> index 9a31cdec6415..67c55153cba8 100644
> --- a/drivers/gpu/drm/radeon/r420.c
> +++ b/drivers/gpu/drm/radeon/r420.c
> @@ -204,7 +204,7 @@ static void r420_clock_resume(struct radeon_device *rdev)
>         WREG32_PLL(R_00000D_SCLK_CNTL, sclk_cntl);
>  }
>
> -static void r420_cp_errata_init(struct radeon_device *rdev)
> +static int r420_cp_errata_init(struct radeon_device *rdev)

You changed the function signature, but you didn't adjust the function
behavior to match.

Alex

>  {
>         int r;
>         struct radeon_ring *ring = &rdev->ring[RADEON_RING_TYPE_GFX_INDEX];
> @@ -215,7 +215,11 @@ static void r420_cp_errata_init(struct radeon_device *rdev)
>          * The proper workaround is to queue a RESYNC at the beginning
>          * of the CP init, apparently.
>          */
> -       radeon_scratch_get(rdev, &rdev->config.r300.resync_scratch);
> +       r = radeon_scratch_get(rdev, &rdev->config.r300.resync_scratch);
> +       if (r) {
> +               DRM_ERROR("failed to get scratch reg (%d).\n", r);
> +               return r;
> +       }
>         r = radeon_ring_lock(rdev, ring, 8);
>         WARN_ON(r);
>         radeon_ring_write(ring, PACKET0(R300_CP_RESYNC_ADDR, 1));
> @@ -290,8 +294,11 @@ static int r420_startup(struct radeon_device *rdev)
>                 dev_err(rdev->dev, "failed initializing CP (%d).\n", r);
>                 return r;
>         }
> -       r420_cp_errata_init(rdev);
> -
> +       r = r420_cp_errata_init(rdev);
> +       if (r) {
> +               dev_err(rdev->dev, "failed initializing CP errata workaround (%d).\n", r);
> +               return r;
> +       }
>         r = radeon_ib_pool_init(rdev);
>         if (r) {
>                 dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
> --
> 2.42.0.windows.2
>