diff mbox

[kvm-unit-tests,4/5] powerpc: check lswx

Message ID 1458141183-27207-5-git-send-email-lvivier@redhat.com
State Superseded
Headers show

Commit Message

Laurent Vivier March 16, 2016, 3:13 p.m. UTC
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 powerpc/emulator.c | 148 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 148 insertions(+)

Comments

Thomas Huth March 18, 2016, 9:09 a.m. UTC | #1
On 16.03.2016 16:13, Laurent Vivier wrote:
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  powerpc/emulator.c | 148 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 148 insertions(+)
> 
> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
> index b66c1d7..dfe5859 100644
> --- a/powerpc/emulator.c
> +++ b/powerpc/emulator.c
> @@ -45,6 +45,153 @@ static void test_64bit(void)
>  	report_prefix_pop();
>  }
>  
> +/*
> + * lswx: Load String Word Indexed X-form
> + *
> + *     lswx RT,RA,RB
> + *
> + * EA = (RA|0) + RB
> + * n  = XER
> + *
> + * Load n bytes from address EA into (n / 4) consecutive registers,
> + * throught RT -> RT + (n / 4) - 1.
> + * - Data are loaded into 4 low order bytes of registers (Word).
> + * - The unfilled bytes are set to 0.
> + * - The sequence of registers wraps around to GPR0.
> + * - if n == 0, content of RT is undefined
> + * - RT <= RA or RB < RT + (n + 4) is invalid or result is undefined
> + * - RT == RA == 0 is invalid
> + *
> + */
> +
> +#define SPR_XER	1
> +
> +static void test_lswx(void)
> +{
> +	int i;
> +	char addr[128];
> +	uint64_t regs[32];
> +
> +	report_prefix_push("lswx");
> +
> +	/* fill memory with sequence */
> +
> +	for (i = 0; i < 128; i++)
> +		addr[i] = 1 + i;
> +
> +	/* check incomplete register filling */
> +
> +	asm volatile ("mtspr %[XER], %[len];"

It's maybe simpler to use the "mtxer" opcode alias here, then you don't
have to pass SPR_XER via the parameters.

> +		      "li r12,-1;"
> +		      "mr r11, r12;"
> +		      "lswx r11, 0, %[addr];"
> +		      "std r11, 0*8(%[regs]);"
> +		      "std r12, 1*8(%[regs]);"
> +		      ::
> +		      [len] "r" (3),
> +		      [XER] "i" (SPR_XER),
> +		      [addr] "r" (addr),
> +		      [regs] "r" (regs)
> +		      :
> +		      /* as 32 is the number of bytes,
> +		       * we should modify 32/4 = 8 regs, from r1
> +		       */

Is that comment a copy-n-paste leftover? It seems not to make much sense
here!?

> +		      "xer", "r11", "r12");

I think you need "memory" in the clobber list, since you write to the
regs buffer.

> +	report("partial", regs[0] == 0x01020300 && regs[1] == (uint64_t)-1);
> +
> +	/* check an old know bug: the number of bytes is used as
> +	 * the number of registers, so try 32 bytes.
> +	 */
> +
> +	asm volatile ("mtspr %[XER], %[len];"
> +		      "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;"
> +		      "lswx r11, 0, %[addr];"
> +		      "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] "r" (32),
> +		      [XER] "i" (SPR_XER),
> +		      [addr] "r" (addr),
> +		      [regs] "r" (regs)
> +		      :
> +		      /* as 32 is the number of bytes,
> +		       * we should modify 32/4 = 8 regs, from r1

... from r11 instead of r1 ?

> +		       */
> +		      "xer", "r11", "r12", "r13", "r14", "r15", "r16", "r17",
> +		      "r18", "r19");

Please also add "memory" here.

> +	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 ("mtspr %[XER], %[len];"
> +		      "li r31,-1;"
> +		      "mr r0, r31;"
> +		      "lswx r31, 0, %[addr];"
> +		      "std r31, 0*8(%[regs]);"
> +		      "std r0, 1*8(%[regs]);"
> +		      ::
> +		      [len] "r" (8),
> +		      [XER] "i" (SPR_XER),
> +		      [addr] "r" (addr),
> +		      [regs] "r" (regs)
> +		      :
> +		      /* as 32 is the number of bytes,
> +		       * we should modify 32/4 = 8 regs, from r1
> +		       */

Comment also needs to be fixed?

> +		      "xer", "r31", "r0");

"memory" missing again

> +	report("wrap around to r0", regs[0] == 0x01020304 &&
> +			            regs[1] == 0x05060708);
> +
> +	/* check wrap around to r0 over RB doesn't break RB */
> +
> +	asm volatile ("mtspr %[XER], %[len];"
> +		      /* adding r1 in the clobber list doesn't protect it... */
> +		      "mr r29,r1;"
> +		      "li r31,-1;"
> +		      "mr r1,r31;"
> +		      "mr r0, %[addr];"
> +		      "lswx r31, 0, r0;"
> +		      "std r31, 0*8(%[regs]);"
> +		      "std r0, 1*8(%[regs]);"
> +		      "std r1, 2*8(%[regs]);"
> +		      "mr r1,r29;"
> +		      ::
> +		      [len] "r" (12),
> +		      [XER] "i" (SPR_XER),
> +		      [addr] "r" (addr),
> +		      [regs] "r" (regs)
> +		      :
> +		      /* as 32 is the number of bytes,
> +		       * we should modify 32/4 = 8 regs, from r1
> +		       */

That comment needs some update, too.

> +		      "xer", "r31", "r0", "r29");

"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 Rb", regs[1] == (uint64_t)addr);

