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

[David Gibson]

On Mon, Dec 23, 2019 at 06:35:30PM -0300, Maxiwell S. Garcia wrote:
> On Mon, Dec 23, 2019 at 05:30:43PM +1100, David Gibson wrote:
> > 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.
> 
> Ok, I will do that. I got this bug using the 'x/i' command on QEMU
> monitor with a LE guest.

Ok.

> > 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.
> > 
> 
> I was changing the code to remove this LE bit from hflags and I found the
> function 'helper_store_hid0_601()' in misc_helper.c, which manipulates the
> 'hflags'. The commit 056401eae6 says:
> 
> "Implement PowerPC 601 HID0 register, needed for little-endian mode support.
> As a consequence, we need to merge hflags coming from MSR with other ones.
> Use little-endian mode from hflags instead of MSR during code translation."
> 
> So, is the 'hflags' necessary here? Can we use MSR instead of hflags to
> change the endianness in this function?

That function alters the LE bit in hflags, but doesn't read it.
*Nothing* reads it, so none of the places that alter it matter.

I strongly suspect we won't properly honour the LE bit in the 601 HID
register, but that's already the case.  I also suspect it's far from
the only way in which 601 emulation is broken.

-- 
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