Message-ID: <20111116070306.GB5433@redhat.com>
Date:	Wed, 16 Nov 2011 09:03:07 +0200
From:	"Michael S. Tsirkin" <mst@...hat.com>
To:	Rusty Russell <rusty@...tcorp.com.au>
Cc:	Sasha Levin <levinsasha928@...il.com>,
	lkml - Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Alexey Kardashevskiy <aik@...abs.ru>,
	Amit Shah <amit.shah@...hat.com>,
	Christian Borntraeger <borntraeger@...ibm.com>,
	Krishna Kumar <krkumar2@...ibm.com>,
	Pawel Moll <pawel.moll@....com>,
	Wang Sheng-Hui <shhuiw@...il.com>,
	virtualization@...ts.linux-foundation.org, kvm@...r.kernel.org,
	avi@...hat.com, penberg@...nel.org
Subject: Re: [PATCHv2 RFC] virtio-spec: flexible configuration layout

On Wed, Nov 16, 2011 at 10:28:05AM +1030, Rusty Russell wrote:
> On Sun, 13 Nov 2011 17:14:27 +0200, "Michael S. Tsirkin" <mst@...hat.com> wrote:
> > On Fri, Nov 11, 2011 at 02:54:31PM +1030, Rusty Russell wrote:
> > > Indeed, I'd like to see two changes to your proposal:
> > > 
> > > (1) It should be all or nothing.  If a driver can find the virtio header
> > >     capability, it should only use the capabilties.  Otherwise, it
> > >     should fall back to legacy.
> > 
> > Okay, but going forward, if we add more capabilities, we probably won't
> > want to require them and fail to load if not there.  That's really why I
> > wanted to make the failover ignore any capability separately - to make
> > this future proof.  I'm not terribly fixated on this, it just seemed a
> > bit more symmetrical to treat all capabilities in the same way. Hmm?
> 
> Sure, a future capbility may not exist.  But once a driver finds that
> virtio header structure in the capability, it should *never* fall back
> to the legacy area.  ie. it can expect that Queue Notify, ISR Status and
> Device Header all exist.
> 
> ie. either use legacy mode, or use capabilities.  Never both.
> 
> > 
> > >     Your draft suggests a mix is possible;
> > >     I prefer a clean failure (ie. one day don't present a BAR 0 *at
> > >     all*, so ancient drivers just fail to load.).
> > 
> > Just to clarify, as written in draft this is possible with the current
> > spec proposal.  So I'm guessing there's some other motivation that you
> > had in mind?
> 
> At the moment you give a hybrid model where both are used.  In five
> years' time, that's going to be particularly ugly.
> > 
> > > (2) There's no huge win in keeping the same layout.  Let's make some
> > >     cleanups.
> > 
> > About this last point - what cleanups do you have in mind?  Just move
> > some registers around?  I guess we could put feature bits near each
> > other, and move device status towards the end to avoid wasting 3
> > bytes.
> 
> > The win seems minimal, but the change does make legacy hypervisor
> > support in guests more cumbersome, as we need to spread coditional code
> > around instead of localizing it in the initialization path.
> 
> But the separation between "legacy" and "modern" will be sharper, making
> it easier to excise the legacy portion later.
> 
> And in five years' time, people implementing virtio will really thank us
> that they can completely ignore the legacy header.

OK, I get it I think.

> > >    There are more users ahead of us then behind us (I
> > >     hope!).
> > 
> > In that case isn't it safe to assume we'll find some uses
> > for the reserved registers?
> 
> How would we tell?  If we use a new capability struct for it, it's
> obvious.  Otherwise, you're going to need to steal more feature bits.

Yes, exactly, if as you suggest, we disable legacy header
when there is a capability - we can use reserved registers
for other stuff.

> Cheers,
> Rusty.
> PS.  Sorry, was off sick for a few days.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists