virtio-comment

During the last TC meeting, we discussed making virtio little endian.
It was suggested that a feature bit can be used for this,
but I now think I see two problems:


1. Features are optional,
in that there's no way for device to communicate to
guest that guest must ack a feature bit, and e.g. fail
if guest does not ack.

On the other hand, it seems likely
that a hardware virtio device might want to *only* implement
little endian format and not both big and little endian.

In other words this would be something Paolo once called
a "negative feature".

2. With virtio-pci we are running out of transport bits,
and need a new config space layout to add extra feature bits.


Thus what I'd like to suggest is a new field reporting to
guest whether device supports native format, little endian format,
or both.

This new field will naturally be only exposed in the new layout.


Thoughts?

--
MST

"Michael S. Tsirkin" <mst@redhat.com> writes:
> During the last TC meeting, we discussed making virtio little endian.
> It was suggested that a feature bit can be used for this,
> but I now think I see two problems:
>
>
> 1. Features are optional,
> in that there's no way for device to communicate to
> guest that guest must ack a feature bit, and e.g. fail
> if guest does not ack.
>
> On the other hand, it seems likely
> that a hardware virtio device might want to *only* implement
> little endian format and not both big and little endian.
>
> In other words this would be something Paolo once called
> a "negative feature".
>
> 2. With virtio-pci we are running out of transport bits,
> and need a new config space layout to add extra feature bits.

The discussion was more in the context of a method for backwards
compatiblity. If we're changing the PCI layout, that itself is
sufficient to trigger LE-only mode.

MMIO is already defined as LE-only. CCW is currently native, ie. big
endian, since it's S/390.

Anthony originally suggested two feature bits: a big endian and a little
endian bit. A device which acks neither needs a heuristic. This is
probably overkill given only CCW will have this problem.

> Thus what I'd like to suggest is a new field reporting to
> guest whether device supports native format, little endian format,
> or both.
>
> This new field will naturally be only exposed in the new layout.

I really don't want to support both endians; I'd rather have the current
endian hacks left for legacy virtio, where they're needed anyway. I
don't think legacy is worth fixing, either.

Cheers,
Rusty.

On Mon, Aug 19, 2013 at 12:21:03PM +0930, Rusty Russell wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > During the last TC meeting, we discussed making virtio little endian.
> > It was suggested that a feature bit can be used for this,
> > but I now think I see two problems:
> >
> >
> > 1. Features are optional,
> > in that there's no way for device to communicate to
> > guest that guest must ack a feature bit, and e.g. fail
> > if guest does not ack.
> >
> > On the other hand, it seems likely
> > that a hardware virtio device might want to *only* implement
> > little endian format and not both big and little endian.
> >
> > In other words this would be something Paolo once called
> > a "negative feature".
> >
> > 2. With virtio-pci we are running out of transport bits,
> > and need a new config space layout to add extra feature bits.
>
> The discussion was more in the context of a method for backwards
> compatiblity. If we're changing the PCI layout, that itself is
> sufficient to trigger LE-only mode.
>
> MMIO is already defined as LE-only.

Sorry, are we talking about only config layout
for now, or ring and headers in guest memory as well?

The later is native for MMIO, isn't it?

> CCW is currently native, ie. big
> endian, since it's S/390.
>
> Anthony originally suggested two feature bits: a big endian and a little
> endian bit. A device which acks neither needs a heuristic. This is
> probably overkill given only CCW will have this problem.
>
> > Thus what I'd like to suggest is a new field reporting to
> > guest whether device supports native format, little endian format,
> > or both.
> >
> > This new field will naturally be only exposed in the new layout.
>
> I really don't want to support both endians; I'd rather have the current
> endian hacks left for legacy virtio, where they're needed anyway. I
> don't think legacy is worth fixing, either.
>
> Cheers,
> Rusty.

"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Mon, Aug 19, 2013 at 12:21:03PM +0930, Rusty Russell wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> > During the last TC meeting, we discussed making virtio little endian.
>> > It was suggested that a feature bit can be used for this,
>> > but I now think I see two problems:
>> >
>> >
>> > 1. Features are optional,
>> > in that there's no way for device to communicate to
>> > guest that guest must ack a feature bit, and e.g. fail
>> > if guest does not ack.
>> >
>> > On the other hand, it seems likely
>> > that a hardware virtio device might want to *only* implement
>> > little endian format and not both big and little endian.
>> >
>> > In other words this would be something Paolo once called
>> > a "negative feature".
>> >
>> > 2. With virtio-pci we are running out of transport bits,
>> > and need a new config space layout to add extra feature bits.
>>
>> The discussion was more in the context of a method for backwards
>> compatiblity. If we're changing the PCI layout, that itself is
>> sufficient to trigger LE-only mode.
>>
>> MMIO is already defined as LE-only.
>
> Sorry, are we talking about only config layout
> for now, or ring and headers in guest memory as well?
>
> The later is native for MMIO, isn't it?

Sorry, you're right.

I was talking about everything, though they're potentially separate
decisions.

Cheers,
Rusty.

On Mon, Aug 19, 2013 at 05:17:30PM +0930, Rusty Russell wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > On Mon, Aug 19, 2013 at 12:21:03PM +0930, Rusty Russell wrote:
> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> > During the last TC meeting, we discussed making virtio little endian.
> >> > It was suggested that a feature bit can be used for this,
> >> > but I now think I see two problems:
> >> >
> >> >
> >> > 1. Features are optional,
> >> > in that there's no way for device to communicate to
> >> > guest that guest must ack a feature bit, and e.g. fail
> >> > if guest does not ack.
> >> >
> >> > On the other hand, it seems likely
> >> > that a hardware virtio device might want to *only* implement
> >> > little endian format and not both big and little endian.
> >> >
> >> > In other words this would be something Paolo once called
> >> > a "negative feature".
> >> >
> >> > 2. With virtio-pci we are running out of transport bits,
> >> > and need a new config space layout to add extra feature bits.
> >>
> >> The discussion was more in the context of a method for backwards
> >> compatiblity. If we're changing the PCI layout, that itself is
> >> sufficient to trigger LE-only mode.
> >>
> >> MMIO is already defined as LE-only.
> >
> > Sorry, are we talking about only config layout
> > for now, or ring and headers in guest memory as well?
> >
> > The later is native for MMIO, isn't it?
>
> Sorry, you're right.
>
> I was talking about everything, though they're potentially separate
> decisions.
>
> Cheers,
> Rusty.

So, I'll focus on the config space for now.
That's also mostly a PCI thing and clearly off datapath so we are safe
from that POV (I think you mentioned some concerns on the last call).

Though we'll need to think about CCW.
It could be benefitial to involve some folks who work on this:
I think cornelia.huck@de.ibm.com and fred.konrad@greensocs.com
were involved in the past.

I'm not sure if we need to follow some procedure
to invite people to get involved - could you
handle that?

--
MST

