On 13/08/2021 06:17, Peter Maydell wrote:
[E-MAIL EXTERNO] Não clique em links ou abra anexos, a menos que você possa
confirmar o remetente e saber que o conteúdo é seguro. Em caso de e-mail
suspeito entre imediatamente em contato com o DTI.
On Thu, 12 Aug 2021 at 21:07, Richard Henderson
wrote:
On 8/12/21 9:10 AM, matheus.fe...@eldorado.org.br wrote:
static bool avr_need_swap(CPUPPCState *env)
{
+bool le;
+#if defined(CONFIG_USER_ONLY)
+le = false;
+#else
+le = msr_le;
+#endif
It certainly doesn't seem like the right fix.
My first guess was that MSR_LE wasn't being properly set up at cpu_reset for
user-only,
but it's there.
This code is confusing because there are multiple possible swaps happening:
(1) gdb_get_regl() and friends assume they are passed a host-endian value
and will tswap to get a value of TARGET_WORDS_BIGENDIAN endianness.
(For the other direction, ldl_p() et al do target-endian accesses.)
(2) for ppc softmmu, TARGET_WORDS_BIGENDIAN is always true, and so
if the CPU is in LE mode then the ppc gdbstub code needs to swap
(ppc_maybe_bswap_register() does this)
(3) for ppc usermode, TARGET_WORDS_BIGENDIAN matches the actual binary's
ordering, so the gdb_get_regl() etc swap is always correct and sufficient
and ppc_maybe_bswap_register() does nothing
(4) the data affected by this avr_need_swap() function is the 128
bit registers, and it has to do with whether we consider the two
64-bit halves as (high, low) or (low, high). (The swapping or not
of each 64-bit half is done with the same steps 1 2 3 above as for a
64-bit value.)
Thanks for this explanation, I think I can better understand the problem
now.
I haven't yet worked through the 128 bit case -- I'd need to look at
whether we store the AVR data in the CPU struct as a pair of uint64
host-order values (Arm does this, it's always index 0 is lo, 1 is hi
regardless of host endianness) or really as a host-order 128 bit integer.
I believe it's the latter. Looking at vsr64_offset in target/ppc/cpu.h,
VsrD macro is used to determine the index of the high element, and the
definition of this macro depends on HOST_WORDS_BIGENDIAN.
But I think the code is pretty confusing, and to make it a bit less
so it would be useful to:
* unify the "do we need to do an extra swap" logic that is currently
split between avr_need_swap() and ppc_maybe_bswap_register() (assuming
that the answer is really the same for both cases, of course...)
I think we can remove avr_need_swap and handle everything in
ppc_maybe_bswap_register. I'll provide another patch with this approach.
* look at whether there is a nicer way to handle the 128 bit
register case than "byteswap the two 64-bit halves and then decide
which order to use them in"
We could use bswap128 from int128.h and something else to handle the
!CONFIG_INT128 case.
* write a good explanatory comment...
-- PMM
IIUC, usermode doesn't need any swap, and system does. What puzzles me
is that the original commit (ea499e71506) mentions that the 64-bit
elements need to be reordered "for both system and user mode". But that
was in 2016, so maybe things have changed since then.
--
Matheus K. Ferst
Instituto de Pesquisas ELDORADO
Analista de Software Júnior
Aviso Legal - Disclaimer
