From patchwork Tue Apr 26 07:34:26 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Xu X-Patchwork-Id: 614796 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3qvFHG2mtrz9t5P for ; Tue, 26 Apr 2016 17:35:06 +1000 (AEST) Received: from localhost ([::1]:36654 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1auxWS-00022c-5M for incoming@patchwork.ozlabs.org; Tue, 26 Apr 2016 03:35:04 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53482) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1auxW3-0001Nf-TU for qemu-devel@nongnu.org; Tue, 26 Apr 2016 03:34:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1auxVx-0008KV-Kl for qemu-devel@nongnu.org; Tue, 26 Apr 2016 03:34:39 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45485) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1auxVx-0008KK-Ch for qemu-devel@nongnu.org; Tue, 26 Apr 2016 03:34:33 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 330ACC049D4F; Tue, 26 Apr 2016 07:34:32 +0000 (UTC) Received: from pxdev.xzpeter.org (dhcp-14-238.nay.redhat.com [10.66.14.238]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u3Q7YQwG013316 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Tue, 26 Apr 2016 03:34:28 -0400 Date: Tue, 26 Apr 2016 15:34:26 +0800 From: Peter Xu To: Jan Kiszka Message-ID: <20160426073426.GD28545@pxdev.xzpeter.org> References: <1461055122-32378-1-git-send-email-peterx@redhat.com> <571DA823.1030003@web.de> <20160425071806.GF3261@pxdev.xzpeter.org> <571DC61C.9020006@web.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <571DC61C.9020006@web.de> User-Agent: Mutt/1.5.24 (2015-08-30) X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: ehabkost@redhat.com, mst@redhat.com, jasowang@redhat.com, rkrcmar@redhat.com, qemu-devel@nongnu.org, alex.williamson@redhat.com, wexu@redhat.com, pbonzini@redhat.com, marcel@redhat.com, imammedo@redhat.com, rth@twiddle.net Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" On Mon, Apr 25, 2016 at 09:24:12AM +0200, Jan Kiszka wrote: > On 2016-04-25 09:18, Peter Xu wrote: > > On Mon, Apr 25, 2016 at 07:16:19AM +0200, Jan Kiszka wrote: > >> On 2016-04-19 10:38, Peter Xu wrote: > > > > [...] > > > >>> By default, IR is disabled to be better compatible with current > >>> QEMU. To enable IR, we can using the following command to boot a > >>> IR-supported VM with virtio-net device with vhost (still do not > >>> support kvm-ioapic, so we need to specify kernel-irqchip={split|off} > >>> here): > >>> > >>> $ qemu-system-x86_64 -M q35,iommu=on,intr=on,kernel-irqchip=split \ > >> > >> "intr" sounds a bit too much like "interrupt", not "interrupt > >> remapping". Why not use the kernel's form, "intremap"? > > > > Sure. It sounds nice to be aligned with the kernel one. Let me take > > it in v5. > > > >> > >>> -enable-kvm -m 1024 \ > >>> -netdev tap,id=net0,vhost=on \ > >>> -device virtio-net-pci,netdev=user.0 \ > >>> -monitor telnet::3333,server,nowait \ > >>> /var/lib/libvirt/images/vm1.qcow2 > >>> > >>> When guest boots, we can verify whether IR enabled by grepping the > >>> dmesg like: > >>> > >>> [root@localhost ~]# journalctl -k | grep "DMAR-IR" > >>> Feb 19 11:21:23 localhost.localdomain kernel: DMAR-IR: IOAPIC id 0 under DRHD base 0xfed90000 IOMMU 0 > >>> Feb 19 11:21:23 localhost.localdomain kernel: DMAR-IR: Enabled IRQ remapping in xapic mode > >>> > >>> Currently supported devices: > >>> > >>> - Emulated/Splitted irqchip > >>> - Generic PCI Devices > >>> - vhost devices > >>> - pass through device support? Not tested, but suppose it should work. > >> > >> I've tested this series against my Jailhouse setup, and it works pretty > >> well! Actually considering to move my test setup over this branch. > > > > This is really encouraging feedback! Btw, thanks for all kinds of > > help on this patchset. :-) > > > >> > >> However, split irqchip still has some issues: When I boot a q35 machine > >> with Linux, the e1000 network adapter only gets a single IRQ delivered. > >> Interestingly, other IOAPIC IRQs like the keyboard work all the time. I > >> didn't debug this in details yet. > > > > I reproduced this problem. It seems that it fails even with > > kernel-irqchip=off. Will try to dig it out. > > Very good. Hope it can be easily fixed. Hi, Jan, The above issue should be caused by EOI missing of level-triggered interrupts. Before that, I was always using edge-triggered interrupts for test, so didn't encounter this one. Would you please help try below patch? It can be applied directly onto the series, and should solve the issue (it works on my test vm, and I'll take it in v5 as well if it also works for you): ------------------------- ------------------------ I am still looking into guest part codes. Although the above patch should solve the issue, there are still issues in guest codes when IR is enabled: - mismatched "vector" in IOAPIC entry and IRTE entry (this is required in vt-d spec 5.1.5.1, and required to correctly deliver EOI broadcast I guess). See intel_irq_remapping_prepare_irte(): ... /* * IO-APIC RTE will be configured with virtual vector. * irq handler will do the explicit EOI to the io-apic. */ entry->vector = info->ioapic_pin; ... - I encountered that level-triggered entries in IOAPIC is marked as edge-triggered interrupt in APIC (which is strange)... This will also affect correct delivery of EOI broadcast. I still need time to figure out why. If EOI broadcast can work, e1000 issue would be solved as well even without above patch. [...] > > > >> > >>> - IR fault reporting > >> > >> Would be welcome! I found a "test case" yesterday: misconfigured IOAPIC > >> ID blocked its IRQs under Jailhouse, and I first had to enable tracing > >> to realize it ;). > > > > Yes, it sounds nice to have guest side feedback on IR faults. Will > > do more reading, and see whether I can add one more patch in v5 to > > do this. > > It's not a must-have for getting things merged. In fact, any additional > feature that could now delay the merge of what you have should rather > wait. Stabilizing, addressing style and structure comments is more > important IMO. Okay, then let me add this into my todo list, and will pick this up when got time. Thanks, -- peterx diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c index b41ab89..de6a8cf 100644 --- a/hw/intc/ioapic.c +++ b/hw/intc/ioapic.c @@ -281,6 +281,36 @@ ioapic_mem_read(void *opaque, hwaddr addr, unsigned int size) return val; } +/* + * This is to satisfy the hack in Linux kernel. One hack of it is to + * simulate clearing the Remote IRR bit of IOAPIC entry using the + * following: + * + * "For IO-APIC's with EOI register, we use that to do an explicit EOI. + * Otherwise, we simulate the EOI message manually by changing the trigger + * mode to edge and then back to level, with RTE being masked during + * this." + * + * (See linux kernel __eoi_ioapic_pin() comment in commit c0205701) + * + * This is based on the assumption that, Remote IRR bit will be + * cleared by IOAPIC hardware for edge-triggered interrupts (I + * believe that's what the IOAPIC version 0x1X hardware does). So + * if we are emulating it, we'd better do it the same here, so that + * the guest kernel hack will work as well on QEMU. + * + * Without this, level-triggered interrupts in IR mode might fail to + * work correctly. + */ +static inline void +ioapic_fix_edge_remote_irr(uint64_t *entry) +{ + if (*entry & IOAPIC_LVT_TRIGGER_MODE) { + /* Level triggered interrupts, make sure remote IRR is zero */ + *entry &= ~((uint64_t)IOAPIC_LVT_REMOTE_IRR); + } +} + static void ioapic_mem_write(void *opaque, hwaddr addr, uint64_t val, unsigned int size) @@ -314,6 +344,7 @@ ioapic_mem_write(void *opaque, hwaddr addr, uint64_t val, s->ioredtbl[index] &= ~0xffffffffULL; s->ioredtbl[index] |= val; } + ioapic_fix_edge_remote_irr(&s->ioredtbl[index]); ioapic_service(s); } }