[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
"Michael S. Tsirkin" <email@example.com> writes: > On Wed, Sep 25, 2013 at 01:50:04PM +0930, Rusty Russell wrote: >> "Michael S. Tsirkin" <firstname.lastname@example.org> writes: >> > On Tue, Sep 24, 2013 at 09:10:09PM +0930, Rusty Russell wrote: >> >> "Michael S. Tsirkin" <email@example.com> writes: >> >> > non-transitional devices should have been able to simply update >> >> > revision ID to make sure legacy drivers are not loaded. >> >> > Unfortunately, mistakes were made: >> >> > - we didn't stress that drivers must check revision ID, >> >> > and of course there's no easy way for drivers to >> >> > test this failure path, >> >> > so older versions of Windows drivers ignored revision >> >> > (latest vision matches revision correctly) >> >> > - CCW lacks revision ID field >> >> > >> >> > Both facts mean a non-transitional device would need >> >> > a separate mechanism to prevent legacy drivers from >> >> > loading. >> >> > We aren't running out of device IDs yet, so >> >> > let's use up some to resolve this. >> >> > >> >> > I incremented all IDs by 0x100 intentionally - >> >> > for the PCI bindings, this should help remind people they can't >> >> > just stick the Subsystem ID into the low byte of the Device ID. >> >> >> >> I don't think this actually works. >> > >> > Hmm what doesn't work, exactly? >> > >> >> If you offer a non-transitional device to a legacy driver, it will >> >> fail anyway (device doesn't work due to missing feature 32). >> > >> > How does it fail? Legacy drivers don't give devices any chance >> > to fail gracefully. >> >> > >> >> There's not much difference between that and it not finding the device. >> > >> > Imagine a non transitional device. >> > It would want to use offset 0 in BAR0 for something else not config. >> > When legacy driver attempts to access it, it can cause >> > all kind of mischief. >> >> As per call, I think we should check that the existing Windows drivers >> fail if BAR0 does not exist, and then make that a recommendation for >> PCI, eg: >> >> On a platform where legacy devices can exist, devices should not offer >> BAR0 at all. >> >> Cheers, >> Rusty. > > Heh no luck at all. My assumption is we'll want to have IO BARs > somewhere. If that happens drivers do all kind of weird stuff. Yeah, yuck! I grabbed the drivers to dig further. It looks like (if we present an IO BAR) the first thing it will do is reset the device by writing a 0 byte to offset 18. If you put virtio_pci_common_cfg at the start of the BAR, that corresponds to a write to the read-only num_queues entry. If it's the ISR BAR it's off the end, and if it's the notification BAR it may be valid if there are 9 virtqueues, but should never be accessed with a single byte write. 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.