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

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2823 bytes --]

On Mon, Dec 23, 2019 at 08:27:49PM -0300, Fabiano Rosas wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
> > 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 see there is:
> 
> static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> {
>     ...
>     ctx->le_mode = !!(env->hflags & (1 << MSR_LE));
>     ...
> }

Ah... good point, I missed that one, sorry.  That makes all the
difference.

My guess is that this bit exists to be a universal flag for endianness
mode, generalizing across the MSR bit on modern cpus, and the old 601
which had it in the HID register.  I'm a bit dubious as to whether our
601 emulation is good enough to warrant bothering with this, but it's
probably best not to mess with it.


> And we call hreg_recompute_hflags in some places:

ITYM hreg_compute_hflags().

> - powerpc_excp (target/ppc/excp_helper.c)
>   Called from TCG do_interrupt
> 
> - ppc_cpu_reset (target/ppc/translate_init.inc.c)
>   Called from spapr_machine_reset
> 
> - hreg_store_msr (target/ppc/helper_regs.h)
>   This is used for migration and for do_rfi, store_msr

Huh... given this, I'm not sure how hflags was getting out of sync
with the MSR in the first place, which brings the initial patch into
question.

> - h_cede (hw/ppc/spapr_hcall.c)
>   QEMU-side H_CEDE hypercall implementation 
> 
> 
> It looks like the hflags MSR_LE is being updated correctly with TCG. But
> with KVM we only touch it on system_reset

Ah.. right.  I think to fix that we'd want an hreg_compute_hflags() at
the end of sucking the state out of KVM.

> (and possibly h_cede? I don't
> know if it is QEMU who handles it).

It's KVM.  If we used the qemu one it would add an awful lot of
latency to cedes.
> 
> So I would let hflags be.
> 
> 
> ... Actually, I don't really know the purpose of hflags. It comes from:
> 
>   commit 3f3373166227b13e762e20d2fb51eadfa6a2d653
>   Author: Fabrice Bellard <fabrice@bellard.org>
>   Date:   Wed Aug 20 23:02:09 2003 +0000
>   
>       pop ss, mov ss, x and sti disable irqs for the next instruction -
>       began dispatch optimization by adding new x86 cpu 'hidden' flags
>       
>       
>       git-svn-id: svn://svn.savannah.nongnu.org/qemu/trunk@372 c046a42c-6fe2-441c-8c8c-71466251a162
> 
> Could any one clarify that?

Not really.  It's really, really old, in the cruft bits of TCG I don't
much understand.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]