Re: [PATCH] target/ppc: fix memory dump endianness in QEMU monitor

[David Gibson]

On Thu, Dec 19, 2019 at 01:38:54PM -0300, Maxiwell S. Garcia wrote:
> The env->hflags is computed in ppc_cpu_reset(), using the MSR register
> as input. But at the point ppc_disas_set_info() is called the MSR_LE bit
> in env->hflags doesn't contain the same information that env->msr.
> 
> Signed-off-by: Maxiwell S. Garcia <address@hidden>
> Signed-off-by: Fabiano Rosas <address@hidden>

I think the change is ok as far as it goes but,

a) the commit message should expand on what the practical effect of
this is.  Looking, I think the only thing this affects is DEBUG_DISAS
output (i.e. very rarely) which is worth noting.

b) AFAICT this is the *only* thing that looks for the LE bit in
hflags. Given that, and the fact that it would be wrong in most cases,
we should remove it from hflags entirely along with this change.

> ---
>  target/ppc/translate_init.inc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
> index d33d65dff7..a0b384da9e 100644
> --- a/target/ppc/translate_init.inc.c
> +++ b/target/ppc/translate_init.inc.c
> @@ -10830,7 +10830,7 @@ static void ppc_disas_set_info(CPUState *cs, 
> disassemble_info *info)
>      PowerPCCPU *cpu = POWERPC_CPU(cs);
>      CPUPPCState *env = &cpu->env;
>  
> -    if ((env->hflags >> MSR_LE) & 1) {
> +    if (msr_le) {
>          info->endian = BFD_ENDIAN_LITTLE;
>      }
>      info->mach = env->bfd_mach;

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature