Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i8771445c:Fastmail
Date:   Wed, 24 Aug 2022 17:48:23 +0200
From:   Maxime Ripard <maxime@cerno.tech>
To:     Danilo Krummrich <dakr@redhat.com>
Cc:     daniel@ffwll.ch, airlied@linux.ie, tzimmermann@suse.de,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH drm-misc-next v2 2/4] drm/vc4: plane: protect device
 resources after removal
Message-ID: <20220824154823.qu3tdwypg5o3ci4z@houat>
References: <20220819110849.192037-1-dakr@redhat.com>
 <20220819110849.192037-3-dakr@redhat.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="i6p2xirgmwjujatv"
Content-Disposition: inline
In-Reply-To: <20220819110849.192037-3-dakr@redhat.com>
Precedence: bulk


--i6p2xirgmwjujatv
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Aug 19, 2022 at 01:08:47PM +0200, Danilo Krummrich wrote:
> (Hardware) resources which are bound to the driver and device lifecycle
> must not be accessed after the device and driver are unbound.
>=20
> However, the DRM device isn't freed as long as the last user closed it,
> hence userspace can still call into the driver.
>=20
> Therefore protect the critical sections which are accessing those
> resources with drm_dev_enter() and drm_dev_exit().
>=20
> Fixes: 9872c7a31921 ("drm/vc4: plane: Switch to drmm_universal_plane_allo=
c()")
> Signed-off-by: Danilo Krummrich <dakr@redhat.com>
> ---
>  drivers/gpu/drm/vc4/vc4_plane.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>=20
> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_pl=
ane.c
> index eff9c63adfa7..c46acb770036 100644
> --- a/drivers/gpu/drm/vc4/vc4_plane.c
> +++ b/drivers/gpu/drm/vc4/vc4_plane.c
> @@ -19,6 +19,7 @@
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_atomic_uapi.h>
>  #include <drm/drm_blend.h>
> +#include <drm/drm_drv.h>
>  #include <drm/drm_fb_dma_helper.h>
>  #include <drm/drm_fourcc.h>
>  #include <drm/drm_framebuffer.h>
> @@ -1219,6 +1220,10 @@ u32 vc4_plane_write_dlist(struct drm_plane *plane,=
 u32 __iomem *dlist)
>  {
>  	struct vc4_plane_state *vc4_state =3D to_vc4_plane_state(plane->state);
>  	int i;
> +	int idx;
> +
> +	if (!drm_dev_enter(plane->dev, &idx))
> +		goto out;
> =20
>  	vc4_state->hw_dlist =3D dlist;
> =20
> @@ -1226,6 +1231,9 @@ u32 vc4_plane_write_dlist(struct drm_plane *plane, =
u32 __iomem *dlist)
>  	for (i =3D 0; i < vc4_state->dlist_count; i++)
>  		writel(vc4_state->dlist[i], &dlist[i]);
> =20
> +	drm_dev_exit(idx);
> +
> +out:
>  	return vc4_state->dlist_count;
>  }
> =20
> @@ -1245,6 +1253,7 @@ void vc4_plane_async_set_fb(struct drm_plane *plane=
, struct drm_framebuffer *fb)
>  	struct vc4_plane_state *vc4_state =3D to_vc4_plane_state(plane->state);
>  	struct drm_gem_dma_object *bo =3D drm_fb_dma_get_gem_obj(fb, 0);
>  	uint32_t addr;
> +	int idx;
> =20
>  	/* We're skipping the address adjustment for negative origin,
>  	 * because this is only called on the primary plane.
> @@ -1252,12 +1261,17 @@ void vc4_plane_async_set_fb(struct drm_plane *pla=
ne, struct drm_framebuffer *fb)
>  	WARN_ON_ONCE(plane->state->crtc_x < 0 || plane->state->crtc_y < 0);
>  	addr =3D bo->dma_addr + fb->offsets[0];
> =20
> +	if (!drm_dev_enter(plane->dev, &idx))
> +		return;
> +
>  	/* Write the new address into the hardware immediately.  The
>  	 * scanout will start from this address as soon as the FIFO
>  	 * needs to refill with pixels.
>  	 */
>  	writel(addr, &vc4_state->hw_dlist[vc4_state->ptr0_offset]);
> =20
> +	drm_dev_exit(idx);
> +

You did change the CRTC patch, but the comment to protect the entire
function also applies to this one.

Maxime

--i6p2xirgmwjujatv
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCYwZIRwAKCRDj7w1vZxhR
xTpMAP9QqYSv8ty8je48B2HuywFGJE+Q2WGNAW/paVlqkHD0WwD/XgOG/VA+mckd
g+GkI3JRlnAhkCqVRyReiq/t0DFeagk=
=OEZa
-----END PGP SIGNATURE-----

--i6p2xirgmwjujatv--