Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/elfload.c                    | 41 +-----------------------
 linux-user/x86_64/target_coredump.c.inc | 42 +++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 40 deletions(-)
 create mode 100644 linux-user/x86_64/target_coredump.c.inc

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 27682f0c81..fc2ee4e0e3 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -159,46 +159,7 @@ typedef abi_int         target_pid_t;
 #define ELF_CLASS      ELFCLASS64
 #define ELF_ARCH       EM_X86_64
 
-#define ELF_NREG    27
-typedef target_elf_greg_t  target_elf_gregset_t[ELF_NREG];
-
-/*
- * Note that ELF_NREG should be 29 as there should be place for
- * TRAPNO and ERR "registers" as well but linux doesn't dump
- * those.
- *
- * See linux kernel: arch/x86/include/asm/elf.h
- */
-static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUX86State *env)
-{
-    (*regs)[0] = tswapreg(env->regs[15]);
-    (*regs)[1] = tswapreg(env->regs[14]);
-    (*regs)[2] = tswapreg(env->regs[13]);
-    (*regs)[3] = tswapreg(env->regs[12]);
-    (*regs)[4] = tswapreg(env->regs[R_EBP]);
-    (*regs)[5] = tswapreg(env->regs[R_EBX]);
-    (*regs)[6] = tswapreg(env->regs[11]);
-    (*regs)[7] = tswapreg(env->regs[10]);
-    (*regs)[8] = tswapreg(env->regs[9]);
-    (*regs)[9] = tswapreg(env->regs[8]);
-    (*regs)[10] = tswapreg(env->regs[R_EAX]);
-    (*regs)[11] = tswapreg(env->regs[R_ECX]);
-    (*regs)[12] = tswapreg(env->regs[R_EDX]);
-    (*regs)[13] = tswapreg(env->regs[R_ESI]);
-    (*regs)[14] = tswapreg(env->regs[R_EDI]);
-    (*regs)[15] = tswapreg(get_task_state(env_cpu_const(env))->orig_ax);
-    (*regs)[16] = tswapreg(env->eip);
-    (*regs)[17] = tswapreg(env->segs[R_CS].selector & 0xffff);
-    (*regs)[18] = tswapreg(env->eflags);
-    (*regs)[19] = tswapreg(env->regs[R_ESP]);
-    (*regs)[20] = tswapreg(env->segs[R_SS].selector & 0xffff);
-    (*regs)[21] = tswapreg(env->segs[R_FS].selector & 0xffff);
-    (*regs)[22] = tswapreg(env->segs[R_GS].selector & 0xffff);
-    (*regs)[23] = tswapreg(env->segs[R_DS].selector & 0xffff);
-    (*regs)[24] = tswapreg(env->segs[R_ES].selector & 0xffff);
-    (*regs)[25] = tswapreg(env->segs[R_FS].selector & 0xffff);
-    (*regs)[26] = tswapreg(env->segs[R_GS].selector & 0xffff);
-}
+#include "target_coredump.c.inc"
 
 #if ULONG_MAX > UINT32_MAX
 #define INIT_GUEST_COMMPAGE
