diff mbox series

[v4,6/6] virtio: Add VIRTIO_F_IN_ORDER property definition

Message ID 20240710125522.4168043-7-jonah.palmer@oracle.com
State New
Headers show
Series virtio,vhost: Add VIRTIO_F_IN_ORDER support | expand

Commit Message

Jonah Palmer July 10, 2024, 12:55 p.m. UTC
Extend the virtio device property definitions to include the
VIRTIO_F_IN_ORDER feature.

The default state of this feature is disabled, allowing it to be
explicitly enabled where it's supported.

Acked-by: Eugenio Pérez <eperezma@redhat.com>
Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
---
 include/hw/virtio/virtio.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Michael S. Tsirkin July 20, 2024, 7:16 p.m. UTC | #1
On Wed, Jul 10, 2024 at 08:55:19AM -0400, Jonah Palmer wrote:
> Extend the virtio device property definitions to include the
> VIRTIO_F_IN_ORDER feature.
> 
> The default state of this feature is disabled, allowing it to be
> explicitly enabled where it's supported.
> 
> Acked-by: Eugenio Pérez <eperezma@redhat.com>
> Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>


Given release is close, it's likely wise.
However, I think we should flip the default in the future
release.

> ---
>  include/hw/virtio/virtio.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index fdc827f82e..d2a1938757 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -373,7 +373,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
>      DEFINE_PROP_BIT64("packed", _state, _field, \
>                        VIRTIO_F_RING_PACKED, false), \
>      DEFINE_PROP_BIT64("queue_reset", _state, _field, \
> -                      VIRTIO_F_RING_RESET, true)
> +                      VIRTIO_F_RING_RESET, true), \
> +    DEFINE_PROP_BIT64("in_order", _state, _field, \
> +                      VIRTIO_F_IN_ORDER, false)
>  
>  hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
>  bool virtio_queue_enabled_legacy(VirtIODevice *vdev, int n);
> -- 
> 2.43.5
Eugenio Pérez July 22, 2024, 11:11 a.m. UTC | #2
On Sat, Jul 20, 2024 at 9:16 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Jul 10, 2024 at 08:55:19AM -0400, Jonah Palmer wrote:
> > Extend the virtio device property definitions to include the
> > VIRTIO_F_IN_ORDER feature.
> >
> > The default state of this feature is disabled, allowing it to be
> > explicitly enabled where it's supported.
> >
> > Acked-by: Eugenio Pérez <eperezma@redhat.com>
> > Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
>
>
> Given release is close, it's likely wise.
> However, I think we should flip the default in the future
> release.
>

Should we post a new version with v9.2 tag enabling it?

> > ---
> >  include/hw/virtio/virtio.h | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > index fdc827f82e..d2a1938757 100644
> > --- a/include/hw/virtio/virtio.h
> > +++ b/include/hw/virtio/virtio.h
> > @@ -373,7 +373,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
> >      DEFINE_PROP_BIT64("packed", _state, _field, \
> >                        VIRTIO_F_RING_PACKED, false), \
> >      DEFINE_PROP_BIT64("queue_reset", _state, _field, \
> > -                      VIRTIO_F_RING_RESET, true)
> > +                      VIRTIO_F_RING_RESET, true), \
> > +    DEFINE_PROP_BIT64("in_order", _state, _field, \
> > +                      VIRTIO_F_IN_ORDER, false)
> >
> >  hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
> >  bool virtio_queue_enabled_legacy(VirtIODevice *vdev, int n);
> > --
> > 2.43.5
>
Eugenio Pérez July 22, 2024, 11:31 a.m. UTC | #3
On Mon, Jul 22, 2024 at 1:11 PM Eugenio Perez Martin
<eperezma@redhat.com> wrote:
>
> On Sat, Jul 20, 2024 at 9:16 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Wed, Jul 10, 2024 at 08:55:19AM -0400, Jonah Palmer wrote:
> > > Extend the virtio device property definitions to include the
> > > VIRTIO_F_IN_ORDER feature.
> > >
> > > The default state of this feature is disabled, allowing it to be
> > > explicitly enabled where it's supported.
> > >
> > > Acked-by: Eugenio Pérez <eperezma@redhat.com>
> > > Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
> >
> >
> > Given release is close, it's likely wise.
> > However, I think we should flip the default in the future
> > release.
> >
>
> Should we post a new version with v9.2 tag enabling it?
>

Sorry, actually I think this needs some more thought. Maybe in_order
hurts the performance of devices that are usually out of order, like
blk. Should we enable only for virtio-net and let each device code
decide?

> > > ---
> > >  include/hw/virtio/virtio.h | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > > index fdc827f82e..d2a1938757 100644
> > > --- a/include/hw/virtio/virtio.h
> > > +++ b/include/hw/virtio/virtio.h
> > > @@ -373,7 +373,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
> > >      DEFINE_PROP_BIT64("packed", _state, _field, \
> > >                        VIRTIO_F_RING_PACKED, false), \
> > >      DEFINE_PROP_BIT64("queue_reset", _state, _field, \
> > > -                      VIRTIO_F_RING_RESET, true)
> > > +                      VIRTIO_F_RING_RESET, true), \
> > > +    DEFINE_PROP_BIT64("in_order", _state, _field, \
> > > +                      VIRTIO_F_IN_ORDER, false)
> > >
> > >  hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
> > >  bool virtio_queue_enabled_legacy(VirtIODevice *vdev, int n);
> > > --
> > > 2.43.5
> >
diff mbox series

Patch

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index fdc827f82e..d2a1938757 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -373,7 +373,9 @@  typedef struct VirtIORNGConf VirtIORNGConf;
     DEFINE_PROP_BIT64("packed", _state, _field, \
                       VIRTIO_F_RING_PACKED, false), \
     DEFINE_PROP_BIT64("queue_reset", _state, _field, \
-                      VIRTIO_F_RING_RESET, true)
+                      VIRTIO_F_RING_RESET, true), \
+    DEFINE_PROP_BIT64("in_order", _state, _field, \
+                      VIRTIO_F_IN_ORDER, false)
 
 hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
 bool virtio_queue_enabled_legacy(VirtIODevice *vdev, int n);