[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]