| Message ID | 20200623144446.4243-1-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | risu: don't do a full register compare for OP_SIGILL | expand |
On 6/23/20 7:44 AM, Alex Bennée wrote: > + } else if (header.pc != get_pc(&ri[APPRENTICE])) { > + res = RES_MISMATCH_REG; You need a new MISMATCH code, because this one implies that you have a reginfo struct to compare. But thanks, I'll incorporate this. r~
On Tue, 23 Jun 2020 at 16:07, Alex Bennée <alex.bennee@linaro.org> wrote: > > OP_SIGILL means we have an unexpected invalid operation. If this is a > load or store the register state may be un-rectified pointing at the > memblock so would be invalid. In this case just compare the PC and > make sure the other end also faulted at the same place. In case of mismatch of the PC do we still print the full register dump? thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On Tue, 23 Jun 2020 at 16:07, Alex Bennée <alex.bennee@linaro.org> wrote: >> >> OP_SIGILL means we have an unexpected invalid operation. If this is a >> load or store the register state may be un-rectified pointing at the >> memblock so would be invalid. In this case just compare the PC and >> make sure the other end also faulted at the same place. > > In case of mismatch of the PC do we still print the full register > dump? As rth points out with a new mismatch code we could do whatever we wanted on a fail although the "master" will not have sent a register dump so we can only dump the local register state. > > thanks > -- PMM -- Alex Bennée
On 6/23/20 9:17 AM, Alex Bennée wrote: > > Peter Maydell <peter.maydell@linaro.org> writes: > >> On Tue, 23 Jun 2020 at 16:07, Alex Bennée <alex.bennee@linaro.org> wrote: >>> >>> OP_SIGILL means we have an unexpected invalid operation. If this is a >>> load or store the register state may be un-rectified pointing at the >>> memblock so would be invalid. In this case just compare the PC and >>> make sure the other end also faulted at the same place. >> >> In case of mismatch of the PC do we still print the full register >> dump? No. If we want that, we should do something else, like remember that the memory pointer is in use and zap it out before reporting the register set. But, generally, if we see SIGILL, then we have not actually executed anything, so the register state doesn't matter too much. What probably does want reporting in this case is the insn opcode. r~
diff --git a/risu.c b/risu.c index 8d907d9..6d6dcf9 100644 --- a/risu.c +++ b/risu.c @@ -124,7 +124,6 @@ static RisuResult send_register_info(void *uc) switch (op) { case OP_TESTEND: case OP_COMPARE: - case OP_SIGILL: header.size = reginfo_size(&ri[MASTER]); extra = &ri[MASTER]; break; @@ -132,6 +131,7 @@ static RisuResult send_register_info(void *uc) header.size = MEMBLOCKLEN; extra = memblock; break; + case OP_SIGILL: case OP_SETMEMBLOCK: case OP_GETMEMBLOCK: header.size = 0; @@ -203,7 +203,6 @@ static RisuResult recv_register_info(struct reginfo *ri) switch (header.risu_op) { case OP_COMPARE: case OP_TESTEND: - case OP_SIGILL: /* If we can't store the data, report invalid size. */ if (header.size > sizeof(*ri)) { return RES_BAD_SIZE; @@ -223,6 +222,7 @@ static RisuResult recv_register_info(struct reginfo *ri) respond(RES_OK); return read_buffer(other_memblock, MEMBLOCKLEN); + case OP_SIGILL: case OP_SETMEMBLOCK: case OP_GETMEMBLOCK: return header.size == 0 ? RES_OK : RES_BAD_SIZE; @@ -250,7 +250,6 @@ static RisuResult recv_and_compare_register_info(void *uc) switch (op) { case OP_COMPARE: case OP_TESTEND: - case OP_SIGILL: /* * If we have nothing to compare against, report an op mismatch. * Otherwise allow the compare to continue, and assume that @@ -270,7 +269,14 @@ static RisuResult recv_and_compare_register_info(void *uc) res = RES_END; } break; - + case OP_SIGILL: + /* We can only check the op and PC */ + if (header.risu_op != OP_SIGILL) { + res = RES_MISMATCH_OP; + } else if (header.pc != get_pc(&ri[APPRENTICE])) { + res = RES_MISMATCH_REG; + } + break; case OP_SETMEMBLOCK: if (op != header.risu_op) { res = RES_MISMATCH_OP;
OP_SIGILL means we have an unexpected invalid operation. If this is a load or store the register state may be un-rectified pointing at the memblock so would be invalid. In this case just compare the PC and make sure the other end also faulted at the same place. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- risu.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) -- 2.20.1