kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [kvm-unit-test PATCH v2] powerpc: Add emulator test for the lswi instruction
@ 2016-04-14 14:49 Thomas Huth
  2016-04-19 12:51 ` Laurent Vivier
  0 siblings, 1 reply; 5+ messages in thread
From: Thomas Huth @ 2016-04-14 14:49 UTC (permalink / raw)
  To: kvm, lvivier; +Cc: kvm-ppc, pbonzini, drjones

This test checks some special cases of the lswi instruction. Test
works fine on real hardware, but in QEMU, this reveals a bug with
the final "don't overwrite Ra" test (RA gets destroyed since the
check in QEMU is still wrong).
The code is based on the lswx test by Laurent Vivier.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 v2: Do not clobber r2 during the final test, since it is needed by
     the exception handler

 powerpc/emulator.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 124 insertions(+)

diff --git a/powerpc/emulator.c b/powerpc/emulator.c
index 3696d83..4dc341f 100644
--- a/powerpc/emulator.c
+++ b/powerpc/emulator.c
@@ -71,6 +71,129 @@ static void test_64bit(void)
 	report_prefix_pop();
 }
 
+/**
+ * Test 'Load String Word Immediate' instruction
+ */
+static void test_lswi(void)
+{
+	int i;
+	char addr[128];
+	uint64_t regs[32];
+
+	report_prefix_push("lswi");
+
+	/* fill memory with sequence */
+	for (i = 0; i < 128; i++)
+		addr[i] = 1 + i;
+
+	/* check incomplete register filling */
+	alignment = 0;
+	asm volatile ("li r12,-1;"
+		      "mr r11, r12;"
+		      "lswi r11, %[addr], %[len];"
+		      "std r11, 0*8(%[regs]);"
+		      "std r12, 1*8(%[regs]);"
+		      ::
+		      [len] "i" (3),
+		      [addr] "b" (addr),
+		      [regs] "r" (regs)
+		      :
+		      "r11", "r12", "memory");
+
+#if  __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+	/*
+	 * lswi is supposed to cause an alignment exception in little endian
+	 * mode, but QEMU does not support it. So in case we do not get an
+	 * exception, this is an expected failure and we run the other tests
+	 */
+	report_xfail("alignment", !alignment, alignment);
+	if (alignment) {
+		report_prefix_pop();
+		return;
+	}
+#endif
+	report("partial", regs[0] == 0x01020300 && regs[1] == (uint64_t)-1);
+
+	/* check NB = 0 ==> 32 bytes. */
+	asm volatile ("li r19,-1;"
+		      "mr r11, r19; mr r12, r19; mr r13, r19;"
+		      "mr r14, r19; mr r15, r19; mr r16, r19;"
+		      "mr r17, r19; mr r18, r19;"
+		      "lswi r11, %[addr], %[len];"
+		      "std r11, 0*8(%[regs]);"
+		      "std r12, 1*8(%[regs]);"
+		      "std r13, 2*8(%[regs]);"
+		      "std r14, 3*8(%[regs]);"
+		      "std r15, 4*8(%[regs]);"
+		      "std r16, 5*8(%[regs]);"
+		      "std r17, 6*8(%[regs]);"
+		      "std r18, 7*8(%[regs]);"
+		      "std r19, 8*8(%[regs]);"
+		      ::
+		      [len] "i" (0),
+		      [addr] "b" (addr),
+		      [regs] "r" (regs)
+		      :
+		      /* as 32 is the number of bytes,
+		       * we should modify 32/4 = 8 regs, from r11 to r18
+		       * We check r19 is unmodified by filling it with 1s
+		       * before the instruction.
+		       */
+		      "r11", "r12", "r13", "r14", "r15", "r16", "r17",
+		      "r18", "r19", "memory");
+
+	report("length", regs[0] == 0x01020304 && regs[1] == 0x05060708 &&
+			 regs[2] == 0x090a0b0c && regs[3] == 0x0d0e0f10 &&
+			 regs[4] == 0x11121314 && regs[5] == 0x15161718 &&
+			 regs[6] == 0x191a1b1c && regs[7] == 0x1d1e1f20 &&
+			 regs[8] == (uint64_t)-1);
+
+	/* check wrap around to r0 */
+	asm volatile ("li r31,-1;"
+		      "mr r0, r31;"
+		      "lswi r31, %[addr], %[len];"
+		      "std r31, 0*8(%[regs]);"
+		      "std r0, 1*8(%[regs]);"
+		      ::
+		      [len] "i" (8),
+		      [addr] "b" (addr),
+		      [regs] "r" (regs)
+		      :
+		      /* modify two registers from r31, wrap around to r0 */
+		      "r31", "r0", "memory");
+
+	report("wrap around to r0", regs[0] == 0x01020304 &&
+			            regs[1] == 0x05060708);
+
+	/* check wrap around doesn't break RA */
+	asm volatile ("mr r29,r1\n"
+		      "li r31,-1\n"
+		      "mr r0,r31\n"
+		      "mr r1, %[addr]\n"
+		      ".long 0x7fe154aa\n"       /* lswi r31, r1, 10 */
+		      "std r31, 0*8(%[regs])\n"
+		      "std r0, 1*8(%[regs])\n"
+		      "std r1, 2*8(%[regs])\n"
+		      "mr r1,r29\n"
+		      ::
+		      [addr] "r" (addr),
+		      [regs] "r" (regs)
+		      :
+		      /* loading three registers from r31 wraps around to r1,
+		       * r1 is saved to r29, as adding it to the clobber
+		       * list doesn't protect it
+		       */
+		      "r0", "r29", "r31", "memory");
+
+	/* doc says it is invalid, real proc stops when it comes to
+	 * overwrite the register.
+	 * In all the cases, the register must stay untouched
+	 */
+	report("Don't overwrite Ra", regs[2] == (uint64_t)addr);
+
+	report_prefix_pop();
+}
+
 /*
  * lswx: Load String Word Indexed X-form
  *
@@ -235,6 +358,7 @@ int main(int argc, char **argv)
 	test_64bit();
 	test_illegal();
 	test_lswx();
+	test_lswi();
 
 	report_prefix_pop();
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [kvm-unit-test PATCH v2] powerpc: Add emulator test for the lswi instruction
  2016-04-14 14:49 [kvm-unit-test PATCH v2] powerpc: Add emulator test for the lswi instruction Thomas Huth
@ 2016-04-19 12:51 ` Laurent Vivier
  2016-04-20  9:24   ` Thomas Huth
  0 siblings, 1 reply; 5+ messages in thread
From: Laurent Vivier @ 2016-04-19 12:51 UTC (permalink / raw)
  To: Thomas Huth, kvm; +Cc: kvm-ppc, pbonzini, drjones



On 14/04/2016 16:49, Thomas Huth wrote:
> This test checks some special cases of the lswi instruction. Test
> works fine on real hardware, but in QEMU, this reveals a bug with
> the final "don't overwrite Ra" test (RA gets destroyed since the
> check in QEMU is still wrong).
> The code is based on the lswx test by Laurent Vivier.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  v2: Do not clobber r2 during the final test, since it is needed by
>      the exception handler
> 
>  powerpc/emulator.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 124 insertions(+)
> 
> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
> index 3696d83..4dc341f 100644
> --- a/powerpc/emulator.c
> +++ b/powerpc/emulator.c
> @@ -71,6 +71,129 @@ static void test_64bit(void)
>  	report_prefix_pop();
>  }
>  
> +/**
> + * Test 'Load String Word Immediate' instruction
> + */
> +static void test_lswi(void)
> +{
> +	int i;
> +	char addr[128];
> +	uint64_t regs[32];
> +
> +	report_prefix_push("lswi");
> +
> +	/* fill memory with sequence */
> +	for (i = 0; i < 128; i++)
> +		addr[i] = 1 + i;
> +
> +	/* check incomplete register filling */
> +	alignment = 0;
> +	asm volatile ("li r12,-1;"
> +		      "mr r11, r12;"
> +		      "lswi r11, %[addr], %[len];"
> +		      "std r11, 0*8(%[regs]);"
> +		      "std r12, 1*8(%[regs]);"
> +		      ::
> +		      [len] "i" (3),
> +		      [addr] "b" (addr),
> +		      [regs] "r" (regs)
> +		      :
> +		      "r11", "r12", "memory");
> +
> +#if  __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> +	/*
> +	 * lswi is supposed to cause an alignment exception in little endian
> +	 * mode, but QEMU does not support it. So in case we do not get an
> +	 * exception, this is an expected failure and we run the other tests
> +	 */
> +	report_xfail("alignment", !alignment, alignment);
> +	if (alignment) {
> +		report_prefix_pop();
> +		return;
> +	}
> +#endif
> +	report("partial", regs[0] == 0x01020300 && regs[1] == (uint64_t)-1);
> +
> +	/* check NB = 0 ==> 32 bytes. */
> +	asm volatile ("li r19,-1;"
> +		      "mr r11, r19; mr r12, r19; mr r13, r19;"
> +		      "mr r14, r19; mr r15, r19; mr r16, r19;"
> +		      "mr r17, r19; mr r18, r19;"
> +		      "lswi r11, %[addr], %[len];"
> +		      "std r11, 0*8(%[regs]);"
> +		      "std r12, 1*8(%[regs]);"
> +		      "std r13, 2*8(%[regs]);"
> +		      "std r14, 3*8(%[regs]);"
> +		      "std r15, 4*8(%[regs]);"
> +		      "std r16, 5*8(%[regs]);"
> +		      "std r17, 6*8(%[regs]);"
> +		      "std r18, 7*8(%[regs]);"
> +		      "std r19, 8*8(%[regs]);"
> +		      ::
> +		      [len] "i" (0),
> +		      [addr] "b" (addr),
> +		      [regs] "r" (regs)
> +		      :
> +		      /* as 32 is the number of bytes,
> +		       * we should modify 32/4 = 8 regs, from r11 to r18
> +		       * We check r19 is unmodified by filling it with 1s
> +		       * before the instruction.
> +		       */
> +		      "r11", "r12", "r13", "r14", "r15", "r16", "r17",
> +		      "r18", "r19", "memory");
> +
> +	report("length", regs[0] == 0x01020304 && regs[1] == 0x05060708 &&
> +			 regs[2] == 0x090a0b0c && regs[3] == 0x0d0e0f10 &&
> +			 regs[4] == 0x11121314 && regs[5] == 0x15161718 &&
> +			 regs[6] == 0x191a1b1c && regs[7] == 0x1d1e1f20 &&
> +			 regs[8] == (uint64_t)-1);
> +
> +	/* check wrap around to r0 */
> +	asm volatile ("li r31,-1;"
> +		      "mr r0, r31;"
> +		      "lswi r31, %[addr], %[len];"
> +		      "std r31, 0*8(%[regs]);"
> +		      "std r0, 1*8(%[regs]);"
> +		      ::
> +		      [len] "i" (8),
> +		      [addr] "b" (addr),
> +		      [regs] "r" (regs)
> +		      :
> +		      /* modify two registers from r31, wrap around to r0 */
> +		      "r31", "r0", "memory");
> +
> +	report("wrap around to r0", regs[0] == 0x01020304 &&
> +			            regs[1] == 0x05060708);
> +
> +	/* check wrap around doesn't break RA */
> +	asm volatile ("mr r29,r1\n"
> +		      "li r31,-1\n"
> +		      "mr r0,r31\n"
> +		      "mr r1, %[addr]\n"
> +		      ".long 0x7fe154aa\n"       /* lswi r31, r1, 10 */

Perhaps you can add a comment explaining why you are using a .long
instead of the mnemonic?

Anyway:

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

> +		      "std r31, 0*8(%[regs])\n"
> +		      "std r0, 1*8(%[regs])\n"
> +		      "std r1, 2*8(%[regs])\n"
> +		      "mr r1,r29\n"
> +		      ::
> +		      [addr] "r" (addr),
> +		      [regs] "r" (regs)
> +		      :
> +		      /* loading three registers from r31 wraps around to r1,
> +		       * r1 is saved to r29, as adding it to the clobber
> +		       * list doesn't protect it
> +		       */
> +		      "r0", "r29", "r31", "memory");
> +
> +	/* doc says it is invalid, real proc stops when it comes to
> +	 * overwrite the register.
> +	 * In all the cases, the register must stay untouched
> +	 */
> +	report("Don't overwrite Ra", regs[2] == (uint64_t)addr);
> +
> +	report_prefix_pop();
> +}
> +
>  /*
>   * lswx: Load String Word Indexed X-form
>   *
> @@ -235,6 +358,7 @@ int main(int argc, char **argv)
>  	test_64bit();
>  	test_illegal();
>  	test_lswx();
> +	test_lswi();
>  
>  	report_prefix_pop();
>  
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [kvm-unit-test PATCH v2] powerpc: Add emulator test for the lswi instruction
  2016-04-19 12:51 ` Laurent Vivier
