Re: [PATCH v8 02/27] drm/vblank: Add a 2-stage version of drm_crtc_arm_vblank_event

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing memory barrier in drm_crtc_arm_prepared_vblank_event() allows premature vblank signaling.
--

commit 876541f26b4001b93fb6f0b7d16d814d3b08e2b0
Author: Maarten Lankhorst <dev@lankhorst.se>

drm/vblank: Add a 2-stage version of drm_crtc_arm_vblank_event

This patch introduces a two-stage mechanism for arming vblank events to
support PREEMPT_RT kernels and increase determinism. It allows the event
to be prepared before hardware programming and armed afterward, avoiding
the need to take vblank locks with local interrupts disabled.

> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index f90fb2d13e423..6856eff97dc97 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c

[ ... ]

> +/**
> + * drm_crtc_arm_prepared_vblank_event - arm prepared vblank event *after* pageflip.
> + * @crtc: the source CRTC of the vblank event
> + * @e: the event to send
> + *
> + * See drm_crtc_prepare_arm_vblank_event(). This function is a 2-stage version of
> + * that call. This function is called directly *AFTER* programming the hardware.
> + *

[ ... ]

> +void drm_crtc_arm_prepared_vblank_event(struct drm_pending_vblank_event *e)
> +{
> +	WARN_ON(!e->postponed);
> +
> +	/* remove the flag to be processed as a normal event */
> +	WRITE_ONCE(e->postponed, false);

[Severity: High]
Could this memory update be reordered with the prior MMIO writes on weakly
ordered architectures?

Since this function is intended to be called immediately after hardware
programming, there might be an ordering issue. For instance, on ARM64,
functions like writel() guarantee that prior memory accesses complete before
the MMIO write, but they do not provide a memory barrier after the write.

This means the CPU could reorder the WRITE_ONCE() update to occur before the
MMIO writes are globally observed. If a concurrent hardware vblank interrupt
is serviced by another CPU, drm_handle_vblank_events() might observe the
cleared postponed flag and signal the event to userspace prematurely.

If userspace receives the event and frees the old framebuffer before the new
hardware state takes effect, could it lead to the hardware scanning out
unmapped memory and triggering IOMMU faults or display corruption? Would
adding a wmb() here ensure the MMIO writes are properly ordered against the
flag update?

> +}
> +EXPORT_SYMBOL(drm_crtc_arm_prepared_vblank_event);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615201846.307297-1-dev@lankhorst.se?part=2