From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sasha Levin Subject: Re: [PATCH] kvm tools: Add MMIO coalescing support Date: Sat, 04 Jun 2011 19:50:29 +0300 Message-ID: <1307206229.2173.3.camel@lappy> References: <20110604101711.GB16292@elte.hu> <1307183318.7239.6.camel@lappy> <20110604103508.GE16292@elte.hu> <20110604104712.GG16292@elte.hu> <20110604112743.GA24146@elte.hu> <20110604144629.GA22189@elte.hu> <1FAEB5EB-57FD-49D7-9D62-3494BC22F4BD@suse.de> <20110604163409.GA1899@elte.hu> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: Alexander Graf , penberg@kernel.org, kvm@vger.kernel.org, asias.hejun@gmail.com, gorcunov@gmail.com, prasadjoshi124@gmail.com To: Ingo Molnar Return-path: Received: from mail-ww0-f44.google.com ([74.125.82.44]:54337 "EHLO mail-ww0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752757Ab1FDQuf (ORCPT ); Sat, 4 Jun 2011 12:50:35 -0400 Received: by wwa36 with SMTP id 36so2577128wwa.1 for ; Sat, 04 Jun 2011 09:50:34 -0700 (PDT) In-Reply-To: <20110604163409.GA1899@elte.hu> Sender: kvm-owner@vger.kernel.org List-ID: On Sat, 2011-06-04 at 18:34 +0200, Ingo Molnar wrote: > * Alexander Graf wrote: > > > > > On 04.06.2011, at 16:46, Ingo Molnar wrote: > > > > > > > > * Alexander Graf wrote: > > > > > >> So the simple rule is: don't register a coalesced MMIO region for a > > >> region where latency matters. [...] > > > > > > So my first suspicion is confirmed. > > > > > > A quick look at Qemu sources shows that lots of drivers are using > > > coalesced_mmio without being aware of the latency effects and only > > > one seems to make use of qemu_flush_coalesced_mmio_buffer(). Drivers > > > like hw/e1000.c sure look latency critical to me. > > > > e1000 maps its NVRAM on coalesced mmio - which is completely ok. > > Ok! > > > > So i maintain my initial opinion: this is a pretty dangerous > > > 'optimization' that should be used with extreme care: i can tell > > > it you with pretty good authority that latency problems are much > > > more easy to introduce than to find and remove ... > > > > Yup, which is why it's very sparsely used in qemu :). Basically, > > it's only e1000 and vga, both of which are heavily used and tested > > drivers. > > Ok, so this change in: > > commit 73389b5ea017288a949ae27536c8cfd298d3e317 > Author: Sasha Levin > Date: Fri Jun 3 22:51:08 2011 +0300 > > kvm tools: Add MMIO coalescing support > > @@ -67,6 +70,16 @@ bool kvm__register_mmio(u64 phys_addr, u64 phys_addr_len, void (*kvm_mmio_callba > .kvm_mmio_callback_fn = kvm_mmio_callback_fn, > }; > > + zone = (struct kvm_coalesced_mmio_zone) { > + .addr = phys_addr, > + .size = phys_addr_len, > + }; > + ret = ioctl(kvm->vm_fd, KVM_REGISTER_COALESCED_MMIO, &zone); > + if (ret < 0) { > + free(mmio); > + return false; > + } > > Seems completely wrong, because it indiscriminately registers *all* > mmio regions as coalesced ones. Yes. I'll add a flag instead of making all of them coalesced. -- Sasha.