diff mbox

[2/4] ppc64-dump: Support dump for little endian ppc64

Message ID 20140428112942.26464.82895.stgit@bahia.lab.toulouse-stg.fr.ibm.com
State New
Headers show

Commit Message

Greg Kurz April 28, 2014, 11:29 a.m. UTC 
From: Bharata B Rao <bharata@linux.vnet.ibm.com>

Fix ppc64 arch specific dump code to work correctly for little endian
guests.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
[ rebased on top of current master branch,
  Greg Kurz <gkurz@linux.vnet.ibm.com> ]
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 target-ppc/arch_dump.c |   62 ++++++++++++++++++++++++++----------------------
 1 file changed, 33 insertions(+), 29 deletions(-)

Comments

Alexander Graf April 29, 2014, 9:19 a.m. UTC | #1 
On 28.04.14 13:29, Greg Kurz wrote:
> From: Bharata B Rao <bharata@linux.vnet.ibm.com>
>
> Fix ppc64 arch specific dump code to work correctly for little endian
> guests.
>
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> [ rebased on top of current master branch,
>    Greg Kurz <gkurz@linux.vnet.ibm.com> ]
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
>   target-ppc/arch_dump.c |   62 ++++++++++++++++++++++++++----------------------
>   1 file changed, 33 insertions(+), 29 deletions(-)
>
> diff --git a/target-ppc/arch_dump.c b/target-ppc/arch_dump.c
> index 9dccf1a..a85c872 100644
> --- a/target-ppc/arch_dump.c
> +++ b/target-ppc/arch_dump.c
> @@ -80,93 +80,95 @@ typedef struct noteStruct {
>   } QEMU_PACKED Note;
>   
>   
> -static void ppc64_write_elf64_prstatus(Note *note, PowerPCCPU *cpu)
> +static void ppc64_write_elf64_prstatus(Note *note, PowerPCCPU *cpu, int endian)
>   {
>       int i;
>       uint64_t cr;
>       struct PPC64ElfPrstatus *prstatus;
>       struct PPC64UserRegStruct *reg;
>   
> -    note->hdr.n_type = cpu_to_be32(NT_PRSTATUS);
> +    note->hdr.n_type = cpu_convert_to_target32(NT_PRSTATUS, endian);
>   
>       prstatus = &note->contents.prstatus;
>       memset(prstatus, 0, sizeof(*prstatus));
>       reg = &prstatus->pr_reg;
>   
>       for (i = 0; i < 32; i++) {
> -        reg->gpr[i] = cpu_to_be64(cpu->env.gpr[i]);
> +        reg->gpr[i] = cpu_convert_to_target64(cpu->env.gpr[i], endian);
>       }
> -    reg->nip = cpu_to_be64(cpu->env.nip);
> -    reg->msr = cpu_to_be64(cpu->env.msr);
> -    reg->ctr = cpu_to_be64(cpu->env.ctr);
> -    reg->link = cpu_to_be64(cpu->env.lr);
> -    reg->xer = cpu_to_be64(cpu_read_xer(&cpu->env));
> +    reg->nip = cpu_convert_to_target64(cpu->env.nip, endian);
> +    reg->msr = cpu_convert_to_target64(cpu->env.msr, endian);
> +    reg->ctr = cpu_convert_to_target64(cpu->env.ctr, endian);
> +    reg->link = cpu_convert_to_target64(cpu->env.lr, endian);
> +    reg->xer = cpu_convert_to_target64(cpu_read_xer(&cpu->env), endian);

I'm not sure we really care about performance here. Can't we just have a 
local endian helper function that gets a Note * pointer and extracts the 
endianness from there?

If not, maybe we should have an encapsulation struct around the Note and 
pass that around as token.


Alex
Greg Kurz April 30, 2014, 9:14 a.m. UTC | #2 
On Tue, 29 Apr 2014 11:19:40 +0200
Alexander Graf <agraf@suse.de> wrote:

> 
> On 28.04.14 13:29, Greg Kurz wrote:
> > From: Bharata B Rao <bharata@linux.vnet.ibm.com>
> >
> > Fix ppc64 arch specific dump code to work correctly for little endian
> > guests.
> >
> > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > [ rebased on top of current master branch,
> >    Greg Kurz <gkurz@linux.vnet.ibm.com> ]
> > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> > ---
> >   target-ppc/arch_dump.c |   62 ++++++++++++++++++++++++++----------------------
> >   1 file changed, 33 insertions(+), 29 deletions(-)
> >
> > diff --git a/target-ppc/arch_dump.c b/target-ppc/arch_dump.c
> > index 9dccf1a..a85c872 100644
> > --- a/target-ppc/arch_dump.c
> > +++ b/target-ppc/arch_dump.c
> > @@ -80,93 +80,95 @@ typedef struct noteStruct {
> >   } QEMU_PACKED Note;
> >   
> >   
> > -static void ppc64_write_elf64_prstatus(Note *note, PowerPCCPU *cpu)
> > +static void ppc64_write_elf64_prstatus(Note *note, PowerPCCPU *cpu, int endian)
> >   {
> >       int i;
> >       uint64_t cr;
> >       struct PPC64ElfPrstatus *prstatus;
> >       struct PPC64UserRegStruct *reg;
> >   
> > -    note->hdr.n_type = cpu_to_be32(NT_PRSTATUS);
> > +    note->hdr.n_type = cpu_convert_to_target32(NT_PRSTATUS, endian);
> >   
> >       prstatus = &note->contents.prstatus;
> >       memset(prstatus, 0, sizeof(*prstatus));
> >       reg = &prstatus->pr_reg;
> >   
> >       for (i = 0; i < 32; i++) {
> > -        reg->gpr[i] = cpu_to_be64(cpu->env.gpr[i]);
> > +        reg->gpr[i] = cpu_convert_to_target64(cpu->env.gpr[i], endian);
> >       }
> > -    reg->nip = cpu_to_be64(cpu->env.nip);
> > -    reg->msr = cpu_to_be64(cpu->env.msr);
> > -    reg->ctr = cpu_to_be64(cpu->env.ctr);
> > -    reg->link = cpu_to_be64(cpu->env.lr);
> > -    reg->xer = cpu_to_be64(cpu_read_xer(&cpu->env));
> > +    reg->nip = cpu_convert_to_target64(cpu->env.nip, endian);
> > +    reg->msr = cpu_convert_to_target64(cpu->env.msr, endian);
> > +    reg->ctr = cpu_convert_to_target64(cpu->env.ctr, endian);
> > +    reg->link = cpu_convert_to_target64(cpu->env.lr, endian);
> > +    reg->xer = cpu_convert_to_target64(cpu_read_xer(&cpu->env), endian);
> 
> I'm not sure we really care about performance here. Can't we just have a 
> local endian helper function that gets a Note * pointer and extracts the 
> endianness from there?
> 

This would be to avoid passing a 3rd argument to Note functions ?

> If not, maybe we should have an encapsulation struct around the Note and 
> pass that around as token.
> 

Yeah we should do that since the Note does not contain anything that could
be used to get the endianness... What about something like:

typedef struct ExtNote {
    Note note;
    DumpState *state;
} ExtNote;

I had proposed to pass ArchDumpInfo * to the endian helpers, but I find
DumpState * better: we end up with less users of DumpState::dump_info.

Thoughts ?

> 
> Alex
> 
>
Alexander Graf April 30, 2014, 9:28 a.m. UTC | #3 
On 30.04.14 11:14, Greg Kurz wrote:
> On Tue, 29 Apr 2014 11:19:40 +0200
> Alexander Graf <agraf@suse.de> wrote:
>
>> On 28.04.14 13:29, Greg Kurz wrote:
>>> From: Bharata B Rao <bharata@linux.vnet.ibm.com>
>>>
>>> Fix ppc64 arch specific dump code to work correctly for little endian
>>> guests.
>>>
>>> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
>>> [ rebased on top of current master branch,
>>>     Greg Kurz <gkurz@linux.vnet.ibm.com> ]
>>> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
>>> ---
>>>    target-ppc/arch_dump.c |   62 ++++++++++++++++++++++++++----------------------
>>>    1 file changed, 33 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/target-ppc/arch_dump.c b/target-ppc/arch_dump.c
>>> index 9dccf1a..a85c872 100644
>>> --- a/target-ppc/arch_dump.c
>>> +++ b/target-ppc/arch_dump.c
>>> @@ -80,93 +80,95 @@ typedef struct noteStruct {
>>>    } QEMU_PACKED Note;
>>>    
>>>    
>>> -static void ppc64_write_elf64_prstatus(Note *note, PowerPCCPU *cpu)
>>> +static void ppc64_write_elf64_prstatus(Note *note, PowerPCCPU *cpu, int endian)
>>>    {
>>>        int i;
>>>        uint64_t cr;
>>>        struct PPC64ElfPrstatus *prstatus;
>>>        struct PPC64UserRegStruct *reg;
>>>    
>>> -    note->hdr.n_type = cpu_to_be32(NT_PRSTATUS);
>>> +    note->hdr.n_type = cpu_convert_to_target32(NT_PRSTATUS, endian);
>>>    
>>>        prstatus = &note->contents.prstatus;
>>>        memset(prstatus, 0, sizeof(*prstatus));
>>>        reg = &prstatus->pr_reg;
>>>    
>>>        for (i = 0; i < 32; i++) {
>>> -        reg->gpr[i] = cpu_to_be64(cpu->env.gpr[i]);
>>> +        reg->gpr[i] = cpu_convert_to_target64(cpu->env.gpr[i], endian);
>>>        }
>>> -    reg->nip = cpu_to_be64(cpu->env.nip);
>>> -    reg->msr = cpu_to_be64(cpu->env.msr);
>>> -    reg->ctr = cpu_to_be64(cpu->env.ctr);
>>> -    reg->link = cpu_to_be64(cpu->env.lr);
>>> -    reg->xer = cpu_to_be64(cpu_read_xer(&cpu->env));
>>> +    reg->nip = cpu_convert_to_target64(cpu->env.nip, endian);
>>> +    reg->msr = cpu_convert_to_target64(cpu->env.msr, endian);
>>> +    reg->ctr = cpu_convert_to_target64(cpu->env.ctr, endian);
>>> +    reg->link = cpu_convert_to_target64(cpu->env.lr, endian);
>>> +    reg->xer = cpu_convert_to_target64(cpu_read_xer(&cpu->env), endian);
>> I'm not sure we really care about performance here. Can't we just have a
>> local endian helper function that gets a Note * pointer and extracts the
>> endianness from there?
>>
> This would be to avoid passing a 3rd argument to Note functions ?
>
>> If not, maybe we should have an encapsulation struct around the Note and
>> pass that around as token.
>>
> Yeah we should do that since the Note does not contain anything that could
> be used to get the endianness... What about something like:
>
> typedef struct ExtNote {
>      Note note;
>      DumpState *state;
> } ExtNote;
>
> I had proposed to pass ArchDumpInfo * to the endian helpers, but I find
> DumpState * better: we end up with less users of DumpState::dump_info.
>
> Thoughts ?

I think you'll see what fits best once you start writing it, so I'll 
trust your taste here :)


Alex
diff mbox

Patch

diff --git a/target-ppc/arch_dump.c b/target-ppc/arch_dump.c
index 9dccf1a..a85c872 100644
--- a/target-ppc/arch_dump.c
+++ b/target-ppc/arch_dump.c
@@ -80,93 +80,95 @@  typedef struct noteStruct {
 } QEMU_PACKED Note;
 
 
-static void ppc64_write_elf64_prstatus(Note *note, PowerPCCPU *cpu)
+static void ppc64_write_elf64_prstatus(Note *note, PowerPCCPU *cpu, int endian)
 {
     int i;
     uint64_t cr;
     struct PPC64ElfPrstatus *prstatus;
     struct PPC64UserRegStruct *reg;
 
-    note->hdr.n_type = cpu_to_be32(NT_PRSTATUS);
+    note->hdr.n_type = cpu_convert_to_target32(NT_PRSTATUS, endian);
 
     prstatus = &note->contents.prstatus;
     memset(prstatus, 0, sizeof(*prstatus));
     reg = &prstatus->pr_reg;
 
     for (i = 0; i < 32; i++) {
-        reg->gpr[i] = cpu_to_be64(cpu->env.gpr[i]);
+        reg->gpr[i] = cpu_convert_to_target64(cpu->env.gpr[i], endian);
     }
-    reg->nip = cpu_to_be64(cpu->env.nip);
-    reg->msr = cpu_to_be64(cpu->env.msr);
-    reg->ctr = cpu_to_be64(cpu->env.ctr);
-    reg->link = cpu_to_be64(cpu->env.lr);
-    reg->xer = cpu_to_be64(cpu_read_xer(&cpu->env));
+    reg->nip = cpu_convert_to_target64(cpu->env.nip, endian);
+    reg->msr = cpu_convert_to_target64(cpu->env.msr, endian);
+    reg->ctr = cpu_convert_to_target64(cpu->env.ctr, endian);
+    reg->link = cpu_convert_to_target64(cpu->env.lr, endian);
+    reg->xer = cpu_convert_to_target64(cpu_read_xer(&cpu->env), endian);
 
     cr = 0;
     for (i = 0; i < 8; i++) {
         cr |= (cpu->env.crf[i] & 15) << (4 * (7 - i));
     }
-    reg->ccr = cpu_to_be64(cr);
+    reg->ccr = cpu_convert_to_target64(cr, endian);
 }
 
-static void ppc64_write_elf64_fpregset(Note *note, PowerPCCPU *cpu)
+static void ppc64_write_elf64_fpregset(Note *note, PowerPCCPU *cpu, int endian)
 {
     int i;
     struct PPC64ElfFpregset  *fpregset;
 
-    note->hdr.n_type = cpu_to_be32(NT_PRFPREG);
+    note->hdr.n_type = cpu_convert_to_target32(NT_PRFPREG, endian);
 
     fpregset = &note->contents.fpregset;
     memset(fpregset, 0, sizeof(*fpregset));
 
     for (i = 0; i < 32; i++) {
-        fpregset->fpr[i] = cpu_to_be64(cpu->env.fpr[i]);
+        fpregset->fpr[i] = cpu_convert_to_target64(cpu->env.fpr[i], endian);
     }
-    fpregset->fpscr = cpu_to_be64(cpu->env.fpscr);
+    fpregset->fpscr = cpu_convert_to_target64(cpu->env.fpscr, endian);
 }
 
-static void ppc64_write_elf64_vmxregset(Note *note, PowerPCCPU *cpu)
+static void ppc64_write_elf64_vmxregset(Note *note, PowerPCCPU *cpu, int endian)
 {
     int i;
     struct PPC64ElfVmxregset *vmxregset;
 
-    note->hdr.n_type = cpu_to_be32(NT_PPC_VMX);
+    note->hdr.n_type = cpu_convert_to_target32(NT_PPC_VMX, endian);
     vmxregset = &note->contents.vmxregset;
     memset(vmxregset, 0, sizeof(*vmxregset));
 
     for (i = 0; i < 32; i++) {
-        vmxregset->avr[i].u64[0] = cpu_to_be64(cpu->env.avr[i].u64[0]);
-        vmxregset->avr[i].u64[1] = cpu_to_be64(cpu->env.avr[i].u64[1]);
+        vmxregset->avr[i].u64[0] =
+            cpu_convert_to_target64(cpu->env.avr[i].u64[0], endian);
+        vmxregset->avr[i].u64[1] =
+            cpu_convert_to_target64(cpu->env.avr[i].u64[1], endian);
     }
-    vmxregset->vscr.u32[3] = cpu_to_be32(cpu->env.vscr);
+    vmxregset->vscr.u32[3] = cpu_convert_to_target32(cpu->env.vscr, endian);
 }
-static void ppc64_write_elf64_vsxregset(Note *note, PowerPCCPU *cpu)
+static void ppc64_write_elf64_vsxregset(Note *note, PowerPCCPU *cpu, int endian)
 {
     int i;
     struct PPC64ElfVsxregset *vsxregset;
 
-    note->hdr.n_type = cpu_to_be32(NT_PPC_VSX);
+    note->hdr.n_type = cpu_convert_to_target32(NT_PPC_VSX, endian);
     vsxregset = &note->contents.vsxregset;
     memset(vsxregset, 0, sizeof(*vsxregset));
 
     for (i = 0; i < 32; i++) {
-        vsxregset->vsr[i] = cpu_to_be64(cpu->env.vsr[i]);
+        vsxregset->vsr[i] = cpu_convert_to_target64(cpu->env.vsr[i], endian);
     }
 }
-static void ppc64_write_elf64_speregset(Note *note, PowerPCCPU *cpu)
+static void ppc64_write_elf64_speregset(Note *note, PowerPCCPU *cpu, int endian)
 {
     struct PPC64ElfSperegset *speregset;
-    note->hdr.n_type = cpu_to_be32(NT_PPC_SPE);
+    note->hdr.n_type = cpu_convert_to_target32(NT_PPC_SPE, endian);
     speregset = &note->contents.speregset;
     memset(speregset, 0, sizeof(*speregset));
 
-    speregset->spe_acc = cpu_to_be64(cpu->env.spe_acc);
-    speregset->spe_fscr = cpu_to_be32(cpu->env.spe_fscr);
+    speregset->spe_acc = cpu_convert_to_target64(cpu->env.spe_acc, endian);
+    speregset->spe_fscr = cpu_convert_to_target32(cpu->env.spe_fscr, endian);
 }
 
 static const struct NoteFuncDescStruct {
     int contents_size;
-    void (*note_contents_func)(Note *note, PowerPCCPU *cpu);
+    void (*note_contents_func)(Note *note, PowerPCCPU *cpu, int endian);
 } note_func[] = {
     {sizeof(((Note *)0)->contents.prstatus),  ppc64_write_elf64_prstatus},
     {sizeof(((Note *)0)->contents.fpregset),  ppc64_write_elf64_fpregset},
@@ -222,13 +224,15 @@  static int ppc64_write_all_elf64_notes(const char *note_name,
     int ret = -1;
     int note_size;
     const NoteFuncDesc *nf;
+    DumpState *s = opaque;
+    int endian = s->dump_info.d_endian;
 
     for (nf = note_func; nf->note_contents_func; nf++) {
-        note.hdr.n_namesz = cpu_to_be32(sizeof(note.name));
-        note.hdr.n_descsz = cpu_to_be32(nf->contents_size);
+        note.hdr.n_namesz = cpu_convert_to_target32(sizeof(note.name), endian);
+        note.hdr.n_descsz = cpu_convert_to_target32(nf->contents_size, endian);
         strncpy(note.name, note_name, sizeof(note.name));
 
-        (*nf->note_contents_func)(&note, cpu);
+        (*nf->note_contents_func)(&note, cpu, endian);
 
         note_size = sizeof(note) - sizeof(note.contents) + nf->contents_size;
         ret = f(&note, note_size, opaque);