| Message ID | 20140214211311.GH29329@ERROL.INI.CMU.EDU (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> Am 14.02.2014 um 22:13 schrieb "Gabriel L. Somlo" <gsomlo@gmail.com>: > >> 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 -- 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
On Fri, Feb 14, 2014 at 04:13:11PM -0500, Gabriel L. Somlo wrote: > 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; Hmm no this is all wrong, from API point of view, devices shoud not care about value of interrupt. They just assert/deassert interrupts. It so happens that 1 means assert 0 means deassert. It's the PCI host or the PIC that needs to flip it. It can't be host ATM because PIC does the logical OR so it must be encoded as 1 == assert. So how about simply - qemu_set_irq(lpc->pic[pic_irq], pic_level); + qemu_set_irq(lpc->pic[pic_irq], !pic_level); plus the ACPI tweaks ... ? > --- > > 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 I don't think we need to make this configurable unless there's real hardware that makes PCI active low. Don't give up yet :)
On Fri, Feb 14, 2014 at 04:13:11PM -0500, Gabriel L. Somlo wrote: > 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); > } > If we really wanted to change it, we could change pci_set_irq to reverse the polarity. But I think doing this in PIC is cleaner. > @@ -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). > > 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
On 16 February 2014 11:34, Michael S. Tsirkin <mst@redhat.com> wrote: > Hmm no this is all wrong, from API point of view, > devices shoud not care about value of interrupt. > They just assert/deassert interrupts. > It so happens that 1 means assert 0 means deassert. Yeah, we generally model things as active-high even if the hardware really treats the signal as active-low. (Among other things there are some issues around how exactly device reset should interact with a signal that is supposed to be high coming out of reset, given you don't know whether the device at the other end of the line has reset yet or not.) This is great up until the point where you have a generic GPIO device one of whose GPIO output lines happens to be wired to an interrupt controller, of course. thanks -- PMM -- 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;