[Qemu-devel] [PATCH 1/2] risu_ppc64le: fix the typo nb => rb

[Qemu-devel] [PATCH 1/2] risu_ppc64le: fix the typo nb => rb

[Nikunj A Dadhania]

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 ppc64.risu | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ppc64.risu b/ppc64.risu
index f7fa3f4..28df9da 100644
--- a/ppc64.risu
+++ b/ppc64.risu
@@ -1029,12 +1029,12 @@ LQ PPC64LE 111000 rtp:5 ra:5 imm:12 0000 \
 !memory { reg_plus_imm($ra, $imm << 4); }
 
 # format:X book:I page:65 v:P1 lswi Load String Word Immediate
-LSWI PPC64LE 011111 rt:5 ra:5 nb:5 10010101010 \
+LSWI PPC64LE 011111 rt:5 ra:5 rb:5 10010101010 \
 !constraints { $ra != 1 && $rb != 1 && $ra != 13 && $rb != 13 && $ra != 0 && $ra != $rb; } \
 !memory { reg_plus_reg($ra, $rb); }
 
 # format:X book:I page:65 v:P1 lswx Load String Word Indexed
-LSWX PPC64LE 011111 rt:5 ra:5 nb:5 10000101010 \
+LSWX PPC64LE 011111 rt:5 ra:5 rb:5 10000101010 \
 !constraints { $ra != 1 && $rb != 1 && $ra != 13 && $rb != 13 && $ra != 0 && $ra != $rb; } \
 !memory { reg_plus_reg($ra, $rb); }
 
@@ -1705,7 +1705,7 @@ STMW PPC64BE 101111 rt:5 ra:5 imm:16 \
 
 # STSWI is not supported in little-endian mode
 # format:X book:I page:66 v:P1 stswi Store String Word Immediate
-STSWI PPC64BE 011111 rs:5 ra:5 nb:5 10110101010 \
+STSWI PPC64BE 011111 rs:5 ra:5 rb:5 10110101010 \
 !constraints { $rs != 1 && $ra != 1 && $rs != 13 && $ra != 13 && $rs != $ra && $ra != 0; } \
 !memory { reg_plus_imm($ra, 0); }
 
-- 
2.7.4

[Qemu-devel] [PATCH 2/2] risu_ppc64le: distinguish real illegal instruction

[Nikunj A Dadhania]

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(-)

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;
 }
-- 
2.7.4

Re: [Qemu-devel] [PATCH 1/2] risu_ppc64le: fix the typo nb => rb

[Peter Maydell]

On 13 February 2017 at 08:59, Nikunj A Dadhania
<nikunj@linux.vnet.ibm.com> wrote:
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  ppc64.risu | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Thanks, applied to risu master.

PS: I nearly missed this patchset entirely, because it doesn't have
a cover letter. If you could provide cover letters for multi-patch
series in future that would be really helpful.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/2] risu_ppc64le: distinguish real illegal instruction

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH 2/2] risu_ppc64le: distinguish real illegal instruction

[Nikunj A Dadhania]

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

Re: [Qemu-devel] [PATCH 1/2] risu_ppc64le: fix the typo nb => rb

[Nikunj A Dadhania]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 13 February 2017 at 08:59, Nikunj A Dadhania
> <nikunj@linux.vnet.ibm.com> wrote:
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> ---
>>  ppc64.risu | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> Thanks, applied to risu master.

Thanks.


> PS: I nearly missed this patchset entirely, because it doesn't have
> a cover letter. If you could provide cover letters for multi-patch
> series in future that would be really helpful.

Sure, will keep that in mind.

Regards
Nikunj

Re: [Qemu-devel] [PATCH 2/2] risu_ppc64le: distinguish real illegal instruction

[Peter Maydell]

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