arch_dump: fix prstatus pid on s390x and ppc

[PATCH 2/2] target/ppc/arch_dump: set prstatus pid to cpuid

Posted by Omar Sandoval 1 year, 7 months ago

Every other architecture does this, and debuggers need it to be able to
identify which prstatus note corresponds to which CPU.

Signed-off-by: Omar Sandoval <osandov@osandov.com>
---
 target/ppc/arch_dump.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
index a8315659d9..78b4205319 100644
--- a/target/ppc/arch_dump.c
+++ b/target/ppc/arch_dump.c
@@ -47,9 +47,11 @@ struct PPCUserRegStruct {
 } QEMU_PACKED;
 
 struct PPCElfPrstatus {
-    char pad1[112];
+    char pad1[32];
+    uint32_t pid;
+    uint8_t pad2[76];
     struct PPCUserRegStruct pr_reg;
-    char pad2[40];
+    char pad3[40];
 } QEMU_PACKED;
 
 
@@ -96,7 +98,7 @@ typedef struct NoteFuncArg {
     DumpState *state;
 } NoteFuncArg;
 
-static void ppc_write_elf_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu)
+static void ppc_write_elf_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu, int id)
 {
     int i;
     reg_t cr;
@@ -109,6 +111,7 @@ static void ppc_write_elf_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu)
 
     prstatus = &note->contents.prstatus;
     memset(prstatus, 0, sizeof(*prstatus));
+    prstatus->pid = cpu_to_dump32(s, id);
     reg = &prstatus->pr_reg;
 
     for (i = 0; i < 32; i++) {
@@ -127,7 +130,7 @@ static void ppc_write_elf_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu)
     reg->ccr = cpu_to_dump_reg(s, cr);
 }
 
-static void ppc_write_elf_fpregset(NoteFuncArg *arg, PowerPCCPU *cpu)
+static void ppc_write_elf_fpregset(NoteFuncArg *arg, PowerPCCPU *cpu, int id)
 {
     int i;
     struct PPCElfFpregset  *fpregset;
@@ -146,7 +149,7 @@ static void ppc_write_elf_fpregset(NoteFuncArg *arg, PowerPCCPU *cpu)
     fpregset->fpscr = cpu_to_dump_reg(s, cpu->env.fpscr);
 }
 
-static void ppc_write_elf_vmxregset(NoteFuncArg *arg, PowerPCCPU *cpu)
+static void ppc_write_elf_vmxregset(NoteFuncArg *arg, PowerPCCPU *cpu, int id)
 {
     int i;
     struct PPCElfVmxregset *vmxregset;
@@ -178,7 +181,7 @@ static void ppc_write_elf_vmxregset(NoteFuncArg *arg, PowerPCCPU *cpu)
     vmxregset->vscr.u32[3] = cpu_to_dump32(s, ppc_get_vscr(&cpu->env));
 }
 
-static void ppc_write_elf_vsxregset(NoteFuncArg *arg, PowerPCCPU *cpu)
+static void ppc_write_elf_vsxregset(NoteFuncArg *arg, PowerPCCPU *cpu, int id)
 {
     int i;
     struct PPCElfVsxregset *vsxregset;
@@ -195,7 +198,7 @@ static void ppc_write_elf_vsxregset(NoteFuncArg *arg, PowerPCCPU *cpu)
     }
 }
 
