Message ID | 20140428112942.26464.82895.stgit@bahia.lab.toulouse-stg.fr.ibm.com |
---|---|
State | New |
Headers | show |
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 = ¬e->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
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 = ¬e->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 > >
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 = ¬e->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 --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 = ¬e->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 = ¬e->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 = ¬e->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 = ¬e->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 = ¬e->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)(¬e, cpu); + (*nf->note_contents_func)(¬e, cpu, endian); note_size = sizeof(note) - sizeof(note.contents) + nf->contents_size; ret = f(¬e, note_size, opaque);