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

 


Help: OASIS Mailing Lists Help | MarkMail Help

virtio 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 Fri, Oct 04, 2013 at 09:14:26AM +0930, Rusty Russell wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > On Wed, Oct 02, 2013 at 08:20:33AM +0300, Michael S. Tsirkin wrote:
> >> On Wed, Oct 02, 2013 at 10:02:28AM +0930, Rusty Russell wrote:
> >> > "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> > > On Thu, Sep 26, 2013 at 11:35:53AM +0930, Rusty Russell wrote:
> >> > >> How about a PCI note something like:
> >> > >> 
> >> > >>         If a device does not support legacy mode, and is on a platform
> >> > >>         where a legacy device with the same ID has previously existed,
> >> > >>         it MUST take the following steps to fail gracefully when a
> >> > >>         legacy driver attempts to drive it:
> >> > >> 
> >> > >>         1) Present no I/O BAR, and no BAR 0, OR
> >> > >>         2) Respond to a single-byte zero write to offset 18 of any I/O
> >> > >>            BAR or BAR0 by presenting zeroes on every BAR and ignoring
> >> > >>            writes.
> >> > >> 
> >> > >> This means it presents 0 virtqueues; at the very least, it makes the
> >> > >> device harmless.
> >> > >> 
> >> > >> Note that I didn't specify that it should only be the first write: think
> >> > >> of the kexec case, where an OS crash causes an older OS to come in and
> >> > >> try to reset the device...
> >> > >> 
> >> > >> Would that work?
> >> > >> Rusty.
> >> > >
> >> > > So I guess we can also suggest revision id > 0 for pci
> >> > > and > 1 for mmio for non transitional devices.
> >> > 
> >> > Only if mmio changes its layout in an incompatible way: that's not
> >> > confimed yet.  Otherwise there's no reason to worry about older drivers
> >> > as the device can fail gracefully when feature 32 isn't acknowledged.
> >> > 
> >> > > At least with recent windows drivers, this will be enough
> >> > > to not make them bind, and it will also work for
> >> > > linux drivers.
> >> > >
> >> > > Question: what to do if we want do create a non transitional
> >> > > device on ccw?
> >> > 
> >> > I think they're happy with their current layout, but I leave this to
> >> > Cornelia (CC'd, but she's still away).
> >> > 
> >> > Cheers,
> >> > Rusty.
> >> 
> >> Well this means they aren't interested in addressing
> >> https://tools.oasis-open.org/issues/browse/VIRTIO-40
> >> ?
> >> Also, endian-ness change if it goes through is enough to
> >> break drivers in weird ways, isn't it?
> >
> > Also, changes like driver initialization change also
> > make for a bunch of interesting bugs.
> > If we let legacy drivers bind to non transitional
> > devices, that just makes for a bunch of weird issues
> > to consider.
> 
> Well, the main issue is that it simply won't work.  Your guest without a
> network or block device or console is usually a useless guest.

Not always though. A new driver can be supplied on floppy/cdrom,
if you don't bind to the old one, the new one can load
more or less automatically.

All this is not exactly top priority since people mostly
will want to use transitional devices.

Except for one user: Anthony actually wanted a way
to disable old drivers for express devices.
I'm not sure that's really all that important.

> We should think through these issues for each transport,
> to make sure
> they handle this case like I suggested for PCI, but I don't think it's
> impossible.
> 
> Cheers,
> Rusty.

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.

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.



-- 
MST


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