Re: [PATCH 5/5] virtio: enable VIRTIO_F_RING_PACKED for all devices
On Fri, May 29, 2020 at 03:15:59PM +0800, Jason Wang wrote: > > On 2020/5/23 上午1:17, Stefan Hajnoczi wrote: > > The packed virtqueue layout was introduced in VIRTIO 1.1. It is a single > > ring instead of a split avail/used ring design. There are CPU cache > > advantages to this layout and it is also suited better to hardware > > implementation. > > > > The vhost-net backend has already supported packed virtqueues for some > > time. Performance benchmarks show that virtio-blk performance on NVMe > > drives is also improved. > > > > Go ahead and enable this feature for all VIRTIO devices. Keep it > > disabled for QEMU 5.0 and earlier machine types. > > > > Signed-off-by: Stefan Hajnoczi > > --- > > include/hw/virtio/virtio.h | 2 +- > > hw/core/machine.c | 18 +- > > 2 files changed, 18 insertions(+), 2 deletions(-) > > > > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > > index b69d517496..fd5b4a2044 100644 > > --- a/include/hw/virtio/virtio.h > > +++ b/include/hw/virtio/virtio.h > > @@ -292,7 +292,7 @@ typedef struct VirtIORNGConf VirtIORNGConf; > > DEFINE_PROP_BIT64("iommu_platform", _state, _field, \ > > VIRTIO_F_IOMMU_PLATFORM, false), \ > > DEFINE_PROP_BIT64("packed", _state, _field, \ > > - VIRTIO_F_RING_PACKED, false) > > + VIRTIO_F_RING_PACKED, true) > > hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n); > > bool virtio_queue_enabled(VirtIODevice *vdev, int n); > > diff --git a/hw/core/machine.c b/hw/core/machine.c > > index bb3a7b18b1..3598c3c825 100644 > > --- a/hw/core/machine.c > > +++ b/hw/core/machine.c > > @@ -28,7 +28,23 @@ > > #include "hw/mem/nvdimm.h" > > #include "migration/vmstate.h" > > -GlobalProperty hw_compat_5_0[] = {}; > > +GlobalProperty hw_compat_5_0[] = { > > +{ "vhost-user-blk", "packed", "off" }, > > +{ "vhost-user-fs-device", "packed", "off" }, > > +{ "vhost-vsock-device", "packed", "off" }, > > +{ "virtio-9p-device", "packed", "off" }, > > +{ "virtio-balloon-device", "packed", "off" }, > > +{ "virtio-blk-device", "packed", "off" }, > > +{ "virtio-crypto-device", "packed", "off" }, > > +{ "virtio-gpu-device", "packed", "off" }, > > +{ "virtio-input-device", "packed", "off" }, > > +{ "virtio-iommu-device", "packed", "off" }, > > +{ "virtio-net-device", "packed", "off" }, > > +{ "virtio-pmem", "packed", "off" }, > > +{ "virtio-rng-device", "packed", "off" }, > > +{ "virtio-scsi-common", "packed", "off" }, > > +{ "virtio-serial-device", "packed", "off" }, > > > Missing "vhost-user-gpu" here? Thanks, you're right. I'll see if virtio-gpu-base works. If not it will be necessary to add all the derived classes. The same is true for virtio-scsi-common, I'd better check it works correctly! Stefan signature.asc Description: PGP signature
Re: [PATCH 5/5] virtio: enable VIRTIO_F_RING_PACKED for all devices
On 2020/5/23 上午1:17, Stefan Hajnoczi wrote: The packed virtqueue layout was introduced in VIRTIO 1.1. It is a single ring instead of a split avail/used ring design. There are CPU cache advantages to this layout and it is also suited better to hardware implementation. The vhost-net backend has already supported packed virtqueues for some time. Performance benchmarks show that virtio-blk performance on NVMe drives is also improved. Go ahead and enable this feature for all VIRTIO devices. Keep it disabled for QEMU 5.0 and earlier machine types. Signed-off-by: Stefan Hajnoczi --- include/hw/virtio/virtio.h | 2 +- hw/core/machine.c | 18 +- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index b69d517496..fd5b4a2044 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -292,7 +292,7 @@ typedef struct VirtIORNGConf VirtIORNGConf; DEFINE_PROP_BIT64("iommu_platform", _state, _field, \ VIRTIO_F_IOMMU_PLATFORM, false), \ DEFINE_PROP_BIT64("packed", _state, _field, \ - VIRTIO_F_RING_PACKED, false) + VIRTIO_F_RING_PACKED, true) hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n); bool virtio_queue_enabled(VirtIODevice *vdev, int n); diff --git a/hw/core/machine.c b/hw/core/machine.c index bb3a7b18b1..3598c3c825 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -28,7 +28,23 @@ #include "hw/mem/nvdimm.h" #include "migration/vmstate.h" -GlobalProperty hw_compat_5_0[] = {}; +GlobalProperty hw_compat_5_0[] = { +{ "vhost-user-blk", "packed", "off" }, +{ "vhost-user-fs-device", "packed", "off" }, +{ "vhost-vsock-device", "packed", "off" }, +{ "virtio-9p-device", "packed", "off" }, +{ "virtio-balloon-device", "packed", "off" }, +{ "virtio-blk-device", "packed", "off" }, +{ "virtio-crypto-device", "packed", "off" }, +{ "virtio-gpu-device", "packed", "off" }, +{ "virtio-input-device", "packed", "off" }, +{ "virtio-iommu-device", "packed", "off" }, +{ "virtio-net-device", "packed", "off" }, +{ "virtio-pmem", "packed", "off" }, +{ "virtio-rng-device", "packed", "off" }, +{ "virtio-scsi-common", "packed", "off" }, +{ "virtio-serial-device", "packed", "off" }, Missing "vhost-user-gpu" here? I try to do something like this in the past but give up since I end up with similar list. It would be better to consider something more smart, probably need some refactor for a common parent class. Thanks +}; const size_t hw_compat_5_0_len = G_N_ELEMENTS(hw_compat_5_0); GlobalProperty hw_compat_4_2[] = {
Re: [PATCH 5/5] virtio: enable VIRTIO_F_RING_PACKED for all devices
* Stefan Hajnoczi (stefa...@redhat.com) wrote: > The packed virtqueue layout was introduced in VIRTIO 1.1. It is a single > ring instead of a split avail/used ring design. There are CPU cache > advantages to this layout and it is also suited better to hardware > implementation. > > The vhost-net backend has already supported packed virtqueues for some > time. Performance benchmarks show that virtio-blk performance on NVMe > drives is also improved. > > Go ahead and enable this feature for all VIRTIO devices. Keep it > disabled for QEMU 5.0 and earlier machine types. > > Signed-off-by: Stefan Hajnoczi Reviewed-by: Dr. David Alan Gilbert > --- > include/hw/virtio/virtio.h | 2 +- > hw/core/machine.c | 18 +- > 2 files changed, 18 insertions(+), 2 deletions(-) > > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > index b69d517496..fd5b4a2044 100644 > --- a/include/hw/virtio/virtio.h > +++ b/include/hw/virtio/virtio.h > @@ -292,7 +292,7 @@ typedef struct VirtIORNGConf VirtIORNGConf; > DEFINE_PROP_BIT64("iommu_platform", _state, _field, \ >VIRTIO_F_IOMMU_PLATFORM, false), \ > DEFINE_PROP_BIT64("packed", _state, _field, \ > - VIRTIO_F_RING_PACKED, false) > + VIRTIO_F_RING_PACKED, true) > > hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n); > bool virtio_queue_enabled(VirtIODevice *vdev, int n); > diff --git a/hw/core/machine.c b/hw/core/machine.c > index bb3a7b18b1..3598c3c825 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -28,7 +28,23 @@ > #include "hw/mem/nvdimm.h" > #include "migration/vmstate.h" > > -GlobalProperty hw_compat_5_0[] = {}; > +GlobalProperty hw_compat_5_0[] = { > +{ "vhost-user-blk", "packed", "off" }, > +{ "vhost-user-fs-device", "packed", "off" }, > +{ "vhost-vsock-device", "packed", "off" }, > +{ "virtio-9p-device", "packed", "off" }, > +{ "virtio-balloon-device", "packed", "off" }, > +{ "virtio-blk-device", "packed", "off" }, > +{ "virtio-crypto-device", "packed", "off" }, > +{ "virtio-gpu-device", "packed", "off" }, > +{ "virtio-input-device", "packed", "off" }, > +{ "virtio-iommu-device", "packed", "off" }, > +{ "virtio-net-device", "packed", "off" }, > +{ "virtio-pmem", "packed", "off" }, > +{ "virtio-rng-device", "packed", "off" }, > +{ "virtio-scsi-common", "packed", "off" }, > +{ "virtio-serial-device", "packed", "off" }, > +}; > const size_t hw_compat_5_0_len = G_N_ELEMENTS(hw_compat_5_0); > > GlobalProperty hw_compat_4_2[] = { > -- > 2.25.3 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
[PATCH 5/5] virtio: enable VIRTIO_F_RING_PACKED for all devices
The packed virtqueue layout was introduced in VIRTIO 1.1. It is a single ring instead of a split avail/used ring design. There are CPU cache advantages to this layout and it is also suited better to hardware implementation. The vhost-net backend has already supported packed virtqueues for some time. Performance benchmarks show that virtio-blk performance on NVMe drives is also improved. Go ahead and enable this feature for all VIRTIO devices. Keep it disabled for QEMU 5.0 and earlier machine types. Signed-off-by: Stefan Hajnoczi --- include/hw/virtio/virtio.h | 2 +- hw/core/machine.c | 18 +- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index b69d517496..fd5b4a2044 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -292,7 +292,7 @@ typedef struct VirtIORNGConf VirtIORNGConf; DEFINE_PROP_BIT64("iommu_platform", _state, _field, \ VIRTIO_F_IOMMU_PLATFORM, false), \ DEFINE_PROP_BIT64("packed", _state, _field, \ - VIRTIO_F_RING_PACKED, false) + VIRTIO_F_RING_PACKED, true) hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n); bool virtio_queue_enabled(VirtIODevice *vdev, int n); diff --git a/hw/core/machine.c b/hw/core/machine.c index bb3a7b18b1..3598c3c825 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -28,7 +28,23 @@ #include "hw/mem/nvdimm.h" #include "migration/vmstate.h" -GlobalProperty hw_compat_5_0[] = {}; +GlobalProperty hw_compat_5_0[] = { +{ "vhost-user-blk", "packed", "off" }, +{ "vhost-user-fs-device", "packed", "off" }, +{ "vhost-vsock-device", "packed", "off" }, +{ "virtio-9p-device", "packed", "off" }, +{ "virtio-balloon-device", "packed", "off" }, +{ "virtio-blk-device", "packed", "off" }, +{ "virtio-crypto-device", "packed", "off" }, +{ "virtio-gpu-device", "packed", "off" }, +{ "virtio-input-device", "packed", "off" }, +{ "virtio-iommu-device", "packed", "off" }, +{ "virtio-net-device", "packed", "off" }, +{ "virtio-pmem", "packed", "off" }, +{ "virtio-rng-device", "packed", "off" }, +{ "virtio-scsi-common", "packed", "off" }, +{ "virtio-serial-device", "packed", "off" }, +}; const size_t hw_compat_5_0_len = G_N_ELEMENTS(hw_compat_5_0); GlobalProperty hw_compat_4_2[] = { -- 2.25.3