[Qemu-devel] [RISU PATCH 00/10] Initial support for SVE

[Qemu-devel] [RISU PATCH 00/10] Initial support for SVE

[Alex Bennée]

Hi,

These patches apply on-top of the last clean-up series:

  Subject: [RISU PATCH 0/7] Add @Group support and some aarch64.risu cleanups
  Date: Tue, 31 Oct 2017 14:54:37 +0000
  Message-Id: <20171031145444.13766-1-alex.bennee@linaro.org>

This series adds support for SVE to RISU. Most of the initial patches
are plumbing changes to better support arch specific option flags
(cleaning up a TODO in the process). I also needed to ensure configure
actually honoured CPPFLAGS so it could be passed yet to be released
headers.

The actual guts of the SVE support is in 3 patches. One to risugen so
it can generate random values for the vector registers. If this isn't
done you get inconsistent runs because of left over junk in the
registers. The remaining two patches add support for copying the SVE
vectors from the signal context and dumping any differences found.

To test this I ran on the Foundation model with:

  ${RTSM_MODEL} --no-secure-memory --gicv3 --data=${BL1_FIRMWARE}@0x0 --data=${FIP_FIRMWARE}@0x8000000 \
                --semihost --network nat --network-nat-ports=8022=22 --block-device ${DISK_IMAGE}

With my hacked up copy of secure firmware to ensure SVE is enabled.
You also need a kernel with SVE support:

  Subject: [PATCH v5 00/30] ARM Scalable Vector Extension (SVE)
  Date: Tue, 31 Oct 2017 15:50:52 +0000
  Message-Id: <1509465082-30427-1-git-send-email-Dave.Martin@arm.com>

My testing so far has been to ensure that each run on the foundation
model is deterministic (same m5sum on each trace) and that differences
are printed out when the user-space vector length is tweaked
(/proc/sys/abi/sve_default_vector_length).


Alex Bennée (10):
  build-all-arches: drop -t (for tty) from docker invocation
  risu.c: split out setting up options
  risu.c: add missing --trace longopt
  risu: move optional args to each architecture
  configure: allow repeated invocation of configure in build dir
  configure: support CPPFLAGS
  risugen: add --sve support
  aarch64.risu: initial SVE instruction
  risu_reginfo_aarch64: add reginfo_copy_sve
  risu_reginfo_aarch64: add SVE support to reginfo_dump_mismatch

 README                 |   6 +++
 aarch64.risu           |  24 ++++++++++
 build-all-archs        |   2 +-
 configure              |   5 +-
 risu.c                 |  57 +++++++++++++++++-----
 risu.h                 |   6 ++-
 risu_reginfo_aarch64.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++++
 risu_reginfo_aarch64.h |   8 ++++
 risu_reginfo_arm.c     |  11 +++++
 risu_reginfo_m68k.c    |   3 ++
 risu_reginfo_ppc64.c   |   3 ++
 risugen                |   3 ++
 risugen_arm.pm         |  57 +++++++++++++++++++---
 13 files changed, 286 insertions(+), 25 deletions(-)

-- 
2.14.2

[Qemu-devel] [RISU PATCH 01/10] build-all-arches: drop -t (for tty) from docker invocation

[Alex Bennée]

This prevents gcc from going nuts with colorizing the compiler output
which looks particularly ugly when invoked via compilation-mode.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 build-all-archs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/build-all-archs b/build-all-archs
index 63918e5..fa2ac90 100755
--- a/build-all-archs
+++ b/build-all-archs
@@ -61,7 +61,7 @@ done
 # If docker is enabled we just brute force the various images until we
 # can set the one that has a workable cross compiler.
 
-DOCKER_RUN="docker run --rm -t -u $(id -u) -v $(pwd):$(pwd) -w $(pwd)"
+DOCKER_RUN="docker run --rm -u $(id -u) -v $(pwd):$(pwd) -w $(pwd)"
 
 program_exists() {
     if [ ! -z "$docker_tags" ]; then
-- 
2.14.2

[Qemu-devel] [RISU PATCH 02/10] risu.c: split out setting up options

[Alex Bennée]

This is a prerequisite to properly handling architecture specific
options.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 risu.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/risu.c b/risu.c
index 47471c6..a5d155d 100644
--- a/risu.c
+++ b/risu.c
@@ -282,6 +282,21 @@ void usage(void)
             "(default 9191)\n");
 }
 
+struct option * setup_options(char **short_opts)
+{
+    static struct option default_longopts[] = {
+        {"help", no_argument, 0, '?'},
+        {"master", no_argument, &ismaster, 1},
+        {"host", required_argument, 0, 'h'},
+        {"port", required_argument, 0, 'p'},
+        {"test-fp-exc", no_argument, &test_fp_exc, 1},
+        {0, 0, 0, 0}
+    };
+
+    *short_opts = "h:p:t:";
+    return default_longopts;
+}
+
 int main(int argc, char **argv)
 {
     /* some handy defaults to make testing easier */
@@ -289,20 +304,14 @@ int main(int argc, char **argv)
     char *hostname = "localhost";
     char *imgfile;
     char *trace_fn = NULL;
+    struct option *longopts;
+    char *shortopts;
 
-    /* TODO clean this up later */
+    longopts = setup_options(&shortopts);
 
     for (;;) {
-        static struct option longopts[] = {
-            {"help", no_argument, 0, '?'},
-            {"master", no_argument, &ismaster, 1},
-            {"host", required_argument, 0, 'h'},
-            {"port", required_argument, 0, 'p'},
-            {"test-fp-exc", no_argument, &test_fp_exc, 1},
-            {0, 0, 0, 0}
-        };
         int optidx = 0;
-        int c = getopt_long(argc, argv, "h:p:t:", longopts, &optidx);
+        int c = getopt_long(argc, argv, shortopts, longopts, &optidx);
         if (c == -1) {
             break;
         }
-- 
2.14.2

[Qemu-devel] [RISU PATCH 03/10] risu.c: add missing --trace longopt

[Alex Bennée]

This got missed when trace support was added.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 risu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/risu.c b/risu.c
index a5d155d..616fc33 100644
--- a/risu.c
+++ b/risu.c
@@ -289,6 +289,7 @@ struct option * setup_options(char **short_opts)
         {"master", no_argument, &ismaster, 1},
         {"host", required_argument, 0, 'h'},
         {"port", required_argument, 0, 'p'},
+        {"trace", required_argument, 0, 't'},
         {"test-fp-exc", no_argument, &test_fp_exc, 1},
         {0, 0, 0, 0}
     };
-- 
2.14.2

[Qemu-devel] [RISU PATCH 04/10] risu: move optional args to each architecture

[Alex Bennée]

The key variables here are: *arch_long_opts and *arch_extra_help. If
they are not NULL then we concatenate the extra options to appropriate
structure to enable the support. Adding architecture short options is
not supported.

This also includes moving the ARM specific test_fp_exc/test-fp-exc
into ARM specific code.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 risu.c                 | 31 ++++++++++++++++++++++++++-----
 risu.h                 |  6 ++++--
 risu_reginfo_aarch64.c |  3 +++
 risu_reginfo_arm.c     | 11 +++++++++++
 risu_reginfo_m68k.c    |  3 +++
 risu_reginfo_ppc64.c   |  3 +++
 6 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/risu.c b/risu.c
index 616fc33..f063093 100644
--- a/risu.c
+++ b/risu.c
@@ -43,9 +43,6 @@ gzFile gz_trace_file;
 
 sigjmp_buf jmpbuf;
 
-/* Should we test for FP exception status bits? */
-int test_fp_exc;
-
 /* Master functions */
 
 int read_sock(void *ptr, size_t bytes)
@@ -280,6 +277,9 @@ void usage(void)
     fprintf(stderr,
             "  -p, --port=PORT   Specify the port to connect to/listen on "
             "(default 9191)\n");
+    if (arch_extra_help) {
+        fprintf(stderr, "%s", arch_extra_help);
+    }
 }
 
 struct option * setup_options(char **short_opts)