On Mon, 19 Aug 2013 12:08:35 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Aug 19, 2013 at 05:17:30PM +0930, Rusty Russell wrote:
> > "Michael S. Tsirkin" <mst@redhat.com> writes:
> > > On Mon, Aug 19, 2013 at 12:21:03PM +0930, Rusty Russell wrote:
> > >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > >> > During the last TC meeting, we discussed making virtio little endian.
> > >> > It was suggested that a feature bit can be used for this,
> > >> > but I now think I see two problems:
> > >> >
> > >> >
> > >> > 1. Features are optional,
> > >> > in that there's no way for device to communicate to
> > >> > guest that guest must ack a feature bit, and e.g. fail
> > >> > if guest does not ack.
> > >> >
> > >> > On the other hand, it seems likely
> > >> > that a hardware virtio device might want to *only* implement
> > >> > little endian format and not both big and little endian.
> > >> >
> > >> > In other words this would be something Paolo once called
> > >> > a "negative feature".
> > >> >
> > >> > 2. With virtio-pci we are running out of transport bits,
> > >> > and need a new config space layout to add extra feature bits.
> > >>
> > >> The discussion was more in the context of a method for backwards
> > >> compatiblity. If we're changing the PCI layout, that itself is
> > >> sufficient to trigger LE-only mode.
> > >>
> > >> MMIO is already defined as LE-only.
> > >
> > > Sorry, are we talking about only config layout
> > > for now, or ring and headers in guest memory as well?
> > >
> > > The later is native for MMIO, isn't it?
> >
> > Sorry, you're right.
> >
> > I was talking about everything, though they're potentially separate
> > decisions.
> >
> > Cheers,
> > Rusty.
>
> So, I'll focus on the config space for now.
> That's also mostly a PCI thing and clearly off datapath so we are safe
> from that POV (I think you mentioned some concerns on the last call).
>
> Though we'll need to think about CCW.
> It could be benefitial to involve some folks who work on this:
> I think cornelia.huck@de.ibm.com and fred.konrad@greensocs.com
> were involved in the past.

I'm already here, just haven't time to respond yet :)

>
> I'm not sure if we need to follow some procedure
> to invite people to get involved - could you
> handle that?
>

On Mon, Aug 19, 2013 at 11:09:44AM +0200, Cornelia Huck wrote:
> On Mon, 19 Aug 2013 12:08:35 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Mon, Aug 19, 2013 at 05:17:30PM +0930, Rusty Russell wrote:
> > > "Michael S. Tsirkin" <mst@redhat.com> writes:
> > > > On Mon, Aug 19, 2013 at 12:21:03PM +0930, Rusty Russell wrote:
> > > >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > > >> > During the last TC meeting, we discussed making virtio little endian.
> > > >> > It was suggested that a feature bit can be used for this,
> > > >> > but I now think I see two problems:
> > > >> >
> > > >> >
> > > >> > 1. Features are optional,
> > > >> > in that there's no way for device to communicate to
> > > >> > guest that guest must ack a feature bit, and e.g. fail
> > > >> > if guest does not ack.
> > > >> >
> > > >> > On the other hand, it seems likely
> > > >> > that a hardware virtio device might want to *only* implement
> > > >> > little endian format and not both big and little endian.
> > > >> >
> > > >> > In other words this would be something Paolo once called
> > > >> > a "negative feature".
> > > >> >
> > > >> > 2. With virtio-pci we are running out of transport bits,
> > > >> > and need a new config space layout to add extra feature bits.
> > > >>
> > > >> The discussion was more in the context of a method for backwards
> > > >> compatiblity. If we're changing the PCI layout, that itself is
> > > >> sufficient to trigger LE-only mode.
> > > >>
> > > >> MMIO is already defined as LE-only.
> > > >
> > > > Sorry, are we talking about only config layout
> > > > for now, or ring and headers in guest memory as well?
> > > >
> > > > The later is native for MMIO, isn't it?
> > >
> > > Sorry, you're right.
> > >
> > > I was talking about everything, though they're potentially separate
> > > decisions.
> > >
> > > Cheers,
> > > Rusty.
> >
> > So, I'll focus on the config space for now.
> > That's also mostly a PCI thing and clearly off datapath so we are safe
> > from that POV (I think you mentioned some concerns on the last call).
> >
> > Though we'll need to think about CCW.
> > It could be benefitial to involve some folks who work on this:
> > I think cornelia.huck@de.ibm.com and fred.konrad@greensocs.com
> > were involved in the past.
>
> I'm already here, just haven't time to respond yet :)

Great.
To put it shortly, what we were discussing for PCI
is using a separate set of addresses for the
virtio config, with a slightly different layout for
the common registers, and with device specific config also
being little endian.

PCI has a 256 "config space" channel that we intend to
use to supply information such as availability of
the new layout and the addresses to use for
common and device specific registers.

I'll work on a spec patch for that, so
if above is not very clear, just wait until
I send that to list.

What kind of mechanism would be appropriate for
virtio CCW?


Actually, virtio-mmio would also be affected:
while I understand that it made config space LE
already, it would need a way to report support for
the new common layout.

Peter, could you comment please?

> >
> > I'm not sure if we need to follow some procedure
> > to invite people to get involved - could you
> > handle that?
> >

On Mon, 19 Aug 2013 12:27:53 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Aug 19, 2013 at 11:09:44AM +0200, Cornelia Huck wrote:
> > On Mon, 19 Aug 2013 12:08:35 +0300
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >
> > > On Mon, Aug 19, 2013 at 05:17:30PM +0930, Rusty Russell wrote:
> > > > "Michael S. Tsirkin" <mst@redhat.com> writes:
> > > > > On Mon, Aug 19, 2013 at 12:21:03PM +0930, Rusty Russell wrote:
> > > > >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > > > >> > During the last TC meeting, we discussed making virtio little endian.
> > > > >> > It was suggested that a feature bit can be used for this,
> > > > >> > but I now think I see two problems:
> > > > >> >
> > > > >> >
> > > > >> > 1. Features are optional,
> > > > >> > in that there's no way for device to communicate to
> > > > >> > guest that guest must ack a feature bit, and e.g. fail
> > > > >> > if guest does not ack.
> > > > >> >
> > > > >> > On the other hand, it seems likely
> > > > >> > that a hardware virtio device might want to *only* implement
> > > > >> > little endian format and not both big and little endian.
> > > > >> >
> > > > >> > In other words this would be something Paolo once called
> > > > >> > a "negative feature".
> > > > >> >
> > > > >> > 2. With virtio-pci we are running out of transport bits,
> > > > >> > and need a new config space layout to add extra feature bits.

Just as point of data: For virtio-ccw we should be able to easily
extend feature bits.

> > > > >>
> > > > >> The discussion was more in the context of a method for backwards
> > > > >> compatiblity. If we're changing the PCI layout, that itself is
> > > > >> sufficient to trigger LE-only mode.
> > > > >>
> > > > >> MMIO is already defined as LE-only.
> > > > >
> > > > > Sorry, are we talking about only config layout
> > > > > for now, or ring and headers in guest memory as well?
> > > > >
> > > > > The later is native for MMIO, isn't it?
> > > >
> > > > Sorry, you're right.
> > > >
> > > > I was talking about everything, though they're potentially separate
> > > > decisions.
> > > >
> > > > Cheers,
> > > > Rusty.
> > >
> > > So, I'll focus on the config space for now.
> > > That's also mostly a PCI thing and clearly off datapath so we are safe
> > > from that POV (I think you mentioned some concerns on the last call).
> > >
> > > Though we'll need to think about CCW.
> > > It could be benefitial to involve some folks who work on this:
> > > I think cornelia.huck@de.ibm.com and fred.konrad@greensocs.com
> > > were involved in the past.
> >
> > I'm already here, just haven't time to respond yet :)
>
> Great.
> To put it shortly, what we were discussing for PCI
> is using a separate set of addresses for the
> virtio config, with a slightly different layout for
> the common registers, and with device specific config also
> being little endian.
>
> PCI has a 256 "config space" channel that we intend to
> use to supply information such as availability of
> the new layout and the addresses to use for
> common and device specific registers.
>
> I'll work on a spec patch for that, so
> if above is not very clear, just wait until
> I send that to list.

