| Message ID | 20191012235859.238387-2-morbo@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | realmode test fixes for clang | expand |
On Sat, Oct 12, 2019 at 4:59 PM Bill Wendling <morbo@google.com> wrote: > > Clang prefers to use a "mempcy" (or equivalent) to copy the "regs" > structure. This doesn't work in 16-bit mode, as it will end up copying > over half the number of bytes. GCC performs a field-by-field copy of the > structure, so force clang to do the same thing. > > Signed-off-by: Bill Wendling <morbo@google.com> > --- > x86/realmode.c | 19 ++++++++++++++++--- > 1 file changed, 16 insertions(+), 3 deletions(-) > > diff --git a/x86/realmode.c b/x86/realmode.c > index 303d093..cf45fd6 100644 > --- a/x86/realmode.c > +++ b/x86/realmode.c > @@ -117,6 +117,19 @@ struct regs { > u32 eip, eflags; > }; > > +#define COPY_REG(name, dst, src) (dst).name = (src).name > +#define COPY_REGS(dst, src) \ > + COPY_REG(eax, dst, src); \ > + COPY_REG(ebx, dst, src); \ > + COPY_REG(ecx, dst, src); \ > + COPY_REG(edx, dst, src); \ > + COPY_REG(esi, dst, src); \ > + COPY_REG(edi, dst, src); \ > + COPY_REG(esp, dst, src); \ > + COPY_REG(ebp, dst, src); \ > + COPY_REG(eip, dst, src); \ > + COPY_REG(eflags, dst, src) > + This seems very fragile, too. Can we introduce our own address-space-size-independent "memcpy" and use that? I'm thinking something like: static void bytecopy(void *dst, void *src, u32 count) { asm volatile("rep movsb" : "+D" (dst), "+S" (src), "+c" (count) : : "cc"); } > struct table_descr { > u16 limit; > void *base; > @@ -148,11 +161,11 @@ static void exec_in_big_real_mode(struct insn_desc *insn) > extern u8 test_insn[], test_insn_end[]; > > for (i = 0; i < insn->len; ++i) > - test_insn[i] = ((u8 *)(unsigned long)insn->ptr)[i]; > + test_insn[i] = ((u8 *)(unsigned long)insn->ptr)[i]; > for (; i < test_insn_end - test_insn; ++i) > test_insn[i] = 0x90; // nop > > - save = inregs; > + COPY_REGS(save, inregs); > asm volatile( > "lgdtl %[gdt_descr] \n\t" > "mov %%cr0, %[tmp] \n\t" > @@ -196,7 +209,7 @@ static void exec_in_big_real_mode(struct insn_desc *insn) > : [gdt_descr]"m"(gdt_descr), [bigseg]"r"((short)16) > : "cc", "memory" > ); > - outregs = save; > + COPY_REGS(outregs, save); > } > > #define R_AX 1 > -- > 2.23.0.700.g56cf767bdb-goog >
On 13/10/19 01:58, Bill Wendling wrote: > Clang prefers to use a "mempcy" (or equivalent) to copy the "regs" > structure. This doesn't work in 16-bit mode, as it will end up copying > over half the number of bytes. GCC performs a field-by-field copy of the > structure, so force clang to do the same thing. Do you mean that: 1) the compiler is compiling a "call memcpy", where the bytes "rep movsl" are interpreted as "rep movsw" 2) the compiler's integrated assembler is including the bytes for "rep movsl", but in 16-bit mode they are "rep movsw" 3) something else The -m16 or -no-integrated-as flags perhaps can help? Paolo > Signed-off-by: Bill Wendling <morbo@google.com> > --- > x86/realmode.c | 19 ++++++++++++++++--- > 1 file changed, 16 insertions(+), 3 deletions(-) > > diff --git a/x86/realmode.c b/x86/realmode.c > index 303d093..cf45fd6 100644 > --- a/x86/realmode.c > +++ b/x86/realmode.c > @@ -117,6 +117,19 @@ struct regs { > u32 eip, eflags; > }; > > +#define COPY_REG(name, dst, src) (dst).name = (src).name > +#define COPY_REGS(dst, src) \ > + COPY_REG(eax, dst, src); \ > + COPY_REG(ebx, dst, src); \ > + COPY_REG(ecx, dst, src); \ > + COPY_REG(edx, dst, src); \ > + COPY_REG(esi, dst, src); \ > + COPY_REG(edi, dst, src); \ > + COPY_REG(esp, dst, src); \ > + COPY_REG(ebp, dst, src); \ > + COPY_REG(eip, dst, src); \ > + COPY_REG(eflags, dst, src) > + > struct table_descr { > u16 limit; > void *base; > @@ -148,11 +161,11 @@ static void exec_in_big_real_mode(struct insn_desc *insn) > extern u8 test_insn[], test_insn_end[]; > > for (i = 0; i < insn->len; ++i) > - test_insn[i] = ((u8 *)(unsigned long)insn->ptr)[i]; > + test_insn[i] = ((u8 *)(unsigned long)insn->ptr)[i]; > for (; i < test_insn_end - test_insn; ++i) > test_insn[i] = 0x90; // nop > > - save = inregs; > + COPY_REGS(save, inregs); > asm volatile( > "lgdtl %[gdt_descr] \n\t" > "mov %%cr0, %[tmp] \n\t" > @@ -196,7 +209,7 @@ static void exec_in_big_real_mode(struct insn_desc *insn) > : [gdt_descr]"m"(gdt_descr), [bigseg]"r"((short)16) > : "cc", "memory" > ); > - outregs = save; > + COPY_REGS(outregs, save); > } > > #define R_AX 1 >
diff --git a/x86/realmode.c b/x86/realmode.c index 303d093..cf45fd6 100644 --- a/x86/realmode.c +++ b/x86/realmode.c @@ -117,6 +117,19 @@ struct regs { u32 eip, eflags; }; +#define COPY_REG(name, dst, src) (dst).name = (src).name +#define COPY_REGS(dst, src) \ + COPY_REG(eax, dst, src); \ + COPY_REG(ebx, dst, src); \ + COPY_REG(ecx, dst, src); \ + COPY_REG(edx, dst, src); \ + COPY_REG(esi, dst, src); \ + COPY_REG(edi, dst, src); \ + COPY_REG(esp, dst, src); \ + COPY_REG(ebp, dst, src); \ + COPY_REG(eip, dst, src); \ + COPY_REG(eflags, dst, src) + struct table_descr { u16 limit; void *base; @@ -148,11 +161,11 @@ static void exec_in_big_real_mode(struct insn_desc *insn) extern u8 test_insn[], test_insn_end[]; for (i = 0; i < insn->len; ++i) - test_insn[i] = ((u8 *)(unsigned long)insn->ptr)[i]; + test_insn[i] = ((u8 *)(unsigned long)insn->ptr)[i]; for (; i < test_insn_end - test_insn; ++i) test_insn[i] = 0x90; // nop - save = inregs; + COPY_REGS(save, inregs); asm volatile( "lgdtl %[gdt_descr] \n\t" "mov %%cr0, %[tmp] \n\t" @@ -196,7 +209,7 @@ static void exec_in_big_real_mode(struct insn_desc *insn) : [gdt_descr]"m"(gdt_descr), [bigseg]"r"((short)16) : "cc", "memory" ); - outregs = save; + COPY_REGS(outregs, save); } #define R_AX 1
Clang prefers to use a "mempcy" (or equivalent) to copy the "regs" structure. This doesn't work in 16-bit mode, as it will end up copying over half the number of bytes. GCC performs a field-by-field copy of the structure, so force clang to do the same thing. Signed-off-by: Bill Wendling <morbo@google.com> --- x86/realmode.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)