@@ -290,12 +290,33 @@ struct option * setup_options(char **short_opts)
         {"host", required_argument, 0, 'h'},
         {"port", required_argument, 0, 'p'},
         {"trace", required_argument, 0, 't'},
-        {"test-fp-exc", no_argument, &test_fp_exc, 1},
         {0, 0, 0, 0}
     };
+    struct option *lopts = &default_longopts[0];
 
     *short_opts = "h:p:t:";
-    return default_longopts;
+
+    if (arch_long_opts) {
+        struct option *dptr, *sptr;
+        size_t osize = sizeof(default_longopts);
+        lopts = malloc(osize);
+        /* Copy default opts */
+        memcpy(lopts, default_longopts, osize);
+        dptr = lopts;
+        while (dptr->name) {
+            dptr++;
+        }
+        /* Copy extra opts */
+        sptr = arch_long_opts;
+        while (sptr->name) {
+            osize += sizeof(struct option);
+            lopts = realloc(lopts, osize);
+            *dptr++ = *sptr++;
+        }
+        memset(dptr, 0, sizeof(struct option));
+    }
+
+    return lopts;
 }
 
 int main(int argc, char **argv)
diff --git a/risu.h b/risu.h
index 1c8ecee..89811f4 100644
--- a/risu.h
+++ b/risu.h
@@ -17,6 +17,10 @@
 #include <ucontext.h>
 #include <stdio.h>
 
+/* Extra option processing for architectures */
+extern void *arch_long_opts;
+extern char *arch_extra_help;
+
 /* GCC computed include to pull in the correct risu_reginfo_*.h for
  * the architecture.
  */
@@ -36,8 +40,6 @@ void send_response_byte(int sock, int resp);
 extern uintptr_t image_start_address;
 extern void *memblock;
 
-extern int test_fp_exc;
-
 /* Ops code under test can request from risu: */
 #define OP_COMPARE 0
 #define OP_TESTEND 1
diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
index e3fadde..38ad338 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -17,6 +17,9 @@
 #include "risu.h"
 #include "risu_reginfo_aarch64.h"
 
