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

[Thomas Huth <thuth@redhat.com>]

On 21.03.2016 12:33, Laurent Vivier wrote:
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> v2:
>     use "mtxer" instead of "mtspr"
>     Fix comments in lswx asm()
>     add "memory" in clobber list
> 
>  powerpc/emulator.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 140 insertions(+)
> 
> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
> index d97090c..8d0dde2 100644
> --- a/powerpc/emulator.c
> +++ b/powerpc/emulator.c
> @@ -56,6 +56,145 @@ 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
> + *
> + */
> +
> +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 ("mtxer %[len];"
> +		      "li r12,-1;"
> +		      "mr r11, r12;"
> +		      "lswx r11, 0, %[addr];"
> +		      "std r11, 0*8(%[regs]);"
> +		      "std r12, 1*8(%[regs]);"
> +		      ::
> +		      [len] "r" (3),
> +		      [addr] "r" (addr),
> +		      [regs] "r" (regs)
> +		      :
> +		      "xer", "r11", "r12", "memory");
> +
> +	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 ("mtxer %[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),
> +		      [addr] "r" (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.
> +		       */
> +		      "xer", "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 ("mtxer %[len];"
> +		      "li r31,-1;"
> +		      "mr r0, r31;"
> +		      "lswx r31, 0, %[addr];"
> +		      "std r31, 0*8(%[regs]);"
> +		      "std r0, 1*8(%[regs]);"
> +		      ::
> +		      [len] "r" (8),
> +		      [addr] "r" (addr),
> +		      [regs] "r" (regs)
> +		      :
> +		      /* modify two registers from r31, wrap around to r0 */
> +		      "xer", "r31", "r0", "memory");
> +
> +	report("wrap around to r0", regs[0] = 0x01020304 &&
> +			            regs[1] = 0x05060708);
> +
> +	/* check wrap around to r0 over RB doesn't break RB */
> +
> +	asm volatile ("mtxer %[len];"
> +		      /* adding r1 in the clobber list doesn't protect it... */

You repeat that again in the comment below, so in case you respin this
series, you could also drop one of the two comments, I think.

> +		      "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),
> +		      [addr] "r" (addr),
> +		      [regs] "r" (regs)
> +		      :
> +		      /* three registers from r31, wrap arount to r1,
> +		       * r1 is saved to r29, as adding it to the clobber
> +		       * list doesn't protect it
> +		       */
> +		      "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);
> +
> +	report_prefix_pop();
> +}
> +
>  int main(int argc, char **argv)
>  {
>  	int i;
> @@ -72,6 +211,7 @@ int main(int argc, char **argv)
>  
>  	test_64bit();
>  	test_illegal();
> +	test_lswx();
>  
>  	report_prefix_pop();
>  
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>