From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42711) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WiNgZ-000189-Ha for qemu-devel@nongnu.org; Thu, 08 May 2014 08:44:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WiNgT-0003Xg-Dv for qemu-devel@nongnu.org; Thu, 08 May 2014 08:44:27 -0400 Message-ID: <536B7C24.1030000@suse.de> Date: Thu, 08 May 2014 14:44:20 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1399537603-6905-1-git-send-email-dougkwan@google.com> <1399537603-6905-3-git-send-email-dougkwan@google.com> <536B42B5.5040300@suse.de> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/3] PPC: Allow little-endian user mode. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Riku Voipio , Doug Kwan , "qemu-ppc@nongnu.org" , QEMU Developers On 05/08/2014 02:39 PM, Peter Maydell wrote: > On 8 May 2014 09:39, Alexander Graf wrote: >> On 05/08/2014 10:26 AM, Doug Kwan wrote: >>> This all running PPC64 little-endian in user mode if target is configured >>> that way. In PPC64 LE user mode we set MSR.LE during initialization. >>> Byteswapping logic is reversed also when QEMU is running in that mode. >>> >>> Signed-off-by: Doug Kwan >> >> I can't say I'm a huge fan of this patch. It allows for really tricky >> subtile mistakes to happen. Can't we leave the target mode configured on big >> endian? > Unfortunately not if you care about linux-user mode: > linux-user mode uses the target-endian setting to figure > out if it needs to do swapping of data in all the syscall > interfaces. > > If you're going to overhaul how PPC deals with endian > dependent loads/stores, I suspect you'll end up with a > cleaner result if you convert to the new "specify endian > setting as part of the memory operation" TCG ops: > So for instance rather than having: > > tcg_gen_qemu_ld16u(arg1, arg2, ctx->mem_idx); > if (unlikely(ctx->le_mode)) { > tcg_gen_bswap16_tl(arg1, arg1); > } > > it would be better to do > TCGMemOp op = MO_UW | (ctx->le_mode ? MO_LE : MO_BE); > tcg_gen_qemu_ld_i32(arg1, arg2, ctx->mem_idx, op); > > This will work regardless of the TARGET_WORD_BIGENDIAN > setting, since we directly ask TCG to do an LE or BE > access, rather than doing a target-endian access and > then swapping. (It's also more efficient if you're > in little-endian mode on a little endian host since > it won't swap at all rather than swapping twice.) Good point. We could store the "default TCGMemOp mask" in ctx and then only add the width per operation. Alex