+void *arch_long_opts;
+char *arch_extra_help;
+
 /* reginfo_init: initialize with a ucontext */
 void reginfo_init(struct reginfo *ri, ucontext_t *uc)
 {
diff --git a/risu_reginfo_arm.c b/risu_reginfo_arm.c
index 6b9ee7b..5acad02 100644
--- a/risu_reginfo_arm.c
+++ b/risu_reginfo_arm.c
@@ -13,12 +13,23 @@
 #include <stdio.h>
 #include <ucontext.h>
 #include <string.h>
+#include <getopt.h>
 
 #include "risu.h"
 #include "risu_reginfo_arm.h"
 
 extern int insnsize(ucontext_t *uc);
 
+/* Should we test for FP exception status bits? */
+static int test_fp_exc;
+static struct option extra_opts[] = {
+    {"test-fp-exc", no_argument, &test_fp_exc, 1},
+    {0, 0, 0, 0}
+};
+
+void *arch_long_opts = &extra_opts[0];
+char *arch_extra_help = "  --test-fp-exc     Check FP exception bits when comparing\n";
+
 static void reginfo_init_vfp(struct reginfo *ri, ucontext_t *uc)
 {
     /* Read VFP registers. These live in uc->uc_regspace, which is
diff --git a/risu_reginfo_m68k.c b/risu_reginfo_m68k.c
index 4ff0aa8..d429502 100644
--- a/risu_reginfo_m68k.c
+++ b/risu_reginfo_m68k.c
@@ -14,6 +14,9 @@
 #include "risu.h"
 #include "risu_reginfo_m68k.h"
 
+void *arch_long_opts;
+char *arch_extra_help;
+
 /* reginfo_init: initialize with a ucontext */
 void reginfo_init(struct reginfo *ri, ucontext_t *uc)
 {
diff --git a/risu_reginfo_ppc64.c b/risu_reginfo_ppc64.c
index eb9c12b..aa5d8c6 100644
--- a/risu_reginfo_ppc64.c
+++ b/risu_reginfo_ppc64.c
@@ -22,6 +22,9 @@
 #define XER 37
 #define CCR 38
 
+void *arch_long_opts;
+char *arch_extra_help;
+
 /* reginfo_init: initialize with a ucontext */
 void reginfo_init(struct reginfo *ri, ucontext_t *uc)
 {
-- 
2.14.2

Re: [Qemu-devel] [RISU PATCH 04/10] risu: move optional args to each architecture

[Richard Henderson]

On 11/07/2017 04:05 PM, Alex Bennée wrote:
> +    if (arch_long_opts) {
> +        struct option *dptr, *sptr;
> +        size_t osize = sizeof(default_longopts);
> +        lopts = malloc(osize);
> +        /* Copy default opts */
> +        memcpy(lopts, default_longopts, osize);
> +        dptr = lopts;
> +        while (dptr->name) {
> +            dptr++;
> +        }
> +        /* Copy extra opts */
> +        sptr = arch_long_opts;
> +        while (sptr->name) {
> +            osize += sizeof(struct option);
> +            lopts = realloc(lopts, osize);

Count default and arch opts first?


r~

[Qemu-devel] [RISU PATCH 05/10] configure: allow repeated invocation of configure in build dir

[Alex Bennée]

Otherwise a second configure run will totally miss out the setting of
BUILD_INC. I made the link -sf as it seemed the easier way around.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 configure | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 1dc527b..c622a5e 100755
--- a/configure
+++ b/configure
@@ -176,10 +176,10 @@ fi
 
 # Are we in a separate build tree? If so, link the Makefile
 # so that 'make' works.
-if test ! -e Makefile; then
+if test ! -e Makefile || test -s Makefile; then
     echo "linking Makefile..."
     BUILD_INC="-I $(pwd)"
-    ln -s "${SRCDIR}/Makefile" .
+    ln -sf "${SRCDIR}/Makefile" .
 fi
 
 generate_config
-- 
2.14.2

[Qemu-devel] [RISU PATCH 06/10] configure: support CPPFLAGS

[Alex Bennée]

Useful for accessing API's that are still brewing, e.g:

  CROSS_PREFIX=aarch64-linux-gnu- \
    CPPFLAGS=-I/home/alex/lsrc/qemu/risu.git/sve-headers/include \
    ../configure

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 README    | 6 ++++++
 configure | 1 +
 2 files changed, 7 insertions(+)

diff --git a/README b/README
index 9946e6e..fbe408d 100644
--- a/README
+++ b/README
@@ -26,6 +26,12 @@ Most useful is
    need this if you're not building on the target system
    (Example: CROSS_PREFIX=arm-linux-gnueabihf- )
 
+Another useful flag is
+ CPPFLAGS= which specified pre-processor flags, usually -I statements
+   for specifying extra include paths. Use this is you need something
+   from new kernel headers not installed on your system.
+   (Example: CPPFLAGS=-I/path/to/sve-kernel-headers/include)
+
 Passing --static will build a statically linked binary which is useful
 if you don't want to mess around with a chroot to run the binary.
 
diff --git a/configure b/configure
index c622a5e..65e1819 100755
--- a/configure
+++ b/configure
@@ -111,6 +111,7 @@ generate_makefilein() {
     echo "# Makefile.in - generated by the 'configure' script" > $m
     echo "ARCH:=${ARCH}" >> $m
     echo "CC:=${CC}" >> $m
+    echo "CPPFLAGS:=${CPPFLAGS}" >> $m
     echo "LDFLAGS:=${LDFLAGS}" >> $m
     echo "AS:=${AS}" >> $m
     echo "OBJCOPY:=${OBJCOPY}" >> $m
-- 
2.14.2

Re: [Qemu-devel] [RISU PATCH 06/10] configure: support CPPFLAGS

[Dave Martin]

On Tue, Nov 07, 2017 at 03:05:54PM +0000, Alex Bennée wrote:
> Useful for accessing API's that are still brewing, e.g:
> 
>   CROSS_PREFIX=aarch64-linux-gnu- \
>     CPPFLAGS=-I/home/alex/lsrc/qemu/risu.git/sve-headers/include \
>     ../configure
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  README    | 6 ++++++
>  configure | 1 +
>  2 files changed, 7 insertions(+)
> 
> diff --git a/README b/README
> index 9946e6e..fbe408d 100644
> --- a/README
> +++ b/README
> @@ -26,6 +26,12 @@ Most useful is
>     need this if you're not building on the target system
>     (Example: CROSS_PREFIX=arm-linux-gnueabihf- )
>  
> +Another useful flag is
> + CPPFLAGS= which specified pre-processor flags, usually -I statements
> +   for specifying extra include paths. Use this is you need something

s/is/if/

> +   from new kernel headers not installed on your system.
> +   (Example: CPPFLAGS=-I/path/to/sve-kernel-headers/include)
> +

You should probably point out that the path should refer to headers
installed by running 'make headers_install' in the Linux source.  This
means we really test the uapi headers properly, and I don't get spurious
bug reports about bad headers ;)

[...]

Cheers
---Dave

Re: [Qemu-devel] [RISU PATCH 06/10] configure: support CPPFLAGS

[Alex Bennée]

Dave Martin <Dave.Martin@arm.com> writes:

> On Tue, Nov 07, 2017 at 03:05:54PM +0000, Alex Bennée wrote:
>> Useful for accessing API's that are still brewing, e.g:
>>
>>   CROSS_PREFIX=aarch64-linux-gnu- \
>>     CPPFLAGS=-I/home/alex/lsrc/qemu/risu.git/sve-headers/include \
>>     ../configure
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  README    | 6 ++++++
>>  configure | 1 +
>>  2 files changed, 7 insertions(+)
>>
>> diff --git a/README b/README
>> index 9946e6e..fbe408d 100644
>> --- a/README
>> +++ b/README
>> @@ -26,6 +26,12 @@ Most useful is
>>     need this if you're not building on the target system
>>     (Example: CROSS_PREFIX=arm-linux-gnueabihf- )
>>
>> +Another useful flag is
>> + CPPFLAGS= which specified pre-processor flags, usually -I statements
>> +   for specifying extra include paths. Use this is you need something
>
> s/is/if/
>
>> +   from new kernel headers not installed on your system.
>> +   (Example: CPPFLAGS=-I/path/to/sve-kernel-headers/include)
>> +
>
> You should probably point out that the path should refer to headers
> installed by running 'make headers_install' in the Linux source.  This
> means we really test the uapi headers properly, and I don't get spurious
> bug reports about bad headers ;)

Will do. Thanks.
>
> [...]
>
> Cheers
> ---Dave


--
Alex Bennée

[Qemu-devel] [RISU PATCH 07/10] risugen: add --sve support

[Alex Bennée]

This is similar to the approach used by the FP/simd data in so far as
we generate a block of random data and then load into it. As there are
no post-index SVE operations we need to emit an additional incp
instruction to generate our offset into the array.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 risugen        |  3 +++
 risugen_arm.pm | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/risugen b/risugen
index aba4bb7..0ac8e86 100755
--- a/risugen
+++ b/risugen
@@ -317,6 +317,7 @@ sub main()
     my $condprob = 0;
     my $fpscr = 0;
     my $fp_enabled = 1;
+    my $sve_enabled = 1;
     my $big_endian = 0;
     my ($infile, $outfile);
 
@@ -334,6 +335,7 @@ sub main()
                 },
                 "be" => sub { $big_endian = 1; },
                 "no-fp" => sub { $fp_enabled = 0; },
+                "sve" => sub { $sve_enabled = 1; },
         ) or return 1;
     # allow "--pattern re,re" and "--pattern re --pattern re"
     @pattern_re = split(/,/,join(',',@pattern_re));
@@ -361,6 +363,7 @@ sub main()
         'fpscr' => $fpscr,
         'numinsns' => $numinsns,
         'fp_enabled' => $fp_enabled,
+        'sve_enabled' => $sve_enabled,
         'outfile' => $outfile,
         'details' => \%insn_details,
         'keys' => \@insn_keys,
diff --git a/risugen_arm.pm b/risugen_arm.pm
index 2f10d58..8d1e1fd 100644
--- a/risugen_arm.pm
+++ b/risugen_arm.pm
@@ -472,9 +472,47 @@ sub write_random_aarch64_fpdata()
     }
 }
 
-sub write_random_aarch64_regdata($)
+sub write_random_aarch64_svedata()
 {
-    my ($fp_enabled) = @_;
+    # Load SVE registers
+    my $align = 16;
+    my $vl = 16;                             # number of vqs
+    my $datalen = (32 * $vl * 16) + $align;
+
+    write_pc_adr(0, (3 * 4) + ($align - 1)); # insn 1
+    write_align_reg(0, $align);              # insn 2
+    write_jump_fwd($datalen);                # insn 3
+
+    # align safety
+    for (my $i = 0; $i < ($align / 4); $i++) {
+        # align with nops
+        insn32(0xd503201f);
+    };
+
+    for (my $rt = 0; $rt <= 31; $rt++) {
+        for (my $q = 0; $q < $vl; $q++) {
+            write_random_fpreg_var(4); # quad
+        }
+    }
+
+    # Reset all the predicate registers to all true
+    for (my $p = 0; $p < 16; $p++) {
+        insn32(0x2518e3e0 | $p);
+    }
+
+    # there is no post index load so we do this by hand
+    write_mov_ri(1, 0);
+    for (my $rt = 0; $rt <= 31; $rt++) {
+        # ld1d    z0.d, p0/z, [x0, x1, lsl #3]
+        insn32(0xa5e14000 | $rt);
+        # incp    x1, p0.d
+        insn32(0x25ec8801);
+    }
+}
+
+sub write_random_aarch64_regdata($$)
+{
+    my ($fp_enabled, $sve_enabled) = @_;
     # clear flags
     insn32(0xd51b421f);     # msr nzcv, xzr
 
@@ -483,6 +521,10 @@ sub write_random_aarch64_regdata($)
         write_random_aarch64_fpdata();
     }
 
+    if ($sve_enabled) {
+        write_random_aarch64_svedata();
+    }
+
     # general purpose registers
     for (my $i = 0; $i <= 30; $i++) {
         # TODO full 64 bit pattern instead of 32
@@ -490,12 +532,12 @@ sub write_random_aarch64_regdata($)
     }
 }
 
-sub write_random_register_data($)
+sub write_random_register_data($$)
 {
-    my ($fp_enabled) = @_;
+    my ($fp_enabled, $sve_enabled) = @_;
 
     if ($is_aarch64) {
-        write_random_aarch64_regdata($fp_enabled);
+        write_random_aarch64_regdata($fp_enabled, $sve_enabled);
     } else {
         write_random_arm_regdata($fp_enabled);
     }
@@ -893,6 +935,7 @@ sub write_test_code($$$$$$$$)
     my $fpscr = $params->{ 'fpscr' };
     my $numinsns = $params->{ 'numinsns' };
     my $fp_enabled = $params->{ 'fp_enabled' };
+    my $sve_enabled = $params->{ 'sve_enabled' };
     my $outfile = $params->{ 'outfile' };
 
     my %insn_details = %{ $params->{ 'details' } };
@@ -918,7 +961,7 @@ sub write_test_code($$$$$$$$)
         write_memblock_setup();
     }
     # memblock setup doesn't clean its registers, so this must come afterwards.