OK. I'll hope I can understand that, given my limited knowledge of pci.

>
> What kind of mechanism would be appropriate for
> virtio CCW?

One idea might be that all devices start out in 'legacy mode' and we
add either a feature bit or a new channel command that switches to
'standard mode'. Old hosts would reject that command, while old guests
would simply not issue it.

Another idea is to look at the read_conf/write_conf channel commands
and introduce a new pair of read_conf_le/write_conf_le commands. Again,
compatibility would be achived via the host doing command rejects.

We also have the possibility to convey extra information about the
control unit (the device) in the 'extended sense id data' via the sense
id command. Basically, we can encode some supported channel commands in
there, but we would need a new command type for that architectured.
We'd still need to rely on the command rejects mentioned above, though.

Regarding little-endian rings, we could probably hack up some code to
measure the overhead that would give us.

On Mon, Aug 19, 2013 at 02:16:11PM +0200, Cornelia Huck wrote:
> On Mon, 19 Aug 2013 12:27:53 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Mon, Aug 19, 2013 at 11:09:44AM +0200, Cornelia Huck wrote:
> > > On Mon, 19 Aug 2013 12:08:35 +0300
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > >
> > > > On Mon, Aug 19, 2013 at 05:17:30PM +0930, Rusty Russell wrote:
> > > > > "Michael S. Tsirkin" <mst@redhat.com> writes:
> > > > > > On Mon, Aug 19, 2013 at 12:21:03PM +0930, Rusty Russell wrote:
> > > > > >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > > > > >> > During the last TC meeting, we discussed making virtio little endian.
> > > > > >> > It was suggested that a feature bit can be used for this,
> > > > > >> > but I now think I see two problems:
> > > > > >> >
> > > > > >> >
> > > > > >> > 1. Features are optional,
> > > > > >> > in that there's no way for device to communicate to
> > > > > >> > guest that guest must ack a feature bit, and e.g. fail
> > > > > >> > if guest does not ack.
> > > > > >> >
> > > > > >> > On the other hand, it seems likely
> > > > > >> > that a hardware virtio device might want to *only* implement
> > > > > >> > little endian format and not both big and little endian.
> > > > > >> >
> > > > > >> > In other words this would be something Paolo once called
> > > > > >> > a "negative feature".
> > > > > >> >
> > > > > >> > 2. With virtio-pci we are running out of transport bits,
> > > > > >> > and need a new config space layout to add extra feature bits.
>
> Just as point of data: For virtio-ccw we should be able to easily
> extend feature bits.
>
> > > > > >>
> > > > > >> The discussion was more in the context of a method for backwards
> > > > > >> compatiblity. If we're changing the PCI layout, that itself is
> > > > > >> sufficient to trigger LE-only mode.
> > > > > >>
> > > > > >> MMIO is already defined as LE-only.
> > > > > >
> > > > > > Sorry, are we talking about only config layout
> > > > > > for now, or ring and headers in guest memory as well?
> > > > > >
> > > > > > The later is native for MMIO, isn't it?
> > > > >
> > > > > Sorry, you're right.
> > > > >
> > > > > I was talking about everything, though they're potentially separate
> > > > > decisions.
> > > > >
> > > > > Cheers,
> > > > > Rusty.
> > > >
> > > > So, I'll focus on the config space for now.
> > > > That's also mostly a PCI thing and clearly off datapath so we are safe
> > > > from that POV (I think you mentioned some concerns on the last call).
> > > >
> > > > Though we'll need to think about CCW.
> > > > It could be benefitial to involve some folks who work on this:
> > > > I think cornelia.huck@de.ibm.com and fred.konrad@greensocs.com
> > > > were involved in the past.
> > >
> > > I'm already here, just haven't time to respond yet :)
> >
> > Great.
> > To put it shortly, what we were discussing for PCI
> > is using a separate set of addresses for the
> > virtio config, with a slightly different layout for
> > the common registers, and with device specific config also
> > being little endian.
> >
> > PCI has a 256 "config space" channel that we intend to
> > use to supply information such as availability of
> > the new layout and the addresses to use for
> > common and device specific registers.
> >
> > I'll work on a spec patch for that, so
> > if above is not very clear, just wait until
> > I send that to list.
>
> OK. I'll hope I can understand that, given my limited knowledge of pci.
>
> >
> > What kind of mechanism would be appropriate for
> > virtio CCW?
>
> One idea might be that all devices start out in 'legacy mode' and we
> add either a feature bit or a new channel command that switches to
> 'standard mode'. Old hosts would reject that command, while old guests
> would simply not issue it.
>
> Another idea is to look at the read_conf/write_conf channel commands
> and introduce a new pair of read_conf_le/write_conf_le commands. Again,
> compatibility would be achived via the host doing command rejects.
>
> We also have the possibility to convey extra information about the
> control unit (the device) in the 'extended sense id data' via the sense
> id command. Basically, we can encode some supported channel commands in
> there, but we would need a new command type for that architectured.
> We'd still need to rely on the command rejects mentioned above, though.

If guest can discover whether host supports the new layout,
why would there be any rejects?


> Regarding little-endian rings, we could probably hack up some code to
> measure the overhead that would give us.

On Mon, 19 Aug 2013 16:05:43 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> > One idea might be that all devices start out in 'legacy mode' and we
> > add either a feature bit or a new channel command that switches to
> > 'standard mode'. Old hosts would reject that command, while old guests
> > would simply not issue it.
> >
> > Another idea is to look at the read_conf/write_conf channel commands
> > and introduce a new pair of read_conf_le/write_conf_le commands. Again,
> > compatibility would be achived via the host doing command rejects.
> >
> > We also have the possibility to convey extra information about the
> > control unit (the device) in the 'extended sense id data' via the sense
> > id command. Basically, we can encode some supported channel commands in
> > there, but we would need a new command type for that architectured.
> > We'd still need to rely on the command rejects mentioned above, though.
>
> If guest can discover whether host supports the new layout,
> why would there be any rejects?

If we want to support old guests on new hosts, we'll have to rely on
this.

On Mon, Aug 19, 2013 at 03:20:49PM +0200, Cornelia Huck wrote:
> On Mon, 19 Aug 2013 16:05:43 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > > One idea might be that all devices start out in 'legacy mode' and we
> > > add either a feature bit or a new channel command that switches to
> > > 'standard mode'. Old hosts would reject that command, while old guests
> > > would simply not issue it.
> > >
> > > Another idea is to look at the read_conf/write_conf channel commands
> > > and introduce a new pair of read_conf_le/write_conf_le commands. Again,
> > > compatibility would be achived via the host doing command rejects.
> > >
> > > We also have the possibility to convey extra information about the
> > > control unit (the device) in the 'extended sense id data' via the sense
> > > id command. Basically, we can encode some supported channel commands in
> > > there, but we would need a new command type for that architectured.
> > > We'd still need to rely on the command rejects mentioned above, though.
> >
> > If guest can discover whether host supports the new layout,
> > why would there be any rejects?
>
> If we want to support old guests on new hosts, we'll have to rely on
> this.

I would expect old guests not to issue old commands, if new
host supports that why would there be rejects?

