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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
>>> ---
>>>  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" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux