On Wed, 21 Jan 2015 12:23:18 +0100
Cornelia Huck [off-list ref] wrote:

On Tue, 20 Jan 2015 11:08:24 +0000
Stefan Hajnoczi [off-list ref] wrote:

quoted

On Thu, Dec 11, 2014 at 02:25:12PM +0100, Cornelia Huck wrote:

quoted

@@ -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?

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.

 Thomas