Re: [kvm-unit-tests PATCH 1/2] x86: realmode: explicitly copy structure to avoid memcpy

[Jim Mattson <jmattson@google.com>]

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
>