| Message ID | 20190517224450.15566-8-jan.bobek@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support for i386/x86_64 with vector extensions | expand |
On 5/17/19 3:44 PM, Jan Bobek wrote: > This allows us to drop dependency on NASM and build the test image > with GCC only. Adds support for x86_64, too. > > Suggested-by: Richard Henderson <richard.henderson@linaro.org> > Signed-off-by: Jan Bobek <jan.bobek@gmail.com> > --- > Makefile | 3 +++ > test_i386.S | 41 +++++++++++++++++++++++++++++++++++++++++ > test_i386.s | 27 --------------------------- > 3 files changed, 44 insertions(+), 27 deletions(-) > create mode 100644 test_i386.S > delete mode 100644 test_i386.s Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
Jan Bobek <jan.bobek@gmail.com> writes: > This allows us to drop dependency on NASM and build the test image > with GCC only. Adds support for x86_64, too. ./risu --master -t run.out test_i386.bin and then: ./risu -t run.out test_i386.bin Gives: loading test image test_i386.bin... starting apprentice image at 0xf7f07000 starting image finished early after 1 checkpoints match status... mismatch on regs! this reginfo: faulting insn fc0b90f Because: Mismatch (master v apprentice): xmm4 : fe76ea16f7d9c58c 000006fc00000000 v: fe76ea16f7d1a58c 000006fc00000000 We probably need to zero or reset the xmm regs both in the test and when risugen dumps it's preamble. > > Suggested-by: Richard Henderson <richard.henderson@linaro.org> > Signed-off-by: Jan Bobek <jan.bobek@gmail.com> > --- > Makefile | 3 +++ > test_i386.S | 41 +++++++++++++++++++++++++++++++++++++++++ > test_i386.s | 27 --------------------------- > 3 files changed, 44 insertions(+), 27 deletions(-) > create mode 100644 test_i386.S > delete mode 100644 test_i386.s > > diff --git a/Makefile b/Makefile > index b362dbe..6ab014a 100644 > --- a/Makefile > +++ b/Makefile > @@ -49,6 +49,9 @@ $(PROG): $(OBJS) > %_$(ARCH).elf: %_$(ARCH).s > $(AS) -o $@ $< > > +%_$(ARCH).elf: %_$(ARCH).S > + $(CC) $(CPPFLAGS) -o $@ -c $< > + > clean: > rm -f $(PROG) $(OBJS) $(BINS) > > diff --git a/test_i386.S b/test_i386.S > new file mode 100644 > index 0000000..456b99c > --- /dev/null > +++ b/test_i386.S > @@ -0,0 +1,41 @@ > +/*############################################################################# > + * Copyright (c) 2010 Linaro Limited > + * All rights reserved. This program and the accompanying materials > + * are made available under the terms of the Eclipse Public License v1.0 > + * which accompanies this distribution, and is available at > + * http://www.eclipse.org/legal/epl-v10.html > + * > + * Contributors: > + * Peter Maydell (Linaro) - initial implementation > + *###########################################################################*/ > + > +/* A trivial test image for x86 */ > + > +/* Initialise the registers to avoid spurious mismatches */ > + xor %eax, %eax > + sahf /* init eflags */ > + > + mov $0x12345678, %eax > + mov $0x9abcdef0, %ebx > + mov $0x97361234, %ecx > + mov $0x84310284, %edx > + mov $0x83624173, %edi > + mov $0xfaebfaeb, %esi > + mov $0x84610123, %ebp > + > +#ifdef __x86_64__ > + movq $0x123456789abcdef0, %r8 > + movq $0xaaaabbbbccccdddd, %r9 > + movq $0x1010101010101010, %r10 > + movq $0x1111111111111111, %r11 > + movq $0x1212121212121212, %r12 > + movq $0x1313131313131313, %r13 > + movq $0x1414141414141414, %r14 > + movq $0x1515151515151515, %r15 > +#endif > + > +/* do compare */ > + ud1 %eax, %eax > + > +/* exit test */ > + ud1 %ecx, %eax > diff --git a/test_i386.s b/test_i386.s > deleted file mode 100644 > index a2140a0..0000000 > --- a/test_i386.s > +++ /dev/null > @@ -1,27 +0,0 @@ > -;############################################################################### > -;# Copyright (c) 2010 Linaro Limited > -;# All rights reserved. This program and the accompanying materials > -;# are made available under the terms of the Eclipse Public License v1.0 > -;# which accompanies this distribution, and is available at > -;# http://www.eclipse.org/legal/epl-v10.html > -;# > -;# Contributors: > -;# Peter Maydell (Linaro) - initial implementation > -;############################################################################### > - > -; A trivial test image for x86 > - > -BITS 32 > -; Initialise the registers to avoid spurious mismatches > -mov eax, 0x12345678 > -mov ebx, 0x9abcdef0 > -mov ecx, 0x97361234 > -mov edx, 0x84310284 > -mov edi, 0x83624173 > -mov esi, 0xfaebfaeb > -mov ebp, 0x84610123 > -; UD1 : do compare > -UD1 > - > -; UD2 : exit test > -UD2 -- Alex Bennée
On Mon, May 20, 2019, 08:17 Alex Bennée <alex.bennee@linaro.org> wrote: > > Jan Bobek <jan.bobek@gmail.com> writes: > > > This allows us to drop dependency on NASM and build the test image > > with GCC only. Adds support for x86_64, too. > > ./risu --master -t run.out test_i386.bin > > and then: > > ./risu -t run.out test_i386.bin > > Gives: > > loading test image test_i386.bin... > starting apprentice image at 0xf7f07000 > starting image > finished early after 1 checkpoints > match status... > mismatch on regs! > this reginfo: > faulting insn fc0b90f > > Because: > > Mismatch (master v apprentice): > xmm4 : fe76ea16f7d9c58c 000006fc00000000 > v: fe76ea16f7d1a58c 000006fc00000000 > > We probably need to zero or reset the xmm regs both in the test and when > risugen dumps it's preamble. > That gets fixed later in the series. r~ >
Richard Henderson <richard.henderson@linaro.org> writes: > On Mon, May 20, 2019, 08:17 Alex Bennée <alex.bennee@linaro.org> wrote: > >> >> Jan Bobek <jan.bobek@gmail.com> writes: >> >> > This allows us to drop dependency on NASM and build the test image >> > with GCC only. Adds support for x86_64, too. >> >> ./risu --master -t run.out test_i386.bin >> >> and then: >> >> ./risu -t run.out test_i386.bin >> >> Gives: >> >> loading test image test_i386.bin... >> starting apprentice image at 0xf7f07000 >> starting image >> finished early after 1 checkpoints >> match status... >> mismatch on regs! >> this reginfo: >> faulting insn fc0b90f >> >> Because: >> >> Mismatch (master v apprentice): >> xmm4 : fe76ea16f7d9c58c 000006fc00000000 >> v: fe76ea16f7d1a58c 000006fc00000000 >> >> We probably need to zero or reset the xmm regs both in the test and when >> risugen dumps it's preamble. >> > > That gets fixed later in the series. So it does, but I'm still seeing the test fail when played back :-/ -- Alex Bennée
On 5/21/19 5:08 AM, Alex Bennée wrote: >>> We probably need to zero or reset the xmm regs both in the test and when >>> risugen dumps it's preamble. >>> >> >> That gets fixed later in the series. > > So it does, but I'm still seeing the test fail when played back :-/ Um, no, I mean this test is extended in patch 9, exactly how you suggest. Are you trying to run the test as seen in patch 7 against the final series? r~
Richard Henderson <richard.henderson@linaro.org> writes: > On 5/21/19 5:08 AM, Alex Bennée wrote: >>>> We probably need to zero or reset the xmm regs both in the test and when >>>> risugen dumps it's preamble. >>>> >>> >>> That gets fixed later in the series. >> >> So it does, but I'm still seeing the test fail when played back :-/ > > Um, no, I mean this test is extended in patch 9, exactly how you suggest. Are > you trying to run the test as seen in patch 7 against the final > series? Running against: commit 555748b35849ad4d354a9a3cd7f8549994b2bea4 (HEAD -> review/i386-support-v2) Author: Jan Bobek <jan.bobek@gmail.com> Date: Fri May 17 18:44:50 2019 -0400 risu_reginfo_i386: accept named feature sets for --xfeature fails for me. -- Alex Bennée
On 5/21/19 11:30 AM, Alex Bennée wrote: > > Richard Henderson <richard.henderson@linaro.org> writes: > >> On 5/21/19 5:08 AM, Alex Bennée wrote: >>>>> We probably need to zero or reset the xmm regs both in the test and when >>>>> risugen dumps it's preamble. >>>>> >>>> >>>> That gets fixed later in the series. >>> >>> So it does, but I'm still seeing the test fail when played back :-/ >> >> Um, no, I mean this test is extended in patch 9, exactly how you suggest. Are >> you trying to run the test as seen in patch 7 against the final >> series? > > Running against: > > commit 555748b35849ad4d354a9a3cd7f8549994b2bea4 (HEAD -> review/i386-support-v2) > Author: Jan Bobek <jan.bobek@gmail.com> > Date: Fri May 17 18:44:50 2019 -0400 > > risu_reginfo_i386: accept named feature sets for --xfeature > > fails for me. I get the same behavior, but it only occurs on 32bit builds of RISU. Specifically, in risu_reginfo_i386.c, lines 172--178: for (i = 0; i < nvecregs; ++i) { #ifdef __x86_64__ memcpy(&ri->vregs[i], &fp->xmm_space[i], 16); #else memcpy(&ri->vregs[i], &fp->_xmm[i * 4], 16); #endif } In the #else branch, fp->_xmm has type _libc_xmmreg[16], and _libc_xmmreg itself is a struct with a 4-element array of uint32s. On my box, this gets fixed by dropping the multiplication from the index, i.e. memcpy(&ri->vregs[i], &fp->_xmm[i], 16); I wonder why Richard wrote it like this in the first place; did fp->_xmm use to be an array of uint32s in previous versions of this API? -Jan
On 5/21/19 12:48 PM, Jan Bobek wrote: > I get the same behavior, but it only occurs on 32bit builds of > RISU. Specifically, in risu_reginfo_i386.c, lines 172--178: > > for (i = 0; i < nvecregs; ++i) { > #ifdef __x86_64__ > memcpy(&ri->vregs[i], &fp->xmm_space[i], 16); > #else > memcpy(&ri->vregs[i], &fp->_xmm[i * 4], 16); > #endif > } > > In the #else branch, fp->_xmm has type _libc_xmmreg[16], and > _libc_xmmreg itself is a struct with a 4-element array of uint32s. On > my box, this gets fixed by dropping the multiplication from the index, > i.e. > > memcpy(&ri->vregs[i], &fp->_xmm[i], 16); > > I wonder why Richard wrote it like this in the first place; did > fp->_xmm use to be an array of uint32s in previous versions of this > API? I dunno what happened, but these indexes are backward. >From <asm/sigcontext.h>: struct _fpstate_32 { ... struct _xmmreg _xmm[8]; struct _fpstate_64 { ... __u32 xmm_space[64]; /* 16x XMM registers, 16 bytes each */ r~
On 5/21/19 12:56 PM, Richard Henderson wrote: > On 5/21/19 12:48 PM, Jan Bobek wrote: >> I get the same behavior, but it only occurs on 32bit builds of >> RISU. Specifically, in risu_reginfo_i386.c, lines 172--178: >> >> for (i = 0; i < nvecregs; ++i) { >> #ifdef __x86_64__ >> memcpy(&ri->vregs[i], &fp->xmm_space[i], 16); >> #else >> memcpy(&ri->vregs[i], &fp->_xmm[i * 4], 16); >> #endif >> } >> >> In the #else branch, fp->_xmm has type _libc_xmmreg[16], and >> _libc_xmmreg itself is a struct with a 4-element array of uint32s. On >> my box, this gets fixed by dropping the multiplication from the index, >> i.e. >> >> memcpy(&ri->vregs[i], &fp->_xmm[i], 16); >> >> I wonder why Richard wrote it like this in the first place; did >> fp->_xmm use to be an array of uint32s in previous versions of this >> API? > > I dunno what happened, but these indexes are backward. > >>From <asm/sigcontext.h>: > > struct _fpstate_32 { > ... > struct _xmmreg _xmm[8]; > > > struct _fpstate_64 { > ... > __u32 xmm_space[64]; /* 16x XMM registers, 16 bytes each */ Indeed; that makes for one more fix in v3. -Jan
diff --git a/Makefile b/Makefile index b362dbe..6ab014a 100644 --- a/Makefile +++ b/Makefile @@ -49,6 +49,9 @@ $(PROG): $(OBJS) %_$(ARCH).elf: %_$(ARCH).s $(AS) -o $@ $< +%_$(ARCH).elf: %_$(ARCH).S + $(CC) $(CPPFLAGS) -o $@ -c $< + clean: rm -f $(PROG) $(OBJS) $(BINS) diff --git a/test_i386.S b/test_i386.S new file mode 100644 index 0000000..456b99c --- /dev/null +++ b/test_i386.S @@ -0,0 +1,41 @@ +/*############################################################################# + * Copyright (c) 2010 Linaro Limited + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Peter Maydell (Linaro) - initial implementation + *###########################################################################*/ + +/* A trivial test image for x86 */ + +/* Initialise the registers to avoid spurious mismatches */ + xor %eax, %eax + sahf /* init eflags */ + + mov $0x12345678, %eax + mov $0x9abcdef0, %ebx + mov $0x97361234, %ecx + mov $0x84310284, %edx + mov $0x83624173, %edi + mov $0xfaebfaeb, %esi + mov $0x84610123, %ebp + +#ifdef __x86_64__ + movq $0x123456789abcdef0, %r8 + movq $0xaaaabbbbccccdddd, %r9 + movq $0x1010101010101010, %r10 + movq $0x1111111111111111, %r11 + movq $0x1212121212121212, %r12 + movq $0x1313131313131313, %r13 + movq $0x1414141414141414, %r14 + movq $0x1515151515151515, %r15 +#endif + +/* do compare */ + ud1 %eax, %eax + +/* exit test */ + ud1 %ecx, %eax diff --git a/test_i386.s b/test_i386.s deleted file mode 100644 index a2140a0..0000000 --- a/test_i386.s +++ /dev/null @@ -1,27 +0,0 @@ -;############################################################################### -;# Copyright (c) 2010 Linaro Limited -;# All rights reserved. This program and the accompanying materials -;# are made available under the terms of the Eclipse Public License v1.0 -;# which accompanies this distribution, and is available at -;# http://www.eclipse.org/legal/epl-v10.html -;# -;# Contributors: -;# Peter Maydell (Linaro) - initial implementation -;############################################################################### - -; A trivial test image for x86 - -BITS 32 -; Initialise the registers to avoid spurious mismatches -mov eax, 0x12345678 -mov ebx, 0x9abcdef0 -mov ecx, 0x97361234 -mov edx, 0x84310284 -mov edi, 0x83624173 -mov esi, 0xfaebfaeb -mov ebp, 0x84610123 -; UD1 : do compare -UD1 - -; UD2 : exit test -UD2
This allows us to drop dependency on NASM and build the test image with GCC only. Adds support for x86_64, too. Suggested-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Jan Bobek <jan.bobek@gmail.com> --- Makefile | 3 +++ test_i386.S | 41 +++++++++++++++++++++++++++++++++++++++++ test_i386.s | 27 --------------------------- 3 files changed, 44 insertions(+), 27 deletions(-) create mode 100644 test_i386.S delete mode 100644 test_i386.s