Re: [igt-dev] [PATCH i-g-t] igt/kms_vblank: Exercise some EINVAL

Daniel Vetter
2018-09-04T21:52:24Z

On Tue, Sep 04, 2018 at 07:51:59PM +0100, Chris Wilson wrote:
> Feed some impossible garbage into drmWaitVblank and expect the kernel to
> report EINVAL.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> tests/kms_vblank.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 45 insertions(+)
>
> diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
> index b3cd2d93c..3e7aff3fe 100644
> --- a/tests/kms_vblank.c
> +++ b/tests/kms_vblank.c
> @@ -455,6 +455,48 @@ static void run_subtests_for_pipe(data_t *data)
> }
> }
>
> +static void invalid_subtest(data_t *data, int fd)
> +{
> + union drm_wait_vblank vbl;
> + unsigned long invalid_flags;
> +
> + igt_display_require_output_on_pipe(&data->display, 0);
> +
> + /* First check all is well with a simple query */
> + memset(&vbl, 0, sizeof(vbl));
> + vbl.request.type = DRM_VBLANK_RELATIVE;
> + igt_assert_eq(wait_vblank(fd, &vbl), 0);
> +
> + invalid_flags = ~(_DRM_VBLANK_TYPES_MASK |
> + _DRM_VBLANK_FLAGS_MASK |
> + _DRM_VBLANK_HIGH_CRTC_MASK);
> +
> + /* pick some interesting invalid permutations */
> + memset(&vbl, 0, sizeof(vbl));
> + vbl.request.type = _DRM_VBLANK_RELATIVE | invalid_flags;
> + igt_assert_eq(wait_vblank(fd, &vbl), -EINVAL);
> + for (int bit = 0; bit < 32; bit++) {
> + int err;
> +
> + if (~invalid_flags & (1 << bit))

Double-take on the ~~, but looks correct. I expect CI to complain about
the dmesg-error on this one, but it won't report because it's not yet
vetted. But should be in the full results somewhere.

> + continue;
> +
> + memset(&vbl, 0, sizeof(vbl));
> + vbl.request.type = _DRM_VBLANK_RELATIVE | (1 << bit);
> + err = wait_vblank(fd, &vbl);
> + igt_assert_f(err == -EINVAL,
> + "vblank wait with invalid request.type bit %d [0x%08x] did not report -EINVAL, got %d\n",
> + bit, 1 << bit, err);
> + }
> +
> + /* check the maximum pipe, nobody should have that many pipes! */
> + memset(&vbl, 0, sizeof(vbl));
> + vbl.request.type = _DRM_VBLANK_RELATIVE;
> + vbl.request.type |= _DRM_VBLANK_SECONDARY;
> + vbl.request.type |= _DRM_VBLANK_FLAGS_MASK;
> + igt_assert_eq(wait_vblank(fd, &vbl), -EINVAL);

Hm just realized that the kernel lets _DRM_VBLANK_SECONDARY and
_DRM_VBLANK_HIGH_CRTC_MASK through at the same time. Not really an issue,
just a bit silly. But for another day.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +}
> +
> igt_main
> {
> int fd;
> @@ -469,6 +511,9 @@ igt_main
> igt_display_require_output(&data.display);
> }
>
> + igt_subtest("invalid")
> + invalid_subtest(&data, fd);
> +
> igt_subtest("crtc-id")
> crtc_id_subtest(&data, fd);
>
> --
> 2.19.0.rc1
>

--
Daniel Vetter
Software Engineer, Intel Corporation