On Mon, 19 Aug 2013 16:24:35 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Aug 19, 2013 at 03:20:49PM +0200, Cornelia Huck wrote:
> > On Mon, 19 Aug 2013 16:05:43 +0300
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >
> > > > One idea might be that all devices start out in 'legacy mode' and we
> > > > add either a feature bit or a new channel command that switches to
> > > > 'standard mode'. Old hosts would reject that command, while old guests
> > > > would simply not issue it.
> > > >
> > > > Another idea is to look at the read_conf/write_conf channel commands
> > > > and introduce a new pair of read_conf_le/write_conf_le commands. Again,
> > > > compatibility would be achived via the host doing command rejects.
> > > >
> > > > We also have the possibility to convey extra information about the
> > > > control unit (the device) in the 'extended sense id data' via the sense
> > > > id command. Basically, we can encode some supported channel commands in
> > > > there, but we would need a new command type for that architectured.
> > > > We'd still need to rely on the command rejects mentioned above, though.
> > >
> > > If guest can discover whether host supports the new layout,
> > > why would there be any rejects?
> >
> > If we want to support old guests on new hosts, we'll have to rely on
> > this.
>
> I would expect old guests not to issue old commands, if new
> host supports that why would there be rejects?
>

Thinko, of course you are right.

Cornelia Huck <cornelia.huck@de.ibm.com> writes:
> On Mon, 19 Aug 2013 12:27:53 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
>> On Mon, Aug 19, 2013 at 11:09:44AM +0200, Cornelia Huck wrote:
>> > On Mon, 19 Aug 2013 12:08:35 +0300
>> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> >
>> > > On Mon, Aug 19, 2013 at 05:17:30PM +0930, Rusty Russell wrote:
>> > > > "Michael S. Tsirkin" <mst@redhat.com> writes:
>> > > > > On Mon, Aug 19, 2013 at 12:21:03PM +0930, Rusty Russell wrote:
>> > > > >> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> > > > >> > During the last TC meeting, we discussed making virtio little endian.
>> > > > >> > It was suggested that a feature bit can be used for this,
>> > > > >> > but I now think I see two problems:
>> > > > >> >
>> > > > >> >
>> > > > >> > 1. Features are optional,
>> > > > >> > in that there's no way for device to communicate to
>> > > > >> > guest that guest must ack a feature bit, and e.g. fail
>> > > > >> > if guest does not ack.
>> > > > >> >
>> > > > >> > On the other hand, it seems likely
>> > > > >> > that a hardware virtio device might want to *only* implement
>> > > > >> > little endian format and not both big and little endian.
>> > > > >> >
>> > > > >> > In other words this would be something Paolo once called
>> > > > >> > a "negative feature".
>> > > > >> >
>> > > > >> > 2. With virtio-pci we are running out of transport bits,
>> > > > >> > and need a new config space layout to add extra feature bits.
>
> Just as point of data: For virtio-ccw we should be able to easily
> extend feature bits.

Yes, I think virtio-ccw and virtio-mmio could simply add a new feature
bit to say "1.0 compliant". In 20 years time that might look a
bit awkward, but that's probably OK unless there's an obvious
out-of-band mechanism to use.

BTW "standards compliant" for proposals so far means:
1) config fields are LE.
2) device-specfic headers are LE (eg. virtio_net_hdr).
3) fields in the ring are LE.

For virtio-ccw, the actual commands to manipulate the virtio devices and
bus (ie. struct ccw1) would remain BE, because that's the nature of CCW.

> Regarding little-endian rings, we could probably hack up some code to
> measure the overhead that would give us.

That would be great!

Thanks,
Rusty.

On Tue, 20 Aug 2013 14:04:10 +0930
Rusty Russell <rusty@au1.ibm.com> wrote:

> Cornelia Huck <cornelia.huck@de.ibm.com> writes:
> > On Mon, 19 Aug 2013 12:27:53 +0300
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >
> >> On Mon, Aug 19, 2013 at 11:09:44AM +0200, Cornelia Huck wrote:
> >> > On Mon, 19 Aug 2013 12:08:35 +0300
> >> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >> >
> >> > > On Mon, Aug 19, 2013 at 05:17:30PM +0930, Rusty Russell wrote:
> >> > > > "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> > > > > On Mon, Aug 19, 2013 at 12:21:03PM +0930, Rusty Russell wrote:
> >> > > > >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> > > > >> > During the last TC meeting, we discussed making virtio little endian.
> >> > > > >> > It was suggested that a feature bit can be used for this,
> >> > > > >> > but I now think I see two problems:
> >> > > > >> >
> >> > > > >> >
> >> > > > >> > 1. Features are optional,
> >> > > > >> > in that there's no way for device to communicate to
> >> > > > >> > guest that guest must ack a feature bit, and e.g. fail
> >> > > > >> > if guest does not ack.
> >> > > > >> >
> >> > > > >> > On the other hand, it seems likely
> >> > > > >> > that a hardware virtio device might want to *only* implement
> >> > > > >> > little endian format and not both big and little endian.
> >> > > > >> >
> >> > > > >> > In other words this would be something Paolo once called
> >> > > > >> > a "negative feature".
> >> > > > >> >
> >> > > > >> > 2. With virtio-pci we are running out of transport bits,
> >> > > > >> > and need a new config space layout to add extra feature bits.
> >
> > Just as point of data: For virtio-ccw we should be able to easily
> > extend feature bits.
>
> Yes, I think virtio-ccw and virtio-mmio could simply add a new feature
> bit to say "1.0 compliant". In 20 years time that might look a
> bit awkward, but that's probably OK unless there's an obvious
> out-of-band mechanism to use.

A feature bit would probably be easier for virtio-ccw - at least,
there's a lot less to architecture on our side (no need for new channel
commands etc.)

>
> BTW "standards compliant" for proposals so far means:
> 1) config fields are LE.
> 2) device-specfic headers are LE (eg. virtio_net_hdr).
> 3) fields in the ring are LE.
>
> For virtio-ccw, the actual commands to manipulate the virtio devices and
> bus (ie. struct ccw1) would remain BE, because that's the nature of CCW.

As they are just a means to do I/O as dictated by the hardware
architecture, anything else would be weird.

>
> > Regarding little-endian rings, we could probably hack up some code to
> > measure the overhead that would give us.
>
> That would be great!

I'll follow up on this when we have some results.

On Tue, Aug 20, 2013 at 09:03:59AM +0200, Cornelia Huck wrote:
> On Tue, 20 Aug 2013 14:04:10 +0930
> Rusty Russell <rusty@au1.ibm.com> wrote:
>
> > Cornelia Huck <cornelia.huck@de.ibm.com> writes:
> > > On Mon, 19 Aug 2013 12:27:53 +0300
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > >
> > >> On Mon, Aug 19, 2013 at 11:09:44AM +0200, Cornelia Huck wrote:
> > >> > On Mon, 19 Aug 2013 12:08:35 +0300
> > >> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > >> >
> > >> > > On Mon, Aug 19, 2013 at 05:17:30PM +0930, Rusty Russell wrote:
> > >> > > > "Michael S. Tsirkin" <mst@redhat.com> writes:
> > >> > > > > On Mon, Aug 19, 2013 at 12:21:03PM +0930, Rusty Russell wrote:
> > >> > > > >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > >> > > > >> > During the last TC meeting, we discussed making virtio little endian.
> > >> > > > >> > It was suggested that a feature bit can be used for this,
> > >> > > > >> > but I now think I see two problems:
> > >> > > > >> >
> > >> > > > >> >
> > >> > > > >> > 1. Features are optional,
> > >> > > > >> > in that there's no way for device to communicate to
> > >> > > > >> > guest that guest must ack a feature bit, and e.g. fail
> > >> > > > >> > if guest does not ack.
> > >> > > > >> >
> > >> > > > >> > On the other hand, it seems likely
> > >> > > > >> > that a hardware virtio device might want to *only* implement
> > >> > > > >> > little endian format and not both big and little endian.
> > >> > > > >> >
> > >> > > > >> > In other words this would be something Paolo once called
> > >> > > > >> > a "negative feature".
> > >> > > > >> >
> > >> > > > >> > 2. With virtio-pci we are running out of transport bits,
> > >> > > > >> > and need a new config space layout to add extra feature bits.
> > >
> > > Just as point of data: For virtio-ccw we should be able to easily
> > > extend feature bits.
> >
> > Yes, I think virtio-ccw and virtio-mmio could simply add a new feature
> > bit to say "1.0 compliant". In 20 years time that might look a
> > bit awkward, but that's probably OK unless there's an obvious
> > out-of-band mechanism to use.
>
> A feature bit would probably be easier for virtio-ccw - at least,
> there's a lot less to architecture on our side (no need for new channel
> commands etc.)

