diff mbox series

[RISU,v5,08/13] risu: add header to trace stream

Message ID 20170619104655.31104-9-alex.bennee@linaro.org
State Superseded
Headers show
Series RISU record/replay patches | expand

Commit Message

Alex Bennée June 19, 2017, 10:46 a.m. UTC 
I've also added a header packet with pc/risu op in it so we can keep
better track of how things are going.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>


---
v5
  - re-base without formatting fixes
  - dropped fprintfs in signal context
v4
  - split from previous patch
---
 reginfo.c      | 94 +++++++++++++++++++++++++++++++++++++---------------------
 risu.h         |  9 ++++++
 risu_aarch64.c |  5 ++++
 risu_arm.c     |  5 ++++
 risu_m68k.c    |  5 ++++
 risu_ppc64.c   |  5 ++++
 6 files changed, 89 insertions(+), 34 deletions(-)

-- 
2.13.0

Comments

Peter Maydell June 20, 2017, 1:55 p.m. UTC | #1 
On 19 June 2017 at 11:46, Alex Bennée <alex.bennee@linaro.org> wrote:
> I've also added a header packet with pc/risu op in it so we can keep

> better track of how things are going.

>

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

>

> ---

> v5

>   - re-base without formatting fixes

>   - dropped fprintfs in signal context

> v4

>   - split from previous patch

> ---

>  reginfo.c      | 94 +++++++++++++++++++++++++++++++++++++---------------------

>  risu.h         |  9 ++++++

>  risu_aarch64.c |  5 ++++

>  risu_arm.c     |  5 ++++

>  risu_m68k.c    |  5 ++++

>  risu_ppc64.c   |  5 ++++

>  6 files changed, 89 insertions(+), 34 deletions(-)

>

> diff --git a/reginfo.c b/reginfo.c

> index 90cea8f..4ff937f 100644

> --- a/reginfo.c

> +++ b/reginfo.c

> @@ -24,10 +24,19 @@ static int packet_mismatch;

>  int send_register_info(write_fn write_fn, void *uc)

