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

[Pawel Moll <pawel.moll@arm.com>]

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. The MMIO
devices are meant to be simple and do one thing and one thing only. The
guest driver can easily handle both versions.

> 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ł