Do you have a revision field somewhere, in case host wants
to disable old drivers?

> >
> > BTW "standards compliant" for proposals so far means:
> > 1) config fields are LE.
> > 2) device-specfic headers are LE (eg. virtio_net_hdr).
> > 3) fields in the ring are LE.
> >
> > For virtio-ccw, the actual commands to manipulate the virtio devices and
> > bus (ie. struct ccw1) would remain BE, because that's the nature of CCW.
>
> As they are just a means to do I/O as dictated by the hardware
> architecture, anything else would be weird.
>
> >
> > > Regarding little-endian rings, we could probably hack up some code to
> > > measure the overhead that would give us.
> >
> > That would be great!
>
> I'll follow up on this when we have some results.

On Tue, 20 Aug 2013 17:01:25 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Tue, Aug 20, 2013 at 09:03:59AM +0200, Cornelia Huck wrote:
> > On Tue, 20 Aug 2013 14:04:10 +0930
> > Rusty Russell <rusty@au1.ibm.com> wrote:
> >
> > > Cornelia Huck <cornelia.huck@de.ibm.com> writes:
> > > > On Mon, 19 Aug 2013 12:27:53 +0300
> > > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > >
> > > >> On Mon, Aug 19, 2013 at 11:09:44AM +0200, Cornelia Huck wrote:
> > > >> > On Mon, 19 Aug 2013 12:08:35 +0300
> > > >> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > >> >
> > > >> > > On Mon, Aug 19, 2013 at 05:17:30PM +0930, Rusty Russell wrote:
> > > >> > > > "Michael S. Tsirkin" <mst@redhat.com> writes:
> > > >> > > > > On Mon, Aug 19, 2013 at 12:21:03PM +0930, Rusty Russell wrote:
> > > >> > > > >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > > >> > > > >> > During the last TC meeting, we discussed making virtio little endian.
> > > >> > > > >> > It was suggested that a feature bit can be used for this,
> > > >> > > > >> > but I now think I see two problems:
> > > >> > > > >> >
> > > >> > > > >> >
> > > >> > > > >> > 1. Features are optional,
> > > >> > > > >> > in that there's no way for device to communicate to
> > > >> > > > >> > guest that guest must ack a feature bit, and e.g. fail
> > > >> > > > >> > if guest does not ack.
> > > >> > > > >> >
> > > >> > > > >> > On the other hand, it seems likely
> > > >> > > > >> > that a hardware virtio device might want to *only* implement
> > > >> > > > >> > little endian format and not both big and little endian.
> > > >> > > > >> >
> > > >> > > > >> > In other words this would be something Paolo once called
> > > >> > > > >> > a "negative feature".
> > > >> > > > >> >
> > > >> > > > >> > 2. With virtio-pci we are running out of transport bits,
> > > >> > > > >> > and need a new config space layout to add extra feature bits.
> > > >
> > > > Just as point of data: For virtio-ccw we should be able to easily
> > > > extend feature bits.
> > >
> > > Yes, I think virtio-ccw and virtio-mmio could simply add a new feature
> > > bit to say "1.0 compliant". In 20 years time that might look a
> > > bit awkward, but that's probably OK unless there's an obvious
> > > out-of-band mechanism to use.
> >
> > A feature bit would probably be easier for virtio-ccw - at least,
> > there's a lot less to architecture on our side (no need for new channel
> > commands etc.)
>
> Do you have a revision field somewhere, in case host wants
> to disable old drivers?

No, we don't. If we wanted to reject old drivers, we'd probably need to
go with the command reject route.

On Tue, Aug 20, 2013 at 04:52:34PM +0200, Cornelia Huck wrote:
> On Tue, 20 Aug 2013 17:01:25 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Tue, Aug 20, 2013 at 09:03:59AM +0200, Cornelia Huck wrote:
> > > On Tue, 20 Aug 2013 14:04:10 +0930
> > > Rusty Russell <rusty@au1.ibm.com> wrote:
> > >
> > > > Cornelia Huck <cornelia.huck@de.ibm.com> writes:
> > > > > On Mon, 19 Aug 2013 12:27:53 +0300
> > > > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > >
> > > > >> On Mon, Aug 19, 2013 at 11:09:44AM +0200, Cornelia Huck wrote:
> > > > >> > On Mon, 19 Aug 2013 12:08:35 +0300
> > > > >> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > >> >
> > > > >> > > On Mon, Aug 19, 2013 at 05:17:30PM +0930, Rusty Russell wrote:
> > > > >> > > > "Michael S. Tsirkin" <mst@redhat.com> writes:
> > > > >> > > > > On Mon, Aug 19, 2013 at 12:21:03PM +0930, Rusty Russell wrote:
> > > > >> > > > >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > > > >> > > > >> > During the last TC meeting, we discussed making virtio little endian.
> > > > >> > > > >> > It was suggested that a feature bit can be used for this,
> > > > >> > > > >> > but I now think I see two problems:
> > > > >> > > > >> >
> > > > >> > > > >> >
> > > > >> > > > >> > 1. Features are optional,
> > > > >> > > > >> > in that there's no way for device to communicate to
> > > > >> > > > >> > guest that guest must ack a feature bit, and e.g. fail
> > > > >> > > > >> > if guest does not ack.
> > > > >> > > > >> >
> > > > >> > > > >> > On the other hand, it seems likely
> > > > >> > > > >> > that a hardware virtio device might want to *only* implement
> > > > >> > > > >> > little endian format and not both big and little endian.
> > > > >> > > > >> >
> > > > >> > > > >> > In other words this would be something Paolo once called
> > > > >> > > > >> > a "negative feature".
> > > > >> > > > >> >
> > > > >> > > > >> > 2. With virtio-pci we are running out of transport bits,
> > > > >> > > > >> > and need a new config space layout to add extra feature bits.
> > > > >
> > > > > Just as point of data: For virtio-ccw we should be able to easily
> > > > > extend feature bits.
> > > >
> > > > Yes, I think virtio-ccw and virtio-mmio could simply add a new feature
> > > > bit to say "1.0 compliant". In 20 years time that might look a
> > > > bit awkward, but that's probably OK unless there's an obvious
> > > > out-of-band mechanism to use.
> > >
> > > A feature bit would probably be easier for virtio-ccw - at least,
> > > there's a lot less to architecture on our side (no need for new channel
> > > commands etc.)
> >
> > Do you have a revision field somewhere, in case host wants
> > to disable old drivers?
>
> No, we don't. If we wanted to reject old drivers, we'd probably need to
> go with the command reject route.

