Re: [Qemu-devel] qemu hw/ppc_oldworld.c target-ppc/cpu.h target-...

[J. Mayer]

On Fri, 2007-11-23 at 21:36 +0000, Paul Brook wrote:
> > Then I took a closer look to the code, to ensure I was not wrong.
> > The PowerPC 32 on 64 bits hosts is implemented the same way that the
> > specification says a PowerPC in 32 bits mode should be. Then higher bits
> > are not garbage. They are what the PowerPC specification say they should
> > be (apart if they are some bugs in the implementation). The fact that
> > they are or not used by computations is another point. The fact is the
> > registers values are correct.
> 
> AFAICS the high bits are never used by anything.

They are used the way the specification tells it should be, ie "when
running in 32 bits mode, all computation are done in 64 bits".

> I think what you mean is that they work the way that ppc64 is defined, to 
> remain compatible with ppc32.  IMHO this is entirely irrelevant as we're 
> emulating a ppc32. You could replace the high bits with garbage and nothing 
> would ever be able to tell the difference.

PowerPC is a 64 bits architecture. PowerPC 32 on 32 bits host is
optimized not to compute the 32 highest bits, the same way it's allowed
to cut down the GPR when implementing a CPU that would not support the
64 bits mode (but this is a tolerance, this is not the architecture is
defined).
PowerPC 32 on 64 bits host is implemented as PowerPC 32 bits mode. This
is a choice that may be discussed but this is the way it's done. Then,
the 32 highest bits are to be computed properly, even if they do not
actually participate in any result seen from the 32 bits application.
Then, print the 64 bits GPR is relevant.
Running the PowerPC 32 emulation on a 64 bits hosts is strictly
equivalent to running the PowerPC 64 emulation in 32 bits mode, as the
architecture specifies it should be. One could then argue the PowerPC 32
targets are not relevant when running on a 64 bits host, which is true.

> > And the fact is that printing a uint64_t on any 64 bits host (x86_64 or
> > any other) using PRIx64 is exactly what is to be done, according to ISO
> > C. Then, pretending that it would crash on any host is completelly
> > pointless.
> 
> We weren't printing a 64-bit value. We were passing a 32-bit target_ulong 
> with 
> a PRIx64 format. Some concrete examples:
> translate.c:6052:
>     cpu_fprintf(f, "MSR " REGX FILL " HID0 " REGX FILL "  HF " REGX FILL
>                 env->msr, env->hflags, env->spr[SPR_HID0],
> 
> All these values are 32-bit tagret_ulong. Using a 64-bit format specifierfor 
> ppc32 targets is just nonsense.

OK. Those are real bugs to be fixed. I'll take a look.... But I'll try
not to break the GPR dump. In fact, GPR should always dumped as 64 bits,
even when runnig on 32 bits hosts. This would be more consistent with
the specification.


> And at line 6069 we even have an explicit cast to a 32-bit type:
> 
>         cpu_fprintf(f, " " REGX, (target_ulong)env->gpr[i]);

OK, this is false. I'll remove this buggy cast.

> > > I see the SPE stuff that uses T0_64 et al, however this still uses
> > > stores the value in the low 32 bits of the {gpr,gprth} pair.
> >
> > SPE dump is the case that does not work properly. Your patch does not solve 
> > anything here, just breaks the main stream case.
> 
> I agree that SPE register dumping does not work, however I also assert that 
> it 
> was never even close to working, and if REGX is supposed to be the solution 
> then most of the uses of REGX are incorrect.
> 
> Please give a concrete example of something that worked before and does not 
> now.

The fact that you cannot dump the full GPR is a bug. When GPR is stored
as 64 bits, they are to be dumped as 64 bits values. If you see bugs in
my code, please tell me I'll try to fix them (and I'll thank you for
this). But not doing weird things that are more buggy than the original
code !
But once again, the biggest problem is that you break my code without
any concertation and without even trying to understand why the code has
been written this way. So, tell me you think there's a bug, or propose a
patch. If I think the patch is OK, I'll tell you. If not, I'll try to
address the bug the way I think ithas to be done.

-- 
J. Mayer <address@hidden>
Never organized