Re: [Qemu-devel] [qemu-s390x] [PATCH v3 1/7] s390x/pci: factor out endianess conversion

From: Cornelia Huck <cohuck@redhat.com>
To: Thomas Huth <thuth@redhat.com>
Cc: Pierre Morel <pmorel@linux.vnet.ibm.com>,
	Yi Min Zhao <zyimin@linux.vnet.ibm.com>,
	pasic@linux.vnet.ibm.com, qemu-devel@nongnu.org, agraf@suse.de,
	borntraeger@de.ibm.com, Qemu-s390x list <qemu-s390x@nongnu.org>
Subject: Re: [Qemu-devel] [qemu-s390x] [PATCH v3 1/7] s390x/pci: factor out endianess conversion
Date: Mon, 27 Nov 2017 15:34:36 +0100	[thread overview]
Message-ID: <20171127153436.200a01c7.cohuck@redhat.com> (raw)
In-Reply-To: <20171127120255.2d36c8f3.cohuck@redhat.com>

On Mon, 27 Nov 2017 12:02:55 +0100
Cornelia Huck <cohuck@redhat.com> wrote:

> On Mon, 27 Nov 2017 07:59:36 +0100
> Thomas Huth <thuth@redhat.com> wrote:
> 
> > On 25.11.2017 14:49, Pierre Morel wrote:  
> > > On 24/11/2017 07:19, Yi Min Zhao wrote:    
> > >>
> > >>
> > >> 在 2017/11/23 下午8:18, Thomas Huth 写道:    
> > >>> On 23.11.2017 13:07, Yi Min Zhao wrote:    
> 
> > >>>> Another question, does 'cpu' in cpu_to_le**() or le**_to_cpu() mean the
> > >>>> host endianess?    
> > >>> Yes, the "cpu" in cpu_to_le or le_to_cpu means the host, indeed. It's
> > >>> confusing :-/
> > >>>    
> > >>>> If the answers to upper two questions are yes, we actually need handle
> > >>>> two cases.
> > >>>> 1) For pcilg, we need to translate the data to little-endian, thus
> > >>>> cpu_to_le**().
> > >>>> 2) For pcistg, we need to translate the data to host endianess, thus
> > >>>> le**_to_cpu().    
> > >>> I think we've got to byte-swap if the host is big endian (s390x), but
> > >>> not if the host is little endian (x86 with TCG).    
> > > 
> > > Here is my comprehension of this funny swapping:
> > > 
> > > - TCG for a BE guest and a le host swap bytes because if we do (register
> > > & 0x01) in the zPCI interception code it must work what ever the
> > > endianess is.    
> > 
> > Uhhh, I might have missed that the value has already been byte-swapped
> > once by TCG for env->regs[r1] ...
> > Now I'm pretty much completely confused ... sorry for the noise if I was
> > wrong... I think it's best you ignore my comment for now (i.e. go with
> > bswapXX() instead of le_to_cpuXX()), and if we later wire up zPCI with
> > TCG, we still can fix this if necessary.  
> 
> I'll try my current pci/tcg patches on LPAR with this (or a v4) on top.
> If it works there (it doesn't yet on my laptop), we do have a
> endianness issue... (unfortunately, the reverse isn't true.)

It does not look too bad: I can get a nice enP1p0s0 device from a
virtio-net-pci with my tcg patches on my laptop (with these patches as
well, of course). So, endianness is likely mostly fine.

> 
> I'll post my pci/tcg patches once I get it mostly working (and it does
> not look like a horror from the crypt anymore).

Time to get out the make up kit for these.