Re: [PATCH v4 03/10] drm/msm: release scanout framebuffers only after a vblank

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/msm: premature unpin of framebuffers causing IOMMU fault
--

commit 86394032733ab2293f0e387866c3cbd603bc3285
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Subject: drm/msm: release scanout framebuffers only after a vblank

This commit defers scanout framebuffer release until a vblank by handing them
to a drm_vblank_work per CRTC. On CRTC disable, pending works are flushed and
framebuffers are unpinned directly from msm_crtc_vblank_off().

> diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c
> index 7e1df472592d3..cb497d419ab83 100644
> --- a/drivers/gpu/drm/msm/msm_kms.c
> +++ b/drivers/gpu/drm/msm/msm_kms.c
[ ... ]
> +/*
> + * The crtc stops fetching here, and with it the vblanks the pending works are
> + * waiting for, so release the framebuffers directly.
> + */
> +static void msm_kms_fb_unpin_release_all(struct msm_kms_fb_unpin *pending)
> +{
[ ... ]
> +		drm_vblank_work_cancel_sync(&unpin->base);
> +		msm_kms_fb_unpin_release(unpin);
> +	}
> +}
> +
> +void msm_crtc_vblank_off(struct drm_crtc *crtc)
> +{
> +	struct msm_drm_private *priv = crtc->dev->dev_private;
> +	struct msm_kms *kms = priv->kms;
> +	unsigned int idx = drm_crtc_index(crtc);
> +
> +	if (kms && idx < ARRAY_SIZE(kms->fb_unpin))
> +		msm_kms_fb_unpin_release_all(&kms->fb_unpin[idx]);
> +
> +	drm_crtc_vblank_off(crtc);
> +}
[ ... ]
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 77a2d1f6700d0..35daf46fc4257 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -1213,7 +1213,7 @@ static void dpu_crtc_disable(struct drm_crtc *crtc,
>  	}
>  
>  	/* Disable/save vblank irq handling */
> -	drm_crtc_vblank_off(crtc);
> +	msm_crtc_vblank_off(crtc);

[Severity: High]
Can this lead to an IOMMU context fault if the display hardware is still 
fetching the frame?

When a userspace application issues an async plane update followed closely by 
a CRTC disable, the ongoing hardware fetch might hit an IOMMU fault because 
the memory is unmapped immediately during the disable sequence.

Because msm_crtc_vblank_off() now synchronously unpins and unmaps all pending 
framebuffers via msm_kms_fb_unpin_release_all(), executing this before the 
hardware is done fetching creates a race window.

Looking at dpu_crtc_disable(), we wait for the hardware to finish its current 
frame fetch much later in the function:

drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c:dpu_crtc_disable() {
    ...
    msm_crtc_vblank_off(crtc);
    ...
    if (_dpu_crtc_wait_for_frame_done(crtc))
        DPU_ERROR(...);
    ...
}

Does this sequence leave a race window where the display is left reading 
unmapped memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261003-fd-kms-fix-smmu-v4-0-2a44d4d8ac77@oss.qualcomm.com?part=3