On Mon, 29 Oct 2012 19:12:54 +0100
Alexander Graf <[email protected]> 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 kvm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
