| Message ID | 1486976352-549-2-git-send-email-nikunj@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 13 February 2017 at 08:59, Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote: > While executing qemu_ppc64le, found an issue that the real illegal > instructions are handled as risu_op which results in wrong info at the > master end. Even the master needs to distinguish real illegal > instructions versus risu_op. > > Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> No, this is deliberate. Otherwise you can't test illegal instructions. What should happen is that both master and apprentice ends end up in the default case, which does a register info compare and continues having stepped the PC past the illegal insn. (If only one end thinks the insn is illegal then there will be a register mismatch on the PC.) thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On 13 February 2017 at 08:59, Nikunj A Dadhania > <nikunj@linux.vnet.ibm.com> wrote: >> While executing qemu_ppc64le, found an issue that the real illegal >> instructions are handled as risu_op which results in wrong info at the >> master end. Even the master needs to distinguish real illegal >> instructions versus risu_op. >> >> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> > > No, this is deliberate. Otherwise you can't test illegal > instructions. What should happen is that both master and > apprentice ends end up in the default case, which does > a register info compare and continues having stepped the > PC past the illegal insn. One of the issue that I had was some of the instruction are implemented in the master and not in apprentice. I think we could then disable them in the ppc64.risu. And enable them only when we have that implemented it in qemu tcg. > (If only one end thinks the insn is illegal then there will > be a register mismatch on the PC.) Yeah, the issue here was it does not come out obviously that there was a real illegal instruction. Maybe a error print at both the ends would help in debugging. Regards Nikunj
On 27 February 2017 at 05:33, Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote: > Peter Maydell <peter.maydell@linaro.org> writes: > >> On 13 February 2017 at 08:59, Nikunj A Dadhania >> <nikunj@linux.vnet.ibm.com> wrote: >>> While executing qemu_ppc64le, found an issue that the real illegal >>> instructions are handled as risu_op which results in wrong info at the >>> master end. Even the master needs to distinguish real illegal >>> instructions versus risu_op. >>> >>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> >> >> No, this is deliberate. Otherwise you can't test illegal >> instructions. What should happen is that both master and >> apprentice ends end up in the default case, which does >> a register info compare and continues having stepped the >> PC past the illegal insn. > > One of the issue that I had was some of the instruction are implemented > in the master and not in apprentice. I think we could then disable them > in the ppc64.risu. And enable them only when we have that implemented it > in qemu tcg. Yes; if you haven't yet implemented an instruction the best approach is just to not try to test it. >> (If only one end thinks the insn is illegal then there will >> be a register mismatch on the PC.) > > Yeah, the issue here was it does not come out obviously that there was a > real illegal instruction. Maybe a error print at both the ends would > help in debugging. It should print "faulting insn mismatch" if the instructions which fault aren't the same thing. This is what the arm and aarch64 implementations of reginfo_dump_mismatch() do, anyway. It looks like the ppc and m68k versions don't do that, though. thanks -- PMM
diff --git a/risu_ppc64le.c b/risu_ppc64le.c index 9c1fafd..54a9bcb 100644 --- a/risu_ppc64le.c +++ b/risu_ppc64le.c @@ -55,7 +55,6 @@ int send_register_info(int sock, void *uc) switch (op) { case OP_COMPARE: case OP_TESTEND: - default: return send_data_pkt(sock, &ri, sizeof(ri)); case OP_SETMEMBLOCK: memblock = (void*)ri.gregs[0]; @@ -66,6 +65,11 @@ int send_register_info(int sock, void *uc) case OP_COMPAREMEM: return send_data_pkt(sock, memblock, MEMBLOCKLEN); break; + default: + fprintf(stderr, "apprentice: Unhandled instruction\n"); + fprintf(stderr, " faulting insn 0x%x\n", ri.faulting_insn); + fprintf(stderr, " insn addr 0x%" PRIx64 "\n\n", ri.nip); + return -1; } return 0; } @@ -85,7 +89,6 @@ int recv_and_compare_register_info(int sock, void *uc) switch (op) { case OP_COMPARE: case OP_TESTEND: - default: if (recv_data_pkt(sock, &apprentice_ri, sizeof(apprentice_ri))) { packet_mismatch = 1; resp = 2; @@ -113,6 +116,11 @@ int recv_and_compare_register_info(int sock, void *uc) } send_response_byte(sock, resp); break; + default: + fprintf(stderr, "master: Unhandled instruction\n"); + fprintf(stderr, " faulting insn 0x%x\n", master_ri.faulting_insn); + fprintf(stderr, " insn addr 0x%" PRIx64 "\n\n", master_ri.nip); + return -1; } return resp; }
While executing qemu_ppc64le, found an issue that the real illegal instructions are handled as risu_op which results in wrong info at the master end. Even the master needs to distinguish real illegal instructions versus risu_op. Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> --- risu_ppc64le.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)