OK, I think architecting something like this would be necessary then.
Can you suggest a clean way that device can currently reject
an existing driver?

--
MST

On Tue, 20 Aug 2013 18:12:05 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Tue, Aug 20, 2013 at 04:52:34PM +0200, Cornelia Huck wrote:
> > On Tue, 20 Aug 2013 17:01:25 +0300
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >
> > > On Tue, Aug 20, 2013 at 09:03:59AM +0200, Cornelia Huck wrote:
> > > > On Tue, 20 Aug 2013 14:04:10 +0930
> > > > Rusty Russell <rusty@au1.ibm.com> wrote:
> > > >
> > > > > Cornelia Huck <cornelia.huck@de.ibm.com> writes:
> > > > > > On Mon, 19 Aug 2013 12:27:53 +0300
> > > > > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > > >
> > > > > >> On Mon, Aug 19, 2013 at 11:09:44AM +0200, Cornelia Huck wrote:
> > > > > >> > On Mon, 19 Aug 2013 12:08:35 +0300
> > > > > >> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > > >> >
> > > > > >> > > On Mon, Aug 19, 2013 at 05:17:30PM +0930, Rusty Russell wrote:
> > > > > >> > > > "Michael S. Tsirkin" <mst@redhat.com> writes:
> > > > > >> > > > > On Mon, Aug 19, 2013 at 12:21:03PM +0930, Rusty Russell wrote:
> > > > > >> > > > >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > > > > >> > > > >> > During the last TC meeting, we discussed making virtio little endian.
> > > > > >> > > > >> > It was suggested that a feature bit can be used for this,
> > > > > >> > > > >> > but I now think I see two problems:
> > > > > >> > > > >> >
> > > > > >> > > > >> >
> > > > > >> > > > >> > 1. Features are optional,
> > > > > >> > > > >> > in that there's no way for device to communicate to
> > > > > >> > > > >> > guest that guest must ack a feature bit, and e.g. fail
> > > > > >> > > > >> > if guest does not ack.
> > > > > >> > > > >> >
> > > > > >> > > > >> > On the other hand, it seems likely
> > > > > >> > > > >> > that a hardware virtio device might want to *only* implement
> > > > > >> > > > >> > little endian format and not both big and little endian.
> > > > > >> > > > >> >
> > > > > >> > > > >> > In other words this would be something Paolo once called
> > > > > >> > > > >> > a "negative feature".
> > > > > >> > > > >> >
> > > > > >> > > > >> > 2. With virtio-pci we are running out of transport bits,
> > > > > >> > > > >> > and need a new config space layout to add extra feature bits.
> > > > > >
> > > > > > Just as point of data: For virtio-ccw we should be able to easily
> > > > > > extend feature bits.
> > > > >
> > > > > Yes, I think virtio-ccw and virtio-mmio could simply add a new feature
> > > > > bit to say "1.0 compliant". In 20 years time that might look a
> > > > > bit awkward, but that's probably OK unless there's an obvious
> > > > > out-of-band mechanism to use.
> > > >
> > > > A feature bit would probably be easier for virtio-ccw - at least,
> > > > there's a lot less to architecture on our side (no need for new channel
> > > > commands etc.)
> > >
> > > Do you have a revision field somewhere, in case host wants
> > > to disable old drivers?
> >
> > No, we don't. If we wanted to reject old drivers, we'd probably need to
> > go with the command reject route.
>
> OK, I think architecting something like this would be necessary then.
> Can you suggest a clean way that device can currently reject
> an existing driver?
>

The basic idea would be:
- Pick one (or more) of the existing channel commands and declare them
v1. Have an equivalent new channel command for v2 (possibly with a
different payload that might include version information).
- For a v1 device, handle the existing command as before. The v2
channel command gets unit check with command reject (which will happen
on old hosts automatically, since they do this for any command they
don't know).
- For a v2 device, accept the v2 command. The existing command will get
unit check with command reject (fencing off old drivers).

(I'll need to think about this a bit more.)

On Tue, Aug 20, 2013 at 02:04:10PM +0930, Rusty Russell wrote:
> Cornelia Huck <cornelia.huck@de.ibm.com> writes:
> > On Mon, 19 Aug 2013 12:27:53 +0300
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >
> >> On Mon, Aug 19, 2013 at 11:09:44AM +0200, Cornelia Huck wrote:
> >> > On Mon, 19 Aug 2013 12:08:35 +0300
> >> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >> >
> >> > > On Mon, Aug 19, 2013 at 05:17:30PM +0930, Rusty Russell wrote:
> >> > > > "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> > > > > On Mon, Aug 19, 2013 at 12:21:03PM +0930, Rusty Russell wrote:
> >> > > > >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> > > > >> > During the last TC meeting, we discussed making virtio little endian.
> >> > > > >> > It was suggested that a feature bit can be used for this,
> >> > > > >> > but I now think I see two problems:
> >> > > > >> >
> >> > > > >> >
> >> > > > >> > 1. Features are optional,
> >> > > > >> > in that there's no way for device to communicate to
> >> > > > >> > guest that guest must ack a feature bit, and e.g. fail
> >> > > > >> > if guest does not ack.
> >> > > > >> >
> >> > > > >> > On the other hand, it seems likely
> >> > > > >> > that a hardware virtio device might want to *only* implement
> >> > > > >> > little endian format and not both big and little endian.
> >> > > > >> >
> >> > > > >> > In other words this would be something Paolo once called
> >> > > > >> > a "negative feature".
> >> > > > >> >
> >> > > > >> > 2. With virtio-pci we are running out of transport bits,
> >> > > > >> > and need a new config space layout to add extra feature bits.
> >
> > Just as point of data: For virtio-ccw we should be able to easily
> > extend feature bits.
>
> Yes, I think virtio-ccw and virtio-mmio could simply add a new feature
> bit to say "1.0 compliant".

Again, this would be a "negative feature", won't it?

> In 20 years time that might look a
> bit awkward, but that's probably OK unless there's an obvious
> out-of-band mechanism to use.
>
> BTW "standards compliant" for proposals so far means:
> 1) config fields are LE.
> 2) device-specfic headers are LE (eg. virtio_net_hdr).
> 3) fields in the ring are LE.
>
> For virtio-ccw, the actual commands to manipulate the virtio devices and
> bus (ie. struct ccw1) would remain BE, because that's the nature of CCW.
>
> > Regarding little-endian rings, we could probably hack up some code to
> > measure the overhead that would give us.
>
> That would be great!
>
> Thanks,
> Rusty.

Il 20/08/2013 10:39, Michael S. Tsirkin ha scritto:
> On Tue, Aug 20, 2013 at 02:04:10PM +0930, Rusty Russell wrote:
>> Cornelia Huck <cornelia.huck@de.ibm.com> writes:
>>> On Mon, 19 Aug 2013 12:27:53 +0300
>>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>>
>>>> On Mon, Aug 19, 2013 at 11:09:44AM +0200, Cornelia Huck wrote:
>>>>> On Mon, 19 Aug 2013 12:08:35 +0300
>>>>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>>>>
>>>>>> On Mon, Aug 19, 2013 at 05:17:30PM +0930, Rusty Russell wrote:
>>>>>>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>>>>>>>> On Mon, Aug 19, 2013 at 12:21:03PM +0930, Rusty Russell wrote:
>>>>>>>>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>>>>>>>>>> During the last TC meeting, we discussed making virtio little endian.
>>>>>>>>>> It was suggested that a feature bit can be used for this,
>>>>>>>>>> but I now think I see two problems:
>>>>>>>>>>
>>>>>>>>>> 1. Features are optional,
>>>>>>>>>> in that there's no way for device to communicate to
>>>>>>>>>> guest that guest must ack a feature bit, and e.g. fail
>>>>>>>>>> if guest does not ack.
>>>>>>>>>>
>>>>>>>>>> On the other hand, it seems likely
>>>>>>>>>> that a hardware virtio device might want to *only* implement
>>>>>>>>>> little endian format and not both big and little endian.
>>>>>>>>>>
>>>>>>>>>> In other words this would be something Paolo once called
>>>>>>>>>> a "negative feature".
>>>>>>>>>>
>>>>>>>>>> 2. With virtio-pci we are running out of transport bits,
>>>>>>>>>> and need a new config space layout to add extra feature bits.
>>>
>>> Just as point of data: For virtio-ccw we should be able to easily
>>> extend feature bits.
>>
>> Yes, I think virtio-ccw and virtio-mmio could simply add a new feature
>> bit to say "1.0 compliant".
>
> Again, this would be a "negative feature", won't it?

