From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH V2] tools/tests: Add EIP check to test_x86_emulator.c Date: Thu, 7 Aug 2014 12:29:03 +0100 Message-ID: <53E362FF.9000305@citrix.com> References: <1407409810-18591-1-git-send-email-rcojocaru@bitdefender.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1407409810-18591-1-git-send-email-rcojocaru@bitdefender.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Razvan Cojocaru , xen-devel@lists.xen.org Cc: ian.jackson@eu.citrix.com, ian.campbell@citrix.com, JBeulich@suse.com, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On 07/08/14 12:10, Razvan Cojocaru wrote: > The test now also checks that EIP was modified after emulating > instructions after (and including) the "movq %mm3,(%ecx)..." > code block. > > Changes since V1: > - Now checking if the value in EIP is correct instead of simply > checking that EIP has been modified. You should state in the commit message that this change depends on Jan's fix to the emulator itself. (I cant remember whether OSSTest runs this test harness) > > Signed-off-by: Razvan Cojocaru > --- > tools/tests/x86_emulator/test_x86_emulator.c | 80 +++++++++++++++++++------- > 1 file changed, 60 insertions(+), 20 deletions(-) > > diff --git a/tools/tests/x86_emulator/test_x86_emulator.c b/tools/tests/x86_emulator/test_x86_emulator.c > index 0a00d5a..8d9894b 100644 > --- a/tools/tests/x86_emulator/test_x86_emulator.c > +++ b/tools/tests/x86_emulator/test_x86_emulator.c > @@ -602,20 +602,24 @@ int main(int argc, char **argv) > printf("%-40s", "Testing movq %mm3,(%ecx)..."); > if ( stack_exec && cpu_has_mmx ) > { > - extern const unsigned char movq_to_mem[]; > + extern const unsigned char movq_to_mem[], movq_to_mem_end[]; > + unsigned long instr_size; > > asm volatile ( "pcmpeqb %%mm3, %%mm3\n" > ".pushsection .test, \"a\", @progbits\n" > "movq_to_mem: movq %%mm3, (%0)\n" > + "movq_to_mem_end:\n" > ".popsection" :: "c" (NULL) ); > > + instr_size = movq_to_mem_end - movq_to_mem; > memcpy(instr, movq_to_mem, 15); > 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) || > + (regs.eip != (unsigned long)&instr[0] + instr_size) ) I presume that this pointer arithmetic works out correctly, but Xen style would insist on brackets, and it would be rather more clear as: (unsigned long)(&instr[0] + instr_size) With that change, Reviewed-by: Andrew Cooper ~Andrew