-static void ppc_write_elf_speregset(NoteFuncArg *arg, PowerPCCPU *cpu)
+static void ppc_write_elf_speregset(NoteFuncArg *arg, PowerPCCPU *cpu, int id)
 {
     struct PPCElfSperegset *speregset;
     Note *note = &arg->note;
@@ -211,7 +214,7 @@ static void ppc_write_elf_speregset(NoteFuncArg *arg, PowerPCCPU *cpu)
 
 static const struct NoteFuncDescStruct {
     int contents_size;
-    void (*note_contents_func)(NoteFuncArg *arg, PowerPCCPU *cpu);
+    void (*note_contents_func)(NoteFuncArg *arg, PowerPCCPU *cpu, int id);
 } note_func[] = {
     {sizeof_field(Note, contents.prstatus),  ppc_write_elf_prstatus},
     {sizeof_field(Note, contents.fpregset),  ppc_write_elf_fpregset},
@@ -282,7 +285,7 @@ static int ppc_write_all_elf_notes(const char *note_name,
         arg.note.hdr.n_descsz = cpu_to_dump32(s, nf->contents_size);
         strncpy(arg.note.name, note_name, sizeof(arg.note.name));
 
-        (*nf->note_contents_func)(&arg, cpu);
+        (*nf->note_contents_func)(&arg, cpu, id);
 
         note_size =
             sizeof(arg.note) - sizeof(arg.note.contents) + nf->contents_size;
-- 
2.45.2

Re: [PATCH 2/2] target/ppc/arch_dump: set prstatus pid to cpuid

Posted by Harsh Prateek Bora 1 year, 7 months ago

Hi Omar,

On 6/19/24 10:30, Omar Sandoval wrote:
> Every other architecture does this, and debuggers need it to be able to
> identify which prstatus note corresponds to which CPU.
> 
> Signed-off-by: Omar Sandoval <osandov@osandov.com>
> ---
>   target/ppc/arch_dump.c | 21 ++++++++++++---------
>   1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
> index a8315659d9..78b4205319 100644
> --- a/target/ppc/arch_dump.c
> +++ b/target/ppc/arch_dump.c
> @@ -47,9 +47,11 @@ struct PPCUserRegStruct {
>   } QEMU_PACKED;
>   
>   struct PPCElfPrstatus {
> -    char pad1[112];
> +    char pad1[32];
> +    uint32_t pid;
> +    uint8_t pad2[76];
>       struct PPCUserRegStruct pr_reg;
> -    char pad2[40];
> +    char pad3[40];
>   } QEMU_PACKED;
>   

Could you please add a comment above the struct providing reference to 
the spec being referred here for member position across the status bits?

With that,

Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>

>   
> @@ -96,7 +98,7 @@ typedef struct NoteFuncArg {
>       DumpState *state;
>   } NoteFuncArg;
>   
> -static void ppc_write_elf_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu)
> +static void ppc_write_elf_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu, int id)
>   {
>       int i;
>       reg_t cr;
> @@ -109,6 +111,7 @@ static void ppc_write_elf_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu)
>   
>       prstatus = &note->contents.prstatus;
>       memset(prstatus, 0, sizeof(*prstatus));
> +    prstatus->pid = cpu_to_dump32(s, id);
>       reg = &prstatus->pr_reg;
>   
>       for (i = 0; i < 32; i++) {
> @@ -127,7 +130,7 @@ static void ppc_write_elf_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu)
>       reg->ccr = cpu_to_dump_reg(s, cr);
>   }
>   
> -static void ppc_write_elf_fpregset(NoteFuncArg *arg, PowerPCCPU *cpu)
> +static void ppc_write_elf_fpregset(NoteFuncArg *arg, PowerPCCPU *cpu, int id)
>   {
>       int i;
>       struct PPCElfFpregset  *fpregset;
> @@ -146,7 +149,7 @@ static void ppc_write_elf_fpregset(NoteFuncArg *arg, PowerPCCPU *cpu)
>       fpregset->fpscr = cpu_to_dump_reg(s, cpu->env.fpscr);
>   }
>   
> -static void ppc_write_elf_vmxregset(NoteFuncArg *arg, PowerPCCPU *cpu)
> +static void ppc_write_elf_vmxregset(NoteFuncArg *arg, PowerPCCPU *cpu, int id)
>   {
>       int i;
>       struct PPCElfVmxregset *vmxregset;
> @@ -178,7 +181,7 @@ static void ppc_write_elf_vmxregset(NoteFuncArg *arg, PowerPCCPU *cpu)
>       vmxregset->vscr.u32[3] = cpu_to_dump32(s, ppc_get_vscr(&cpu->env));
>   }
>   
> -static void ppc_write_elf_vsxregset(NoteFuncArg *arg, PowerPCCPU *cpu)
> +static void ppc_write_elf_vsxregset(NoteFuncArg *arg, PowerPCCPU *cpu, int id)
>   {
>       int i;
>       struct PPCElfVsxregset *vsxregset;
> @@ -195,7 +198,7 @@ static void ppc_write_elf_vsxregset(NoteFuncArg *arg, PowerPCCPU *cpu)
>       }
>   }
>   
> -static void ppc_write_elf_speregset(NoteFuncArg *arg, PowerPCCPU *cpu)
> +static void ppc_write_elf_speregset(NoteFuncArg *arg, PowerPCCPU *cpu, int id)
>   {
>       struct PPCElfSperegset *speregset;
>       Note *note = &arg->note;
> @@ -211,7 +214,7 @@ static void ppc_write_elf_speregset(NoteFuncArg *arg, PowerPCCPU *cpu)
>   
>   static const struct NoteFuncDescStruct {
>       int contents_size;
> -    void (*note_contents_func)(NoteFuncArg *arg, PowerPCCPU *cpu);
> +    void (*note_contents_func)(NoteFuncArg *arg, PowerPCCPU *cpu, int id);
>   } note_func[] = {
>       {sizeof_field(Note, contents.prstatus),  ppc_write_elf_prstatus},
>       {sizeof_field(Note, contents.fpregset),  ppc_write_elf_fpregset},
> @@ -282,7 +285,7 @@ static int ppc_write_all_elf_notes(const char *note_name,
>           arg.note.hdr.n_descsz = cpu_to_dump32(s, nf->contents_size);
>           strncpy(arg.note.name, note_name, sizeof(arg.note.name));
>   
> -        (*nf->note_contents_func)(&arg, cpu);
> +        (*nf->note_contents_func)(&arg, cpu, id);
>   
>           note_size =
>               sizeof(arg.note) - sizeof(arg.note.contents) + nf->contents_size;

Re: [PATCH 2/2] target/ppc/arch_dump: set prstatus pid to cpuid

Posted by Thomas Huth 1 year, 7 months ago

On 19/06/2024 07.00, Omar Sandoval wrote:
> Every other architecture does this, and debuggers need it to be able to
> identify which prstatus note corresponds to which CPU.
> 
> Signed-off-by: Omar Sandoval <osandov@osandov.com>
> ---
>   target/ppc/arch_dump.c | 21 ++++++++++++---------
>   1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
> index a8315659d9..78b4205319 100644
> --- a/target/ppc/arch_dump.c
> +++ b/target/ppc/arch_dump.c
> @@ -47,9 +47,11 @@ struct PPCUserRegStruct {
>   } QEMU_PACKED;
>   
>   struct PPCElfPrstatus {
> -    char pad1[112];
> +    char pad1[32];
> +    uint32_t pid;
> +    uint8_t pad2[76];
>       struct PPCUserRegStruct pr_reg;
> -    char pad2[40];
> +    char pad3[40];
>   } QEMU_PACKED;
>   
>   
> @@ -96,7 +98,7 @@ typedef struct NoteFuncArg {
>       DumpState *state;
>   } NoteFuncArg;
>   
> -static void ppc_write_elf_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu)
> +static void ppc_write_elf_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu, int id)
>   {
>       int i;
>       reg_t cr;
> @@ -109,6 +111,7 @@ static void ppc_write_elf_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu)
>   
>       prstatus = &note->contents.prstatus;
>       memset(prstatus, 0, sizeof(*prstatus));
> +    prstatus->pid = cpu_to_dump32(s, id);
>       reg = &prstatus->pr_reg;
>   
>       for (i = 0; i < 32; i++) {
> @@ -127,7 +130,7 @@ static void ppc_write_elf_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu)
>       reg->ccr = cpu_to_dump_reg(s, cr);
>   }
>   
> -static void ppc_write_elf_fpregset(NoteFuncArg *arg, PowerPCCPU *cpu)
> +static void ppc_write_elf_fpregset(NoteFuncArg *arg, PowerPCCPU *cpu, int id)
>   {
>       int i;
>       struct PPCElfFpregset  *fpregset;
> @@ -146,7 +149,7 @@ static void ppc_write_elf_fpregset(NoteFuncArg *arg, PowerPCCPU *cpu)
>       fpregset->fpscr = cpu_to_dump_reg(s, cpu->env.fpscr);
>   }
>   
> -static void ppc_write_elf_vmxregset(NoteFuncArg *arg, PowerPCCPU *cpu)
> +static void ppc_write_elf_vmxregset(NoteFuncArg *arg, PowerPCCPU *cpu, int id)
>   {
>       int i;
>       struct PPCElfVmxregset *vmxregset;
> @@ -178,7 +181,7 @@ static void ppc_write_elf_vmxregset(NoteFuncArg *arg, PowerPCCPU *cpu)
>       vmxregset->vscr.u32[3] = cpu_to_dump32(s, ppc_get_vscr(&cpu->env));
>   }
>   
> -static void ppc_write_elf_vsxregset(NoteFuncArg *arg, PowerPCCPU *cpu)
> +static void ppc_write_elf_vsxregset(NoteFuncArg *arg, PowerPCCPU *cpu, int id)
>   {
>       int i;
>       struct PPCElfVsxregset *vsxregset;
> @@ -195,7 +198,7 @@ static void ppc_write_elf_vsxregset(NoteFuncArg *arg, PowerPCCPU *cpu)
>       }
>   }
>   
> -static void ppc_write_elf_speregset(NoteFuncArg *arg, PowerPCCPU *cpu)
> +static void ppc_write_elf_speregset(NoteFuncArg *arg, PowerPCCPU *cpu, int id)
>   {
>       struct PPCElfSperegset *speregset;
>       Note *note = &arg->note;
> @@ -211,7 +214,7 @@ static void ppc_write_elf_speregset(NoteFuncArg *arg, PowerPCCPU *cpu)
>   
>   static const struct NoteFuncDescStruct {
>       int contents_size;
> -    void (*note_contents_func)(NoteFuncArg *arg, PowerPCCPU *cpu);
> +    void (*note_contents_func)(NoteFuncArg *arg, PowerPCCPU *cpu, int id);
>   } note_func[] = {
>       {sizeof_field(Note, contents.prstatus),  ppc_write_elf_prstatus},
>       {sizeof_field(Note, contents.fpregset),  ppc_write_elf_fpregset},
> @@ -282,7 +285,7 @@ static int ppc_write_all_elf_notes(const char *note_name,
>           arg.note.hdr.n_descsz = cpu_to_dump32(s, nf->contents_size);
>           strncpy(arg.note.name, note_name, sizeof(arg.note.name));
>   
> -        (*nf->note_contents_func)(&arg, cpu);
> +        (*nf->note_contents_func)(&arg, cpu, id);
>   
>           note_size =
>               sizeof(arg.note) - sizeof(arg.note.contents) + nf->contents_size;

Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH 1/2] target/s390x/arch_dump: use correct byte order for pid
[PATCH 2/2] target/ppc/arch_dump: set prstatus pid to cpuid