Yes, unless the device model implements both pre-standard and
post-standard layouts.

Perhaps we can reserve the high 16 bits of the device ID for the
standard revision? Thus pre-standard layout would have device IDs like
0x0000xxxx (as it is now). Post-standard layout would be 0x0001xxxx.

Paolo

On Tue, 20 Aug 2013 10:47:37 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 20/08/2013 10:39, Michael S. Tsirkin ha scritto:
> > On Tue, Aug 20, 2013 at 02:04:10PM +0930, Rusty Russell wrote:
> >> Cornelia Huck <cornelia.huck@de.ibm.com> writes:
> >>> On Mon, 19 Aug 2013 12:27:53 +0300
> >>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >>>
> >>>> On Mon, Aug 19, 2013 at 11:09:44AM +0200, Cornelia Huck wrote:
> >>>>> On Mon, 19 Aug 2013 12:08:35 +0300
> >>>>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >>>>>
> >>>>>> On Mon, Aug 19, 2013 at 05:17:30PM +0930, Rusty Russell wrote:
> >>>>>>> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >>>>>>>> On Mon, Aug 19, 2013 at 12:21:03PM +0930, Rusty Russell wrote:
> >>>>>>>>> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >>>>>>>>>> During the last TC meeting, we discussed making virtio little endian.
> >>>>>>>>>> It was suggested that a feature bit can be used for this,
> >>>>>>>>>> but I now think I see two problems:
> >>>>>>>>>>
> >>>>>>>>>> 1. Features are optional,
> >>>>>>>>>> in that there's no way for device to communicate to
> >>>>>>>>>> guest that guest must ack a feature bit, and e.g. fail
> >>>>>>>>>> if guest does not ack.
> >>>>>>>>>>
> >>>>>>>>>> On the other hand, it seems likely
> >>>>>>>>>> that a hardware virtio device might want to *only* implement
> >>>>>>>>>> little endian format and not both big and little endian.
> >>>>>>>>>>
> >>>>>>>>>> In other words this would be something Paolo once called
> >>>>>>>>>> a "negative feature".
> >>>>>>>>>>
> >>>>>>>>>> 2. With virtio-pci we are running out of transport bits,
> >>>>>>>>>> and need a new config space layout to add extra feature bits.
> >>>
> >>> Just as point of data: For virtio-ccw we should be able to easily
> >>> extend feature bits.
> >>
> >> Yes, I think virtio-ccw and virtio-mmio could simply add a new feature
> >> bit to say "1.0 compliant".
> >
> > Again, this would be a "negative feature", won't it?
>
> Yes, unless the device model implements both pre-standard and
> post-standard layouts.
>
> Perhaps we can reserve the high 16 bits of the device ID for the
> standard revision? Thus pre-standard layout would have device IDs like
> 0x0000xxxx (as it is now). Post-standard layout would be 0x0001xxxx.

This device ID (in this format) is PCI-specific, isn't it?

On Tue, 2013-08-20 at 05:34 +0100, Rusty Russell wrote:
> Yes, I think virtio-ccw and virtio-mmio could simply add a new feature
> bit to say "1.0 compliant".

MMIO devices have "version" register. Currently it's 1. The OASIS spec
will bump it to 2 (main change will be the addressing scheme). So the
guest drivers may support both versions.

Pawel

On Tue, Aug 20, 2013 at 10:23:13AM +0100, Pawel Moll wrote:
> On Tue, 2013-08-20 at 05:34 +0100, Rusty Russell wrote:
> > Yes, I think virtio-ccw and virtio-mmio could simply add a new feature
> > bit to say "1.0 compliant".
>
> MMIO devices have "version" register. Currently it's 1. The OASIS spec
> will bump it to 2 (main change will be the addressing scheme). So the
> guest drivers may support both versions.
>
> Pawel

No, updating revision is a way to disable compatibility
in drivers.

So old drivers currently do:
if revision != 1 -> exit
new drivers will do
if revision < 1 || revision > 2 -> exit

Now a host can disable loading old drivers
by setting revision to 2.

However this must not be required by spec,
it must be possible to write a device that
implements both old and new spec.


I'll have to look at how mmio and ccw do
feature bits to figure out if a feature bit
will work for this.


--
MST

> However this must not be required by spec,
> it must be possible to write a device that
> implements both old and new spec.

