From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
To: David Gibson <david@gibson.dropbear.id.au>,
"Paul A. Clarke" <pc@us.ibm.com>
Cc: Giuseppe Musacchio <thatlemon@gmail.com>,
qemu-ppc@nongnu.org,
Richard Henderson <richard.henderson@linaro.org>,
qemu-devel@nongnu.org
Subject: Re: [PATCH] Fix `lxvdsx` (issue #212)
Date: Tue, 18 May 2021 07:53:26 +0100 [thread overview]
Message-ID: <25618d64-b40c-ff7e-8f69-1cddcd3863f1@ilande.co.uk> (raw)
In-Reply-To: <YKMZwVmfec0IocfV@yekko>
On 18/05/2021 02:34, David Gibson wrote:
> I'm having a hard time convincing myself this is correct in all cases.
> Have you tested it with all combinations of BE/LE host and BE/LE guest
> code?
>
> The description in the ISA is pretty inscrutable, since it's in terms
> of the confusing numbering if different element types in BE vs LE
> mode.
>
> It looks to me like before bcb0b7b1a1c0 this originally resolved to
> MO_Q modified by ctx->default_tcg_memop_mask, which appears to depend
> on the current guest endian mode. That's pretty hard to trace through
> the various layers of macros, but for reference, before bcb0b7b1a1c0
> this used gen_qemu_ld64_i64(), which appears to be constructed by the
> line GEN_QEMU_LOAD_64(ld64, DEF_MEMOP(MO_Q)) in translate.c.
>
> Richard or Giuseppe, care to weigh in?
>
>> ---
>> target/ppc/translate/vsx-impl.c.inc | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc
>> index b817d31260bb..46f97c029ca8 100644
>> --- a/target/ppc/translate/vsx-impl.c.inc
>> +++ b/target/ppc/translate/vsx-impl.c.inc
>> @@ -162,7 +162,7 @@ static void gen_lxvdsx(DisasContext *ctx)
>> gen_addr_reg_index(ctx, EA);
>>
>> data = tcg_temp_new_i64();
>> - tcg_gen_qemu_ld_i64(data, EA, ctx->mem_idx, MO_TEQ);
>> + tcg_gen_qemu_ld_i64(data, EA, ctx->mem_idx, MO_LEQ);
>> tcg_gen_gvec_dup_i64(MO_Q, vsr_full_offset(xT(ctx->opcode)), 16, 16, data);
>>
>> tcg_temp_free(EA);
Right. I think what is happening here is currently the load uses MO_TE (i.e. target
endian) which defaults to big endian for PPC and that's why you're seeing the byte
swap. The reason this is required for the vector instructions is because the vector
registers need to be stored in host byte-order to allow them to make use of the
host's vector instructions.
A quick look around the same file at gen_lxvw4x() suggests that you need a solution
like this to work correctly with both big and little endian:
if (ctx->le_mode) {
tcg_gen_qemu_ld_i64(data, EA, ctx->mem_idx, MO_LEQ);
} else {
tcg_gen_qemu_ld_i64(data, EA, ctx->mem_idx, MO_BEQ);
}
The original commit message for bcb0b7b1a1c0 mentions that the implementation is
based upon that of the lxvwsx instruction, so I'm fairly sure that gen_lxvwsx() is
also broken for little endian and will need a similar fix too.
ATB,
Mark.
prev parent reply other threads:[~2021-05-18 6:55 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-17 21:40 [PATCH] Fix `lxvdsx` (issue #212) Paul A. Clarke
2021-05-18 1:34 ` David Gibson
2021-05-18 6:40 ` Giuseppe Musacchio
2021-05-18 7:30 ` Greg Kurz
2021-05-19 0:46 ` David Gibson
2021-05-18 6:53 ` Mark Cave-Ayland [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=25618d64-b40c-ff7e-8f69-1cddcd3863f1@ilande.co.uk \
--to=mark.cave-ayland@ilande.co.uk \
--cc=david@gibson.dropbear.id.au \
--cc=pc@us.ibm.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=thatlemon@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).