OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

 


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-dev message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]


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


On Mon, 7 Oct 2013 15:16:11 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Oct 07, 2013 at 02:00:55PM +0200, Cornelia Huck wrote:
> > 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.
> > 
> 
> I see virtio as an internal implementation detail
> of the device.
> For example, virtio pci devices are regular pci devices -
> not a controller with a virtio bus.
> 
> Generally a "bus" or "controller" with always a single device
> behind it always looks like over-engineering to me ...

For channel I/O, the control unit and the device have traditionally
been different things. Some channel commands will be targeted at the
control unit, some at the device, and we can have status reports on the
subchannel from both the control unit and the device (which makes
sense, if you picture the device being in a different box on the other
side of your raised floor). It also makes it possible to have
device-less control units for special purposes or to replace a real
device with an emulation. Of course, there are integrated devices as
well, which would be more similar to a pci device than the channel
'device'. (In general, the control unit vs. device stuff is more
complex than what I need for virtio.)

I'm not too opposed to adding a 'device' for non-transitional devices,
though; it only seems slightly odd.

> 
> > 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.
> 
> Would all this need host kernel changes in kvm?
> It's been a while since I looked at kvm s390.

The host kvm only contains some small bits for channel I/O interrupts;
host virtio is all implemented in user space (read, qemu).



[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]