From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <JBeulich@suse.com>,
xen-devel <xen-devel@lists.xenproject.org>
Cc: ian.jackson@eu.citrix.com, Keir Fraser <keir@xen.org>,
ian.campbell@citrix.com,
Razvan Cojocaru <rcojocaru@bitdefender.com>,
stefano.stabellini@eu.citrix.com
Subject: Re: [PATCH] test_x86_emulator: add missing EIP checks
Date: Thu, 7 Aug 2014 16:52:43 +0100 [thread overview]
Message-ID: <53E3A0CB.5@citrix.com> (raw)
In-Reply-To: <53E3BB5E020000780002A313@mail.emea.novell.com>
[-- Attachment #1.1: Type: text/plain, Size: 13395 bytes --]
On 07/08/14 16:46, Jan Beulich wrote:
> The MMX, SSE, and AVX instruction tests didn't validate that EIP got
> properly updated by the emulator (which indeed it failed to do).
>
> At once macroize the resepctive code quite a bit, hopefully making it
> easier to add further tests when the need arises.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
This is much neater than before.
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> I guess I'll append this to the actual emulator fix.
>
> --- a/tools/tests/x86_emulator/test_x86_emulator.c
> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
> @@ -599,23 +599,32 @@ int main(int argc, char **argv)
> printf("skipped\n");
> #endif
>
> +#define decl_insn(which) extern const unsigned char which[], which##_len[]
> +#define put_insn(which, insn) ".pushsection .test, \"ax\", @progbits\n" \
> + #which ": " insn "\n" \
> + ".equ " #which "_len, .-" #which "\n" \
> + ".popsection"
> +#define set_insn(which) (regs.eip = (unsigned long)memcpy(instr, which, \
> + (unsigned long)which##_len))
> +#define check_eip(which) (regs.eip == (unsigned long)instr + \
> + (unsigned long)which##_len)
> +
> printf("%-40s", "Testing movq %mm3,(%ecx)...");
> if ( stack_exec && cpu_has_mmx )
> {
> - extern const unsigned char movq_to_mem[];
> + decl_insn(movq_to_mem);
>
> asm volatile ( "pcmpeqb %%mm3, %%mm3\n"
> - ".pushsection .test, \"a\", @progbits\n"
> - "movq_to_mem: movq %%mm3, (%0)\n"
> - ".popsection" :: "c" (NULL) );
> + put_insn(movq_to_mem, "movq %%mm3, (%0)")
> + :: "c" (NULL) );
>
> - memcpy(instr, movq_to_mem, 15);
> + set_insn(movq_to_mem);
> memset(res, 0x33, 64);
> memset(res + 8, 0xff, 8);
> - regs.eip = (unsigned long)&instr[0];
> regs.ecx = (unsigned long)res;
> rc = x86_emulate(&ctxt, &emulops);
> - if ( (rc != X86EMUL_OKAY) || memcmp(res, res + 8, 32) )
> + if ( (rc != X86EMUL_OKAY) || memcmp(res, res + 8, 32) ||
> + !check_eip(movq_to_mem) )
> goto fail;
> printf("okay\n");
> }
> @@ -625,19 +634,17 @@ int main(int argc, char **argv)
> printf("%-40s", "Testing movq (%edx),%mm5...");
> if ( stack_exec && cpu_has_mmx )
> {
> - extern const unsigned char movq_from_mem[];
> + decl_insn(movq_from_mem);
>
> asm volatile ( "pcmpgtb %%mm5, %%mm5\n"
> - ".pushsection .test, \"a\", @progbits\n"
> - "movq_from_mem: movq (%0), %%mm5\n"
> - ".popsection" :: "d" (NULL) );
> + put_insn(movq_from_mem, "movq (%0), %%mm5")
> + :: "d" (NULL) );
>
> - memcpy(instr, movq_from_mem, 15);
> - regs.eip = (unsigned long)&instr[0];
> + set_insn(movq_from_mem);
> regs.ecx = 0;
> regs.edx = (unsigned long)res;
> rc = x86_emulate(&ctxt, &emulops);
> - if ( rc != X86EMUL_OKAY )
> + if ( rc != X86EMUL_OKAY || !check_eip(movq_from_mem) )
> goto fail;
> asm ( "pcmpeqb %%mm3, %%mm3\n\t"
> "pcmpeqb %%mm5, %%mm3\n\t"
> @@ -652,20 +659,19 @@ int main(int argc, char **argv)
> printf("%-40s", "Testing movdqu %xmm2,(%ecx)...");
> if ( stack_exec && cpu_has_sse2 )
> {
> - extern const unsigned char movdqu_to_mem[];
> + decl_insn(movdqu_to_mem);
>
> asm volatile ( "pcmpeqb %%xmm2, %%xmm2\n"
> - ".pushsection .test, \"a\", @progbits\n"
> - "movdqu_to_mem: movdqu %%xmm2, (%0)\n"
> - ".popsection" :: "c" (NULL) );
> + put_insn(movdqu_to_mem, "movdqu %%xmm2, (%0)")
> + :: "c" (NULL) );
>
> - memcpy(instr, movdqu_to_mem, 15);
> + set_insn(movdqu_to_mem);
> memset(res, 0x55, 64);
> memset(res + 8, 0xff, 16);
> - regs.eip = (unsigned long)&instr[0];
> regs.ecx = (unsigned long)res;
> rc = x86_emulate(&ctxt, &emulops);
> - if ( (rc != X86EMUL_OKAY) || memcmp(res, res + 8, 32) )
> + if ( (rc != X86EMUL_OKAY) || memcmp(res, res + 8, 32) ||
> + !check_eip(movdqu_to_mem) )
> goto fail;
> printf("okay\n");
> }
> @@ -675,19 +681,17 @@ int main(int argc, char **argv)
> printf("%-40s", "Testing movdqu (%edx),%xmm4...");
> if ( stack_exec && cpu_has_sse2 )
> {
> - extern const unsigned char movdqu_from_mem[];
> + decl_insn(movdqu_from_mem);
>
> asm volatile ( "pcmpgtb %%xmm4, %%xmm4\n"
> - ".pushsection .test, \"a\", @progbits\n"
> - "movdqu_from_mem: movdqu (%0), %%xmm4\n"
> - ".popsection" :: "d" (NULL) );
> + put_insn(movdqu_from_mem, "movdqu (%0), %%xmm4")
> + :: "d" (NULL) );
>
> - memcpy(instr, movdqu_from_mem, 15);
> - regs.eip = (unsigned long)&instr[0];
> + set_insn(movdqu_from_mem);
> regs.ecx = 0;
> regs.edx = (unsigned long)res;
> rc = x86_emulate(&ctxt, &emulops);
> - if ( rc != X86EMUL_OKAY )
> + if ( rc != X86EMUL_OKAY || !check_eip(movdqu_from_mem) )
> goto fail;
> asm ( "pcmpeqb %%xmm2, %%xmm2\n\t"
> "pcmpeqb %%xmm4, %%xmm2\n\t"
> @@ -702,21 +706,20 @@ int main(int argc, char **argv)
> printf("%-40s", "Testing vmovdqu %ymm2,(%ecx)...");
> if ( stack_exec && cpu_has_avx )
> {
> - extern const unsigned char vmovdqu_to_mem[];
> + decl_insn(vmovdqu_to_mem);
>
> asm volatile ( "vpcmpeqb %%xmm2, %%xmm2, %%xmm2\n"
> - ".pushsection .test, \"a\", @progbits\n"
> - "vmovdqu_to_mem: vmovdqu %%ymm2, (%0)\n"
> - ".popsection" :: "c" (NULL) );
> + put_insn(vmovdqu_to_mem, "vmovdqu %%ymm2, (%0)")
> + :: "c" (NULL) );
>
> - memcpy(instr, vmovdqu_to_mem, 15);
> + set_insn(vmovdqu_to_mem);
> memset(res, 0x55, 128);
> memset(res + 16, 0xff, 16);
> memset(res + 20, 0x00, 16);
> - regs.eip = (unsigned long)&instr[0];
> regs.ecx = (unsigned long)res;
> rc = x86_emulate(&ctxt, &emulops);
> - if ( (rc != X86EMUL_OKAY) || memcmp(res, res + 16, 64) )
> + if ( (rc != X86EMUL_OKAY) || memcmp(res, res + 16, 64) ||
> + !check_eip(vmovdqu_to_mem) )
> goto fail;
> printf("okay\n");
> }
> @@ -726,7 +729,7 @@ int main(int argc, char **argv)
> printf("%-40s", "Testing vmovdqu (%edx),%ymm4...");
> if ( stack_exec && cpu_has_avx )
> {
> - extern const unsigned char vmovdqu_from_mem[];
> + decl_insn(vmovdqu_from_mem);
>
> #if 0 /* Don't use AVX2 instructions for now */
> asm volatile ( "vpcmpgtb %%ymm4, %%ymm4, %%ymm4\n"
> @@ -734,17 +737,15 @@ int main(int argc, char **argv)
> asm volatile ( "vpcmpgtb %%xmm4, %%xmm4, %%xmm4\n\t"
> "vinsertf128 $1, %%xmm4, %%ymm4, %%ymm4\n"
> #endif
> - ".pushsection .test, \"a\", @progbits\n"
> - "vmovdqu_from_mem: vmovdqu (%0), %%ymm4\n"
> - ".popsection" :: "d" (NULL) );
> + put_insn(vmovdqu_from_mem, "vmovdqu (%0), %%ymm4")
> + :: "d" (NULL) );
>
> - memcpy(instr, vmovdqu_from_mem, 15);
> + set_insn(vmovdqu_from_mem);
> memset(res + 4, 0xff, 16);
> - regs.eip = (unsigned long)&instr[0];
> regs.ecx = 0;
> regs.edx = (unsigned long)res;
> rc = x86_emulate(&ctxt, &emulops);
> - if ( rc != X86EMUL_OKAY )
> + if ( rc != X86EMUL_OKAY || !check_eip(vmovdqu_from_mem) )
> goto fail;
> #if 0 /* Don't use AVX2 instructions for now */
> asm ( "vpcmpeqb %%ymm2, %%ymm2, %%ymm2\n\t"
> @@ -771,20 +772,19 @@ int main(int argc, char **argv)
> memset(res + 10, 0x66, 8);
> if ( stack_exec && cpu_has_sse2 )
> {
> - extern const unsigned char movsd_to_mem[];
> + decl_insn(movsd_to_mem);
>
> asm volatile ( "movlpd %0, %%xmm5\n\t"
> "movhpd %0, %%xmm5\n"
> - ".pushsection .test, \"a\", @progbits\n"
> - "movsd_to_mem: movsd %%xmm5, (%1)\n"
> - ".popsection" :: "m" (res[10]), "c" (NULL) );
> + put_insn(movsd_to_mem, "movsd %%xmm5, (%1)")
> + :: "m" (res[10]), "c" (NULL) );
>
> - memcpy(instr, movsd_to_mem, 15);
> - regs.eip = (unsigned long)&instr[0];
> + set_insn(movsd_to_mem);
> regs.ecx = (unsigned long)(res + 2);
> regs.edx = 0;
> rc = x86_emulate(&ctxt, &emulops);
> - if ( (rc != X86EMUL_OKAY) || memcmp(res, res + 8, 32) )
> + if ( (rc != X86EMUL_OKAY) || memcmp(res, res + 8, 32) ||
> + !check_eip(movsd_to_mem) )
> goto fail;
> printf("okay\n");
> }
> @@ -797,19 +797,17 @@ int main(int argc, char **argv)
> printf("%-40s", "Testing movaps (%edx),%xmm7...");
> if ( stack_exec && cpu_has_sse )
> {
> - extern const unsigned char movaps_from_mem[];
> + decl_insn(movaps_from_mem);
>
> asm volatile ( "xorps %%xmm7, %%xmm7\n"
> - ".pushsection .test, \"a\", @progbits\n"
> - "movaps_from_mem: movaps (%0), %%xmm7\n"
> - ".popsection" :: "d" (NULL) );
> + put_insn(movaps_from_mem, "movaps (%0), %%xmm7")
> + :: "d" (NULL) );
>
> - memcpy(instr, movaps_from_mem, 15);
> - regs.eip = (unsigned long)&instr[0];
> + set_insn(movaps_from_mem);
> regs.ecx = 0;
> regs.edx = (unsigned long)res;
> rc = x86_emulate(&ctxt, &emulops);
> - if ( rc != X86EMUL_OKAY )
> + if ( rc != X86EMUL_OKAY || !check_eip(movaps_from_mem) )
> goto fail;
> asm ( "cmpeqps %1, %%xmm7\n\t"
> "movmskps %%xmm7, %0" : "=r" (rc) : "m" (res[8]) );
> @@ -825,19 +823,18 @@ int main(int argc, char **argv)
> memset(res + 10, 0x77, 8);
> if ( stack_exec && cpu_has_avx )
> {
> - extern const unsigned char vmovsd_to_mem[];
> + decl_insn(vmovsd_to_mem);
>
> asm volatile ( "vbroadcastsd %0, %%ymm5\n"
> - ".pushsection .test, \"a\", @progbits\n"
> - "vmovsd_to_mem: vmovsd %%xmm5, (%1)\n"
> - ".popsection" :: "m" (res[10]), "c" (NULL) );
> + put_insn(vmovsd_to_mem, "vmovsd %%xmm5, (%1)")
> + :: "m" (res[10]), "c" (NULL) );
>
> - memcpy(instr, vmovsd_to_mem, 15);
> - regs.eip = (unsigned long)&instr[0];
> + set_insn(vmovsd_to_mem);
> regs.ecx = (unsigned long)(res + 2);
> regs.edx = 0;
> rc = x86_emulate(&ctxt, &emulops);
> - if ( (rc != X86EMUL_OKAY) || memcmp(res, res + 8, 32) )
> + if ( (rc != X86EMUL_OKAY) || memcmp(res, res + 8, 32) ||
> + !check_eip(vmovsd_to_mem) )
> goto fail;
> printf("okay\n");
> }
> @@ -850,19 +847,17 @@ int main(int argc, char **argv)
> printf("%-40s", "Testing vmovaps (%edx),%ymm7...");
> if ( stack_exec && cpu_has_avx )
> {
> - extern const unsigned char vmovaps_from_mem[];
> + decl_insn(vmovaps_from_mem);
>
> asm volatile ( "vxorps %%ymm7, %%ymm7, %%ymm7\n"
> - ".pushsection .test, \"a\", @progbits\n"
> - "vmovaps_from_mem: vmovaps (%0), %%ymm7\n"
> - ".popsection" :: "d" (NULL) );
> + put_insn(vmovaps_from_mem, "vmovaps (%0), %%ymm7")
> + :: "d" (NULL) );
>
> - memcpy(instr, vmovaps_from_mem, 15);
> - regs.eip = (unsigned long)&instr[0];
> + set_insn(vmovaps_from_mem);
> regs.ecx = 0;
> regs.edx = (unsigned long)res;
> rc = x86_emulate(&ctxt, &emulops);
> - if ( rc != X86EMUL_OKAY )
> + if ( rc != X86EMUL_OKAY || !check_eip(vmovaps_from_mem) )
> goto fail;
> asm ( "vcmpeqps %1, %%ymm7, %%ymm0\n\t"
> "vmovmskps %%ymm0, %0" : "=r" (rc) : "m" (res[8]) );
> @@ -873,6 +868,11 @@ int main(int argc, char **argv)
> else
> printf("skipped\n");
>
> +#undef decl_insn
> +#undef put_insn
> +#undef set_insn
> +#undef check_eip
> +
> for ( j = 1; j <= 2; j++ )
> {
> #if defined(__i386__)
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
[-- Attachment #1.2: Type: text/html, Size: 13934 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
next prev parent reply other threads:[~2014-08-07 15:53 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-07 11:38 [PATCH V3] tools/tests: Add EIP check to test_x86_emulator.c Razvan Cojocaru
2014-08-07 11:50 ` Jan Beulich
2014-08-07 15:46 ` [PATCH] test_x86_emulator: add missing EIP checks Jan Beulich
2014-08-07 15:52 ` Andrew Cooper [this message]
2014-08-07 16:02 ` Jan Beulich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53E3A0CB.5@citrix.com \
--to=andrew.cooper3@citrix.com \
--cc=JBeulich@suse.com \
--cc=ian.campbell@citrix.com \
--cc=ian.jackson@eu.citrix.com \
--cc=keir@xen.org \
--cc=rcojocaru@bitdefender.com \
--cc=stefano.stabellini@eu.citrix.com \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.