* [Qemu-devel] [PATCH] hw/virtio: enable common virtio feature for mmio device @ 2014-06-04 2:05 Ming Lei 2014-06-09 8:00 ` Ming Lei ` (3 more replies) 0 siblings, 4 replies; 11+ messages in thread From: Ming Lei @ 2014-06-04 2:05 UTC (permalink / raw) To: Peter Maydell, qemu-devel; +Cc: Ming Lei Both 'indirect_desc' and 'event_idx' are bus independent features, and they should be enabled for mmio devices too. On arm64 quad core VM(qemu-kvm), the patch can increase block I/O performance a lot with latest linux tree: - without the patch: 14K IOPS - with the patch: 34K IOPS fio script: [global] direct=1 bsrange=4k-4k timeout=10 numjobs=4 ioengine=libaio iodepth=64 filename=/dev/vdc group_reporting=1 [f1] rw=randread Cc: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Ming Lei <ming.lei@canonical.com> --- hw/virtio/virtio-mmio.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c index 8829eb0..18c6e5b 100644 --- a/hw/virtio/virtio-mmio.c +++ b/hw/virtio/virtio-mmio.c @@ -369,10 +369,16 @@ static void virtio_mmio_realizefn(DeviceState *d, Error **errp) sysbus_init_mmio(sbd, &proxy->iomem); } +static Property virtio_mmio_properties[] = { + DEFINE_VIRTIO_COMMON_FEATURES(VirtIOMMIOProxy, host_features), + DEFINE_PROP_END_OF_LIST(), +}; + static void virtio_mmio_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); + dc->props = virtio_mmio_properties; dc->realize = virtio_mmio_realizefn; dc->reset = virtio_mmio_reset; set_bit(DEVICE_CATEGORY_MISC, dc->categories); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/virtio: enable common virtio feature for mmio device 2014-06-04 2:05 [Qemu-devel] [PATCH] hw/virtio: enable common virtio feature for mmio device Ming Lei @ 2014-06-09 8:00 ` Ming Lei 2014-06-16 7:54 ` Paolo Bonzini 2014-06-30 9:59 ` Ming Lei ` (2 subsequent siblings) 3 siblings, 1 reply; 11+ messages in thread From: Ming Lei @ 2014-06-09 8:00 UTC (permalink / raw) To: Peter Maydell, qemu-devel; +Cc: Ming Lei On Wed, Jun 4, 2014 at 10:05 AM, Ming Lei <ming.lei@canonical.com> wrote: > Both 'indirect_desc' and 'event_idx' are bus independent features, > and they should be enabled for mmio devices too. > > On arm64 quad core VM(qemu-kvm), the patch can increase block I/O > performance a lot with latest linux tree: > - without the patch: 14K IOPS > - with the patch: 34K IOPS > > fio script: > [global] > direct=1 > bsrange=4k-4k > timeout=10 > numjobs=4 > ioengine=libaio > iodepth=64 > > filename=/dev/vdc > group_reporting=1 > > [f1] > rw=randread > > Cc: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Ming Lei <ming.lei@canonical.com> > --- > hw/virtio/virtio-mmio.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c > index 8829eb0..18c6e5b 100644 > --- a/hw/virtio/virtio-mmio.c > +++ b/hw/virtio/virtio-mmio.c > @@ -369,10 +369,16 @@ static void virtio_mmio_realizefn(DeviceState *d, Error **errp) > sysbus_init_mmio(sbd, &proxy->iomem); > } > > +static Property virtio_mmio_properties[] = { > + DEFINE_VIRTIO_COMMON_FEATURES(VirtIOMMIOProxy, host_features), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > static void virtio_mmio_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > > + dc->props = virtio_mmio_properties; > dc->realize = virtio_mmio_realizefn; > dc->reset = virtio_mmio_reset; > set_bit(DEVICE_CATEGORY_MISC, dc->categories); > -- > 1.7.9.5 > Hi Peter, Gentle ping... Thanks, -- Ming Lei ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/virtio: enable common virtio feature for mmio device 2014-06-09 8:00 ` Ming Lei @ 2014-06-16 7:54 ` Paolo Bonzini 2014-06-16 9:26 ` Ming Lei 0 siblings, 1 reply; 11+ messages in thread From: Paolo Bonzini @ 2014-06-16 7:54 UTC (permalink / raw) To: Ming Lei, Peter Maydell, qemu-devel Il 09/06/2014 10:00, Ming Lei ha scritto: > On Wed, Jun 4, 2014 at 10:05 AM, Ming Lei <ming.lei@canonical.com> wrote: >> Both 'indirect_desc' and 'event_idx' are bus independent features, >> and they should be enabled for mmio devices too. >> >> On arm64 quad core VM(qemu-kvm), the patch can increase block I/O >> performance a lot with latest linux tree: >> - without the patch: 14K IOPS >> - with the patch: 34K IOPS >> >> fio script: >> [global] >> direct=1 >> bsrange=4k-4k >> timeout=10 >> numjobs=4 >> ioengine=libaio >> iodepth=64 >> >> filename=/dev/vdc >> group_reporting=1 >> >> [f1] >> rw=randread >> >> Cc: Peter Maydell <peter.maydell@linaro.org> >> Signed-off-by: Ming Lei <ming.lei@canonical.com> >> --- >> hw/virtio/virtio-mmio.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c >> index 8829eb0..18c6e5b 100644 >> --- a/hw/virtio/virtio-mmio.c >> +++ b/hw/virtio/virtio-mmio.c >> @@ -369,10 +369,16 @@ static void virtio_mmio_realizefn(DeviceState *d, Error **errp) >> sysbus_init_mmio(sbd, &proxy->iomem); >> } >> >> +static Property virtio_mmio_properties[] = { >> + DEFINE_VIRTIO_COMMON_FEATURES(VirtIOMMIOProxy, host_features), >> + DEFINE_PROP_END_OF_LIST(), >> +}; >> + >> static void virtio_mmio_class_init(ObjectClass *klass, void *data) >> { >> DeviceClass *dc = DEVICE_CLASS(klass); >> >> + dc->props = virtio_mmio_properties; >> dc->realize = virtio_mmio_realizefn; >> dc->reset = virtio_mmio_reset; >> set_bit(DEVICE_CATEGORY_MISC, dc->categories); >> -- >> 1.7.9.5 Looks good. Can you look into moving DEFINE_VIRTIO_COMMON_FEATURES from all virtio pci devices to TYPE_VIRTIO_PCI, too? Thanks, Paolo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/virtio: enable common virtio feature for mmio device 2014-06-16 7:54 ` Paolo Bonzini @ 2014-06-16 9:26 ` Ming Lei 2014-06-16 9:50 ` Paolo Bonzini 2014-06-30 10:09 ` Michael S. Tsirkin 0 siblings, 2 replies; 11+ messages in thread From: Ming Lei @ 2014-06-16 9:26 UTC (permalink / raw) To: Paolo Bonzini; +Cc: Peter Maydell, qemu-devel [-- Attachment #1: Type: text/plain, Size: 2153 bytes --] On Mon, Jun 16, 2014 at 3:54 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 09/06/2014 10:00, Ming Lei ha scritto: > >> On Wed, Jun 4, 2014 at 10:05 AM, Ming Lei <ming.lei@canonical.com> wrote: >>> >>> Both 'indirect_desc' and 'event_idx' are bus independent features, >>> and they should be enabled for mmio devices too. >>> >>> On arm64 quad core VM(qemu-kvm), the patch can increase block I/O >>> performance a lot with latest linux tree: >>> - without the patch: 14K IOPS >>> - with the patch: 34K IOPS >>> >>> fio script: >>> [global] >>> direct=1 >>> bsrange=4k-4k >>> timeout=10 >>> numjobs=4 >>> ioengine=libaio >>> iodepth=64 >>> >>> filename=/dev/vdc >>> group_reporting=1 >>> >>> [f1] >>> rw=randread >>> >>> Cc: Peter Maydell <peter.maydell@linaro.org> >>> Signed-off-by: Ming Lei <ming.lei@canonical.com> >>> --- >>> hw/virtio/virtio-mmio.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c >>> index 8829eb0..18c6e5b 100644 >>> --- a/hw/virtio/virtio-mmio.c >>> +++ b/hw/virtio/virtio-mmio.c >>> @@ -369,10 +369,16 @@ static void virtio_mmio_realizefn(DeviceState *d, >>> Error **errp) >>> sysbus_init_mmio(sbd, &proxy->iomem); >>> } >>> >>> +static Property virtio_mmio_properties[] = { >>> + DEFINE_VIRTIO_COMMON_FEATURES(VirtIOMMIOProxy, host_features), >>> + DEFINE_PROP_END_OF_LIST(), >>> +}; >>> + >>> static void virtio_mmio_class_init(ObjectClass *klass, void *data) >>> { >>> DeviceClass *dc = DEVICE_CLASS(klass); >>> >>> + dc->props = virtio_mmio_properties; >>> dc->realize = virtio_mmio_realizefn; >>> dc->reset = virtio_mmio_reset; >>> set_bit(DEVICE_CATEGORY_MISC, dc->categories); >>> -- >>> 1.7.9.5 > > > Looks good. Paolo, thanks for your review. > Can you look into moving DEFINE_VIRTIO_COMMON_FEATURES > from all virtio pci devices to TYPE_VIRTIO_PCI, too? OK, that looks a good cleanup, how about the attached patch? If it is OK, I will prepare a formal one for submitting. Thanks, -- Ming Lei [-- Attachment #2: handle_virtio_common_features_per_bus.patch --] [-- Type: text/x-patch, Size: 4381 bytes --] diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index b437f19..af2e1c3 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -917,7 +917,6 @@ static Property virtio_9p_pci_properties[] = { DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2), - DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), DEFINE_VIRTIO_9P_PROPERTIES(V9fsPCIState, vdev.fsconf), DEFINE_PROP_END_OF_LIST(), }; @@ -1038,11 +1037,17 @@ static void virtio_pci_reset(DeviceState *qdev) proxy->flags &= ~VIRTIO_PCI_FLAG_BUS_MASTER_BUG; } +static Property virtio_pci_properties[] = { + DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), + DEFINE_PROP_END_OF_LIST(), +}; + static void virtio_pci_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); + dc->props = virtio_pci_properties; k->init = virtio_pci_init; k->exit = virtio_pci_exit; k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET; @@ -1071,7 +1076,6 @@ static Property virtio_blk_pci_properties[] = { DEFINE_PROP_BIT("x-data-plane", VirtIOBlkPCI, blk.data_plane, 0, false), DEFINE_PROP_UINT32("num_queues", VirtIOBlkPCI, blk.num_queues, 1), #endif - DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features), DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkPCI, blk), DEFINE_PROP_END_OF_LIST(), }; @@ -1195,7 +1199,6 @@ static const TypeInfo virtio_scsi_pci_info = { static Property vhost_scsi_pci_properties[] = { DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, DEV_NVECTORS_UNSPECIFIED), - DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), DEFINE_VHOST_SCSI_PROPERTIES(VHostSCSIPCI, vdev.parent_obj.conf), DEFINE_PROP_END_OF_LIST(), }; @@ -1276,7 +1279,6 @@ static void balloon_pci_stats_set_poll_interval(Object *obj, struct Visitor *v, } static Property virtio_balloon_pci_properties[] = { - DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0), DEFINE_PROP_END_OF_LIST(), }; @@ -1379,7 +1381,6 @@ static Property virtio_serial_pci_properties[] = { VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2), DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0), - DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), DEFINE_VIRTIO_SERIAL_PROPERTIES(VirtIOSerialPCI, vdev.serial), DEFINE_PROP_END_OF_LIST(), }; @@ -1475,7 +1476,6 @@ static const TypeInfo virtio_net_pci_info = { /* virtio-rng-pci */ static Property virtio_rng_pci_properties[] = { - DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), DEFINE_VIRTIO_RNG_PROPERTIES(VirtIORngPCI, vdev.conf), DEFINE_PROP_END_OF_LIST(), }; diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h index df60f16..bf65b26 100644 --- a/include/hw/virtio/virtio-net.h +++ b/include/hw/virtio/virtio-net.h @@ -242,7 +242,6 @@ struct virtio_net_ctrl_mq { #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET 0 #define DEFINE_VIRTIO_NET_FEATURES(_state, _field) \ - DEFINE_VIRTIO_COMMON_FEATURES(_state, _field), \ DEFINE_PROP_BIT("any_layout", _state, _field, VIRTIO_F_ANY_LAYOUT, true), \ DEFINE_PROP_BIT("csum", _state, _field, VIRTIO_NET_F_CSUM, true), \ DEFINE_PROP_BIT("guest_csum", _state, _field, VIRTIO_NET_F_GUEST_CSUM, true), \ diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h index 42b1024..367afc6 100644 --- a/include/hw/virtio/virtio-scsi.h +++ b/include/hw/virtio/virtio-scsi.h @@ -180,7 +180,6 @@ typedef struct { DEFINE_PROP_UINT32("cmd_per_lun", _state, _conf_field.cmd_per_lun, 128) #define DEFINE_VIRTIO_SCSI_FEATURES(_state, _feature_field) \ - DEFINE_VIRTIO_COMMON_FEATURES(_state, _feature_field), \ DEFINE_PROP_BIT("hotplug", _state, _feature_field, VIRTIO_SCSI_F_HOTPLUG, \ true), \ DEFINE_PROP_BIT("param_change", _state, _feature_field, \ ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/virtio: enable common virtio feature for mmio device 2014-06-16 9:26 ` Ming Lei @ 2014-06-16 9:50 ` Paolo Bonzini 2014-06-30 10:09 ` Michael S. Tsirkin 1 sibling, 0 replies; 11+ messages in thread From: Paolo Bonzini @ 2014-06-16 9:50 UTC (permalink / raw) To: Ming Lei; +Cc: Peter Maydell, qemu-devel Il 16/06/2014 11:26, Ming Lei ha scritto: > On Mon, Jun 16, 2014 at 3:54 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: >> Il 09/06/2014 10:00, Ming Lei ha scritto: >> >>> On Wed, Jun 4, 2014 at 10:05 AM, Ming Lei <ming.lei@canonical.com> wrote: >>>> >>>> Both 'indirect_desc' and 'event_idx' are bus independent features, >>>> and they should be enabled for mmio devices too. >>>> >>>> On arm64 quad core VM(qemu-kvm), the patch can increase block I/O >>>> performance a lot with latest linux tree: >>>> - without the patch: 14K IOPS >>>> - with the patch: 34K IOPS >>>> >>>> fio script: >>>> [global] >>>> direct=1 >>>> bsrange=4k-4k >>>> timeout=10 >>>> numjobs=4 >>>> ioengine=libaio >>>> iodepth=64 >>>> >>>> filename=/dev/vdc >>>> group_reporting=1 >>>> >>>> [f1] >>>> rw=randread >>>> >>>> Cc: Peter Maydell <peter.maydell@linaro.org> >>>> Signed-off-by: Ming Lei <ming.lei@canonical.com> >>>> --- >>>> hw/virtio/virtio-mmio.c | 6 ++++++ >>>> 1 file changed, 6 insertions(+) >>>> >>>> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c >>>> index 8829eb0..18c6e5b 100644 >>>> --- a/hw/virtio/virtio-mmio.c >>>> +++ b/hw/virtio/virtio-mmio.c >>>> @@ -369,10 +369,16 @@ static void virtio_mmio_realizefn(DeviceState *d, >>>> Error **errp) >>>> sysbus_init_mmio(sbd, &proxy->iomem); >>>> } >>>> >>>> +static Property virtio_mmio_properties[] = { >>>> + DEFINE_VIRTIO_COMMON_FEATURES(VirtIOMMIOProxy, host_features), >>>> + DEFINE_PROP_END_OF_LIST(), >>>> +}; >>>> + >>>> static void virtio_mmio_class_init(ObjectClass *klass, void *data) >>>> { >>>> DeviceClass *dc = DEVICE_CLASS(klass); >>>> >>>> + dc->props = virtio_mmio_properties; >>>> dc->realize = virtio_mmio_realizefn; >>>> dc->reset = virtio_mmio_reset; >>>> set_bit(DEVICE_CATEGORY_MISC, dc->categories); >>>> -- >>>> 1.7.9.5 >> >> >> Looks good. > > Paolo, thanks for your review. > >> Can you look into moving DEFINE_VIRTIO_COMMON_FEATURES >> from all virtio pci devices to TYPE_VIRTIO_PCI, too? > > OK, that looks a good cleanup, how about the attached patch? Since you touched DEFINE_VIRTIO_SCSI_FEATURES and DEFINE_VIRTIO_NET_FEATURES, you need to do the same change in hw/s390x/s390-virtio-bus.c and hw/s390x/virtio-ccw.c as well. Otherwise, the PCI parts of the patch are okay. Thanks, Paolo > If it is OK, I will prepare a formal one for submitting. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/virtio: enable common virtio feature for mmio device 2014-06-16 9:26 ` Ming Lei 2014-06-16 9:50 ` Paolo Bonzini @ 2014-06-30 10:09 ` Michael S. Tsirkin 2014-06-30 10:13 ` Ming Lei 1 sibling, 1 reply; 11+ messages in thread From: Michael S. Tsirkin @ 2014-06-30 10:09 UTC (permalink / raw) To: Ming Lei; +Cc: Paolo Bonzini, qemu-devel, Peter Maydell On Mon, Jun 16, 2014 at 05:26:33PM +0800, Ming Lei wrote: > On Mon, Jun 16, 2014 at 3:54 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: > > Il 09/06/2014 10:00, Ming Lei ha scritto: > > > >> On Wed, Jun 4, 2014 at 10:05 AM, Ming Lei <ming.lei@canonical.com> wrote: > >>> > >>> Both 'indirect_desc' and 'event_idx' are bus independent features, > >>> and they should be enabled for mmio devices too. > >>> > >>> On arm64 quad core VM(qemu-kvm), the patch can increase block I/O > >>> performance a lot with latest linux tree: > >>> - without the patch: 14K IOPS > >>> - with the patch: 34K IOPS > >>> > >>> fio script: > >>> [global] > >>> direct=1 > >>> bsrange=4k-4k > >>> timeout=10 > >>> numjobs=4 > >>> ioengine=libaio > >>> iodepth=64 > >>> > >>> filename=/dev/vdc > >>> group_reporting=1 > >>> > >>> [f1] > >>> rw=randread > >>> > >>> Cc: Peter Maydell <peter.maydell@linaro.org> > >>> Signed-off-by: Ming Lei <ming.lei@canonical.com> > >>> --- > >>> hw/virtio/virtio-mmio.c | 6 ++++++ > >>> 1 file changed, 6 insertions(+) > >>> > >>> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c > >>> index 8829eb0..18c6e5b 100644 > >>> --- a/hw/virtio/virtio-mmio.c > >>> +++ b/hw/virtio/virtio-mmio.c > >>> @@ -369,10 +369,16 @@ static void virtio_mmio_realizefn(DeviceState *d, > >>> Error **errp) > >>> sysbus_init_mmio(sbd, &proxy->iomem); > >>> } > >>> > >>> +static Property virtio_mmio_properties[] = { > >>> + DEFINE_VIRTIO_COMMON_FEATURES(VirtIOMMIOProxy, host_features), > >>> + DEFINE_PROP_END_OF_LIST(), > >>> +}; > >>> + > >>> static void virtio_mmio_class_init(ObjectClass *klass, void *data) > >>> { > >>> DeviceClass *dc = DEVICE_CLASS(klass); > >>> > >>> + dc->props = virtio_mmio_properties; > >>> dc->realize = virtio_mmio_realizefn; > >>> dc->reset = virtio_mmio_reset; > >>> set_bit(DEVICE_CATEGORY_MISC, dc->categories); > >>> -- > >>> 1.7.9.5 > > > > > > Looks good. > > Paolo, thanks for your review. > > > Can you look into moving DEFINE_VIRTIO_COMMON_FEATURES > > from all virtio pci devices to TYPE_VIRTIO_PCI, too? > > OK, that looks a good cleanup, how about the attached patch? > If it is OK, I will prepare a formal one for submitting. > > > Thanks, > -- > Ming Lei I applied the original patch for now. Pls address Paolo's comments and resubmit this one. > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index b437f19..af2e1c3 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -917,7 +917,6 @@ static Property virtio_9p_pci_properties[] = { > DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, > VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), > DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2), > - DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), > DEFINE_VIRTIO_9P_PROPERTIES(V9fsPCIState, vdev.fsconf), > DEFINE_PROP_END_OF_LIST(), > }; > @@ -1038,11 +1037,17 @@ static void virtio_pci_reset(DeviceState *qdev) > proxy->flags &= ~VIRTIO_PCI_FLAG_BUS_MASTER_BUG; > } > > +static Property virtio_pci_properties[] = { > + DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > static void virtio_pci_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > + dc->props = virtio_pci_properties; > k->init = virtio_pci_init; > k->exit = virtio_pci_exit; > k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET; > @@ -1071,7 +1076,6 @@ static Property virtio_blk_pci_properties[] = { > DEFINE_PROP_BIT("x-data-plane", VirtIOBlkPCI, blk.data_plane, 0, false), > DEFINE_PROP_UINT32("num_queues", VirtIOBlkPCI, blk.num_queues, 1), > #endif > - DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features), > DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkPCI, blk), > DEFINE_PROP_END_OF_LIST(), > }; > @@ -1195,7 +1199,6 @@ static const TypeInfo virtio_scsi_pci_info = { > static Property vhost_scsi_pci_properties[] = { > DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, > DEV_NVECTORS_UNSPECIFIED), > - DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), > DEFINE_VHOST_SCSI_PROPERTIES(VHostSCSIPCI, vdev.parent_obj.conf), > DEFINE_PROP_END_OF_LIST(), > }; > @@ -1276,7 +1279,6 @@ static void balloon_pci_stats_set_poll_interval(Object *obj, struct Visitor *v, > } > > static Property virtio_balloon_pci_properties[] = { > - DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), > DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0), > DEFINE_PROP_END_OF_LIST(), > }; > @@ -1379,7 +1381,6 @@ static Property virtio_serial_pci_properties[] = { > VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), > DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2), > DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0), > - DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), > DEFINE_VIRTIO_SERIAL_PROPERTIES(VirtIOSerialPCI, vdev.serial), > DEFINE_PROP_END_OF_LIST(), > }; > @@ -1475,7 +1476,6 @@ static const TypeInfo virtio_net_pci_info = { > /* virtio-rng-pci */ > > static Property virtio_rng_pci_properties[] = { > - DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), > DEFINE_VIRTIO_RNG_PROPERTIES(VirtIORngPCI, vdev.conf), > DEFINE_PROP_END_OF_LIST(), > }; > diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h > index df60f16..bf65b26 100644 > --- a/include/hw/virtio/virtio-net.h > +++ b/include/hw/virtio/virtio-net.h > @@ -242,7 +242,6 @@ struct virtio_net_ctrl_mq { > #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET 0 > > #define DEFINE_VIRTIO_NET_FEATURES(_state, _field) \ > - DEFINE_VIRTIO_COMMON_FEATURES(_state, _field), \ > DEFINE_PROP_BIT("any_layout", _state, _field, VIRTIO_F_ANY_LAYOUT, true), \ > DEFINE_PROP_BIT("csum", _state, _field, VIRTIO_NET_F_CSUM, true), \ > DEFINE_PROP_BIT("guest_csum", _state, _field, VIRTIO_NET_F_GUEST_CSUM, true), \ > diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h > index 42b1024..367afc6 100644 > --- a/include/hw/virtio/virtio-scsi.h > +++ b/include/hw/virtio/virtio-scsi.h > @@ -180,7 +180,6 @@ typedef struct { > DEFINE_PROP_UINT32("cmd_per_lun", _state, _conf_field.cmd_per_lun, 128) > > #define DEFINE_VIRTIO_SCSI_FEATURES(_state, _feature_field) \ > - DEFINE_VIRTIO_COMMON_FEATURES(_state, _feature_field), \ > DEFINE_PROP_BIT("hotplug", _state, _feature_field, VIRTIO_SCSI_F_HOTPLUG, \ > true), \ > DEFINE_PROP_BIT("param_change", _state, _feature_field, \ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/virtio: enable common virtio feature for mmio device 2014-06-30 10:09 ` Michael S. Tsirkin @ 2014-06-30 10:13 ` Ming Lei 0 siblings, 0 replies; 11+ messages in thread From: Ming Lei @ 2014-06-30 10:13 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: Paolo Bonzini, qemu-devel, Peter Maydell On Mon, Jun 30, 2014 at 6:09 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > On Mon, Jun 16, 2014 at 05:26:33PM +0800, Ming Lei wrote: >> On Mon, Jun 16, 2014 at 3:54 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: >> > Il 09/06/2014 10:00, Ming Lei ha scritto: >> > >> >> On Wed, Jun 4, 2014 at 10:05 AM, Ming Lei <ming.lei@canonical.com> wrote: >> >>> >> >>> Both 'indirect_desc' and 'event_idx' are bus independent features, >> >>> and they should be enabled for mmio devices too. >> >>> >> >>> On arm64 quad core VM(qemu-kvm), the patch can increase block I/O >> >>> performance a lot with latest linux tree: >> >>> - without the patch: 14K IOPS >> >>> - with the patch: 34K IOPS >> >>> >> >>> fio script: >> >>> [global] >> >>> direct=1 >> >>> bsrange=4k-4k >> >>> timeout=10 >> >>> numjobs=4 >> >>> ioengine=libaio >> >>> iodepth=64 >> >>> >> >>> filename=/dev/vdc >> >>> group_reporting=1 >> >>> >> >>> [f1] >> >>> rw=randread >> >>> >> >>> Cc: Peter Maydell <peter.maydell@linaro.org> >> >>> Signed-off-by: Ming Lei <ming.lei@canonical.com> >> >>> --- >> >>> hw/virtio/virtio-mmio.c | 6 ++++++ >> >>> 1 file changed, 6 insertions(+) >> >>> >> >>> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c >> >>> index 8829eb0..18c6e5b 100644 >> >>> --- a/hw/virtio/virtio-mmio.c >> >>> +++ b/hw/virtio/virtio-mmio.c >> >>> @@ -369,10 +369,16 @@ static void virtio_mmio_realizefn(DeviceState *d, >> >>> Error **errp) >> >>> sysbus_init_mmio(sbd, &proxy->iomem); >> >>> } >> >>> >> >>> +static Property virtio_mmio_properties[] = { >> >>> + DEFINE_VIRTIO_COMMON_FEATURES(VirtIOMMIOProxy, host_features), >> >>> + DEFINE_PROP_END_OF_LIST(), >> >>> +}; >> >>> + >> >>> static void virtio_mmio_class_init(ObjectClass *klass, void *data) >> >>> { >> >>> DeviceClass *dc = DEVICE_CLASS(klass); >> >>> >> >>> + dc->props = virtio_mmio_properties; >> >>> dc->realize = virtio_mmio_realizefn; >> >>> dc->reset = virtio_mmio_reset; >> >>> set_bit(DEVICE_CATEGORY_MISC, dc->categories); >> >>> -- >> >>> 1.7.9.5 >> > >> > >> > Looks good. >> >> Paolo, thanks for your review. >> >> > Can you look into moving DEFINE_VIRTIO_COMMON_FEATURES >> > from all virtio pci devices to TYPE_VIRTIO_PCI, too? >> >> OK, that looks a good cleanup, how about the attached patch? >> If it is OK, I will prepare a formal one for submitting. >> >> >> Thanks, >> -- >> Ming Lei > > I applied the original patch for now. > Pls address Paolo's comments and resubmit this one. I have addresses all comments for the virtio-pci changes, and will reply you on that thread. Thanks, -- Ming Lei ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/virtio: enable common virtio feature for mmio device 2014-06-04 2:05 [Qemu-devel] [PATCH] hw/virtio: enable common virtio feature for mmio device Ming Lei 2014-06-09 8:00 ` Ming Lei @ 2014-06-30 9:59 ` Ming Lei 2014-06-30 10:08 ` Michael S. Tsirkin 2014-07-02 12:34 ` Michael S. Tsirkin 3 siblings, 0 replies; 11+ messages in thread From: Ming Lei @ 2014-06-30 9:59 UTC (permalink / raw) To: Peter Maydell, qemu-devel; +Cc: Paolo Bonzini, Ming Lei, Michael S. Tsirkin Hi Guys, On Wed, Jun 4, 2014 at 10:05 AM, Ming Lei <ming.lei@canonical.com> wrote: > Both 'indirect_desc' and 'event_idx' are bus independent features, > and they should be enabled for mmio devices too. > > On arm64 quad core VM(qemu-kvm), the patch can increase block I/O > performance a lot with latest linux tree: > - without the patch: 14K IOPS > - with the patch: 34K IOPS > > fio script: > [global] > direct=1 > bsrange=4k-4k > timeout=10 > numjobs=4 > ioengine=libaio > iodepth=64 > > filename=/dev/vdc > group_reporting=1 > > [f1] > rw=randread > > Cc: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Ming Lei <ming.lei@canonical.com> > --- > hw/virtio/virtio-mmio.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c > index 8829eb0..18c6e5b 100644 > --- a/hw/virtio/virtio-mmio.c > +++ b/hw/virtio/virtio-mmio.c > @@ -369,10 +369,16 @@ static void virtio_mmio_realizefn(DeviceState *d, Error **errp) > sysbus_init_mmio(sbd, &proxy->iomem); > } > > +static Property virtio_mmio_properties[] = { > + DEFINE_VIRTIO_COMMON_FEATURES(VirtIOMMIOProxy, host_features), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > static void virtio_mmio_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > > + dc->props = virtio_mmio_properties; > dc->realize = virtio_mmio_realizefn; > dc->reset = virtio_mmio_reset; > set_bit(DEVICE_CATEGORY_MISC, dc->categories); > -- Gentle ping... Thanks, -- Ming Lei ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/virtio: enable common virtio feature for mmio device 2014-06-04 2:05 [Qemu-devel] [PATCH] hw/virtio: enable common virtio feature for mmio device Ming Lei 2014-06-09 8:00 ` Ming Lei 2014-06-30 9:59 ` Ming Lei @ 2014-06-30 10:08 ` Michael S. Tsirkin 2014-07-02 12:34 ` Michael S. Tsirkin 3 siblings, 0 replies; 11+ messages in thread From: Michael S. Tsirkin @ 2014-06-30 10:08 UTC (permalink / raw) To: Ming Lei; +Cc: Peter Maydell, qemu-devel On Wed, Jun 04, 2014 at 10:05:55AM +0800, Ming Lei wrote: > Both 'indirect_desc' and 'event_idx' are bus independent features, > and they should be enabled for mmio devices too. > > On arm64 quad core VM(qemu-kvm), the patch can increase block I/O > performance a lot with latest linux tree: > - without the patch: 14K IOPS > - with the patch: 34K IOPS > > fio script: > [global] > direct=1 > bsrange=4k-4k > timeout=10 > numjobs=4 > ioengine=libaio > iodepth=64 > > filename=/dev/vdc > group_reporting=1 > > [f1] > rw=randread > > Cc: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Ming Lei <ming.lei@canonical.com> Applied, thanks! > --- > hw/virtio/virtio-mmio.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c > index 8829eb0..18c6e5b 100644 > --- a/hw/virtio/virtio-mmio.c > +++ b/hw/virtio/virtio-mmio.c > @@ -369,10 +369,16 @@ static void virtio_mmio_realizefn(DeviceState *d, Error **errp) > sysbus_init_mmio(sbd, &proxy->iomem); > } > > +static Property virtio_mmio_properties[] = { > + DEFINE_VIRTIO_COMMON_FEATURES(VirtIOMMIOProxy, host_features), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > static void virtio_mmio_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > > + dc->props = virtio_mmio_properties; > dc->realize = virtio_mmio_realizefn; > dc->reset = virtio_mmio_reset; > set_bit(DEVICE_CATEGORY_MISC, dc->categories); > -- > 1.7.9.5 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/virtio: enable common virtio feature for mmio device 2014-06-04 2:05 [Qemu-devel] [PATCH] hw/virtio: enable common virtio feature for mmio device Ming Lei ` (2 preceding siblings ...) 2014-06-30 10:08 ` Michael S. Tsirkin @ 2014-07-02 12:34 ` Michael S. Tsirkin 2014-07-02 12:36 ` Ming Lei 3 siblings, 1 reply; 11+ messages in thread From: Michael S. Tsirkin @ 2014-07-02 12:34 UTC (permalink / raw) To: Ming Lei; +Cc: Peter Maydell, qemu-devel On Wed, Jun 04, 2014 at 10:05:55AM +0800, Ming Lei wrote: > Both 'indirect_desc' and 'event_idx' are bus independent features, > and they should be enabled for mmio devices too. > > On arm64 quad core VM(qemu-kvm), the patch can increase block I/O > performance a lot with latest linux tree: > - without the patch: 14K IOPS > - with the patch: 34K IOPS > > fio script: > [global] > direct=1 > bsrange=4k-4k > timeout=10 > numjobs=4 > ioengine=libaio > iodepth=64 > > filename=/dev/vdc > group_reporting=1 > > [f1] > rw=randread > > Cc: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Ming Lei <ming.lei@canonical.com> Applied, thanks! I'm guessing this doesn't affect cross-version migration since virtio-mmio isn't used on a PC and that's the only versioned machine. > --- > hw/virtio/virtio-mmio.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c > index 8829eb0..18c6e5b 100644 > --- a/hw/virtio/virtio-mmio.c > +++ b/hw/virtio/virtio-mmio.c > @@ -369,10 +369,16 @@ static void virtio_mmio_realizefn(DeviceState *d, Error **errp) > sysbus_init_mmio(sbd, &proxy->iomem); > } > > +static Property virtio_mmio_properties[] = { > + DEFINE_VIRTIO_COMMON_FEATURES(VirtIOMMIOProxy, host_features), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > static void virtio_mmio_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > > + dc->props = virtio_mmio_properties; > dc->realize = virtio_mmio_realizefn; > dc->reset = virtio_mmio_reset; > set_bit(DEVICE_CATEGORY_MISC, dc->categories); > -- > 1.7.9.5 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/virtio: enable common virtio feature for mmio device 2014-07-02 12:34 ` Michael S. Tsirkin @ 2014-07-02 12:36 ` Ming Lei 0 siblings, 0 replies; 11+ messages in thread From: Ming Lei @ 2014-07-02 12:36 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: Peter Maydell, qemu-devel On Wed, Jul 2, 2014 at 8:34 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > On Wed, Jun 04, 2014 at 10:05:55AM +0800, Ming Lei wrote: >> Both 'indirect_desc' and 'event_idx' are bus independent features, >> and they should be enabled for mmio devices too. >> >> On arm64 quad core VM(qemu-kvm), the patch can increase block I/O >> performance a lot with latest linux tree: >> - without the patch: 14K IOPS >> - with the patch: 34K IOPS >> >> fio script: >> [global] >> direct=1 >> bsrange=4k-4k >> timeout=10 >> numjobs=4 >> ioengine=libaio >> iodepth=64 >> >> filename=/dev/vdc >> group_reporting=1 >> >> [f1] >> rw=randread >> >> Cc: Peter Maydell <peter.maydell@linaro.org> >> Signed-off-by: Ming Lei <ming.lei@canonical.com> > > Applied, thanks! > I'm guessing this doesn't affect cross-version > migration since virtio-mmio isn't used on > a PC and that's the only versioned machine. Yes, I think so. Even ARM/ARM64 will use virtio-pci if kernel support is ready because virtio-pci has better support than virtio-mmio. Thanks, ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-07-02 12:37 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-06-04 2:05 [Qemu-devel] [PATCH] hw/virtio: enable common virtio feature for mmio device Ming Lei 2014-06-09 8:00 ` Ming Lei 2014-06-16 7:54 ` Paolo Bonzini 2014-06-16 9:26 ` Ming Lei 2014-06-16 9:50 ` Paolo Bonzini 2014-06-30 10:09 ` Michael S. Tsirkin 2014-06-30 10:13 ` Ming Lei 2014-06-30 9:59 ` Ming Lei 2014-06-30 10:08 ` Michael S. Tsirkin 2014-07-02 12:34 ` Michael S. Tsirkin 2014-07-02 12:36 ` Ming Lei
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).