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.ben...@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
