From patchwork Fri Feb 14 21:13:11 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Gabriel L. Somlo" X-Patchwork-Id: 3654141 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id F3145BF13A for ; Fri, 14 Feb 2014 21:14:55 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id EED21201C7 for ; Fri, 14 Feb 2014 21:14:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id DFBFD201BF for ; Fri, 14 Feb 2014 21:14:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752354AbaBNVOu (ORCPT ); Fri, 14 Feb 2014 16:14:50 -0500 Received: from mail-qc0-f172.google.com ([209.85.216.172]:42983 "EHLO mail-qc0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752071AbaBNVOt (ORCPT ); Fri, 14 Feb 2014 16:14:49 -0500 Received: by mail-qc0-f172.google.com with SMTP id c9so21261973qcz.31 for ; Fri, 14 Feb 2014 13:14:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=QOYWQQEoVOEj+vlYX9oV/h51wmMgKrpY1iDgXpYRrW8=; b=kvKFbOtGQRtWgWH07H/KsvF7Q5jRa0BcK2DrZ0+ysxGwmvA8KoBKlE1mpx+QP53Xze 902AjfhLAo0qPulM3wQ8lwHULVs/oMmSqljF+TofnCgbnalhDliMgsjYXw4LRfzEjaNq bTWy3LfnY513l40hmMsBJL8yv32QZtbHVZS+1V5Kg2dBeaEGdbqAbeSGAyXr9B76NvKt JkpCn6JN/jQXS9zRilDiUwAbLIlQ4g5Qg1MuF3BWf20czixLhIcEdOaEjQFfWxsp3zZo AjIf9bsYYqLwPvsECoFV5vnrahr1k2/k0CSAq7SdKmxVCos53wKGRk50p62h9rfRH3fz EM6g== X-Received: by 10.224.137.5 with SMTP id u5mr17513127qat.12.1392412488329; Fri, 14 Feb 2014 13:14:48 -0800 (PST) Received: from ERROL.INI.CMU.EDU (ERROL.INI.CMU.EDU. [128.2.16.43]) by mx.google.com with ESMTPSA id b12sm19027661qag.23.2014.02.14.13.14.48 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Fri, 14 Feb 2014 13:14:48 -0800 (PST) Date: Fri, 14 Feb 2014 16:13:11 -0500 From: "Gabriel L. Somlo" To: "Michael S. Tsirkin" Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org, eddie.dong@intel.com, agraf@suse.de Subject: Re: RFC: ioapic polarity vs. qemu os-x guest Message-ID: <20140214211311.GH29329@ERROL.INI.CMU.EDU> References: <20140130204423.GK29329@ERROL.INI.CMU.EDU> <20140211182330.GC29329@ERROL.INI.CMU.EDU> <20140211195444.GB10951@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20140211195444.GB10951@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Feb 11, 2014 at 09:54:44PM +0200, Michael S. Tsirkin wrote: > On Tue, Feb 11, 2014 at 01:23:31PM -0500, Gabriel L. Somlo wrote: > > 1. Regarding KVM and the polarity xor line in the patch above: Does > > anyone have experience with any *other* guests which insist on setting > > level-triggered interrupt polarity to 1/active-low ? Is that xor line > > actually doing anything useful in practice, for any other guest, on > > either QEMU or any other platform ? > > > > > > 2. Is there anything in QEMU (besides the ACPI DSDT .dsl files) which > > has a hardcoded assumption re. "polarity == 0", or active-high, for > > level-triggered interrupts? I tried to dig through hw/i386/kvm/ioapic.c > > and a bunch of other files, but couldn't isolate anything that I could > > "flip" to fix things in userspace. > > > > > > Any ideas or suggestions about the appropriate way to move forward would > > be much appreciated !!! > > > > > > Thanks much, > > --Gabriel > > I think changing ACPI is the right thing to > do really. But we'll need to fix some things > first of course. So I followed your advice, and was able to boot OS X just fine (but booting Linux after this patch still resulted in multiple "no one cared" complaints on IRQs 17, 18, 19, etc.: --- At this point, I'm beginning to realize that the "ActiveHigh" assumption is rather pervasively baked in throughout the QEMU source code... And I'm wondering whether a ton of changes to make it either programatically configurable or change the hard-codded assumption to "ActiveLow" would be feasible, welcome, etc... I personally would prefer configurable (e.g. "-machine foo,kernel_irqchip=on,polarity=high", or some such). Thanks much for any ideas, --Gabriel -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/hw/i386/q35-acpi-dsdt.dsl b/hw/i386/q35-acpi-dsdt.dsl index d618e9e..9c52f64 100644 --- a/hw/i386/q35-acpi-dsdt.dsl +++ b/hw/i386/q35-acpi-dsdt.dsl @@ -353,7 +353,7 @@ DefinitionBlock ( Method(IQCR, 1, Serialized) { // _CRS method - get current settings Name(PRR0, ResourceTemplate() { - Interrupt(, Level, ActiveHigh, Shared) { 0 } + Interrupt(, Level, ActiveLow, Shared) { 0 } }) CreateDWordField(PRR0, 0x05, PRRI) Store(And(Arg0, 0x0F), PRRI) @@ -365,7 +365,7 @@ DefinitionBlock ( Name(_HID, EISAID("PNP0C0F")) \ Name(_UID, uid) \ Name(_PRS, ResourceTemplate() { \ - Interrupt(, Level, ActiveHigh, Shared) { \ + Interrupt(, Level, ActiveLow, Shared) { \ 5, 10, 11 \ } \ }) \ @@ -398,12 +398,12 @@ DefinitionBlock ( Name(_HID, EISAID("PNP0C0F")) \ Name(_UID, uid) \ Name(_PRS, ResourceTemplate() { \ - Interrupt(, Level, ActiveHigh, Shared) { \ + Interrupt(, Level, ActiveLow, Shared) { \ gsi \ } \ }) \ Name(_CRS, ResourceTemplate() { \ - Interrupt(, Level, ActiveHigh, Shared) { \ + Interrupt(, Level, ActiveLow, Shared) { \ gsi \ } \ }) \ diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c index 51ce12d..fe1527a 100644 --- a/hw/isa/lpc_ich9.c +++ b/hw/isa/lpc_ich9.c @@ -206,17 +206,17 @@ static void ich9_lpc_update_pic(ICH9LPCState *lpc, int pic_irq) int i, pic_level; /* The pic level is the logical OR of all the PCI irqs mapped to it */ - pic_level = 0; + pic_level = 1; for (i = 0; i < ICH9_LPC_NB_PIRQS; i++) { int tmp_irq; int tmp_dis; ich9_lpc_pic_irq(lpc, i, &tmp_irq, &tmp_dis); if (!tmp_dis && pic_irq == tmp_irq) { - pic_level |= pci_bus_get_irq_level(lpc->d.bus, i); + pic_level &= !pci_bus_get_irq_level(lpc->d.bus, i); } } if (pic_irq == ich9_lpc_sci_irq(lpc)) { - pic_level |= lpc->sci_level; + pic_level &= !lpc->sci_level; } qemu_set_irq(lpc->pic[pic_irq], pic_level); -- However, even on OS X, the Ethernet (e1000) card won't link up at all. Fixing that requires another patch: diff --git a/hw/net/e1000.c b/hw/net/e1000.c index 58ba93b..c7a2c07 100644 --- a/hw/net/e1000.c +++ b/hw/net/e1000.c @@ -301,7 +301,7 @@ set_interrupt_cause(E1000State *s, int index, uint32_t val) s->mac_reg[ICS] = val; pending_ints = (s->mac_reg[IMS] & s->mac_reg[ICR]); - if (!s->mit_irq_level && pending_ints) { + if (s->mit_irq_level && pending_ints) { /* * Here we detect a potential raising edge. We postpone raising the * interrupt line if we are inside the mitigation delay window @@ -339,7 +339,7 @@ set_interrupt_cause(E1000State *s, int index, uint32_t val) } } - s->mit_irq_level = (pending_ints != 0); + s->mit_irq_level = (pending_ints == 0); pci_set_irq(d, s->mit_irq_level); } @@ -393,7 +393,7 @@ static void e1000_reset(void *opaque) timer_del(d->autoneg_timer); timer_del(d->mit_timer); d->mit_timer_on = 0; - d->mit_irq_level = 0; + d->mit_irq_level = 1; d->mit_ide = 0; memset(d->phy_reg, 0, sizeof d->phy_reg); memmove(d->phy_reg, phy_reg_init, sizeof phy_reg_init); @@ -1324,7 +1324,7 @@ static int e1000_post_load(void *opaque, int version_id) if (!(s->compat_flags & E1000_FLAG_MIT)) { s->mac_reg[ITR] = s->mac_reg[RDTR] = s->mac_reg[RADV] = s->mac_reg[TADV] = 0; - s->mit_irq_level = false; + s->mit_irq_level = true; } s->mit_ide = 0; s->mit_timer_on = false;