I don't think so. It must be possible to write a driver that drivers
both the old and the new specs. I don't see any value in making the
device implementation more complicated. It provides a functionality of
its (or rather: of the host's environment author) choice. Then it's
guest's decision to use it or not.

Pawel

On Tue, 2013-08-20 at 15:03 +0100, Pawel Moll wrote:
> It must be possible to write a driver that drivers
> both the old and the new specs.

Should read: "... that drives devices complaint to both the old and the
new specs".

Pawel

Il 20/08/2013 16:09, Pawel Moll ha scritto:
> On Tue, 2013-08-20 at 15:03 +0100, Pawel Moll wrote:
>> It must be possible to write a driver that drivers
>> both the old and the new specs.
>
> Should read: "... that drives devices complaint to both the old and the
> new specs".

Both ways should be possible: write a driver for both old and new specs,
and write a device for both old and new specs.

In both cases, the differences between old and new specs should be
ideally hidden in the virtio layer, or more precisely in the virtio
transport layer. The virtio devices and drivers themselves should be
unaffected.

Paolo

On Tue, 2013-08-20 at 15:11 +0100, Paolo Bonzini wrote:
> Il 20/08/2013 16:09, Pawel Moll ha scritto:
> > On Tue, 2013-08-20 at 15:03 +0100, Pawel Moll wrote:
> >> It must be possible to write a driver that drivers
> >> both the old and the new specs.
> >
> > Should read: "... that drives devices complaint to both the old and the
> > new specs".
>
> Both ways should be possible: write a driver for both old and new specs,
> and write a device for both old and new specs.
>
> In both cases, the differences between old and new specs should be
> ideally hidden in the virtio layer, or more precisely in the virtio
> transport layer. The virtio devices and drivers themselves should be
> unaffected.

First of all, I disagree with the assumption that the OASIS spec must
not bring any changes to the environment. I actually consider it an
opportunity to fix what was mis-designed, even for a price of changes
needed in at both sides of the equation.

Now, as to the "hidden in the transport layer"... By "driver" I meant
"virtio_mmio.c", which - I believe - is the transport layer you are
referring to. And this is what I want to change indeed with the
assumption that virtio_mmio v2 does *not* have to be compatible with
virtio_mmio v1. And I will be strongly against any attempts of making it
(*the MMIO device*) more complicated.

Pawel

On Tue, Aug 20, 2013 at 03:25:17PM +0100, Pawel Moll wrote:
> On Tue, 2013-08-20 at 15:11 +0100, Paolo Bonzini wrote:
> > Il 20/08/2013 16:09, Pawel Moll ha scritto:
> > > On Tue, 2013-08-20 at 15:03 +0100, Pawel Moll wrote:
> > >> It must be possible to write a driver that drivers
> > >> both the old and the new specs.
> > >
> > > Should read: "... that drives devices complaint to both the old and the
> > > new specs".
> >
> > Both ways should be possible: write a driver for both old and new specs,
> > and write a device for both old and new specs.
> >
> > In both cases, the differences between old and new specs should be
> > ideally hidden in the virtio layer, or more precisely in the virtio
> > transport layer. The virtio devices and drivers themselves should be
> > unaffected.
>
> First of all, I disagree with the assumption that the OASIS spec must
> not bring any changes to the environment. I actually consider it an
> opportunity to fix what was mis-designed, even for a price of changes
> needed in at both sides of the equation.
>
> Now, as to the "hidden in the transport layer"... By "driver" I meant
> "virtio_mmio.c", which - I believe - is the transport layer you are
> referring to. And this is what I want to change indeed with the
> assumption that virtio_mmio v2 does *not* have to be compatible with
> virtio_mmio v1. And I will be strongly against any attempts of making it
> (*the MMIO device*) more complicated.
>
> Pawel
>

Confused.
You want to support old and new devices in the same
driver, don't you? Isn't that making it more complicated?


--
MST

On Tue, 2013-08-20 at 16:08 +0100, Michael S. Tsirkin wrote:
> Confused.
> You want to support old and new devices in the same
> driver, don't you? Isn't that making it more complicated?

It will make the driver more complicated, yes (slightly, as the *MMIO*
changes are not going to be too extensive). But to my knowledge only one
virtio_mmio driver has been ever implemented (the Linux one), where I
can do the right thing quite easily and this is a common pattern for the
kernel drivers (a single one can driver a family of almost, but not
quite, identical devices). I just don't want to make life harder for the
guys implementing the device component (I'll say this again: virtio_mmio
interface was conceived to be as simple as possible :-) in qemu, kvm
tools, proprietary modelling environments, <pick your choice here>.

Pawel

On Tue, Aug 20, 2013 at 04:14:09PM +0100, Pawel Moll wrote:
> On Tue, 2013-08-20 at 16:08 +0100, Michael S. Tsirkin wrote:
> > Confused.
> > You want to support old and new devices in the same
> > driver, don't you? Isn't that making it more complicated?
>
> It will make the driver more complicated, yes (slightly, as the *MMIO*
> changes are not going to be too extensive). But to my knowledge only one
> virtio_mmio driver has been ever implemented (the Linux one), where I
> can do the right thing quite easily and this is a common pattern for the
> kernel drivers (a single one can driver a family of almost, but not
> quite, identical devices). I just don't want to make life harder for the
> guys implementing the device component (I'll say this again: virtio_mmio
> interface was conceived to be as simple as possible :-) in qemu, kvm
> tools, proprietary modelling environments, <pick your choice here>.
>
> Pawel

Right.

I think spec should give devices the choice.

If they don't want old drivers to work,
they can bump version number.
If they want old drivers to work, don't
bump version number.

This implies that another mechanism (besides version number)
is used by driver and device to detect new spec.

Actually feature bit starts looking OK to me
as long as it doesn't mean "v2 driver required",
instead meaning "v2 driver supported".
We specify that v2 driver always acks this bit.

So if you (like Pawel) don't want to support old drivers,
just bump revision and set this bit.
Then if driver does not ack this bit -> it's a driver bug.

If you want to support old drivers - don't bump
revision, and set this feature bit.
You can look at the feature bit to distinguish old
drivers from new ones.

--
MST

"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Tue, Aug 20, 2013 at 04:14:09PM +0100, Pawel Moll wrote:
>> On Tue, 2013-08-20 at 16:08 +0100, Michael S. Tsirkin wrote:
>> > Confused.
>> > You want to support old and new devices in the same
>> > driver, don't you? Isn't that making it more complicated?
>>
>> It will make the driver more complicated, yes (slightly, as the *MMIO*
>> changes are not going to be too extensive). But to my knowledge only one
>> virtio_mmio driver has been ever implemented (the Linux one), where I
>> can do the right thing quite easily and this is a common pattern for the
>> kernel drivers (a single one can driver a family of almost, but not
>> quite, identical devices). I just don't want to make life harder for the
>> guys implementing the device component (I'll say this again: virtio_mmio
>> interface was conceived to be as simple as possible :-) in qemu, kvm
>> tools, proprietary modelling environments, <pick your choice here>.
>>
>> Pawel
>
> Right.
>
> I think spec should give devices the choice.
>
> If they don't want old drivers to work,
> they can bump version number.
> If they want old drivers to work, don't
> bump version number.
>
> This implies that another mechanism (besides version number)
> is used by driver and device to detect new spec.
>
> Actually feature bit starts looking OK to me
> as long as it doesn't mean "v2 driver required",
> instead meaning "v2 driver supported".
> We specify that v2 driver always acks this bit.

Yes, precisely.

> So if you (like Pawel) don't want to support old drivers,
> just bump revision and set this bit.
> Then if driver does not ack this bit -> it's a driver bug.
>
> If you want to support old drivers - don't bump
> revision, and set this feature bit.
> You can look at the feature bit to distinguish old
> drivers from new ones.

That scheme would have ultimate flexibility, but I'm not sure
that it's any better in practice than just giving up if you only
implement "standard" virtio and the driver doesn't ack the bit.

Cheers,
Rusty.

On Tue, Aug 20, 2013 at 03:09:20PM +0100, Pawel Moll wrote:
> On Tue, 2013-08-20 at 15:03 +0100, Pawel Moll wrote:
> > It must be possible to write a driver that drivers
> > both the old and the new specs.
>
> Should read: "... that drives devices complaint to both the old and the
> new specs".
>
> Pawel
>

This actually doesn't have a lot of value IMO.
If two drivers are required, that will do the job
equally well.

On Tue, 2013-08-20 at 16:05 +0100, Michael S. Tsirkin wrote:
> On Tue, Aug 20, 2013 at 03:09:20PM +0100, Pawel Moll wrote:
> > On Tue, 2013-08-20 at 15:03 +0100, Pawel Moll wrote:
> > > It must be possible to write a driver that drivers
> > > both the old and the new specs.
> >
> > Should read: "... that drives devices complaint to both the old and the
> > new specs".
>
> This actually doesn't have a lot of value IMO.
> If two drivers are required, that will do the job
> equally well.

Fair enough, I won't argue this statement. I'll just make sure that in
case of MMIO there will be one driver, capable of handling both v1 and
v2 devices :-)

Pawel

On Tue, Aug 20, 2013 at 03:03:04PM +0100, Pawel Moll wrote:
> > However this must not be required by spec,
> > it must be possible to write a device that
> > implements both old and new spec.
>
> I don't think so. It must be possible to write a driver that drivers
> both the old and the new specs. I don't see any value in making the
> device implementation more complicated. It provides a functionality of
> its (or rather: of the host's environment author) choice. Then it's
> guest's decision to use it or not.
>
> Pawel
>

That's exactly what I am saying.
A device should be able to implement both old
and new spec, or only the new spec.