Re: [virtio] [OASIS Issue Tracker] Created: (VIRTIO-35) race condition with multi-dword config accesses

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Oct 08, 2013 at 02:06:52PM +0100, Pawel Moll wrote:
> On Tue, 2013-10-08 at 13:10 +0100, Michael S. Tsirkin wrote:
> > On Tue, Oct 08, 2013 at 11:39:10AM +0100, Pawel Moll wrote:
> > > On Fri, 2013-10-04 at 04:36 +0100, Rusty Russell wrote:
> > > > +• 0x018 | R | ConfigGeneration
> > > > + Configuration atomicity value.
> > > > + Changes every time the configuration noticeably changes.  This means the
> > > > + device may only change the value after a configuration read operation,
> > > > + but it must change if there is any risk of a device seeing an inconsistent
> > > > + configuration state.
> > > 
> > > How about moving it closer to the config space? Something like 0x0f0 or
> > > even 0x0fc?
> > > 
> > > > +2.3.2.2.1. Legacy Interface: MMIO Device Layout
> > > > +--------------------------
> > > > +The ConfigGeneration field does not exist in legacy devices; fortunately
> > > > +it would return 0 if accessed.  
> > > 
> > > Not on "my" models, no. It would result in external abort (think
> > > SIGBUS).
> > > 
> > > Not a big problem - the device version number will be bumped to 2 anyway
> > > (working on it right now).
> > > 
> > > Paweł
> > > 
> > 
> > Hmm do current guests check version number?
> 
> Yes.
> 
>         /* Check device version */
>         vm_dev->version = readl(vm_dev->base + VIRTIO_MMIO_VERSION);
>         if (vm_dev->version != 1) {
>                 dev_err(&pdev->dev, "Version %ld not supported!\n",
>                                 vm_dev->version);
>                 return -ENXIO;
>         }
> 
> > Transitional devices should support both old and
> > new guests.
> 
> We've discussed it before and I have a different opinion.

It's merely a definition (we voted this into spec btw :) ).
If you don't support legacy drivers
you have a non transitional device.
That's fine I guess.

> The MMIO
> devices are meant to be simple and do one thing and one thing only. The
> guest driver can easily handle both versions.

To clarify, I'm not asking you to code up a transitional device.  I'm
merely suggesting that the option is there in the spec.
Makes sense?

> > For transitional I think the right thing to do is
> > to key off the VIRTIO_1 feature bit and/or FEATURES_OK.
> 
> One does not preclude another. MMIO devices ver.2 may have this as
> requirement.
> 
> Paweł
>