Huh, how can this KVM unit test ever finish successfully if real
processor stops? Should this last test maybe be optional and only be
triggered if this kvm-unit-test is run with certain parameters?

> +	report_prefix_pop();
> +}
> +
>  int main(void)
>  {
>  	handle_exception(0x700, program_check_handler, (void *)&is_invalid);
> @@ -53,6 +200,7 @@ int main(void)
>  
>  	test_64bit();
>  	test_illegal();
> +	test_lswx();
>  
>  	report_prefix_pop();

 Thomas


--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laurent Vivier March 18, 2016, 10:01 a.m. UTC | #2
On 18/03/2016 10:09, Thomas Huth wrote:
> On 16.03.2016 16:13, Laurent Vivier wrote:
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>>  powerpc/emulator.c | 148 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 148 insertions(+)
>>
>> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
>> index b66c1d7..dfe5859 100644
>> --- a/powerpc/emulator.c
>> +++ b/powerpc/emulator.c
>> @@ -45,6 +45,153 @@ static void test_64bit(void)
>>  	report_prefix_pop();
>>  }
>>  
>> +/*
>> + * lswx: Load String Word Indexed X-form
>> + *
>> + *     lswx RT,RA,RB
>> + *
>> + * EA = (RA|0) + RB
>> + * n  = XER
>> + *
>> + * Load n bytes from address EA into (n / 4) consecutive registers,
>> + * throught RT -> RT + (n / 4) - 1.
>> + * - Data are loaded into 4 low order bytes of registers (Word).
>> + * - The unfilled bytes are set to 0.
>> + * - The sequence of registers wraps around to GPR0.
>> + * - if n == 0, content of RT is undefined
>> + * - RT <= RA or RB < RT + (n + 4) is invalid or result is undefined
>> + * - RT == RA == 0 is invalid
>> + *
>> + */
>> +
>> +#define SPR_XER	1
>> +
>> +static void test_lswx(void)
>> +{
>> +	int i;
>> +	char addr[128];
>> +	uint64_t regs[32];
>> +
>> +	report_prefix_push("lswx");
>> +
>> +	/* fill memory with sequence */
>> +
>> +	for (i = 0; i < 128; i++)
>> +		addr[i] = 1 + i;
>> +
>> +	/* check incomplete register filling */
>> +
>> +	asm volatile ("mtspr %[XER], %[len];"
> 
> It's maybe simpler to use the "mtxer" opcode alias here, then you don't
> have to pass SPR_XER via the parameters.

OK.

>> +		      "li r12,-1;"
>> +		      "mr r11, r12;"
>> +		      "lswx r11, 0, %[addr];"
>> +		      "std r11, 0*8(%[regs]);"
>> +		      "std r12, 1*8(%[regs]);"
>> +		      ::
>> +		      [len] "r" (3),
>> +		      [XER] "i" (SPR_XER),
>> +		      [addr] "r" (addr),
>> +		      [regs] "r" (regs)
>> +		      :
>> +		      /* as 32 is the number of bytes,
>> +		       * we should modify 32/4 = 8 regs, from r1
>> +		       */
> 
> Is that comment a copy-n-paste leftover? It seems not to make much sense
> here!?

Yes, it's completely wrong...

>> +		      "xer", "r11", "r12");
> 
> I think you need "memory" in the clobber list, since you write to the
> regs buffer.

ok

>> +	report("partial", regs[0] == 0x01020300 && regs[1] == (uint64_t)-1);
>> +
>> +	/* check an old know bug: the number of bytes is used as
>> +	 * the number of registers, so try 32 bytes.
>> +	 */
>> +
>> +	asm volatile ("mtspr %[XER], %[len];"
>> +		      "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;"
>> +		      "lswx r11, 0, %[addr];"
>> +		      "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] "r" (32),
>> +		      [XER] "i" (SPR_XER),
>> +		      [addr] "r" (addr),
>> +		      [regs] "r" (regs)
>> +		      :
>> +		      /* as 32 is the number of bytes,
>> +		       * we should modify 32/4 = 8 regs, from r1
> 
> ... from r11 instead of r1 ?

always a bad cut'n'paste...

>> +		       */
>> +		      "xer", "r11", "r12", "r13", "r14", "r15", "r16", "r17",
>> +		      "r18", "r19");
> 
> Please also add "memory" here.


ok

>> +	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 ("mtspr %[XER], %[len];"
>> +		      "li r31,-1;"
>> +		      "mr r0, r31;"
>> +		      "lswx r31, 0, %[addr];"
>> +		      "std r31, 0*8(%[regs]);"
>> +		      "std r0, 1*8(%[regs]);"
>> +		      ::
>> +		      [len] "r" (8),
>> +		      [XER] "i" (SPR_XER),
>> +		      [addr] "r" (addr),
>> +		      [regs] "r" (regs)
>> +		      :
>> +		      /* as 32 is the number of bytes,
>> +		       * we should modify 32/4 = 8 regs, from r1
>> +		       */
> 
> Comment also needs to be fixed?

yes,

> 
>> +		      "xer", "r31", "r0");
> 
> "memory" missing again

ok,

> 
>> +	report("wrap around to r0", regs[0] == 0x01020304 &&
>> +			            regs[1] == 0x05060708);
>> +
>> +	/* check wrap around to r0 over RB doesn't break RB */
>> +
>> +	asm volatile ("mtspr %[XER], %[len];"
>> +		      /* adding r1 in the clobber list doesn't protect it... */
>> +		      "mr r29,r1;"
>> +		      "li r31,-1;"
>> +		      "mr r1,r31;"
>> +		      "mr r0, %[addr];"
>> +		      "lswx r31, 0, r0;"
>> +		      "std r31, 0*8(%[regs]);"
>> +		      "std r0, 1*8(%[regs]);"
>> +		      "std r1, 2*8(%[regs]);"
>> +		      "mr r1,r29;"
>> +		      ::
>> +		      [len] "r" (12),
>> +		      [XER] "i" (SPR_XER),
>> +		      [addr] "r" (addr),
>> +		      [regs] "r" (regs)
>> +		      :
>> +		      /* as 32 is the number of bytes,
>> +		       * we should modify 32/4 = 8 regs, from r1
>> +		       */
> 
> That comment needs some update, too.

yes,

> 
>> +		      "xer", "r31", "r0", "r29");
> 
> "memory"

ok

>> +	/* 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 Rb", regs[1] == (uint64_t)addr);
> 
> Huh, how can this KVM unit test ever finish successfully if real
> processor stops? Should this last test maybe be optional and only be
> triggered if this kvm-unit-test is run with certain parameters?

Well, my bad, I've not been accurate: the processor doesn't stop, but
the processing of the instruction is stopped. Only registers until Rb
(not included) are updated, and then the processor continue with the
next instruction. So we have just to test Rb is not modified. I'll
update the comment.

> 
>> +	report_prefix_pop();
>> +}
>> +
>>  int main(void)
>>  {
>>  	handle_exception(0x700, program_check_handler, (void *)&is_invalid);
>> @@ -53,6 +200,7 @@ int main(void)
>>  
>>  	test_64bit();
>>  	test_illegal();
>> +	test_lswx();
>>  
>>  	report_prefix_pop();
> 
>  Thomas
> 


Thanks,
Laurent
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thomas Huth March 18, 2016, 10:06 a.m. UTC | #3
On 18.03.2016 11:01, Laurent Vivier wrote:
> 
> 
> On 18/03/2016 10:09, Thomas Huth wrote:
>> On 16.03.2016 16:13, Laurent Vivier wrote:
>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>> ---
>>>  powerpc/emulator.c | 148 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 148 insertions(+)
>>>
>>> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
>>> index b66c1d7..dfe5859 100644
>>> --- a/powerpc/emulator.c
>>> +++ b/powerpc/emulator.c
>>> @@ -45,6 +45,153 @@ static void test_64bit(void)
>>>  	report_prefix_pop();
>>>  }
>>>  
>>> +/*
>>> + * lswx: Load String Word Indexed X-form
>>> + *
>>> + *     lswx RT,RA,RB
>>> + *
>>> + * EA = (RA|0) + RB
>>> + * n  = XER
>>> + *
>>> + * Load n bytes from address EA into (n / 4) consecutive registers,
>>> + * throught RT -> RT + (n / 4) - 1.
>>> + * - Data are loaded into 4 low order bytes of registers (Word).
>>> + * - The unfilled bytes are set to 0.
>>> + * - The sequence of registers wraps around to GPR0.
>>> + * - if n == 0, content of RT is undefined
>>> + * - RT <= RA or RB < RT + (n + 4) is invalid or result is undefined
>>> + * - RT == RA == 0 is invalid
>>> + *
>>> + */
>>> +
>>> +#define SPR_XER	1
>>> +
>>> +static void test_lswx(void)
>>> +{
>>> +	int i;
>>> +	char addr[128];
>>> +	uint64_t regs[32];
>>> +
>>> +	report_prefix_push("lswx");
>>> +
>>> +	/* fill memory with sequence */
>>> +
>>> +	for (i = 0; i < 128; i++)
>>> +		addr[i] = 1 + i;
>>> +
>>> +	/* check incomplete register filling */
>>> +
>>> +	asm volatile ("mtspr %[XER], %[len];"
>>
>> It's maybe simpler to use the "mtxer" opcode alias here, then you don't
>> have to pass SPR_XER via the parameters.
> 
> OK.
> 
>>> +		      "li r12,-1;"
>>> +		      "mr r11, r12;"
>>> +		      "lswx r11, 0, %[addr];"
>>> +		      "std r11, 0*8(%[regs]);"
>>> +		      "std r12, 1*8(%[regs]);"
>>> +		      ::
>>> +		      [len] "r" (3),
>>> +		      [XER] "i" (SPR_XER),
>>> +		      [addr] "r" (addr),
>>> +		      [regs] "r" (regs)
>>> +		      :
>>> +		      /* as 32 is the number of bytes,
>>> +		       * we should modify 32/4 = 8 regs, from r1
>>> +		       */
>>
>> Is that comment a copy-n-paste leftover? It seems not to make much sense
>> here!?
> 
> Yes, it's completely wrong...
> 
>>> +		      "xer", "r11", "r12");
>>
>> I think you need "memory" in the clobber list, since you write to the
>> regs buffer.
> 
> ok
> 
>>> +	report("partial", regs[0] == 0x01020300 && regs[1] == (uint64_t)-1);
>>> +
>>> +	/* check an old know bug: the number of bytes is used as
>>> +	 * the number of registers, so try 32 bytes.
>>> +	 */
>>> +
>>> +	asm volatile ("mtspr %[XER], %[len];"
>>> +		      "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;"
>>> +		      "lswx r11, 0, %[addr];"
>>> +		      "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] "r" (32),
>>> +		      [XER] "i" (SPR_XER),
>>> +		      [addr] "r" (addr),
>>> +		      [regs] "r" (regs)
>>> +		      :
>>> +		      /* as 32 is the number of bytes,
>>> +		       * we should modify 32/4 = 8 regs, from r1
>>
>> ... from r11 instead of r1 ?
> 
> always a bad cut'n'paste...
> 
>>> +		       */
>>> +		      "xer", "r11", "r12", "r13", "r14", "r15", "r16", "r17",
>>> +		      "r18", "r19");
>>
>> Please also add "memory" here.
> 
> 
> ok
> 
>>> +	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 ("mtspr %[XER], %[len];"
>>> +		      "li r31,-1;"
>>> +		      "mr r0, r31;"
>>> +		      "lswx r31, 0, %[addr];"
>>> +		      "std r31, 0*8(%[regs]);"
>>> +		      "std r0, 1*8(%[regs]);"
>>> +		      ::
>>> +		      [len] "r" (8),
>>> +		      [XER] "i" (SPR_XER),
>>> +		      [addr] "r" (addr),
>>> +		      [regs] "r" (regs)
>>> +		      :
>>> +		      /* as 32 is the number of bytes,
>>> +		       * we should modify 32/4 = 8 regs, from r1
>>> +		       */
>>
>> Comment also needs to be fixed?
> 
> yes,
> 
>>
>>> +		      "xer", "r31", "r0");
>>
>> "memory" missing again
> 
> ok,
> 
>>
>>> +	report("wrap around to r0", regs[0] == 0x01020304 &&
>>> +			            regs[1] == 0x05060708);
>>> +
>>> +	/* check wrap around to r0 over RB doesn't break RB */
>>> +
>>> +	asm volatile ("mtspr %[XER], %[len];"
>>> +		      /* adding r1 in the clobber list doesn't protect it... */
>>> +		      "mr r29,r1;"
>>> +		      "li r31,-1;"
>>> +		      "mr r1,r31;"
>>> +		      "mr r0, %[addr];"
>>> +		      "lswx r31, 0, r0;"
>>> +		      "std r31, 0*8(%[regs]);"
>>> +		      "std r0, 1*8(%[regs]);"
>>> +		      "std r1, 2*8(%[regs]);"
>>> +		      "mr r1,r29;"
>>> +		      ::
>>> +		      [len] "r" (12),
>>> +		      [XER] "i" (SPR_XER),
>>> +		      [addr] "r" (addr),
>>> +		      [regs] "r" (regs)
>>> +		      :
>>> +		      /* as 32 is the number of bytes,
>>> +		       * we should modify 32/4 = 8 regs, from r1
>>> +		       */
>>
>> That comment needs some update, too.
> 
> yes,
> 
>>
>>> +		      "xer", "r31", "r0", "r29");
>>
>> "memory"
> 
> ok
> 
>>> +	/* 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 Rb", regs[1] == (uint64_t)addr);
>>
>> Huh, how can this KVM unit test ever finish successfully if real
>> processor stops? Should this last test maybe be optional and only be
>> triggered if this kvm-unit-test is run with certain parameters?
> 
> Well, my bad, I've not been accurate: the processor doesn't stop, but
> the processing of the instruction is stopped. Only registers until Rb
> (not included) are updated, and then the processor continue with the
> next instruction. So we have just to test Rb is not modified. I'll
> update the comment.

Ok, now I've got it, I think. Maybe you should also check the
"is_invalid" variable here? And it would also be interesting to see
whether regs[0] (i.e. r31) has been changed or not?

 Thomas

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laurent Vivier March 18, 2016, 10:18 a.m. UTC | #4
On 18/03/2016 11:06, Thomas Huth wrote:
> On 18.03.2016 11:01, Laurent Vivier wrote:
...
>>>> +	/* 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 Rb", regs[1] == (uint64_t)addr);
>>>
>>> Huh, how can this KVM unit test ever finish successfully if real
>>> processor stops? Should this last test maybe be optional and only be
>>> triggered if this kvm-unit-test is run with certain parameters?
>>
>> Well, my bad, I've not been accurate: the processor doesn't stop, but
>> the processing of the instruction is stopped. Only registers until Rb
>> (not included) are updated, and then the processor continue with the
>> next instruction. So we have just to test Rb is not modified. I'll
>> update the comment.
> 
> Ok, now I've got it, I think. Maybe you should also check the
> "is_invalid" variable here? And it would also be interesting to see
> whether regs[0] (i.e. r31) has been changed or not?

In fact, I've tested this with kvm PR and HV, on POWER8 and ppc970, and
there is never an illegal exception. It's why I don't check that.

Checking regs[1] is correct in the case of the doc (illegal instruction)
and of the real processor (it doesn't update it).

Checking regs[0] can be tricky: in the case of the doc (illegal) it
should not be updated, in the case of the real processor it is updated.

I'd like this test checks only if TCG behaves like a real processor,
it's why I prefer to check this result against the result of a real
processor than against the result described in the doc.

Laurent
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thomas Huth March 18, 2016, 10:34 a.m. UTC | #5
On 18.03.2016 11:18, Laurent Vivier wrote:
> 
> 
> On 18/03/2016 11:06, Thomas Huth wrote:
>> On 18.03.2016 11:01, Laurent Vivier wrote:
> ...
>>>>> +	/* 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 Rb", regs[1] == (uint64_t)addr);
>>>>
>>>> Huh, how can this KVM unit test ever finish successfully if real
>>>> processor stops? Should this last test maybe be optional and only be
>>>> triggered if this kvm-unit-test is run with certain parameters?
>>>
>>> Well, my bad, I've not been accurate: the processor doesn't stop, but
>>> the processing of the instruction is stopped. Only registers until Rb
>>> (not included) are updated, and then the processor continue with the
>>> next instruction. So we have just to test Rb is not modified. I'll
>>> update the comment.
>>
>> Ok, now I've got it, I think. Maybe you should also check the
>> "is_invalid" variable here? And it would also be interesting to see
>> whether regs[0] (i.e. r31) has been changed or not?
> 
> In fact, I've tested this with kvm PR and HV, on POWER8 and ppc970, and
> there is never an illegal exception. It's why I don't check that.
> 
> Checking regs[1] is correct in the case of the doc (illegal instruction)
> and of the real processor (it doesn't update it).
> 
> Checking regs[0] can be tricky: in the case of the doc (illegal) it
> should not be updated, in the case of the real processor it is updated.
> 
> I'd like this test checks only if TCG behaves like a real processor,
> it's why I prefer to check this result against the result of a real
> processor than against the result described in the doc.

Ah, ok ... I just saw that there is even a comment in the QEMU sources
about this in front of the helper_lswx() function ... then better keep
the unit test code in its current shape!

 Thomas

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/powerpc/emulator.c b/powerpc/emulator.c
index b66c1d7..dfe5859 100644
--- a/powerpc/emulator.c
+++ b/powerpc/emulator.c
@@ -45,6 +45,153 @@  static void test_64bit(void)
 	report_prefix_pop();
 }
 
+/*
+ * lswx: Load String Word Indexed X-form
+ *
+ *     lswx RT,RA,RB
+ *
+ * EA = (RA|0) + RB
+ * n  = XER
+ *
+ * Load n bytes from address EA into (n / 4) consecutive registers,
+ * throught RT -> RT + (n / 4) - 1.
+ * - Data are loaded into 4 low order bytes of registers (Word).
+ * - The unfilled bytes are set to 0.
+ * - The sequence of registers wraps around to GPR0.
+ * - if n == 0, content of RT is undefined
+ * - RT <= RA or RB < RT + (n + 4) is invalid or result is undefined
+ * - RT == RA == 0 is invalid
+ *
+ */
+
+#define SPR_XER	1
+
+static void test_lswx(void)
+{
+	int i;
+	char addr[128];
+	uint64_t regs[32];
+
+	report_prefix_push("lswx");
+
+	/* fill memory with sequence */
+
+	for (i = 0; i < 128; i++)
+		addr[i] = 1 + i;
+
+	/* check incomplete register filling */
+
+	asm volatile ("mtspr %[XER], %[len];"
+		      "li r12,-1;"
+		      "mr r11, r12;"
+		      "lswx r11, 0, %[addr];"
+		      "std r11, 0*8(%[regs]);"
+		      "std r12, 1*8(%[regs]);"
+		      ::
+		      [len] "r" (3),
+		      [XER] "i" (SPR_XER),
+		      [addr] "r" (addr),
+		      [regs] "r" (regs)
+		      :
+		      /* as 32 is the number of bytes,
+		       * we should modify 32/4 = 8 regs, from r1
+		       */
+		      "xer", "r11", "r12");
+
+	report("partial", regs[0] == 0x01020300 && regs[1] == (uint64_t)-1);
+
+	/* check an old know bug: the number of bytes is used as
+	 * the number of registers, so try 32 bytes.
+	 */
+
+	asm volatile ("mtspr %[XER], %[len];"
+		      "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;"
+		      "lswx r11, 0, %[addr];"
+		      "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] "r" (32),
+		      [XER] "i" (SPR_XER),
+		      [addr] "r" (addr),
+		      [regs] "r" (regs)
+		      :
+		      /* as 32 is the number of bytes,
+		       * we should modify 32/4 = 8 regs, from r1
+		       */
+		      "xer", "r11", "r12", "r13", "r14", "r15", "r16", "r17",
+		      "r18", "r19");
+
+	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 ("mtspr %[XER], %[len];"
+		      "li r31,-1;"
+		      "mr r0, r31;"
+		      "lswx r31, 0, %[addr];"
+		      "std r31, 0*8(%[regs]);"
+		      "std r0, 1*8(%[regs]);"
+		      ::
+		      [len] "r" (8),
+		      [XER] "i" (SPR_XER),
+		      [addr] "r" (addr),
+		      [regs] "r" (regs)
+		      :
+		      /* as 32 is the number of bytes,
+		       * we should modify 32/4 = 8 regs, from r1
+		       */
+		      "xer", "r31", "r0");
+
+	report("wrap around to r0", regs[0] == 0x01020304 &&
+			            regs[1] == 0x05060708);
+
+	/* check wrap around to r0 over RB doesn't break RB */
+
+	asm volatile ("mtspr %[XER], %[len];"
+		      /* adding r1 in the clobber list doesn't protect it... */
+		      "mr r29,r1;"
+		      "li r31,-1;"
+		      "mr r1,r31;"
+		      "mr r0, %[addr];"
+		      "lswx r31, 0, r0;"
+		      "std r31, 0*8(%[regs]);"
+		      "std r0, 1*8(%[regs]);"
+		      "std r1, 2*8(%[regs]);"
+		      "mr r1,r29;"
+		      ::
+		      [len] "r" (12),
+		      [XER] "i" (SPR_XER),
+		      [addr] "r" (addr),
+		      [regs] "r" (regs)
+		      :
+		      /* as 32 is the number of bytes,
+		       * we should modify 32/4 = 8 regs, from r1
+		       */
+		      "xer", "r31", "r0", "r29");
+
+	/* 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 Rb", regs[1] == (uint64_t)addr);
+
+	report_prefix_pop();
+}
+
 int main(void)
 {
 	handle_exception(0x700, program_check_handler, (void *)&is_invalid);
@@ -53,6 +200,7 @@  int main(void)
 
 	test_64bit();
 	test_illegal();
+	test_lswx();
 
 	report_prefix_pop();