From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57961) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WwTZA-00034B-SP for qemu-devel@nongnu.org; Mon, 16 Jun 2014 05:51:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WwTZ6-0007y1-G1 for qemu-devel@nongnu.org; Mon, 16 Jun 2014 05:51:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55711) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WwTZ6-0007xx-7K for qemu-devel@nongnu.org; Mon, 16 Jun 2014 05:51:00 -0400 Message-ID: <539EBDFD.2040406@redhat.com> Date: Mon, 16 Jun 2014 11:50:53 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1401847555-8306-1-git-send-email-ming.lei@canonical.com> <539EA2B5.3000808@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] hw/virtio: enable common virtio feature for mmio device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ming Lei Cc: Peter Maydell , qemu-devel@nongnu.org Il 16/06/2014 11:26, Ming Lei ha scritto: > On Mon, Jun 16, 2014 at 3:54 PM, Paolo Bonzini wrote: >> Il 09/06/2014 10:00, Ming Lei ha scritto: >> >>> On Wed, Jun 4, 2014 at 10:05 AM, 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 >>>> Signed-off-by: Ming Lei >>>> --- >>>> 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.