From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH v4 4/9] KVM: Switch assigned device IRQ forwarding to threaded handler Date: Tue, 09 Nov 2010 13:36:23 +0100 Message-ID: <4CD94047.8090704@siemens.com> References: <0c712006c559bbe090c7e7060c545c697d18e155.1289215310.git.jan.kiszka@siemens.com> <4CD93E00.6060609@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , kvm , Alex Williamson , "Michael S. Tsirkin" To: Avi Kivity Return-path: Received: from goliath.siemens.de ([192.35.17.28]:20806 "EHLO goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750987Ab0KIMgl (ORCPT ); Tue, 9 Nov 2010 07:36:41 -0500 In-Reply-To: <4CD93E00.6060609@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Am 09.11.2010 13:26, Avi Kivity wrote: > On 11/08/2010 01:21 PM, Jan Kiszka wrote: >> This improves the IRQ forwarding for assigned devices: By using the >> kernel's threaded IRQ scheme, we can get rid of the latency-prone work >> queue and simplify the code in the same run. >> >> Moreover, we no longer have to hold assigned_dev_lock while raising the >> guest IRQ, which can be a lenghty operation as we may have to iterate >> over all VCPUs. The lock is now only used for synchronizing masking vs. >> unmasking of INTx-type IRQs, thus is renames to intx_lock. > > Nice stuff. > >> >> -#define KVM_ASSIGNED_MSIX_PENDING 0x1 >> -struct kvm_guest_msix_entry { >> - u32 vector; >> - u16 entry; >> - u16 flags; >> -}; >> - > > Ok, so .flags was used to handle the delay between the interrupt handler > and the work queue, which is now done automatically by threaded > interrupts. Good. Yes, a nice consolidating side effect. > >> @@ -313,10 +276,10 @@ static int assigned_device_enable_host_msix(struct kvm *kvm, >> return r; >> >> for (i = 0; i< dev->entries_nr; i++) { >> - r = request_irq(dev->host_msix_entries[i].vector, >> - kvm_assigned_dev_intr, 0, >> - "kvm_assigned_msix_device", >> - (void *)dev); >> + r = request_threaded_irq(dev->host_msix_entries[i].vector, >> + NULL, kvm_assigned_dev_thread, >> + 0, "kvm_assigned_msix_device", >> + (void *)dev); >> if (r) >> goto err; >> } > > > Should eventually be done from interrupt context. msix delivery only > needs a routing table lookup and smp reschedule IPI, which can be done > from interrupt context. > Yes, definitely. But we would have to catch the case when the guest configures the MSI-X message to be a broadcast to all its trillion VCPUs (once we support trillions of them). Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux