From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH v2 0/9] qemu-kvm: Clean up and enhance MSI irqchip support Date: Wed, 27 Apr 2011 15:12:22 +0300 Message-ID: <4DB80826.5010706@redhat.com> References: <4DB7C56D.8040503@redhat.com> <4DB7DB24.8060403@siemens.com> <4DB7DC11.1010308@redhat.com> <4DB7DC8B.1030402@siemens.com> <4DB7DE7A.4000608@redhat.com> <4DB7FC21.8000203@siemens.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , "kvm@vger.kernel.org" , "Michael S. Tsirkin" To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:64054 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755561Ab1D0MM2 (ORCPT ); Wed, 27 Apr 2011 08:12:28 -0400 In-Reply-To: <4DB7FC21.8000203@siemens.com> Sender: kvm-owner@vger.kernel.org List-ID: On 04/27/2011 02:21 PM, Jan Kiszka wrote: > On 2011-04-27 11:14, Avi Kivity wrote: > > On 04/27/2011 12:06 PM, Jan Kiszka wrote: > >>> > >>> We can simply drop all route entries that are used exclusively in qemu > >>> (i.e. not bound to an irqfd) and let the cache rebuild itself. > >> > >> When should they be dropped? > > > > Whenever we need to allocate a new routing entry, but cannot because it > > is full. > > > > def kvm_send_msi_message(addr, val): > > gsi = route_cache.get((addr, val), None) > > if gsi is None: > > expire_volatile_route_cache_entries_if_full() > > gsi = alloc_gsi_cache_entry() > > route_cache[(addr, val)] = gsi > > update_route_cache() > > kvm_irq_line(gsi, 1) > > kvm_irq_line(gsi, 0) > > > > The code would have to be in kvm.c, where it can track whether an entry > > is volatile or persistent. > > Yeah, what I forgot is the other side of this caching: Looking up the > GSI from the MSI addr/data tuple. That's more complex than the current > O(1) way via device.msitable[vector]. Well, we could use some hash > table. But is it worth it? IMHO only if we could radically simplify the > PCI device hooking as well (HPET is negligible compared to that). But > I'm not yet sure if we can given what vhost needs. A hash table is indeed overcomplicated for this. How about a replacement for stl_phys() for the MSI case: - stl_phys(timer->fsb >> 32, timer->fsb & 0xffffffff); + msi_stl_phys(timer->fsb >> 32, timer->fsb & 0xffffffff, &timer->msi_cache); msi_stl_phys(target_phys_addr_t addr, uint32_t data, MSICache *cache) { if (kvm_msi_enabled() && addr & MSI_ADDR_MASK == msi_base_addr) { if (cache->addr != addr || cache->data != data) { kvm_update_msi_cache(cache, addr, data); } kvm_irq_line(cache->gsi, 1); kvm_irq_line(cache->gsi, 0); return; } stl_phys(addr, data); } This bit of code would need to be updated for IOMMU and interrupt remapping, but at least it means that devices don't need significant change for kvm support. We could also allocate a single gsi for use in hw/apic.c so hacks like using DMA to generate an MSI will work (will be slow, though). -- error compiling committee.c: too many arguments to function