From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42465) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V7nUR-00038a-Ih for qemu-devel@nongnu.org; Fri, 09 Aug 2013 10:16:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V7nUM-0000kg-CA for qemu-devel@nongnu.org; Fri, 09 Aug 2013 10:16:27 -0400 Received: from mail-ie0-f175.google.com ([209.85.223.175]:50579) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V7nUM-0000kV-1Q for qemu-devel@nongnu.org; Fri, 09 Aug 2013 10:16:22 -0400 Received: by mail-ie0-f175.google.com with SMTP id s9so4159189iec.20 for ; Fri, 09 Aug 2013 07:16:21 -0700 (PDT) From: Anthony Liguori In-Reply-To: <87vc3fo9j1.fsf@rustcorp.com.au> References: <1375938949-22622-1-git-send-email-rusty@rustcorp.com.au> <1375938949-22622-2-git-send-email-rusty@rustcorp.com.au> <87li4cgvh1.fsf@codemonkey.ws> <5203AB19.9070505@suse.de> <87r4e4p4wj.fsf@codemonkey.ws> <20130808154507.GX14664@redhat.com> <87pptof9nn.fsf@codemonkey.ws> <87vc3fo9j1.fsf@rustcorp.com.au> Date: Fri, 09 Aug 2013 09:16:18 -0500 Message-ID: <8738qjlzkd.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH 1/7] virtio: allow byte swapping for vring and config access List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Rusty Russell , "Daniel P. Berrange" Cc: Peter Maydell , Andreas =?utf-8?Q?F=C3=A4rber?= , anton@samba.org, qemu-devel@nongnu.org Rusty Russell writes: > Anthony Liguori writes: >> "Daniel P. Berrange" writes: >> >> The distinction is important in QEMU. ppc64 is still >> TARGET_WORDS_BIGENDIAN. We still want most stl_phys to treat integers >> as big endian. There's just this extra concept that CPU loads/stores >> are sometimes byte swapped. That affects virtio but not a lot else. > > You've redefined endian here; please don't do that. Endian is the order > in memory which a CPU does loads and stores. From any reasonable > definition, PPC is bi-endian. > > It's actually a weird thing for the qemu core to know at all: almost > everything which cares is in target-specific code. The exceptions are > gdb stubs and virtio, both of which are "native endian" (and that weird > code in exec.c: what is notdirty_mem_write?). > > Your argument that we shouldn't fix stl_* might be justifiable (ie. just > hack virtio and gdb as one-offs), but it's neither clear nor "least > surprise". That's not what I'm suggesting. I'm suggesting that we should introduce multiple variants of {ld,st}* for different types of memory access. These are bad names, but I'm thinking along the lines of: /* Read a word as the load/store instructions would */ cpu_ldst_ldw() /* Read a word as the instruction fetch unit would */ cpu_fetch_ldw() /* Read a word as the hardware MMU would */ cpu_mmu_ldw() Peter was suggesting that instead of having separate functions, we should use a context: ldw(cpu->ldst, ..) ldw(cpu->fetch, ..) ... I think I prefer functions though over a context. But this is really about TCG, not virtio. As Ben pointed out, virtio endianness needs to be independent of CPUs. We process the ring outside of a specific CPU context and it's possible that if we pick an arbitrary one, it will be in the wrong context (if running BE userspace). The only real problem I have with your original patch is putting virtio knowledge in the target code. I think your adjusted version is fine. Regards, Anthony Liguori > > Chers, > Rusty.