kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rusty Russell <rusty@rustcorp.com.au>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Krishna Kumar <krkumar2@in.ibm.com>,
	kvm@vger.kernel.org, Pawel Moll <pawel.moll@arm.com>,
	Wang Sheng-Hui <shhuiw@gmail.com>,
	Alexey Kardashevskiy <aik@ozlabs.ru>,
	lkml - Kernel Mailing List <linux-kernel@vger.kernel.org>,
	virtualization@lists.linux-foundation.org, penberg@kernel.org,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	Sasha Levin <levinsasha928@gmail.com>,
	Amit Shah <amit.shah@redhat.com>,
	avi@redhat.com
Subject: Re: [PATCHv2 RFC] virtio-spec: flexible configuration layout
Date: Wed, 16 Nov 2011 10:28:05 +1030	[thread overview]
Message-ID: <87bosdos4y.fsf@rustcorp.com.au> (raw)
In-Reply-To: <20111113151427.GA16749@redhat.com>

On Sun, 13 Nov 2011 17:14:27 +0200, "Michael S. Tsirkin" <mst@redhat.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.

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

Cheers,
Rusty.
PS.  Sorry, was off sick for a few days.

  parent reply	other threads:[~2011-11-15 23:58 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <87wrbkvh3v.fsf@rustcorp.com.au>
2011-11-01 11:45 ` [PULL] virtio Michael S. Tsirkin
2011-11-01 12:33   ` Sasha Levin
2011-11-01 12:42     ` Michael S. Tsirkin
2011-11-01 12:45       ` Sasha Levin
2011-11-02  1:09       ` Rusty Russell
2011-11-02  4:52         ` Sasha Levin
2011-11-02 22:07           ` Rusty Russell
2011-11-02 23:31         ` [PATCH RFC] virtio-pci: flexible configuration layout Michael S. Tsirkin
2011-11-03  0:19           ` Sasha Levin
2011-11-03 10:33             ` Michael S. Tsirkin
2011-11-03 11:09               ` Sasha Levin
2011-11-03 11:36                 ` Michael S. Tsirkin
2011-11-03 13:30                 ` Michael S. Tsirkin
2011-11-03 10:37           ` Avi Kivity
2011-11-03 12:11             ` Michael S. Tsirkin
2011-11-03 13:37               ` Avi Kivity
2011-11-03 13:53                 ` Michael S. Tsirkin
2011-11-03 14:59           ` Jesse Barnes
2011-11-08 21:40           ` [PATCH RFC] virtio-spec: " Michael S. Tsirkin
2011-11-08 21:41             ` Michael S. Tsirkin
2011-11-09 10:21               ` Avi Kivity
2011-11-09  8:46             ` Sasha Levin
2011-11-09 10:13               ` Michael S. Tsirkin
2011-11-09 10:26                 ` Sasha Levin
2011-11-09 10:49                   ` Michael S. Tsirkin
2011-11-09 12:25                 ` Pekka Enberg
2011-11-09 12:28                   ` Sasha Levin
2011-11-09 12:36                     ` Pekka Enberg
2011-11-09 15:33                       ` Michael S. Tsirkin
2012-06-18 11:54                       ` Michael S. Tsirkin
2012-06-18 12:05                         ` Sasha Levin
2012-06-18 12:07                           ` Michael S. Tsirkin
2011-11-09 12:38               ` Avi Kivity
2011-11-09 12:48                 ` Sasha Levin
2011-11-09 15:19                   ` Michael S. Tsirkin
2011-11-13 14:07                     ` Ronen Hod
2011-11-09 15:51                   ` Michael S. Tsirkin
2011-11-13 20:40                 ` Vadim Rozenfeld
2011-11-09  9:55             ` Sasha Levin
2011-11-09 10:18               ` Michael S. Tsirkin
2011-11-09 10:20                 ` Sasha Levin
2011-11-09 10:47                   ` Pawel Moll
2011-11-09 10:55                     ` Sasha Levin
2011-11-09 11:06                       ` Pawel Moll
2011-11-09 11:39                         ` Peter Maydell
2011-11-09 12:07                         ` Sasha Levin
2011-11-09 19:59             ` [PATCHv2 " Michael S. Tsirkin
2011-11-09 20:00               ` Michael S. Tsirkin
2011-11-09 20:24               ` Sasha Levin
2011-11-09 20:52                 ` Michael S. Tsirkin
2011-11-09 20:57                   ` Sasha Levin
2011-11-09 21:14                     ` Michael S. Tsirkin
2011-11-09 21:13                       ` Sasha Levin
2011-11-10  8:55                         ` Michael S. Tsirkin
2011-11-11  4:24                     ` Rusty Russell
2011-11-11  7:39                       ` Sasha Levin
2011-11-11 12:59                         ` Michael S. Tsirkin
2011-11-11 13:06                           ` Pawel Moll
2011-11-15 23:58                         ` Rusty Russell
2011-11-16  7:21                           ` Michael S. Tsirkin
2011-11-16  8:17                             ` Sasha Levin
2011-11-16  9:09                               ` Michael S. Tsirkin
2011-11-11 13:03                       ` Michael S. Tsirkin
2011-11-13 15:14                       ` Michael S. Tsirkin
2011-11-14  6:59                         ` Michael S. Tsirkin
2011-11-15 23:58                         ` Rusty Russell [this message]
2011-11-16  7:03                           ` Michael S. Tsirkin
2011-11-10 12:24               ` [PATCHv3 " Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87bosdos4y.fsf@rustcorp.com.au \
    --to=rusty@rustcorp.com.au \
    --cc=aik@ozlabs.ru \
    --cc=amit.shah@redhat.com \
    --cc=avi@redhat.com \
    --cc=borntraeger@de.ibm.com \
    --cc=krkumar2@in.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=levinsasha928@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=pawel.moll@arm.com \
    --cc=penberg@kernel.org \
    --cc=shhuiw@gmail.com \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).