-    write_random_register_data($fp_enabled);
+    write_random_register_data($fp_enabled, $sve_enabled);
     write_switch_to_test_mode();
 
     for my $i (1..$numinsns) {
@@ -930,7 +973,7 @@ sub write_test_code($$$$$$$$)
         # Rewrite the registers periodically. This avoids the tendency
         # for the VFP registers to decay to NaNs and zeroes.
         if ($periodic_reg_random && ($i % 100) == 0) {
-            write_random_register_data($fp_enabled);
+            write_random_register_data($fp_enabled, $sve_enabled);
             write_switch_to_test_mode();
         }
         progress_update($i);
-- 
2.14.2

Re: [Qemu-devel] [RISU PATCH 07/10] risugen: add --sve support

[Richard Henderson]

On 11/07/2017 04:05 PM, Alex Bennée wrote:
> @@ -483,6 +521,10 @@ sub write_random_aarch64_regdata($)
>          write_random_aarch64_fpdata();
>      }
>  
> +    if ($sve_enabled) {
> +        write_random_aarch64_svedata();
> +    }

Should write either sve or fpdata, but not both, since the registers overlap.


r~

Re: [Qemu-devel] [RISU PATCH 07/10] risugen: add --sve support

[Dave Martin]

On Tue, Nov 07, 2017 at 03:05:55PM +0000, Alex Bennée wrote:
> This is similar to the approach used by the FP/simd data in so far as
> we generate a block of random data and then load into it. As there are
> no post-index SVE operations we need to emit an additional incp
> instruction to generate our offset into the array.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  risugen        |  3 +++
>  risugen_arm.pm | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 53 insertions(+), 7 deletions(-)
> 
> diff --git a/risugen b/risugen
> index aba4bb7..0ac8e86 100755
> --- a/risugen
> +++ b/risugen
> @@ -317,6 +317,7 @@ sub main()
>      my $condprob = 0;
>      my $fpscr = 0;
>      my $fp_enabled = 1;
> +    my $sve_enabled = 1;
>      my $big_endian = 0;
>      my ($infile, $outfile);
>  
> @@ -334,6 +335,7 @@ sub main()
>                  },
>                  "be" => sub { $big_endian = 1; },
>                  "no-fp" => sub { $fp_enabled = 0; },
> +                "sve" => sub { $sve_enabled = 1; },
>          ) or return 1;
>      # allow "--pattern re,re" and "--pattern re --pattern re"
>      @pattern_re = split(/,/,join(',',@pattern_re));
> @@ -361,6 +363,7 @@ sub main()
>          'fpscr' => $fpscr,
>          'numinsns' => $numinsns,
>          'fp_enabled' => $fp_enabled,
> +        'sve_enabled' => $sve_enabled,
>          'outfile' => $outfile,
>          'details' => \%insn_details,
>          'keys' => \@insn_keys,
> diff --git a/risugen_arm.pm b/risugen_arm.pm
> index 2f10d58..8d1e1fd 100644
> --- a/risugen_arm.pm
> +++ b/risugen_arm.pm
> @@ -472,9 +472,47 @@ sub write_random_aarch64_fpdata()
>      }
>  }
>  
> -sub write_random_aarch64_regdata($)
> +sub write_random_aarch64_svedata()
>  {
> -    my ($fp_enabled) = @_;
> +    # Load SVE registers
> +    my $align = 16;
> +    my $vl = 16;                             # number of vqs

Would this be better phrased

	my $vq = 16;				# quadwords per vector

> +    my $datalen = (32 * $vl * 16) + $align;
> +
> +    write_pc_adr(0, (3 * 4) + ($align - 1)); # insn 1
> +    write_align_reg(0, $align);              # insn 2
> +    write_jump_fwd($datalen);                # insn 3
> +
> +    # align safety
> +    for (my $i = 0; $i < ($align / 4); $i++) {
> +        # align with nops
> +        insn32(0xd503201f);
> +    };
> +
> +    for (my $rt = 0; $rt <= 31; $rt++) {
> +        for (my $q = 0; $q < $vl; $q++) {
> +            write_random_fpreg_var(4); # quad
> +        }
> +    }
> +
> +    # Reset all the predicate registers to all true
> +    for (my $p = 0; $p < 16; $p++) {
> +        insn32(0x2518e3e0 | $p);
> +    }
> +
> +    # there is no post index load so we do this by hand
> +    write_mov_ri(1, 0);
> +    for (my $rt = 0; $rt <= 31; $rt++) {
> +        # ld1d    z0.d, p0/z, [x0, x1, lsl #3]
> +        insn32(0xa5e14000 | $rt);
> +        # incp    x1, p0.d
> +        insn32(0x25ec8801);

You could avoid this with the unpredicated form LDR (vector).
(LD1x scalar+immediate doesn't provide enough immediate range).

	# ldr	z$rt, [x0, #$rt, mul vl]
	insn32(0x85804000 + $rt + (($rt & 7) << 10) + (($rt & 0x18) << 13));

which is what the kernel does.

No harm in exercising different instructions though!  The kernel uses
embarrassingly few.


Does it matter that the stride will depend on the actual current VL?
If x0 just points to a block of random data, I guess it doesn't matter:
some trailing data remains unused, but that doesn't make the used data
any less random.

[...]

Cheers
---Dave

Re: [Qemu-devel] [RISU PATCH 07/10] risugen: add --sve support

[Alex Bennée]

Dave Martin <Dave.Martin@arm.com> writes:

> On Tue, Nov 07, 2017 at 03:05:55PM +0000, Alex Bennée wrote:
>> This is similar to the approach used by the FP/simd data in so far as
>> we generate a block of random data and then load into it. As there are
>> no post-index SVE operations we need to emit an additional incp
>> instruction to generate our offset into the array.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  risugen        |  3 +++
>>  risugen_arm.pm | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++-------
>>  2 files changed, 53 insertions(+), 7 deletions(-)
>>
>> diff --git a/risugen b/risugen
>> index aba4bb7..0ac8e86 100755
>> --- a/risugen
>> +++ b/risugen
>> @@ -317,6 +317,7 @@ sub main()
>>      my $condprob = 0;
>>      my $fpscr = 0;
>>      my $fp_enabled = 1;
>> +    my $sve_enabled = 1;
>>      my $big_endian = 0;
>>      my ($infile, $outfile);
>>
>> @@ -334,6 +335,7 @@ sub main()
>>                  },
>>                  "be" => sub { $big_endian = 1; },
>>                  "no-fp" => sub { $fp_enabled = 0; },
>> +                "sve" => sub { $sve_enabled = 1; },
>>          ) or return 1;
>>      # allow "--pattern re,re" and "--pattern re --pattern re"
>>      @pattern_re = split(/,/,join(',',@pattern_re));
>> @@ -361,6 +363,7 @@ sub main()
>>          'fpscr' => $fpscr,
>>          'numinsns' => $numinsns,
>>          'fp_enabled' => $fp_enabled,
>> +        'sve_enabled' => $sve_enabled,
>>          'outfile' => $outfile,
>>          'details' => \%insn_details,
>>          'keys' => \@insn_keys,
>> diff --git a/risugen_arm.pm b/risugen_arm.pm
>> index 2f10d58..8d1e1fd 100644
>> --- a/risugen_arm.pm
>> +++ b/risugen_arm.pm
>> @@ -472,9 +472,47 @@ sub write_random_aarch64_fpdata()
>>      }
>>  }
>>
>> -sub write_random_aarch64_regdata($)
>> +sub write_random_aarch64_svedata()
>>  {
>> -    my ($fp_enabled) = @_;
>> +    # Load SVE registers
>> +    my $align = 16;
>> +    my $vl = 16;                             # number of vqs
>
> Would this be better phrased
>
> 	my $vq = 16;				# quadwords per vector
>
>> +    my $datalen = (32 * $vl * 16) + $align;
>> +
>> +    write_pc_adr(0, (3 * 4) + ($align - 1)); # insn 1
>> +    write_align_reg(0, $align);              # insn 2
>> +    write_jump_fwd($datalen);                # insn 3
>> +
>> +    # align safety
>> +    for (my $i = 0; $i < ($align / 4); $i++) {
>> +        # align with nops
>> +        insn32(0xd503201f);
>> +    };
>> +
>> +    for (my $rt = 0; $rt <= 31; $rt++) {
>> +        for (my $q = 0; $q < $vl; $q++) {
>> +            write_random_fpreg_var(4); # quad
>> +        }
>> +    }
>> +
>> +    # Reset all the predicate registers to all true
>> +    for (my $p = 0; $p < 16; $p++) {
>> +        insn32(0x2518e3e0 | $p);
>> +    }
>> +
>> +    # there is no post index load so we do this by hand
>> +    write_mov_ri(1, 0);
>> +    for (my $rt = 0; $rt <= 31; $rt++) {
>> +        # ld1d    z0.d, p0/z, [x0, x1, lsl #3]
>> +        insn32(0xa5e14000 | $rt);
>> +        # incp    x1, p0.d
>> +        insn32(0x25ec8801);
>
> You could avoid this with the unpredicated form LDR (vector).
> (LD1x scalar+immediate doesn't provide enough immediate range).
>
> 	# ldr	z$rt, [x0, #$rt, mul vl]
> 	insn32(0x85804000 + $rt + (($rt & 7) << 10) + (($rt & 0x18) << 13));
>
> which is what the kernel does.

Ahh thanks.

> No harm in exercising different instructions though!  The kernel uses
> embarrassingly few.

Well that's what the actual random instructions are for. I haven't yet
considered if we need to group things together to properly exercise the
predicate code. So far RISU doesn't exercise conditional branches as
it's hard to do that without generating code that would lock up. It's
very much the "easy 80%" of instructions we target.

> Does it matter that the stride will depend on the actual current VL?
> If x0 just points to a block of random data, I guess it doesn't matter:
> some trailing data remains unused, but that doesn't make the used data
> any less random.

Not really. We do try and re-generate every N instructions so all the
floats don't just denormalise.

>
> [...]
>
> Cheers
> ---Dave


--
Alex Bennée

[Qemu-devel] [RISU PATCH 08/10] aarch64.risu: initial SVE instruction

[Alex Bennée]

---
 aarch64.risu | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/aarch64.risu b/aarch64.risu
index 838bded..bfa4ff8 100644
--- a/aarch64.risu
+++ b/aarch64.risu
@@ -2937,3 +2937,27 @@ FCVTZUsi_RES A64_V sf:1 0011110 1 type:1 1 11 001 000000 rn:5 rd:5
 # End of:
 # Data processing - SIMD and floating point
 # Data processing - Scalar Floating-Point and Advanced SIMD
+
+# SVE Instructions
+# Top-level Encodings
+#
+# 31            24 | 23 22 | 21      17 | 16 | 15    10 | 9                 0
+#  y y y y y y y m |  -  - |  m m m m m |  - |          | - - - - - - - - - -
+#
+@SVE
+
+# SVE Integer Arithmetic - Binary Predicated Group
+# 31            24 | 23  22 | 21 | 20  19 | 18 16 | 15  13 | 12           0
+#  0 0 0 0 0 1 0 0 |  size  |  0 |   op   |  ---  |  0 0 0 | ------------- |
+
+SVE_INTA_PRED_UNALL A64_V 00000100 ig:2 010 001 000 ignore2:13
+
+# - SVE integer add/subtract vectors (predicated)
+# 31            24 | 23 22 | 21  19 | 18 16 | 15  13 | 12  10 | 9  5 | 4  0 |
+#  0 0 0 0 0 1 0 0 |  size |  0 0 0 |  opc  |  0 0 0 |   Pg   |  Zm  |  Zn  |
+# opc 010/1xx are UNALLOCATED
+
+SVE_INT_ADDSUB_PRED A64_V 00000100 sz:2 000 opc:3 000 pg:3 zm:5 zn:5
+
+@
+# End of SVE instructions
-- 
2.14.2

[Qemu-devel] [RISU PATCH 09/10] risu_reginfo_aarch64: add reginfo_copy_sve

[Alex Bennée]

Add the ability to save SVE registers from the signal context. This is
controlled with an optional flag --test-sve. The whole thing is
conditionally compiled when SVE support is in the sigcontext headers.

Technically SVE registers could be beyond an EXTRA_MAGIC section. I've
not seen this on the model so currently we abort() if we encounter the
EXTRA_MAGIC section.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 risu_reginfo_aarch64.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++
 risu_reginfo_aarch64.h |  8 ++++++
 2 files changed, 82 insertions(+)

diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
index 38ad338..7c97790 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -13,12 +13,78 @@
 #include <stdio.h>
 #include <ucontext.h>
 #include <string.h>
+#include <getopt.h>
+#include <stdlib.h>
+#include <stdbool.h>
 
 #include "risu.h"
 #include "risu_reginfo_aarch64.h"
 
+#ifndef SVE_MAGIC
 void *arch_long_opts;
 char *arch_extra_help;
+#else
+/* Should we test SVE register state */
+static int test_sve;
+static struct option extra_opts[] = {
+    {"test-sve", no_argument, &test_sve, 1},
+    {0, 0, 0, 0}
+};
+
+void *arch_long_opts = &extra_opts[0];
+char *arch_extra_help = "  --test-sve        Compare SVE registers\n";
+
+/* Extra SVE copy function, only called with --test-sve */
+static void reginfo_copy_sve(struct reginfo *ri, struct _aarch64_ctx *ctx)
+{
+    struct sve_context *sve;
+    int r, vq;
+    bool found = false;
+
+    while (!found) {
+       switch (ctx->magic)
+       {
+          case SVE_MAGIC:
+             found = true;
+             break;
+          case EXTRA_MAGIC:
+             fprintf(stderr, "%s: found EXTRA_MAGIC\n", __func__);
+             abort();
+          case 0:
+             /* We might not have an SVE context */
+             fprintf(stderr, "%s: reached end of ctx, no joy (%d)\n", __func__, ctx->size);
+             return;
+          default:
+             ctx = (struct _aarch64_ctx *)((void *)ctx + ctx->size);
+             break;
+       }
+
+    }
+
+    sve = (struct sve_context *) ctx;
+    ri->vl = sve->vl;
+    vq = sve_vq_from_vl(sve->vl); /* number of quads for whole vl */
+
+    /* Copy ZREG's one at a time */
+    for (r = 0; r < SVE_NUM_ZREGS; r++) {
+        memcpy(&ri->zregs[r],
+               (char *)sve + SVE_SIG_ZREG_OFFSET(vq, r),
+               SVE_SIG_ZREG_SIZE(vq));
+    }
+
+    /* Copy PREG's one at a time */
+    for (r = 0; r < SVE_NUM_PREGS; r++) {
+        memcpy(&ri->pregs[r],
+               (char *)sve + SVE_SIG_PREG_OFFSET(vq, r),
+               SVE_SIG_PREG_SIZE(vq));
+    }
+
+    /* Finally the FFR */
+    memcpy(&ri->ffr,(char *)sve + SVE_SIG_FFR_OFFSET(vq),
+               SVE_SIG_FFR_SIZE(vq));
+
+}
+#endif
 
 /* reginfo_init: initialize with a ucontext */
 void reginfo_init(struct reginfo *ri, ucontext_t *uc)
@@ -26,6 +92,7 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
     int i;
     struct _aarch64_ctx *ctx;
     struct fpsimd_context *fp;
+
     /* necessary to be able to compare with memcmp later */
     memset(ri, 0, sizeof(*ri));
 
@@ -59,6 +126,13 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
     for (i = 0; i < 32; i++) {
         ri->vregs[i] = fp->vregs[i];
     }
+
+#ifdef SVE_MAGIC
+    if (test_sve) {
+        ctx = (struct _aarch64_ctx *) &uc->uc_mcontext.__reserved[0];
+        reginfo_copy_sve(ri, ctx);
+    }
+#endif
 };
 
 /* reginfo_is_eq: compare the reginfo structs, returns nonzero if equal */
diff --git a/risu_reginfo_aarch64.h b/risu_reginfo_aarch64.h
index a05fb4e..317383f 100644
--- a/risu_reginfo_aarch64.h
+++ b/risu_reginfo_aarch64.h
@@ -25,6 +25,14 @@ struct reginfo {
     uint32_t fpsr;
     uint32_t fpcr;
     __uint128_t vregs[32];
+
+#ifdef SVE_MAGIC
+    /* SVE */
+    uint16_t    vl; /* current VL */
+    __uint128_t zregs[SVE_NUM_ZREGS][SVE_VQ_MAX];
+    uint16_t    pregs[SVE_NUM_PREGS][SVE_VQ_MAX];
+    uint16_t    ffr[SVE_VQ_MAX];
+#endif
 };
 
 #endif /* RISU_REGINFO_AARCH64_H */
-- 
2.14.2

Re: [Qemu-devel] [RISU PATCH 09/10] risu_reginfo_aarch64: add reginfo_copy_sve

[Dave Martin]

On Tue, Nov 07, 2017 at 03:05:57PM +0000, Alex Bennée wrote:
> Add the ability to save SVE registers from the signal context. This is
> controlled with an optional flag --test-sve. The whole thing is
> conditionally compiled when SVE support is in the sigcontext headers.
> 
> Technically SVE registers could be beyond an EXTRA_MAGIC section. I've
> not seen this on the model so currently we abort() if we encounter the
> EXTRA_MAGIC section.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  risu_reginfo_aarch64.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  risu_reginfo_aarch64.h |  8 ++++++
>  2 files changed, 82 insertions(+)
> 
> diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
> index 38ad338..7c97790 100644
> --- a/risu_reginfo_aarch64.c
> +++ b/risu_reginfo_aarch64.c
> @@ -13,12 +13,78 @@
>  #include <stdio.h>
>  #include <ucontext.h>
>  #include <string.h>
> +#include <getopt.h>
> +#include <stdlib.h>
> +#include <stdbool.h>
>  
>  #include "risu.h"
>  #include "risu_reginfo_aarch64.h"
>  
> +#ifndef SVE_MAGIC
>  void *arch_long_opts;
>  char *arch_extra_help;
> +#else
> +/* Should we test SVE register state */
> +static int test_sve;
> +static struct option extra_opts[] = {
> +    {"test-sve", no_argument, &test_sve, 1},
> +    {0, 0, 0, 0}
> +};
> +
> +void *arch_long_opts = &extra_opts[0];
> +char *arch_extra_help = "  --test-sve        Compare SVE registers\n";
> +
> +/* Extra SVE copy function, only called with --test-sve */
> +static void reginfo_copy_sve(struct reginfo *ri, struct _aarch64_ctx *ctx)
> +{
> +    struct sve_context *sve;
> +    int r, vq;
> +    bool found = false;
> +
> +    while (!found) {
> +       switch (ctx->magic)
> +       {
> +          case SVE_MAGIC:
> +             found = true;
> +             break;
> +          case EXTRA_MAGIC:
> +             fprintf(stderr, "%s: found EXTRA_MAGIC\n", __func__);
> +             abort();
> +          case 0:
> +             /* We might not have an SVE context */
> +             fprintf(stderr, "%s: reached end of ctx, no joy (%d)\n", __func__, ctx->size);
> +             return;
> +          default:
> +             ctx = (struct _aarch64_ctx *)((void *)ctx + ctx->size);
> +             break;
> +       }
> +
> +    }
> +
> +    sve = (struct sve_context *) ctx;
> +    ri->vl = sve->vl;
> +    vq = sve_vq_from_vl(sve->vl); /* number of quads for whole vl */
> +
> +    /* Copy ZREG's one at a time */
> +    for (r = 0; r < SVE_NUM_ZREGS; r++) {
> +        memcpy(&ri->zregs[r],
> +               (char *)sve + SVE_SIG_ZREG_OFFSET(vq, r),
> +               SVE_SIG_ZREG_SIZE(vq));
> +    }
> +
> +    /* Copy PREG's one at a time */
> +    for (r = 0; r < SVE_NUM_PREGS; r++) {
> +        memcpy(&ri->pregs[r],
> +               (char *)sve + SVE_SIG_PREG_OFFSET(vq, r),
> +               SVE_SIG_PREG_SIZE(vq));
> +    }
> +
> +    /* Finally the FFR */
> +    memcpy(&ri->ffr,(char *)sve + SVE_SIG_FFR_OFFSET(vq),
> +               SVE_SIG_FFR_SIZE(vq));
> +
> +}
> +#endif
>  
>  /* reginfo_init: initialize with a ucontext */
>  void reginfo_init(struct reginfo *ri, ucontext_t *uc)
> @@ -26,6 +92,7 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
>      int i;
>      struct _aarch64_ctx *ctx;
>      struct fpsimd_context *fp;
> +
>      /* necessary to be able to compare with memcmp later */
>      memset(ri, 0, sizeof(*ri));
>  
> @@ -59,6 +126,13 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
>      for (i = 0; i < 32; i++) {
>          ri->vregs[i] = fp->vregs[i];
>      }
> +

Where do you get uc from?

If it comes straight out of a signal frame that's OK, but if it's copied
around as a ucontext_t or manipulated by getcontext() etc this is going
to go wrong when extra_context is present.

This code may get used by people as a reference, so at least adding a
comment to the effect that this only works on the signal frame would be
a good idea.


When you add support for extra_context, you should probably add a
sanity-check to ensure that the datap pointer really does point to the
right place -- see <asm/sigcontext.h>.

Even though the kernel _should_ never violate this, it can be violated
by ucontext_t manipulation in userspace, so any function that doesn't
know where its ucontext_t came from should do this check.

[...]

Cheers
---Dave

[Qemu-devel] [RISU PATCH 10/10] risu_reginfo_aarch64: add SVE support to reginfo_dump_mismatch

[Alex Bennée]

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 risu_reginfo_aarch64.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
index 7c97790..8aba192 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -141,6 +141,18 @@ int reginfo_is_eq(struct reginfo *r1, struct reginfo *r2)
     return memcmp(r1, r2, sizeof(*r1)) == 0;
 }
 
+#ifdef SVE_MAGIC
+static int sve_zreg_is_eq(struct reginfo *r1, struct reginfo *r2, int z)
+{
+    return memcmp(r1->zregs[z], r2->zregs[z], sizeof(*r1->zregs[z])) == 0;
+}
+
+static int sve_preg_is_eq(struct reginfo *r1, struct reginfo *r2, int p)
+{
+    return memcmp(r1->pregs[p], r2->pregs[p], sizeof(*r1->pregs[p])) == 0;
+}
+#endif
+
 /* reginfo_dump: print state to a stream, returns nonzero on success */
 int reginfo_dump(struct reginfo *ri, FILE * f)
 {
@@ -216,5 +228,42 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
         }
     }
 
