From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=41937 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q90uQ-0006wv-L0 for qemu-devel@nongnu.org; Sun, 10 Apr 2011 16:06:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q90uP-0004ry-FO for qemu-devel@nongnu.org; Sun, 10 Apr 2011 16:06:58 -0400 Received: from hall.aurel32.net ([88.191.126.93]:57796) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q90uP-0004rg-6B for qemu-devel@nongnu.org; Sun, 10 Apr 2011 16:06:57 -0400 Date: Sun, 10 Apr 2011 22:06:53 +0200 From: Aurelien Jarno Subject: Re: [Qemu-devel] [PATCH 01/15] s390x: fix virtio feature bitmap Message-ID: <20110410200653.GE4551@volta.aurel32.net> References: <1301927544-32767-1-git-send-email-agraf@suse.de> <1301927544-32767-2-git-send-email-agraf@suse.de> <20110410192531.GC4551@volta.aurel32.net> <2F05F566-871D-48B5-805A-B5FF48432193@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <2F05F566-871D-48B5-805A-B5FF48432193@suse.de> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: peter.maydell@linaro.org, QEMU-devel Developers , Richard Henderson On Sun, Apr 10, 2011 at 09:26:26PM +0200, Alexander Graf wrote: > > On 10.04.2011, at 21:25, Aurelien Jarno wrote: > > > On Mon, Apr 04, 2011 at 04:32:10PM +0200, Alexander Graf wrote: > >> The feature bitmap in the s390 virtio machine is little endian. To > >> address for that, we need to bswap the values after reading them out. > >> > >> Signed-off-by: Alexander Graf > >> --- > >> hw/s390-virtio-bus.c | 4 ++-- > >> 1 files changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c > >> index 58af164..60e0135 100644 > >> --- a/hw/s390-virtio-bus.c > >> +++ b/hw/s390-virtio-bus.c > >> @@ -223,7 +223,7 @@ void s390_virtio_device_sync(VirtIOS390Device *dev) > >> cur_offs += num_vq * VIRTIO_VQCONFIG_LEN; > >> > >> /* Sync feature bitmap */ > >> - stl_phys(cur_offs, dev->host_features); > >> + stl_phys(cur_offs, bswap32(dev->host_features)); > > > > Is bswap32 correct here for both big and little endian guests? I don't > > really understand the reason why a bswap is needed here, especially > > given that AFAIK this code was already used when using KVM. > > This is target specific code. The s390-virtio-bus is s390 specific. And yes, the code was also broken with KVM. That's how I first found it actually. > So in short, s390-virtio-bus is specific to big endian machines, but is using little endian? That looks a bit weired. I guess it's probably to late to change the specification anyway... -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net