Re: [Qemu-devel] [PATCH v3 2/4] ppc64-dump: Support dump for little endian ppc64

[Alexander Graf <agraf@suse.de>]

On 05/05/2014 10:05 AM, 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.
>
> We introduce a NoteFuncArg type to avoid adding extra arguments to all note
> functions.
>
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> [ rebased on top of current master branch,
>    introduced NoteFuncArg,
>    use new cpu_to_dump{16,32,64} endian helpers,
>    Greg Kurz <gkurz@linux.vnet.ibm.com> ]
> Reviewed-by: Alexander Graf <agraf@suse.de>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
>
> Changes in v3:
> - better taste with the endian helpers naming
>
>   target-ppc/arch_dump.c |   82 +++++++++++++++++++++++++++++-------------------
>   1 file changed, 49 insertions(+), 33 deletions(-)
>
> diff --git a/target-ppc/arch_dump.c b/target-ppc/arch_dump.c
> index 9dccf1a..5487b61 100644
> --- a/target-ppc/arch_dump.c
> +++ b/target-ppc/arch_dump.c
> @@ -79,94 +79,109 @@ typedef struct noteStruct {
>       } contents;
>   } QEMU_PACKED Note;
>   
> +typedef struct NoteFuncArg {
> +    Note note;
> +    DumpState *state;
> +} NoteFuncArg;
>   
> -static void ppc64_write_elf64_prstatus(Note *note, PowerPCCPU *cpu)
> +static void ppc64_write_elf64_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu)
>   {
>       int i;
>       uint64_t cr;
>       struct PPC64ElfPrstatus *prstatus;
>       struct PPC64UserRegStruct *reg;
> +    Note *note = &arg->note;
> +    DumpState *s = arg->state;
>   
> -    note->hdr.n_type = cpu_to_be32(NT_PRSTATUS);
> +    note->hdr.n_type = cpu_to_dump32(s, NT_PRSTATUS);
>   
>       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_to_dump64(s, cpu->env.gpr[i]);
>       }
> -    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_to_dump64(s, cpu->env.nip);
> +    reg->msr = cpu_to_dump64(s, cpu->env.msr);
> +    reg->ctr = cpu_to_dump64(s, cpu->env.ctr);
> +    reg->link = cpu_to_dump64(s, cpu->env.lr);
> +    reg->xer = cpu_to_dump64(s, cpu_read_xer(&cpu->env));
>   
>       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_to_dump64(s, cr);
>   }
>   
> -static void ppc64_write_elf64_fpregset(Note *note, PowerPCCPU *cpu)
> +static void ppc64_write_elf64_fpregset(NoteFuncArg *arg, PowerPCCPU *cpu)
>   {
>       int i;
>       struct PPC64ElfFpregset  *fpregset;
> +    Note *note = &arg->note;
> +    DumpState *s = arg->state;
>   
> -    note->hdr.n_type = cpu_to_be32(NT_PRFPREG);
> +    note->hdr.n_type = cpu_to_dump32(s, NT_PRFPREG);
>   
>       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_to_dump64(s, cpu->env.fpr[i]);
>       }
> -    fpregset->fpscr = cpu_to_be64(cpu->env.fpscr);
> +    fpregset->fpscr = cpu_to_dump64(s, cpu->env.fpscr);
>   }
>   
> -static void ppc64_write_elf64_vmxregset(Note *note, PowerPCCPU *cpu)
> +static void ppc64_write_elf64_vmxregset(NoteFuncArg *arg, PowerPCCPU *cpu)
>   {
>       int i;
>       struct PPC64ElfVmxregset *vmxregset;
> +    Note *note = &arg->note;
> +    DumpState *s = arg->state;
>   
> -    note->hdr.n_type = cpu_to_be32(NT_PPC_VMX);
> +    note->hdr.n_type = cpu_to_dump32(s, NT_PPC_VMX);
>       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_to_dump64(s, cpu->env.avr[i].u64[0]);
> +        vmxregset->avr[i].u64[1] = cpu_to_dump64(s, cpu->env.avr[i].u64[1]);

Is this correct? Tom, could you please ack if it is?


Alex