From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33874) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YDuZK-0003br-KN for qemu-devel@nongnu.org; Wed, 21 Jan 2015 07:39:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YDuZG-0007RE-Hy for qemu-devel@nongnu.org; Wed, 21 Jan 2015 07:39:34 -0500 Received: from e06smtp14.uk.ibm.com ([195.75.94.110]:60764) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YDuZG-0007Qy-9E for qemu-devel@nongnu.org; Wed, 21 Jan 2015 07:39:30 -0500 Received: from /spool/local by e06smtp14.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 21 Jan 2015 12:39:28 -0000 Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by d06dlp02.portsmouth.uk.ibm.com (Postfix) with ESMTP id 7FDC82190046 for ; Wed, 21 Jan 2015 12:39:23 +0000 (GMT) Received: from d06av11.portsmouth.uk.ibm.com (d06av11.portsmouth.uk.ibm.com [9.149.37.252]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t0LCdPwj60620842 for ; Wed, 21 Jan 2015 12:39:25 GMT Received: from d06av11.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av11.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t0LCdOdr014750 for ; Wed, 21 Jan 2015 05:39:24 -0700 Date: Wed, 21 Jan 2015 13:39:22 +0100 From: Cornelia Huck Message-ID: <20150121133922.1b3e7ceb.cornelia.huck@de.ibm.com> In-Reply-To: <20150121125141.5d905254@oc7435384737.ibm.com> References: <1418304322-7546-1-git-send-email-cornelia.huck@de.ibm.com> <1418304322-7546-11-git-send-email-cornelia.huck@de.ibm.com> <20150120110824.GJ17631@stefanha-thinkpad.redhat.com> <20150121122318.22a77751.cornelia.huck@de.ibm.com> <20150121125141.5d905254@oc7435384737.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH RFC v6 10/20] s390x/virtio-ccw: add virtio set-revision call List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth Cc: Stefan Hajnoczi , virtualization@lists.linux-foundation.org, qemu-devel@nongnu.org, mst@redhat.com On Wed, 21 Jan 2015 12:51:41 +0100 Thomas Huth wrote: > On Wed, 21 Jan 2015 12:23:18 +0100 > Cornelia Huck wrote: > > > On Tue, 20 Jan 2015 11:08:24 +0000 > > Stefan Hajnoczi wrote: > > > > > On Thu, Dec 11, 2014 at 02:25:12PM +0100, Cornelia Huck wrote: > > > > @@ -608,6 +631,25 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) > > > > } > > > > } > > > > break; > > > > + case CCW_CMD_SET_VIRTIO_REV: > > > > + len = sizeof(revinfo); > > > > + if (ccw.count < len || (check_len && ccw.count > len)) { > > > > + ret = -EINVAL; > > > > + break; > > > > + } > > > > + if (!ccw.cda) { > > > > + ret = -EFAULT; > > > > + break; > > > > + } > > > > + cpu_physical_memory_read(ccw.cda, &revinfo, len); > > > > + if (dev->revision >= 0 || > > > > + revinfo.revision > virtio_ccw_rev_max(dev)) { > > > > > > In the next patch virtio_ccw_handle_set_vq() uses big-endian memory > > > access functions to load a struct from guest memory. > > > > > > Here you just copy the struct in without byteswaps. > > > > > > Are the byteswaps missing here? (I guess this normally runs big-endian > > > guests on big-endian hosts so it's not noticable.) > > > > Indeed, these are supposed to be big-endian. I'll double check the > > other payloads. > > Right. Cornelia, can you take care of this or shall I rework the patch? Currently already working on a patch :) > NB: Actually, there are a couple of "XXX config space endianness" > comments in that virtio_ccw_cb() function, so there are likely a bunch > of problems when this code should be run on little endian hosts one day. > So far, this code only runs with big-endian guests on big-endian hosts > since the virtio-ccw machine is currently KVM-only as far as I know, > that's likely why nobody complained about this yet. The transport can't take care of the config space endianness, this needs to be done by the individual devices. Probably best to simply ditch the comments.