From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Subject: Re: RFC: ioapic polarity vs. qemu os-x guest Date: Fri, 14 Feb 2014 22:21:09 +0100 Message-ID: References: <20140130204423.GK29329@ERROL.INI.CMU.EDU> <20140211182330.GC29329@ERROL.INI.CMU.EDU> <20140211195444.GB10951@redhat.com> <20140214211311.GH29329@ERROL.INI.CMU.EDU> Mime-Version: 1.0 (1.0) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8BIT Cc: "Michael S. Tsirkin" , "kvm@vger.kernel.org" , "qemu-devel@nongnu.org" , "eddie.dong@intel.com" To: "Gabriel L. Somlo" Return-path: Received: from cantor2.suse.de ([195.135.220.15]:33736 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752739AbaBNVVO convert rfc822-to-8bit (ORCPT ); Fri, 14 Feb 2014 16:21:14 -0500 In-Reply-To: <20140214211311.GH29329@ERROL.INI.CMU.EDU> Sender: kvm-owner@vger.kernel.org List-ID: > Am 14.02.2014 um 22:13 schrieb "Gabriel L. Somlo" : > >> 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.: > > 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; > --- > > 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). Can't you just turn the polarity around in the pci host adapter? Alex > > Thanks much for any ideas, > --Gabriel From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52167) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WEQCJ-0005bb-2y for qemu-devel@nongnu.org; Fri, 14 Feb 2014 16:21:30 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WEQCB-000664-P7 for qemu-devel@nongnu.org; Fri, 14 Feb 2014 16:21:23 -0500 Received: from cantor2.suse.de ([195.135.220.15]:52815 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WEQCB-00065b-FF for qemu-devel@nongnu.org; Fri, 14 Feb 2014 16:21:15 -0500 Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (1.0) From: Alexander Graf In-Reply-To: <20140214211311.GH29329@ERROL.INI.CMU.EDU> Date: Fri, 14 Feb 2014 22:21:09 +0100 Content-Transfer-Encoding: quoted-printable Message-Id: References: <20140130204423.GK29329@ERROL.INI.CMU.EDU> <20140211182330.GC29329@ERROL.INI.CMU.EDU> <20140211195444.GB10951@redhat.com> <20140214211311.GH29329@ERROL.INI.CMU.EDU> Subject: Re: [Qemu-devel] RFC: ioapic polarity vs. qemu os-x guest List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Gabriel L. Somlo" Cc: "eddie.dong@intel.com" , "qemu-devel@nongnu.org" , "kvm@vger.kernel.org" , "Michael S. Tsirkin" > Am 14.02.2014 um 22:13 schrieb "Gabriel L. Somlo" : >=20 >> 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 ? >>>=20 >>>=20 >>> 2. Is there anything in QEMU (besides the ACPI DSDT .dsl files) which >>> has a hardcoded assumption re. "polarity =3D=3D 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. >>>=20 >>>=20 >>> Any ideas or suggestions about the appropriate way to move forward would= >>> be much appreciated !!! >>>=20 >>>=20 >>> Thanks much, >>> --Gabriel >>=20 >> I think changing ACPI is the right thing to >> do really. But we'll need to fix some things >> first of course. >=20 > 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.: >=20 > 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, i= nt pic_irq) > int i, pic_level; >=20 > /* The pic level is the logical OR of all the PCI irqs mapped to it */= > - pic_level =3D 0; > + pic_level =3D 1; > for (i =3D 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 =3D=3D tmp_irq) { > - pic_level |=3D pci_bus_get_irq_level(lpc->d.bus, i); > + pic_level &=3D !pci_bus_get_irq_level(lpc->d.bus, i); > } > } > if (pic_irq =3D=3D ich9_lpc_sci_irq(lpc)) { > - pic_level |=3D lpc->sci_level; > + pic_level &=3D !lpc->sci_level; > } >=20 > qemu_set_irq(lpc->pic[pic_irq], pic_level); > -- >=20 > However, even on OS X, the Ethernet (e1000) card won't link up at all. > Fixing that requires another patch: >=20 > 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] =3D val; >=20 > pending_ints =3D (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 th= e > * interrupt line if we are inside the mitigation delay window > @@ -339,7 +339,7 @@ set_interrupt_cause(E1000State *s, int index, uint32_t= val) > } > } >=20 > - s->mit_irq_level =3D (pending_ints !=3D 0); > + s->mit_irq_level =3D (pending_ints =3D=3D 0); > pci_set_irq(d, s->mit_irq_level); > } >=20 > @@ -393,7 +393,7 @@ static void e1000_reset(void *opaque) > timer_del(d->autoneg_timer); > timer_del(d->mit_timer); > d->mit_timer_on =3D 0; > - d->mit_irq_level =3D 0; > + d->mit_irq_level =3D 1; > d->mit_ide =3D 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] =3D s->mac_reg[RDTR] =3D s->mac_reg[RADV] =3D > s->mac_reg[TADV] =3D 0; > - s->mit_irq_level =3D false; > + s->mit_irq_level =3D true; > } > s->mit_ide =3D 0; > s->mit_timer_on =3D false; > --- >=20 > At this point, I'm beginning to realize that the "ActiveHigh" > assumption is rather pervasively baked in throughout the QEMU > source code... >=20 > 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=3Don,polarity=3Dhigh", or some such). Can't you just turn the polarity around in the pci host adapter? Alex >=20 > Thanks much for any ideas, > --Gabriel