Re: [PATCH v1 24/28] tests/tcg/aarch64: add SVE iotcl test

[Aleksandar Markovic]

уто, 17. мар 2020. у 11:33 Philippe Mathieu-Daudé <address@hidden>
је написао/ла:
>
> On 3/16/20 6:21 PM, Alex Bennée wrote:
> > This is a fairly bare-bones test of setting the various vector sizes
> > for SVE which will only fail if the PR_SVE_SET_VL can't reduce the
> > user-space vector length by powers of 2.
> >
> > However we will also be able to use it in a future test which
> > exercises the GDB stub.
> >
> > Signed-off-by: Alex Bennée <address@hidden>
>
> Tested-by: Philippe Mathieu-Daudé <address@hidden>
>

I think it would be clearer and better if you used used the
word "prctl" rather than "ioctl" in both title/commit message
and code (including comments).

I think it is also a little problematic that tests like this reside
and belong in tests/tcg - since they deal more with elements
of kernel interface (system call prctl()), rather than TCG.
Still, I see there are many other similar cases in tests/tcg,
so probably (at this moment) one should't bother about it in
the context of this patch. However, in general, the location
/tests/tcg for all these cases seems wrong to me.

Sincerely,
Aleksandar

> >
> > ---
> > v3
> >    - use index to fill zreg
> >    - CROSS_CC_HAS_SVE
> > v5
> >    - merge conflicts
> > v6
> >    - drop id check
> > ---
> >   tests/tcg/aarch64/sve-ioctls.c    | 70 +++++++++++++++++++++++++++++++
> >   tests/tcg/aarch64/Makefile.target |  4 ++
> >   2 files changed, 74 insertions(+)
> >   create mode 100644 tests/tcg/aarch64/sve-ioctls.c
> >
> > diff --git a/tests/tcg/aarch64/sve-ioctls.c b/tests/tcg/aarch64/sve-ioctls.c
> > new file mode 100644
> > index 00000000000..9544dffa0ee
> > --- /dev/null
> > +++ b/tests/tcg/aarch64/sve-ioctls.c
> > @@ -0,0 +1,70 @@
> > +/*
> > + * SVE ioctls tests
> > + *
> > + * Test the SVE width setting ioctls work and provide a base for
> > + * testing the gdbstub.
> > + *
> > + * Copyright (c) 2019 Linaro Ltd
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +#include <sys/prctl.h>
> > +#include <asm/hwcap.h>
> > +#include <stdio.h>
> > +#include <sys/auxv.h>
> > +#include <stdint.h>
> > +#include <stdlib.h>
> > +
> > +#ifndef HWCAP_CPUID
> > +#define HWCAP_CPUID (1 << 11)
> > +#endif
> > +
> > +#define SVE_MAX_QUADS  (2048 / 128)
> > +#define BYTES_PER_QUAD (128 / 8)
> > +
> > +#define get_cpu_reg(id) ({                                      \
> > +            unsigned long __val;                                \
> > +            asm("mrs %0, "#id : "=r" (__val));                  \
> > +            __val;                                              \
> > +        })
> > +
> > +static int do_sve_ioctl_test(void)
> > +{
> > +    int i, res, init_vq;
> > +
> > +    res = prctl(PR_SVE_GET_VL, 0, 0, 0, 0);
> > +    if (res < 0) {
> > +        printf("FAILED to PR_SVE_GET_VL (%d)", res);
> > +        return -1;
> > +    }
> > +    init_vq = res & PR_SVE_VL_LEN_MASK;
> > +
> > +    for (i = init_vq; i > 15; i /= 2) {
> > +        printf("Checking PR_SVE_SET_VL=%d\n", i);
> > +        res = prctl(PR_SVE_SET_VL, i, 0, 0, 0, 0);
> > +        if (res < 0) {
> > +            printf("FAILED to PR_SVE_SET_VL (%d)", res);
> > +            return -1;
> > +        }
> > +        asm("index z0.b, #0, #1\n"
> > +            ".global __sve_ld_done\n"
> > +            "__sve_ld_done:\n"
> > +            "mov z0.b, #0\n"
> > +            : /* no outputs kept */
> > +            : /* no inputs */
> > +            : "memory", "z0");
> > +    }
> > +    printf("PASS\n");
> > +    return 0;
> > +}
> > +
> > +int main(int argc, char **argv)
> > +{
> > +    /* we also need to probe for the ioctl support */
> > +    if (getauxval(AT_HWCAP) & HWCAP_SVE) {
> > +        return do_sve_ioctl_test();
> > +    } else {
> > +        printf("SKIP: no HWCAP_SVE on this system\n");
> > +        return 0;
> > +    }
> > +}
> > diff --git a/tests/tcg/aarch64/Makefile.target 
> > b/tests/tcg/aarch64/Makefile.target
> > index b61b53e4dd1..c879932ff6c 100644
> > --- a/tests/tcg/aarch64/Makefile.target
> > +++ b/tests/tcg/aarch64/Makefile.target
> > @@ -47,6 +47,10 @@ ifneq ($(DOCKER_IMAGE)$(CROSS_CC_HAS_SVE),)
> >   AARCH64_TESTS += sysregs
> >   sysregs: CFLAGS+=-march=armv8.1-a+sve
> >
> > +# SVE ioctl test
> > +AARCH64_TESTS += sve-ioctls
> > +sve-ioctls: CFLAGS+=-march=armv8.1-a+sve
> > +
> >   ifneq ($(HAVE_GDB_BIN),)
> >   GDB_SCRIPT=$(SRC_PATH)/tests/guest-debug/run-test.py
> >
> >
>
>