@ 2016-04-20  9:24   ` Thomas Huth
  2016-05-10 12:05     ` Paolo Bonzini
  0 siblings, 1 reply; 5+ messages in thread
From: Thomas Huth @ 2016-04-20  9:24 UTC (permalink / raw)
  To: Laurent Vivier, kvm, pbonzini; +Cc: kvm-ppc, drjones

On 19.04.2016 14:51, Laurent Vivier wrote:
> 
> 
> On 14/04/2016 16:49, Thomas Huth wrote:
>> This test checks some special cases of the lswi instruction. Test
>> works fine on real hardware, but in QEMU, this reveals a bug with
>> the final "don't overwrite Ra" test (RA gets destroyed since the
>> check in QEMU is still wrong).
>> The code is based on the lswx test by Laurent Vivier.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  v2: Do not clobber r2 during the final test, since it is needed by
>>      the exception handler
>>
>>  powerpc/emulator.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 124 insertions(+)
>>
>> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
>> index 3696d83..4dc341f 100644
>> --- a/powerpc/emulator.c
>> +++ b/powerpc/emulator.c
>> @@ -71,6 +71,129 @@ static void test_64bit(void)
>>  	report_prefix_pop();
>>  }
>>  
>> +/**
>> + * Test 'Load String Word Immediate' instruction
>> + */
>> +static void test_lswi(void)
>> +{
>> +	int i;
>> +	char addr[128];
>> +	uint64_t regs[32];
>> +
>> +	report_prefix_push("lswi");
>> +
>> +	/* fill memory with sequence */
>> +	for (i = 0; i < 128; i++)
>> +		addr[i] = 1 + i;
>> +
>> +	/* check incomplete register filling */
>> +	alignment = 0;
>> +	asm volatile ("li r12,-1;"
>> +		      "mr r11, r12;"
>> +		      "lswi r11, %[addr], %[len];"
>> +		      "std r11, 0*8(%[regs]);"
>> +		      "std r12, 1*8(%[regs]);"
>> +		      ::
>> +		      [len] "i" (3),
>> +		      [addr] "b" (addr),
>> +		      [regs] "r" (regs)
>> +		      :
>> +		      "r11", "r12", "memory");
>> +
>> +#if  __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
>> +	/*
>> +	 * lswi is supposed to cause an alignment exception in little endian
>> +	 * mode, but QEMU does not support it. So in case we do not get an
>> +	 * exception, this is an expected failure and we run the other tests
>> +	 */
>> +	report_xfail("alignment", !alignment, alignment);
>> +	if (alignment) {
>> +		report_prefix_pop();
>> +		return;
>> +	}
>> +#endif
>> +	report("partial", regs[0] == 0x01020300 && regs[1] == (uint64_t)-1);
>> +
>> +	/* check NB = 0 ==> 32 bytes. */
>> +	asm volatile ("li r19,-1;"
>> +		      "mr r11, r19; mr r12, r19; mr r13, r19;"
>> +		      "mr r14, r19; mr r15, r19; mr r16, r19;"
>> +		      "mr r17, r19; mr r18, r19;"
>> +		      "lswi r11, %[addr], %[len];"
>> +		      "std r11, 0*8(%[regs]);"
>> +		      "std r12, 1*8(%[regs]);"
>> +		      "std r13, 2*8(%[regs]);"
>> +		      "std r14, 3*8(%[regs]);"
>> +		      "std r15, 4*8(%[regs]);"
>> +		      "std r16, 5*8(%[regs]);"
>> +		      "std r17, 6*8(%[regs]);"
>> +		      "std r18, 7*8(%[regs]);"
>> +		      "std r19, 8*8(%[regs]);"
>> +		      ::
>> +		      [len] "i" (0),
>> +		      [addr] "b" (addr),
>> +		      [regs] "r" (regs)
>> +		      :
>> +		      /* as 32 is the number of bytes,
>> +		       * we should modify 32/4 = 8 regs, from r11 to r18
>> +		       * We check r19 is unmodified by filling it with 1s
>> +		       * before the instruction.
>> +		       */
>> +		      "r11", "r12", "r13", "r14", "r15", "r16", "r17",
>> +		      "r18", "r19", "memory");
>> +
>> +	report("length", regs[0] == 0x01020304 && regs[1] == 0x05060708 &&
>> +			 regs[2] == 0x090a0b0c && regs[3] == 0x0d0e0f10 &&
>> +			 regs[4] == 0x11121314 && regs[5] == 0x15161718 &&
>> +			 regs[6] == 0x191a1b1c && regs[7] == 0x1d1e1f20 &&
>> +			 regs[8] == (uint64_t)-1);
>> +
>> +	/* check wrap around to r0 */
>> +	asm volatile ("li r31,-1;"
>> +		      "mr r0, r31;"
>> +		      "lswi r31, %[addr], %[len];"
>> +		      "std r31, 0*8(%[regs]);"
>> +		      "std r0, 1*8(%[regs]);"
>> +		      ::
>> +		      [len] "i" (8),
>> +		      [addr] "b" (addr),
>> +		      [regs] "r" (regs)
>> +		      :
>> +		      /* modify two registers from r31, wrap around to r0 */
>> +		      "r31", "r0", "memory");
>> +
>> +	report("wrap around to r0", regs[0] == 0x01020304 &&
>> +			            regs[1] == 0x05060708);
>> +
>> +	/* check wrap around doesn't break RA */
>> +	asm volatile ("mr r29,r1\n"
>> +		      "li r31,-1\n"
>> +		      "mr r0,r31\n"
>> +		      "mr r1, %[addr]\n"
>> +		      ".long 0x7fe154aa\n"       /* lswi r31, r1, 10 */
> 
> Perhaps you can add a comment explaining why you are using a .long
> instead of the mnemonic?

The compiler is smart enough to detect that r1 is in the range of
registers that get clobbered, and thus rejects that mnemonic. You
quickly notice that when replacing the .long with the mnemonic, so I'm
not sure whether it's worth to add a verbose comment here... Paolo, what
do you prefer?

 Thomas


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [kvm-unit-test PATCH v2] powerpc: Add emulator test for the lswi instruction
  2016-04-20  9:24   ` Thomas Huth
@ 2016-05-10 12:05     ` Paolo Bonzini
  2016-05-10 15:23       ` Thomas Huth
  0 siblings, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2016-05-10 12:05 UTC (permalink / raw)
  To: Thomas Huth, Laurent Vivier, kvm; +Cc: kvm-ppc, drjones



On 20/04/2016 11:24, Thomas Huth wrote:
>>> +		      ".long 0x7fe154aa\n"       /* lswi r31, r1, 10 */
>>
>> Perhaps you can add a comment explaining why you are using a .long
>> instead of the mnemonic?
>
> The compiler is smart enough to detect that r1 is in the range of
> registers that get clobbered, and thus rejects that mnemonic. You
> quickly notice that when replacing the .long with the mnemonic, so I'm
> not sure whether it's worth to add a verbose comment here... Paolo, what
> do you prefer?

The assembler does this, not the compiler.  Is this a valid operation at
all, or is it undefined?  (aka, what does it do on real hardware)? O_o

Paolo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [kvm-unit-test PATCH v2] powerpc: Add emulator test for the lswi instruction
  2016-05-10 12:05     ` Paolo Bonzini
@ 2016-05-10 15:23       ` Thomas Huth
  0 siblings, 0 replies; 5+ messages in thread
From: Thomas Huth @ 2016-05-10 15:23 UTC (permalink / raw)
  To: Paolo Bonzini, Laurent Vivier, kvm; +Cc: kvm-ppc, drjones

On 10.05.2016 14:05, Paolo Bonzini wrote:
> 
> 
> On 20/04/2016 11:24, Thomas Huth wrote:
>>>> +		      ".long 0x7fe154aa\n"       /* lswi r31, r1, 10 */
>>>
>>> Perhaps you can add a comment explaining why you are using a .long
>>> instead of the mnemonic?
>>
>> The compiler is smart enough to detect that r1 is in the range of
>> registers that get clobbered, and thus rejects that mnemonic. You
>> quickly notice that when replacing the .long with the mnemonic, so I'm
>> not sure whether it's worth to add a verbose comment here... Paolo, what
>> do you prefer?
> 
> The assembler does this, not the compiler.

Right, of course.

> Is this a valid operation at all, or is it undefined?  (aka, what
> does it do on real hardware)? O_o

The specification (PowerISA) says "If RA is in the range of registers to
be loaded, including the case in which RA=0, the instruction form is
invalid." ... which sounds like the CPU is supposed to raise an invalid
instruction exception.

However, on real hardware (POWER8), the CPU simply stops before loading
that register and continues with the next instruction. In any case, the
contents of the RA register should not be destroyed, and this is what
we're testing here. (and this is also already mentioned in a comment
later in the patch already, right before the report() line).

 Thomas


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-05-10 15:23 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-14 14:49 [kvm-unit-test PATCH v2] powerpc: Add emulator test for the lswi instruction Thomas Huth
2016-04-19 12:51 ` Laurent Vivier
2016-04-20  9:24   ` Thomas Huth
2016-05-10 12:05     ` Paolo Bonzini
2016-05-10 15:23       ` Thomas Huth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).