Re: [virtio-dev] [PATCH] transitional issues: add new IDs for all devices

[Cornelia Huck <cornelia.huck@de.ibm.com>]

On Mon, 7 Oct 2013 13:03:12 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> By the way, I looked at ccw. It seems to use this field
> for device ID:
> 
> struct ccw_device_id {
>         __u16   match_flags;    /* which fields to match against */
> 
>         __u16   cu_type;        /* control unit type     */
>         __u16   dev_type;       /* device type           */
>         __u8    cu_model;       /* control unit model    */
>         __u8    dev_model;      /* device model          */
> 
>         kernel_ulong_t driver_info;
> };
> 
> and
> 
>        dev_set_drvdata(&cdev->dev, vcdev);
>        vcdev->vdev.id.vendor = cdev->id.cu_type;
>        vcdev->vdev.id.device = cdev->id.cu_model;
> 
> 
> That's broken: we'll run out of an 8 bit field
> and we'll have problems.

Well, I did not anticipate the device number exceeding 8 bit...

> 
> How about we put device id in dev_type instead?
> 
> We can use cu_type kind of like a revision field:
> any value 0-10 is for transitional devices,
> value 11 and up is for non transitional devices.

The thing I don't like about this is that we end up mixing control unit
information (the cu_ fields) with device information (the dev_ fields).
Conceptionally, I think of the virtio proxy device of something akin to
a traditional control unit of a certain type (0x3832 == virtio-ccw) and
a certain model (blk, net, ...). I did not provide a device since what
I see as a 'device' (virtio-blk, virtio-net, ...) is not a device in
the s390 channel sense (it doesn't talk ccw, only the 'control unit'
does). If a 'device' would suddenly appear for the same control unit
type, it would look all weird to me.

If we can't handle this with an extra ccw for the revision type, I'd
vote for a new control unit type (which we'd have to reserve
internally) and keeping the model at device number - 0x100. If this
wouldn't work, we could have a fixed control unit model with this new
type and encode the device number in the device type. Still a bit odd,
but less odd than when keeping the control unit type.