[igt-dev] [PATCH 2/2] tests/fbdev: Add tests for read/write ops on framebuffer

Daniel Vetter daniel.vetter at ffwll.ch
Tue Nov 3 12:10:00 UTC 2020


On Tue, Nov 3, 2020 at 1:08 PM Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
>
> On Tue, Nov 3, 2020 at 11:48 AM Thomas Zimmermann <tzimmermann at suse.de> wrote:
> >
> > Read and write tests check the read and written buffer against the
> > content of the mapped framebuffer. Fbdev has some specific behavior
> > when reading/writing near the EOF, which the eof test checks.
> >
> > Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> > ---
> >  tests/fbdev.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 80 insertions(+)
> >
> > diff --git a/tests/fbdev.c b/tests/fbdev.c
> > index 11df954d..045b0874 100644
> > --- a/tests/fbdev.c
> > +++ b/tests/fbdev.c
> > @@ -25,6 +25,7 @@
> >
> >  #include "igt.h"
> >
> > +#include <errno.h>
> >  #include <fcntl.h>
> >  #include <string.h>
> >  #include <sys/ioctl.h>
> > @@ -43,6 +44,7 @@ igt_main
> >         struct fb_fix_screeninfo fix_info;
> >         int fd = -1;
> >         void *map = NULL;
> > +       void *buf = NULL;
>
> Same thing here as with the shared map, this blows up if you only run
> specific subtests.
>
> Also I think this is getting unwieldy, and we should extract subtests
> into functions that get called for local scoping and everything.
>
> >
> >         /*
> >          * Should this test focus on the fbdev independent of any drm driver,
> > @@ -82,7 +84,85 @@ igt_main
> >                 memset(map, 0, fix_info.smem_len);
> >         }
> >
> > +       igt_subtest("read") {
> > +               ssize_t ret;
> > +               int cmp;
> > +
> > +               /* allocate two additional byte for eof test */
> > +               buf = malloc(fix_info.smem_len + 2);
> > +               igt_assert(buf);
> > +
> > +               /* framebuffer should be 0 from mmap test */
> > +               ret = pread(fd, buf, fix_info.smem_len, 0);
> > +               igt_require_f(ret == (ssize_t)fix_info.smem_len, "pread failed\n");
> > +               cmp = memcmp(map, buf, fix_info.smem_len);
> > +               igt_require_f(!cmp, "read buffer differs from mapped framebuffer for 0\n");
> > +
> > +               /* fill framebuffer with 0x55 and compare again */
> > +               memset(map, 0x55, fix_info.smem_len);
> > +               ret = pread(fd, buf, fix_info.smem_len, 0);
> > +               igt_require_f(ret == (ssize_t)fix_info.smem_len, "pread failed\n");
> > +               cmp = memcmp(map, buf, fix_info.smem_len);
> > +               igt_require_f(!cmp, "read buffer differs from mapped framebuffer for 0x55\n");
> > +       }
> > +
> > +       igt_subtest("write") {
> > +               ssize_t ret;
> > +               int cmp;
> > +
> > +               /* clear framebuffer and compare again */
> > +               memset(buf, 0, fix_info.smem_len);
> > +               ret = pwrite(fd, buf, fix_info.smem_len, 0);
> > +               igt_require_f(ret == (ssize_t)fix_info.smem_len, "pwrite failed\n");
> > +               cmp = memcmp(map, buf, fix_info.smem_len);
> > +               igt_require_f(!cmp, "write buffer differs from mapped framebuffer for 0\n");
> > +       }
> > +
> > +       igt_subtest("eof") {
> > +               unsigned long lastindex = fix_info.smem_len - 1;
> > +               unsigned char *maplast = &((unsigned char *)map)[lastindex];
> > +               unsigned char *buflast = &((unsigned char *)buf)[lastindex];
> > +               long long ret;
> > +
> > +               *buflast = 0x55;
> > +
> > +               /* write across EOF; set remaining bytes */
> > +               ret = (long long)pwrite(fd, buflast, 2, lastindex);
> > +               igt_require_f(ret == 1, "write crossed EOF, ret=%lld\n", ret);
> > +               igt_require_f(*maplast == *buflast, "write buffer differs from mapped framebuffer at final byte, "
> > +                                                   "maplast=%u buflast=%u\n", *maplast, *buflast);
> > +
> > +               /* write at EOF; get ENOSPC */
> > +               ret = (long long)pwrite(fd, &buflast[1], 1, lastindex + 1);
> > +               igt_require_f((ret == -1) && (errno == ENOSPC), "write at EOF, ret=%lld\n", ret);
> > +
> > +               *maplast = 0;
> > +
> > +               /* write final byte */
> > +               ret = (long long)pwrite(fd, buflast, 1, lastindex);
> > +               igt_require_f(ret == 1, "write before EOF, ret=%lld\n", ret);
> > +               igt_require_f(*maplast == *buflast, "write buffer differs from mapped framebuffer at final byte, "
> > +                                                   "maplast=%u buflast=%u\n", *maplast, *buflast);
> > +
> > +               /* write after EOF; get EFBIG */
> > +               ret = (long long)pwrite(fd, &buflast[2], 1, lastindex + 2);
> > +               igt_require_f((ret == -1) && (errno == EFBIG), "write after EOF, ret=%lld\n", ret);
> > +
> > +               *maplast = 0;
> > +
> > +               /* read across the EOF; get remaining bytes */
> > +               ret = (long long)pread(fd, buflast, 2, lastindex);
> > +               igt_require_f(ret == 1, "read before EOF\n");
> > +               igt_require_f(*maplast == *buflast, "read buffer differs from mapped framebuffer at final byte, "
> > +                                                   "maplast=%u buflast=%u\n", *maplast, *buflast);
> > +
> > +               /* read after EOF; get 0 */
> > +               ret = (long long)pread(fd, &buflast[1], 1, lastindex + 1);
> > +               igt_require_f(ret == 0, "read at EOF, ret=%lld\n", ret);
>
> Maybe also a NULL pointer tests which checks we get an EFAULT?
>
> Another corner case is a read/write that's not PAGE aligned, e.g. from
> PAGE_SIZE/2 to PAGE_SIZE + PAGE_SIZE/2 (assuming the buffer is at
> least 2 pages ofc).

Uh that gives us a read/write size of PAGE_SIZE, so not too
interesting. Would need to be more misaligned (maybe even make it odd
or something nasty like that, and bigger than one size so we go
through the loop once in the kernel code).
-Daniel

> Otherwise test logic looks complete to me. Thanks for creating these.
> -Daniel
>
> > +       }
> > +
> >         igt_fixture {
> > +               free(buf);
> >                 if (map && map != MAP_FAILED)
> >                         munmap(map, fix_info.smem_len);
> >                 close(fd);
> > --
> > 2.29.0
> >
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the igt-dev mailing list