On Mon, 29 Oct 2012 19:12:54 +0100 Alexander Graf <agraf@xxxxxxx> wrote: > > On 29.10.2012, at 14:07, Cornelia Huck wrote: > > +static void virtio_ccw_kvm_notify(struct virtqueue *vq) > > +{ > > + struct virtio_ccw_vq_info *info = vq->priv; > > + struct virtio_ccw_device *vcdev; > > + struct subchannel_id schid; > > + __u32 reg2; > > + > > + vcdev = to_vc_device(info->vq->vdev); > > + ccw_device_get_schid(vcdev->cdev, &schid); > > + reg2 = *(__u32 *)&schid; > > That cast looks quite ugly. Can't you just access the field in there you need? Or if it's multiple fields do a union over them? Or assemble them by hand in C? I think the cast looks less ugly than using a union to morph it around. I want the schid with all fields filled out anyway, since this is what identifies the subchannel. > > > + kvm_hypercall2(3 /* CCW_NOTIFY */, reg2, info->queue_index); > > This wants to be a #define :) Probably :) > > > +} > > + > > +static int virtio_ccw_read_vq_conf(struct virtio_ccw_device *vcdev, int index) > > +{ > > + vcdev->config_block->index = index; > > + vcdev->ccw->cmd_code = CCW_CMD_READ_VQ_CONF; > > + vcdev->ccw->flags = 0; > > + vcdev->ccw->count = sizeof(struct vq_config_block); > > + vcdev->ccw->cda = (__u32)(unsigned long)(vcdev->config_block); > > Is this casting a pointer to a u32? What if this is in highmem? Ah, I just saw the comment that ccw memory needs to be <2GB. Phew. Any plans to get rid of that limitation? Well, we could do full-blown IDAW handling to get to 64bit addresses - which would need a lot of extra code in the host. I doubt whether it would be worth it. (Well, we'll probably want IDAWs sometime in the future - I just think it's overkill for those tiny snippets.) -- To unsubscribe from this list: send the line "unsubscribe linux-s390" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html