>  {

>      struct reginfo ri;

> +    trace_header_t header;

>      int op;

> +

>      reginfo_init(&ri, uc);

>      op = get_risuop(&ri);

>

> +    /* Write a header with PC/op to keep in sync */

> +    header.pc = get_pc(&ri);

> +    header.risu_op = op;

> +    if (write_fn(&header, sizeof(header)) != 0) {

> +        return -1;

> +    }

> +

>      switch (op) {

>      case OP_TESTEND:

>          write_fn(&ri, sizeof(ri));

> @@ -63,46 +72,63 @@ int send_register_info(write_fn write_fn, void *uc)

>  int recv_and_compare_register_info(read_fn read_fn, respond_fn resp_fn, void *uc)

>  {

>      int resp = 0, op;

> +    trace_header_t header;

>

>      reginfo_init(&master_ri, uc);

>      op = get_risuop(&master_ri);

>

> -    switch (op) {

> -    case OP_COMPARE:

> -    case OP_TESTEND:

> -    default:

> -        /* Do a simple register compare on (a) explicit request

> -         * (b) end of test (c) a non-risuop UNDEF

> -         */

> -        if (read_fn(&apprentice_ri, sizeof(apprentice_ri))) {

> -            packet_mismatch = 1;

> -            resp = 2;

> -        } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {

> -            /* register mismatch */

> -            resp = 2;

> -        } else if (op == OP_TESTEND) {

> -            resp = 1;

> -        }

> -        resp_fn(resp);

> -        break;

> -    case OP_SETMEMBLOCK:

> -        memblock = (void *)(uintptr_t)get_reginfo_paramreg(&master_ri);

> -        break;

> -    case OP_GETMEMBLOCK:

> -        set_ucontext_paramreg(uc, get_reginfo_paramreg(&master_ri) +

> -                              (uintptr_t)memblock);

> -        break;

> -    case OP_COMPAREMEM:

> -        mem_used = 1;

> -        if (read_fn(apprentice_memblock, MEMBLOCKLEN)) {

> -            packet_mismatch = 1;

> -            resp = 2;

> -        } else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {

> -            /* memory mismatch */

> -            resp = 2;

> +    if (read_fn(&header, sizeof(header)) != 0) {

> +        return -1;

> +    }

> +

> +    if (header.risu_op == op ) {


Stray extra space.

> +

> +        /* send OK for the header */

> +        resp_fn(0);

> +

> +        switch (op) {

> +        case OP_COMPARE:

> +        case OP_TESTEND:

> +        default:

> +            /* Do a simple register compare on (a) explicit request

> +             * (b) end of test (c) a non-risuop UNDEF

> +             */

> +            if (read_fn(&apprentice_ri, sizeof(apprentice_ri))) {

> +                packet_mismatch = 1;

> +                resp = 2;

> +

> +            } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {

> +                /* register mismatch */

> +                resp = 2;

> +

> +            } else if (op == OP_TESTEND) {

> +                resp = 1;

> +            }

> +            resp_fn(resp);

> +            break;

> +        case OP_SETMEMBLOCK:

> +            memblock = (void *)(uintptr_t)get_reginfo_paramreg(&master_ri);

> +            break;

> +        case OP_GETMEMBLOCK:

> +            set_ucontext_paramreg(uc, get_reginfo_paramreg(&master_ri) +

> +                                  (uintptr_t)memblock);

> +            break;

> +        case OP_COMPAREMEM:

> +            mem_used = 1;

> +            if (read_fn(apprentice_memblock, MEMBLOCKLEN)) {

> +                packet_mismatch = 1;

> +                resp = 2;

> +            } else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {

> +                /* memory mismatch */

> +                resp = 2;

> +            }

> +            resp_fn(resp);

> +            break;

>          }

> +    } else {

> +        /* We are out of sync */

> +        resp = 2;

>          resp_fn(resp);

> -        break;


This is probably easier to read if structured the other way
up, so:

     if (header.risu_op != op) {
        /* out of sync */
        ...
        return resp;
     }

and then you can leave the rest of the function alone.


>      }

>

>      return resp;

> diff --git a/risu.h b/risu.h

> index 32241bc..e9e70ab 100644

> --- a/risu.h

> +++ b/risu.h

> @@ -51,6 +51,12 @@ extern int ismaster;

>

>  struct reginfo;

>

> +typedef struct

> +{

> +   uintptr_t pc;

> +   uint32_t risu_op;

> +} trace_header_t;

> +

>  /* Functions operating on reginfo */

>

>  /* To keep the read/write logic from multiplying across all arches

> @@ -102,6 +108,9 @@ uint64_t get_reginfo_paramreg(struct reginfo *ri);

>   */

>  int get_risuop(struct reginfo *ri);

>

> +/* Return the PC from a reginfo */

> +uintptr_t get_pc(struct reginfo *ri);

> +

>  /* initialize structure from a ucontext */

>  void reginfo_init(struct reginfo *ri, ucontext_t *uc);

>

> diff --git a/risu_aarch64.c b/risu_aarch64.c

> index 9c6809d..492d141 100644

> --- a/risu_aarch64.c

> +++ b/risu_aarch64.c

> @@ -40,3 +40,8 @@ int get_risuop(struct reginfo *ri)

>      uint32_t risukey = 0x00005af0;

>      return (key != risukey) ? -1 : op;

>  }

> +

> +uintptr_t get_pc(struct reginfo *ri)

> +{

> +   return ri->pc;


Indent here (and in some of the other versions of this
function) isn't matching the standard set up in earlier patches.

> +}

> diff --git a/risu_arm.c b/risu_arm.c

> index a55c6c2..5fcb2a5 100644

> --- a/risu_arm.c

> +++ b/risu_arm.c

> @@ -68,3 +68,8 @@ int get_risuop(struct reginfo *ri)

>      uint32_t risukey = (isz == 2) ? 0xdee0 : 0xe7fe5af0;

>      return (key != risukey) ? -1 : op;

>  }

> +

> +uintptr_t get_pc(struct reginfo *ri)

> +{

> +   return ri->gpreg[15];

> +}

> diff --git a/risu_m68k.c b/risu_m68k.c

> index 0bf5c14..1056eef 100644

> --- a/risu_m68k.c

> +++ b/risu_m68k.c

> @@ -33,3 +33,8 @@ int get_risuop(struct reginfo *ri)

>      uint32_t risukey = 0x4afc7000;

>      return (key != risukey) ? -1 : op;

>  }

> +

> +uintptr_t get_pc(struct reginfo *ri)

> +{

> +    return ri->gregs[R_PC];

> +}

> diff --git a/risu_ppc64.c b/risu_ppc64.c

> index eb60573..83f8d1f 100644

> --- a/risu_ppc64.c

> +++ b/risu_ppc64.c

> @@ -38,3 +38,8 @@ int get_risuop(struct reginfo *ri)

>      uint32_t risukey = 0x00005af0;

>      return (key != risukey) ? -1 : op;

>  }

> +

> +uintptr_t get_pc(struct reginfo *ri)

> +{

> +   return ri->nip;

> +}

> --


Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>


thanks
-- PMM
diff mbox series

Patch

diff --git a/reginfo.c b/reginfo.c
index 90cea8f..4ff937f 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -24,10 +24,19 @@  static int packet_mismatch;
 int send_register_info(write_fn write_fn, void *uc)
 {
     struct reginfo ri;
+    trace_header_t header;
     int op;
+
     reginfo_init(&ri, uc);
     op = get_risuop(&ri);
 
+    /* Write a header with PC/op to keep in sync */
+    header.pc = get_pc(&ri);
+    header.risu_op = op;
+    if (write_fn(&header, sizeof(header)) != 0) {
+        return -1;
+    }
+
     switch (op) {
     case OP_TESTEND:
         write_fn(&ri, sizeof(ri));
@@ -63,46 +72,63 @@  int send_register_info(write_fn write_fn, void *uc)
 int recv_and_compare_register_info(read_fn read_fn, respond_fn resp_fn, void *uc)
 {
     int resp = 0, op;
+    trace_header_t header;
 
     reginfo_init(&master_ri, uc);
     op = get_risuop(&master_ri);
 
-    switch (op) {
-    case OP_COMPARE:
-    case OP_TESTEND:
-    default:
-        /* Do a simple register compare on (a) explicit request
-         * (b) end of test (c) a non-risuop UNDEF
-         */
-        if (read_fn(&apprentice_ri, sizeof(apprentice_ri))) {
-            packet_mismatch = 1;
-            resp = 2;
-        } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
-            /* register mismatch */
-            resp = 2;
-        } else if (op == OP_TESTEND) {
-            resp = 1;
-        }
-        resp_fn(resp);
-        break;
-    case OP_SETMEMBLOCK:
-        memblock = (void *)(uintptr_t)get_reginfo_paramreg(&master_ri);
-        break;
-    case OP_GETMEMBLOCK:
-        set_ucontext_paramreg(uc, get_reginfo_paramreg(&master_ri) +
-                              (uintptr_t)memblock);
-        break;
-    case OP_COMPAREMEM:
-        mem_used = 1;
-        if (read_fn(apprentice_memblock, MEMBLOCKLEN)) {
-            packet_mismatch = 1;
-            resp = 2;
-        } else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {
-            /* memory mismatch */
-            resp = 2;
+    if (read_fn(&header, sizeof(header)) != 0) {
+        return -1;
+    }
+
+    if (header.risu_op == op ) {
+
+        /* send OK for the header */
+        resp_fn(0);
+
+        switch (op) {
+        case OP_COMPARE:
+        case OP_TESTEND:
+        default:
+            /* Do a simple register compare on (a) explicit request
+             * (b) end of test (c) a non-risuop UNDEF
+             */
+            if (read_fn(&apprentice_ri, sizeof(apprentice_ri))) {
+                packet_mismatch = 1;
+                resp = 2;
+
+            } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
+                /* register mismatch */
+                resp = 2;
+
+            } else if (op == OP_TESTEND) {
+                resp = 1;
+            }
+            resp_fn(resp);
+            break;
+        case OP_SETMEMBLOCK:
+            memblock = (void *)(uintptr_t)get_reginfo_paramreg(&master_ri);
+            break;
+        case OP_GETMEMBLOCK:
+            set_ucontext_paramreg(uc, get_reginfo_paramreg(&master_ri) +
+                                  (uintptr_t)memblock);
+            break;
+        case OP_COMPAREMEM:
+            mem_used = 1;
+            if (read_fn(apprentice_memblock, MEMBLOCKLEN)) {
+                packet_mismatch = 1;
+                resp = 2;
+            } else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {
+                /* memory mismatch */
+                resp = 2;
+            }
+            resp_fn(resp);
+            break;
         }
+    } else {
+        /* We are out of sync */
+        resp = 2;
         resp_fn(resp);
-        break;
     }
 
     return resp;
diff --git a/risu.h b/risu.h
index 32241bc..e9e70ab 100644
--- a/risu.h
+++ b/risu.h
@@ -51,6 +51,12 @@  extern int ismaster;
 
 struct reginfo;
 
+typedef struct
+{
+   uintptr_t pc;
+   uint32_t risu_op;
+} trace_header_t;
+
 /* Functions operating on reginfo */
 
 /* To keep the read/write logic from multiplying across all arches
@@ -102,6 +108,9 @@  uint64_t get_reginfo_paramreg(struct reginfo *ri);
  */
 int get_risuop(struct reginfo *ri);
 
+/* Return the PC from a reginfo */
+uintptr_t get_pc(struct reginfo *ri);
+
 /* initialize structure from a ucontext */
 void reginfo_init(struct reginfo *ri, ucontext_t *uc);
 
diff --git a/risu_aarch64.c b/risu_aarch64.c
index 9c6809d..492d141 100644
--- a/risu_aarch64.c
+++ b/risu_aarch64.c
@@ -40,3 +40,8 @@  int get_risuop(struct reginfo *ri)
     uint32_t risukey = 0x00005af0;
     return (key != risukey) ? -1 : op;
 }
+
+uintptr_t get_pc(struct reginfo *ri)
+{
+   return ri->pc;
+}
diff --git a/risu_arm.c b/risu_arm.c
index a55c6c2..5fcb2a5 100644
--- a/risu_arm.c
+++ b/risu_arm.c
@@ -68,3 +68,8 @@  int get_risuop(struct reginfo *ri)
     uint32_t risukey = (isz == 2) ? 0xdee0 : 0xe7fe5af0;
     return (key != risukey) ? -1 : op;
 }
+
+uintptr_t get_pc(struct reginfo *ri)
+{
+   return ri->gpreg[15];
+}
diff --git a/risu_m68k.c b/risu_m68k.c
index 0bf5c14..1056eef 100644
--- a/risu_m68k.c
+++ b/risu_m68k.c
@@ -33,3 +33,8 @@  int get_risuop(struct reginfo *ri)
     uint32_t risukey = 0x4afc7000;
     return (key != risukey) ? -1 : op;
 }
+
+uintptr_t get_pc(struct reginfo *ri)
+{
+    return ri->gregs[R_PC];
+}
diff --git a/risu_ppc64.c b/risu_ppc64.c
index eb60573..83f8d1f 100644
--- a/risu_ppc64.c
+++ b/risu_ppc64.c
@@ -38,3 +38,8 @@  int get_risuop(struct reginfo *ri)
     uint32_t risukey = 0x00005af0;
     return (key != risukey) ? -1 : op;
 }
+
+uintptr_t get_pc(struct reginfo *ri)
+{
+   return ri->nip;
+}