linux-kernel - Re: [PATCH 1/2] drm/vkms: Bugfix extra vblank frame

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190203205707.7pqv6s5xuhbxduqv@smtp.gmail.com>
Date:   Sun, 3 Feb 2019 18:57:07 -0200
From:   Rodrigo Siqueira <rodrigosiqueiramelo@...il.com>
To:     Shayenne Moura <shayenneluzmoura@...il.com>
Cc:     Haneen Mohammed <hamohammed.sa@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        David Airlie <airlied@...ux.ie>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] drm/vkms: Bugfix extra vblank frame

On 01/30, Shayenne Moura wrote:
> kms_flip tests are breaking on vkms when simulate vblank because vblank
> event sequence count returns one extra frame after arm vblank event to
> make a page flip.
> 
> When vblank interrupt happens, userspace processes the vblank event and
> issues the next page flip command. Kernel calls queue_work to call
> commit_planes and arm the new page flip. The next vblank picks up the
> newly armed vblank event and vblank interrupt happens again.
> 
> The arm and vblank event are asynchronous, then, on the next vblank, we
> receive x+2 from `get_vblank_timestamp`, instead x+1, although timestamp
> and vblank seqno matches.
> 
> Function `get_vblank_timestamp` is reached by 2 ways:
> 
>   - from `drm_mode_page_flip_ioctl`: driver is doing one atomic operation
>     to synchronize planes in the same output. There is no vblank simulation,
>     the `drm_crtc_arm_vblank_event` function adds 1 on vblank count, and the
>     variable in_vblank_irq is false
>   - from `vkms_vblank_simulate`: since the driver is doing a vblank simulation,
>     the variable in_vblank_irq is true.
> 
> Fix this problem subtracting one vblank period from vblank_time when
> `get_vblank_timestamp` is called from trace `drm_mode_page_flip_ioctl`,
> i.e., is not a real vblank interrupt, and getting the timestamp and vblank
> seqno when it is a real vblank interrupt.
> 
> The reason for all this is that get_vblank_timestamp always supplies the
> timestamp for the next vblank event. The hrtimer is the vblank simulator,
> and it needs the correct previous value to present the next vblank. Since
> this is how hw timestamp registers work and what the vblank core expects.
> 
> Signed-off-by: Shayenne Moura <shayenneluzmoura@...il.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@...el.com>
> 
> ---
>  drivers/gpu/drm/vkms/vkms_crtc.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> index d44bfc392491..23146ff2a25b 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -87,6 +87,9 @@ bool vkms_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,
>  
>  	*vblank_time = output->vblank_hrtimer.node.expires;
>  
> +	if (!in_vblank_irq)
> +		*vblank_time -= output->period_ns;
> +
>  	return true;
>  }
>  
> -- 
> 2.17.1
> 

Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@...il.com>

-- 
Rodrigo Siqueira
https://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)