+#ifdef SVE_MAGIC
+    if (test_sve) {
+        if (m->vl != a->vl) {
+            fprintf(f, "  SVE VL  : %d vs %d\n", m->vl, a->vl);
+        }
+        for (i = 0; i < SVE_NUM_PREGS; i++) {
+           if (!sve_preg_is_eq(m, a, i)) {
+              int q;
+              fprintf(f, "  P%2d   : ", i);
+              for (q = 0; q < sve_vq_from_vl(m->vl); q++) {
+                 fprintf(f, "%04x", m->pregs[i][q]);
+              }
+              fprintf(f, " vs ");
+              for (q = 0; q < sve_vq_from_vl(m->vl); q++) {
+                 fprintf(f, "%04x", a->pregs[i][q]);
+              }
+              fprintf(f, "\n");
+            }
+        }
+        for (i = 0; i < SVE_NUM_ZREGS; i++) {
+           if (!sve_zreg_is_eq(m, a, i)) {
+              int q;
+              char *pad="";
+              fprintf(f, "  Z%2d   : ", i);
+              for (q = 0; q < sve_vq_from_vl(m->vl); q++) {
+                 if (m->zregs[i][q] != a->zregs[i][q]) {
+                    fprintf(f, "%sq%02d: %016" PRIx64 "%016" PRIx64 " vs %016" PRIx64 "%016" PRIx64"\n", pad, q,
+                            (uint64_t) (m->zregs[i][q] >> 64), (uint64_t) m->zregs[i][q],
+                            (uint64_t) (a->zregs[i][q] >> 64), (uint64_t) a->zregs[i][q]);
+                    pad = "          ";
+                 }
+              }
+           }
+        }
+    }
+#endif
+
     return !ferror(f);
 }
