From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim Krcmar Subject: Re: [PATCH] KVM: x86: implement IOAPIC_REG_EOI for directed EOI support Date: Thu, 13 Apr 2017 16:15:14 +0200 Message-ID: <20170413141513.GA20221@potion> References: <20170411141115.4314-1-lprosek@redhat.com> <20170412064029.GD16464@pxdev.xzpeter.org> <20170412090644.GG16464@pxdev.xzpeter.org> <20170412205243.GG20145@potion> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: Peter Xu , KVM list To: Ladi Prosek Return-path: Received: from mx1.redhat.com ([209.132.183.28]:36520 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750745AbdDMOPT (ORCPT ); Thu, 13 Apr 2017 10:15:19 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D3B897F7B1 for ; Thu, 13 Apr 2017 14:15:18 +0000 (UTC) Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: 2017-04-13 08:36+0200, Ladi Prosek: > On Wed, Apr 12, 2017 at 10:52 PM, Radim Krcmar wrote: >> 2017-04-12 17:06+0800, Peter Xu: >>> On Wed, Apr 12, 2017 at 09:36:58AM +0200, Ladi Prosek wrote: >>> > On Wed, Apr 12, 2017 at 8:40 AM, Peter Xu wrote: >>> > > On Tue, Apr 11, 2017 at 04:11:15PM +0200, Ladi Prosek wrote: >>> > >> If the guest takes advantage of the directed EOI feature by setting >>> > >> APIC_SPIV_DIRECTED_EOI, it is expected to signal EOI by writing to >>> > >> the EOI register of the respective IOAPIC. >>> > >> >>> > >> From Intel's x2APIC Specification: >>> > >> "following the EOI to the local x2APIC unit for a level triggered >>> > >> interrupt, perform a directed EOI to the IOxAPIC generating the >>> > >> interrupt by writing to its EOI register." >>> > >> >>> > >> Commit fc61b800f950 ("KVM: Add Directed EOI support to APIC emulation") >>> > >> inhibited EOI on LAPIC EOI register write but didn't add the IOAPIC >>> > >> part. IOAPIC_REG_EOI writes were handled only on IA64 and the code >>> > >> was later removed with the rest of IA64 support. >>> > >> >>> > >> The bug has gone undetected for a long time because Linux writes to >>> > >> IOAPIC_REG_EOI only if the IOAPIC version is >=0x20. Windows doesn't >>> > >> seem to perform such a check. >>> > > >>> > > Hi, Ladi, >>> > >>> > Hi Peter, >>> > >>> > > Not sure I'm understanding it correctly... I see "direct EOI" is a >>> > > feature for IOAPIC version 0x20, while "suppress EOI-broadcast" is >>> > > another feature for APIC. They are not the same feature, so it may not >>> > > be required to have them all together. IIUC current x86 kvm is just >>> > > the case - it supports EOI broadcast suppression on APIC, but it does >>> > > not support direct EOI on kernel IOAPIC. >>> > >>> > Thanks, that makes perfect sense and explains why Linux behaves the >>> > way it does (__eoi_ioapic_pin in arch/x86/kernel/apic/io_apic.c). >>> > >>> > This document makes it look like suppress EOI-broadcast always implies >>> > directed EOI, though: >>> > >>> > http://www.intel.com/content/dam/doc/specification-update/64-architecture-x2apic-specification.pdf >>> > >>> > NB "The support for Directed EOI capability can be detected by means >>> > of bit 24 in the Local APIC Version Register. " >>> > >>> > There is no mention of APIC version or any other detection mechanism >>> > for directed EOI. Maybe the chip being x2APIC implies version >= 0x20 >>> > but I don't see that in the document either. >>> > >>> > I suspect that Microsoft implemented EOI by following this same spec. >>> > Level-triggered interrupts don't work right on Windows Server 2016 >>> > with Hyper-V enabled without this patch. >>> >>> Yes, the documents for IOAPIC is always hard to find, at least for >>> me... >>> >>> There is some pages mentioned IOAPIC in ICH9 manual on chap 13.5 here: >>> http://www.intel.com/content/dam/doc/datasheet/io-controller-hub-9-datasheet.pdf >>> >>> However I see nothing related to how the IOAPIC version is defined. In >>> that sense, the comment above __eoi_ioapic_pin() seems to be better. :) >> >> Yeah, it is officially described in ICH9 datasheet as: >> >> 13.5.6 VER—Version Register (LPC I/F—D31:F0) >> Default Value: 00170020h >> >> The one we emulate in KVM is in 82093AA datasheet: >> >> 3.2.2. IOAPICVER—IOAPIC VERSION REGISTER >> Default Value: 00170011h >> >> The former has the EOI register, the latter doesn't. > > Got it. Do you want me to resubmit with a different commit message and > a comment blaming the guest OS instead of calling it a KVM bug? :) Or > do you think it's worth exploring Peter's suggestion to make a more > invasive fix? I would experiment with reverting the APIC suppress-EOI-broadcast first. It doesn't currently work well and is pointless in theory. >> --- >> I don't like the idea behind the patch, but it is acceptable and >> thinking about good solutions gets us into compatibility nightmares ... >> (We could remove support for directed EOI, because it is a detectable >> feature and makes little sense in KVM, or we could implement the IOAPIC >> version 0x20, but both would be tricky to migrate.) >> >> People should switch to userspace IOAPIC anyway. :) > > For the record, QEMU with kernel-irqchip=split works fine as it > emulates version 0x20 with the IOAPIC_EOI register by default. Great, thanks. > kernel-irqchip=off does not seem to work, I will look into it. Well, good luck. :)