From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Huth Date: Wed, 20 Apr 2016 09:24:09 +0000 Subject: Re: [kvm-unit-test PATCH v2] powerpc: Add emulator test for the lswi instruction Message-Id: <57174AB9.2090900@redhat.com> List-Id: References: <1460645382-31616-1-git-send-email-thuth@redhat.com> <571629BC.6000605@redhat.com> In-Reply-To: <571629BC.6000605@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Laurent Vivier , kvm@vger.kernel.org, pbonzini@redhat.com Cc: kvm-ppc@vger.kernel.org, drjones@redhat.com 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 >> --- >> 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Huth Subject: Re: [kvm-unit-test PATCH v2] powerpc: Add emulator test for the lswi instruction Date: Wed, 20 Apr 2016 11:24:09 +0200 Message-ID: <57174AB9.2090900@redhat.com> References: <1460645382-31616-1-git-send-email-thuth@redhat.com> <571629BC.6000605@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: kvm-ppc@vger.kernel.org, drjones@redhat.com To: Laurent Vivier , kvm@vger.kernel.org, pbonzini@redhat.com Return-path: Received: from mx1.redhat.com ([209.132.183.28]:44638 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750941AbcDTJYN (ORCPT ); Wed, 20 Apr 2016 05:24:13 -0400 In-Reply-To: <571629BC.6000605@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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 >> --- >> 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