-- 
2.14.2

Re: [Qemu-devel] [RISU PATCH 10/10] risu_reginfo_aarch64: add SVE support to reginfo_dump_mismatch

[Dave Martin]

On Tue, Nov 07, 2017 at 03:05:58PM +0000, Alex Bennée wrote:
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  risu_reginfo_aarch64.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
> index 7c97790..8aba192 100644
> --- a/risu_reginfo_aarch64.c
> +++ b/risu_reginfo_aarch64.c
> @@ -141,6 +141,18 @@ int reginfo_is_eq(struct reginfo *r1, struct reginfo *r2)
>      return memcmp(r1, r2, sizeof(*r1)) == 0;
>  }
>  
> +#ifdef SVE_MAGIC
> +static int sve_zreg_is_eq(struct reginfo *r1, struct reginfo *r2, int z)
> +{
> +    return memcmp(r1->zregs[z], r2->zregs[z], sizeof(*r1->zregs[z])) == 0;
> +}
> +
> +static int sve_preg_is_eq(struct reginfo *r1, struct reginfo *r2, int p)
> +{
> +    return memcmp(r1->pregs[p], r2->pregs[p], sizeof(*r1->pregs[p])) == 0;
> +}
> +#endif
> +
>  /* reginfo_dump: print state to a stream, returns nonzero on success */
>  int reginfo_dump(struct reginfo *ri, FILE * f)
>  {
> @@ -216,5 +228,42 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
>          }
>      }
>  
> +#ifdef SVE_MAGIC
> +    if (test_sve) {
> +        if (m->vl != a->vl) {
> +            fprintf(f, "  SVE VL  : %d vs %d\n", m->vl, a->vl);
> +        }
> +        for (i = 0; i < SVE_NUM_PREGS; i++) {
> +           if (!sve_preg_is_eq(m, a, i)) {
> +              int q;
> +              fprintf(f, "  P%2d   : ", i);
> +              for (q = 0; q < sve_vq_from_vl(m->vl); q++) {
> +                 fprintf(f, "%04x", m->pregs[i][q]);
> +              }
> +              fprintf(f, " vs ");
> +              for (q = 0; q < sve_vq_from_vl(m->vl); q++) {
> +                 fprintf(f, "%04x", a->pregs[i][q]);
> +              }
> +              fprintf(f, "\n");
> +            }
> +        }
> +        for (i = 0; i < SVE_NUM_ZREGS; i++) {
> +           if (!sve_zreg_is_eq(m, a, i)) {
> +              int q;
> +              char *pad="";
> +              fprintf(f, "  Z%2d   : ", i);
> +              for (q = 0; q < sve_vq_from_vl(m->vl); q++) {
> +                 if (m->zregs[i][q] != a->zregs[i][q]) {
> +                    fprintf(f, "%sq%02d: %016" PRIx64 "%016" PRIx64 " vs %016" PRIx64 "%016" PRIx64"\n", pad, q,
> +                            (uint64_t) (m->zregs[i][q] >> 64), (uint64_t) m->zregs[i][q],
> +                            (uint64_t) (a->zregs[i][q] >> 64), (uint64_t) a->zregs[i][q]);
> +                    pad = "          ";
> +                 }
> +              }
> +           }
> +        }

No FFR?

Perhaps I should have explicitly encoded FFR as "P16" -- that sort of
works and saves some open-coding of the extra special case, but it feels
less correct.

You could do

static int sve_preg_is_eq(uint16_t const (*p1)[SVE_VQ_MAX],
				uint16_t const (*p2)[SVE_VQ_MAX])
{
	return memcmp(p1, p2, sizeof *p1) == 0;
}

/* ... */

sve_preg_is_eq(&r1->pregs[p], &r2->pregs[p])

/* ... */

sve_preg_is_eq(&r1->ffr, &r2->ffr)

(or some variation on this theme).  ffr is a specialised predicate
register, so I think you can assume that it really does have the same
type as pregs[p].  Coding the above way will give a typcheck error if
not.

Cheers
---Dave

Re: [Qemu-devel] [RISU PATCH 10/10] risu_reginfo_aarch64: add SVE support to reginfo_dump_mismatch

[Alex Bennée]

Dave Martin <Dave.Martin@arm.com> writes:

> On Tue, Nov 07, 2017 at 03:05:58PM +0000, Alex Bennée wrote:
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  risu_reginfo_aarch64.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 49 insertions(+)
>>
>> diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
>> index 7c97790..8aba192 100644
>> --- a/risu_reginfo_aarch64.c
>> +++ b/risu_reginfo_aarch64.c
>> @@ -141,6 +141,18 @@ int reginfo_is_eq(struct reginfo *r1, struct reginfo *r2)
>>      return memcmp(r1, r2, sizeof(*r1)) == 0;
>>  }
>>
>> +#ifdef SVE_MAGIC
>> +static int sve_zreg_is_eq(struct reginfo *r1, struct reginfo *r2, int z)
>> +{
>> +    return memcmp(r1->zregs[z], r2->zregs[z], sizeof(*r1->zregs[z])) == 0;
>> +}
>> +
>> +static int sve_preg_is_eq(struct reginfo *r1, struct reginfo *r2, int p)
>> +{
>> +    return memcmp(r1->pregs[p], r2->pregs[p], sizeof(*r1->pregs[p])) == 0;
>> +}
>> +#endif
>> +
>>  /* reginfo_dump: print state to a stream, returns nonzero on success */
>>  int reginfo_dump(struct reginfo *ri, FILE * f)
>>  {
>> @@ -216,5 +228,42 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
>>          }
>>      }
>>
>> +#ifdef SVE_MAGIC
>> +    if (test_sve) {
>> +        if (m->vl != a->vl) {
>> +            fprintf(f, "  SVE VL  : %d vs %d\n", m->vl, a->vl);
>> +        }
>> +        for (i = 0; i < SVE_NUM_PREGS; i++) {
>> +           if (!sve_preg_is_eq(m, a, i)) {
>> +              int q;
>> +              fprintf(f, "  P%2d   : ", i);
>> +              for (q = 0; q < sve_vq_from_vl(m->vl); q++) {
>> +                 fprintf(f, "%04x", m->pregs[i][q]);
>> +              }
>> +              fprintf(f, " vs ");
>> +              for (q = 0; q < sve_vq_from_vl(m->vl); q++) {
>> +                 fprintf(f, "%04x", a->pregs[i][q]);
>> +              }
>> +              fprintf(f, "\n");
>> +            }
>> +        }
>> +        for (i = 0; i < SVE_NUM_ZREGS; i++) {
>> +           if (!sve_zreg_is_eq(m, a, i)) {
>> +              int q;
>> +              char *pad="";
>> +              fprintf(f, "  Z%2d   : ", i);
>> +              for (q = 0; q < sve_vq_from_vl(m->vl); q++) {
>> +                 if (m->zregs[i][q] != a->zregs[i][q]) {
>> +                    fprintf(f, "%sq%02d: %016" PRIx64 "%016" PRIx64 " vs %016" PRIx64 "%016" PRIx64"\n", pad, q,
>> +                            (uint64_t) (m->zregs[i][q] >> 64), (uint64_t) m->zregs[i][q],
>> +                            (uint64_t) (a->zregs[i][q] >> 64), (uint64_t) a->zregs[i][q]);
>> +                    pad = "          ";
>> +                 }
>> +              }
>> +           }
>> +        }
>
> No FFR?
>
> Perhaps I should have explicitly encoded FFR as "P16" -- that sort of
> works and saves some open-coding of the extra special case, but it feels
> less correct.

Oops, yeah missed that out. I'll add it to the next version.

Good catch!

>
> You could do
>
> static int sve_preg_is_eq(uint16_t const (*p1)[SVE_VQ_MAX],
> 				uint16_t const (*p2)[SVE_VQ_MAX])
> {
> 	return memcmp(p1, p2, sizeof *p1) == 0;
> }
>
> /* ... */
>
> sve_preg_is_eq(&r1->pregs[p], &r2->pregs[p])
>
> /* ... */
>
> sve_preg_is_eq(&r1->ffr, &r2->ffr)
>
> (or some variation on this theme).  ffr is a specialised predicate
> register, so I think you can assume that it really does have the same
> type as pregs[p].  Coding the above way will give a typcheck error if
> not.

Ahh ok thanks.

>
> Cheers
> ---Dave


--
Alex Bennée

Re: [Qemu-devel] [RISU PATCH 00/10] Initial support for SVE

[Dave Martin]

On Tue, Nov 07, 2017 at 03:05:48PM +0000, Alex Bennée wrote:
> Hi,
> 
> These patches apply on-top of the last clean-up series:
> 
>   Subject: [RISU PATCH 0/7] Add @Group support and some aarch64.risu cleanups
>   Date: Tue, 31 Oct 2017 14:54:37 +0000
>   Message-Id: <20171031145444.13766-1-alex.bennee@linaro.org>
> 
> This series adds support for SVE to RISU. Most of the initial patches
> are plumbing changes to better support arch specific option flags
> (cleaning up a TODO in the process). I also needed to ensure configure
> actually honoured CPPFLAGS so it could be passed yet to be released
> headers.

Should there be a getauxval(AT_HWCAP) & HWCAP_SVE check in this series
somewhere?

I don't know enough about how RISU is structured to know whether/where
this is needed.

[...]

Cheers
---Dave

Re: [Qemu-devel] [RISU PATCH 00/10] Initial support for SVE

[Alex Bennée]

Dave Martin <Dave.Martin@arm.com> writes:

> On Tue, Nov 07, 2017 at 03:05:48PM +0000, Alex Bennée wrote:
>> Hi,
>>
>> These patches apply on-top of the last clean-up series:
>>
>>   Subject: [RISU PATCH 0/7] Add @Group support and some aarch64.risu cleanups
>>   Date: Tue, 31 Oct 2017 14:54:37 +0000
>>   Message-Id: <20171031145444.13766-1-alex.bennee@linaro.org>
>>
>> This series adds support for SVE to RISU. Most of the initial patches
>> are plumbing changes to better support arch specific option flags
>> (cleaning up a TODO in the process). I also needed to ensure configure
>> actually honoured CPPFLAGS so it could be passed yet to be released
>> headers.
>
> Should there be a getauxval(AT_HWCAP) & HWCAP_SVE check in this series
> somewhere?
>
> I don't know enough about how RISU is structured to know whether/where
> this is needed.

That would be a saner runtime check to do but it's a balance as RISU is
a fairly specialist tool which kind of assumes people know what they are
doing.

The current check is on SVE_MAGIC in the header files which does mean a
binary compiled on an SVE headered system is now carrying about a much
larger register dump even when run without the --test-sve flag.

Whether it makes sense to be more flexible is a call I'll leave up to
Peter.

>
> [...]
>
> Cheers
> ---Dave


--
Alex Bennée

Re: [Qemu-devel] [RISU PATCH 00/10] Initial support for SVE

[Dave Martin]

On Wed, Nov 08, 2017 at 11:02:15AM +0000, Alex Bennée wrote:
> 
> Dave Martin <Dave.Martin@arm.com> writes:
> 
> > On Tue, Nov 07, 2017 at 03:05:48PM +0000, Alex Bennée wrote:
> >> Hi,
> >>
> >> These patches apply on-top of the last clean-up series:
> >>
> >>   Subject: [RISU PATCH 0/7] Add @Group support and some aarch64.risu cleanups
> >>   Date: Tue, 31 Oct 2017 14:54:37 +0000
> >>   Message-Id: <20171031145444.13766-1-alex.bennee@linaro.org>
> >>
> >> This series adds support for SVE to RISU. Most of the initial patches
> >> are plumbing changes to better support arch specific option flags
> >> (cleaning up a TODO in the process). I also needed to ensure configure
> >> actually honoured CPPFLAGS so it could be passed yet to be released
> >> headers.
> >
> > Should there be a getauxval(AT_HWCAP) & HWCAP_SVE check in this series
> > somewhere?
> >
> > I don't know enough about how RISU is structured to know whether/where
> > this is needed.
> 
> That would be a saner runtime check to do but it's a balance as RISU is
> a fairly specialist tool which kind of assumes people know what they are
> doing.
> 
> The current check is on SVE_MAGIC in the header files which does mean a
> binary compiled on an SVE headered system is now carrying about a much
> larger register dump even when run without the --test-sve flag.
> 
> Whether it makes sense to be more flexible is a call I'll leave up to
> Peter.

Fair enough.  If there is anywhere useful to put it, it would serve as
a useful example -- but as you point out, this is not a typical piece
of userspace software...

Cheers
---Dave

Re: [Qemu-devel] [RISU PATCH 00/10] Initial support for SVE

[Peter Maydell]

On 7 November 2017 at 15:05, Alex Bennée <alex.bennee@linaro.org> wrote:
> Hi,
>
> These patches apply on-top of the last clean-up series:
>
>   Subject: [RISU PATCH 0/7] Add @Group support and some aarch64.risu cleanups
>   Date: Tue, 31 Oct 2017 14:54:37 +0000
>   Message-Id: <20171031145444.13766-1-alex.bennee@linaro.org>
>
> This series adds support for SVE to RISU. Most of the initial patches
> are plumbing changes to better support arch specific option flags
> (cleaning up a TODO in the process). I also needed to ensure configure
> actually honoured CPPFLAGS so it could be passed yet to be released
> headers.
>
> The actual guts of the SVE support is in 3 patches. One to risugen so
> it can generate random values for the vector registers. If this isn't
> done you get inconsistent runs because of left over junk in the
> registers. The remaining two patches add support for copying the SVE
> vectors from the signal context and dumping any differences found.
>
> To test this I ran on the Foundation model with:
>
>   ${RTSM_MODEL} --no-secure-memory --gicv3 --data=${BL1_FIRMWARE}@0x0 --data=${FIP_FIRMWARE}@0x8000000 \
>                 --semihost --network nat --network-nat-ports=8022=22 --block-device ${DISK_IMAGE}
>
> With my hacked up copy of secure firmware to ensure SVE is enabled.
> You also need a kernel with SVE support:
>
>   Subject: [PATCH v5 00/30] ARM Scalable Vector Extension (SVE)
>   Date: Tue, 31 Oct 2017 15:50:52 +0000
>   Message-Id: <1509465082-30427-1-git-send-email-Dave.Martin@arm.com>
>
> My testing so far has been to ensure that each run on the foundation
> model is deterministic (same m5sum on each trace) and that differences
> are printed out when the user-space vector length is tweaked
> (/proc/sys/abi/sve_default_vector_length).
>
>
> Alex Bennée (10):
>   build-all-arches: drop -t (for tty) from docker invocation
>   risu.c: split out setting up options
>   risu.c: add missing --trace longopt
>   risu: move optional args to each architecture
>   configure: allow repeated invocation of configure in build dir
>   configure: support CPPFLAGS
>   risugen: add --sve support
>   aarch64.risu: initial SVE instruction
>   risu_reginfo_aarch64: add reginfo_copy_sve
>   risu_reginfo_aarch64: add SVE support to reginfo_dump_mismatch

I've applied patches 1, 2, 3, 5, 6 to risu master.

thanks
-- PMM