From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sasha Levin Subject: Re: [PATCH] IO: Intelligent device lookup on bus Date: Thu, 21 Jul 2011 11:43:24 +0300 Message-ID: <1311237804.29708.17.camel@lappy> References: <1311163918-14334-1-git-send-email-levinsasha928@gmail.com> <4E26CD66.1080407@siemens.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, Avi Kivity , Marcelo Tosatti To: Jan Kiszka Return-path: Received: from mail-fx0-f52.google.com ([209.85.161.52]:46478 "EHLO mail-fx0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751905Ab1GUInw (ORCPT ); Thu, 21 Jul 2011 04:43:52 -0400 Received: by fxd18 with SMTP id 18so3211979fxd.11 for ; Thu, 21 Jul 2011 01:43:51 -0700 (PDT) In-Reply-To: <4E26CD66.1080407@siemens.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, 2011-07-20 at 14:43 +0200, Jan Kiszka wrote: > On 2011-07-20 14:11, Sasha Levin wrote: > > Currently the method of dealing with an IO operation on a bus (PIO/MMIO) > > is to call the read or write callback for each device registered > > on the bus until we find a device which handles it. > > > > Since the number of devices on a bus can be significant due to ioeventfds > > and coalesced MMIO zones, this leads to a lot of overhead on each IO > > operation. > > > > Instead of registering devices, we now register ranges which points to > > a device. Lookup is done using an efficient bsearch instead of a linear > > search. > > > > This should speed up all IO operations generated by the guest. > > > > Cc: Avi Kivity > > Cc: Marcelo Tosatti > > Signed-off-by: Sasha Levin > > --- > > > > This patch depends on '[PATCH v3] MMIO: Make coalesced mmio use a device > > per zone'. > > > > arch/x86/kvm/i8254.c | 4 +- > > arch/x86/kvm/i8259.c | 4 +- > > include/linux/kvm_host.h | 18 ++++---- > > virt/kvm/coalesced_mmio.c | 6 +-- > > virt/kvm/eventfd.c | 3 +- > > virt/kvm/ioapic.c | 13 +----- > > virt/kvm/kvm_main.c | 107 +++++++++++++++++++++++++++++++++++++++----- > > 7 files changed, 115 insertions(+), 40 deletions(-) > > > > diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c > > index efad723..61d193c 100644 > > --- a/arch/x86/kvm/i8254.c > > +++ b/arch/x86/kvm/i8254.c > > @@ -713,13 +713,15 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags) > > kvm_register_irq_mask_notifier(kvm, 0, &pit->mask_notifier); > > > > kvm_iodevice_init(&pit->dev, &pit_dev_ops); > > - ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS, &pit->dev); > > + ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS, KVM_PIT_BASE_ADDRESS, > > + KVM_PIT_MEM_LENGTH, &pit->dev); > > if (ret < 0) > > goto fail; > > > > if (flags & KVM_PIT_SPEAKER_DUMMY) { > > kvm_iodevice_init(&pit->speaker_dev, &speaker_dev_ops); > > ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS, > > + KVM_SPEAKER_BASE_ADDRESS, 4, > > &pit->speaker_dev); > > if (ret < 0) > > goto fail_unregister; > > diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c > > index 19fe855..c2295af 100644 > > --- a/arch/x86/kvm/i8259.c > > +++ b/arch/x86/kvm/i8259.c > > @@ -562,7 +562,9 @@ struct kvm_pic *kvm_create_pic(struct kvm *kvm) > > */ > > kvm_iodevice_init(&s->dev, &picdev_ops); > > mutex_lock(&kvm->slots_lock); > > - ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS, &s->dev); > > + ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS, 0x20, 2, &s->dev); > > + ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS, 0xa0, 2, &s->dev); > > + ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS, 0x4d0, 2, &s->dev); > > This made me wonder if you are incorrectly registering the same device > multiple times. kvm_io_bus_register_dev is no longer optimally > describing what is done here: the registration of an IO range with the > bus. And that range is handled by a specific device. Conceptually, a > device is only attached once to some bus. I agree. My initial thoughts were to leave it this way to simplify registrations, but as you said - it makes it somewhat confusing. I'll modify kvm_io_bus_register_dev to receive an array or ranges instead of a single range. -- Sasha.