diff --git a/linux-user/x86_64/target_coredump.c.inc b/linux-user/x86_64/target_coredump.c.inc
new file mode 100644
index 0000000000..b85ee22669
--- /dev/null
+++ b/linux-user/x86_64/target_coredump.c.inc
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#define ELF_NREG    27
+typedef target_elf_greg_t  target_elf_gregset_t[ELF_NREG];
+
+/*
+ * Note that ELF_NREG should be 29 as there should be place for
+ * TRAPNO and ERR "registers" as well but linux doesn't dump those.
+ *
+ * See linux kernel: arch/x86/include/asm/elf.h
+ */
+static void elf_core_copy_regs(target_elf_gregset_t *regs,
+                               const CPUX86State *env)
+{
+    (*regs)[0] = tswapreg(env->regs[15]);
+    (*regs)[1] = tswapreg(env->regs[14]);
+    (*regs)[2] = tswapreg(env->regs[13]);
+    (*regs)[3] = tswapreg(env->regs[12]);
+    (*regs)[4] = tswapreg(env->regs[R_EBP]);
+    (*regs)[5] = tswapreg(env->regs[R_EBX]);
+    (*regs)[6] = tswapreg(env->regs[11]);
+    (*regs)[7] = tswapreg(env->regs[10]);
+    (*regs)[8] = tswapreg(env->regs[9]);
+    (*regs)[9] = tswapreg(env->regs[8]);
+    (*regs)[10] = tswapreg(env->regs[R_EAX]);
+    (*regs)[11] = tswapreg(env->regs[R_ECX]);
+    (*regs)[12] = tswapreg(env->regs[R_EDX]);
+    (*regs)[13] = tswapreg(env->regs[R_ESI]);
+    (*regs)[14] = tswapreg(env->regs[R_EDI]);
+    (*regs)[15] = tswapreg(get_task_state(env_cpu_const(env))->orig_ax);
+    (*regs)[16] = tswapreg(env->eip);
+    (*regs)[17] = tswapreg(env->segs[R_CS].selector & 0xffff);
+    (*regs)[18] = tswapreg(env->eflags);
+    (*regs)[19] = tswapreg(env->regs[R_ESP]);
+    (*regs)[20] = tswapreg(env->segs[R_SS].selector & 0xffff);
+    (*regs)[21] = tswapreg(env->segs[R_FS].selector & 0xffff);
+    (*regs)[22] = tswapreg(env->segs[R_GS].selector & 0xffff);
+    (*regs)[23] = tswapreg(env->segs[R_DS].selector & 0xffff);
+    (*regs)[24] = tswapreg(env->segs[R_ES].selector & 0xffff);
+    (*regs)[25] = tswapreg(env->segs[R_FS].selector & 0xffff);
+    (*regs)[26] = tswapreg(env->segs[R_GS].selector & 0xffff);
+}
-- 
2.43.0

On Wed, 30 Jul 2025 at 01:24, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/elfload.c                    | 41 +-----------------------
>  linux-user/x86_64/target_coredump.c.inc | 42 +++++++++++++++++++++++++
>  2 files changed, 43 insertions(+), 40 deletions(-)
>  create mode 100644 linux-user/x86_64/target_coredump.c.inc
>

Maybe we can come back to this at some point to convert the
.c.inc into a proper .c / .h file.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

On 8/2/25 03:45, Peter Maydell wrote:
> On Wed, 30 Jul 2025 at 01:24, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   linux-user/elfload.c                    | 41 +-----------------------
>>   linux-user/x86_64/target_coredump.c.inc | 42 +++++++++++++++++++++++++
>>   2 files changed, 43 insertions(+), 40 deletions(-)
>>   create mode 100644 linux-user/x86_64/target_coredump.c.inc
>>
> 
> Maybe we can come back to this at some point to convert the
> .c.inc into a proper .c / .h file.

Are you thinking of something like ELF_NREG + target_elf_gregset_t in the header and 
elf_core_copy_regs in the c file?

I suppose there's no reason not to do that now, putting the function in the new 
target/elfload.c, and the one or two declarations in target_elf.h.


r~

On Fri, 1 Aug 2025 at 22:44, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 8/2/25 03:45, Peter Maydell wrote:
> > On Wed, 30 Jul 2025 at 01:24, Richard Henderson
> > <richard.henderson@linaro.org> wrote:
> >>
> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >> ---
> >>   linux-user/elfload.c                    | 41 +-----------------------
> >>   linux-user/x86_64/target_coredump.c.inc | 42 +++++++++++++++++++++++++
> >>   2 files changed, 43 insertions(+), 40 deletions(-)
> >>   create mode 100644 linux-user/x86_64/target_coredump.c.inc
> >>
> >
> > Maybe we can come back to this at some point to convert the
> > .c.inc into a proper .c / .h file.
>
> Are you thinking of something like ELF_NREG + target_elf_gregset_t in the header and
> elf_core_copy_regs in the c file?
>
> I suppose there's no reason not to do that now, putting the function in the new
> target/elfload.c, and the one or two declarations in target_elf.h.

I hadn't looked closely enough at the details to have a concrete
idea; this was just my preference for avoiding .c.inc files
unless we really need them (e.g. we're including the same file
multiple times or it's just too painful to split a huge file
any other way).

If it's easy to put the function in the .c file and the declarations
in the .h file then I think that's better than the .c.inc approach.

-- PMM