On Thu, Jun 29, 2023 at 4:53 PM Akihiko Odaki <[email protected]> wrote: > > On 2023/06/02 16:25, Akihiko Odaki wrote: > > The datasheet does not say what happens when interrupt was asserted > > (ICR.INT_ASSERT=1) and auto mask is *not* active. > > However, section of 13.3.27 the PCIe* GbE Controllers Open Source > > Software Developer’s Manual, which were written for older devices, > > namely 631xESB/632xESB, 82563EB/82564EB, 82571EB/82572EI & > > 82573E/82573V/82573L, does say: > >> If IMS = 0b, then the ICR register is always clear-on-read. If IMS is > >> not 0b, but some ICR bit is set where the corresponding IMS bit is not > >> set, then a read does not clear the ICR register. For example, if > >> IMS = 10101010b and ICR = 01010101b, then a read to the ICR register > >> does not clear it. If IMS = 10101010b and ICR = 0101011b, then a read > >> to the ICR register clears it entirely (ICR.INT_ASSERTED = 1b). > > > > Linux does no longer activate auto mask since commit > > 0a8047ac68e50e4ccbadcfc6b6b070805b976885 and the real hardware clears > > ICR even in such a case so we also should do so. > > > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1707441 > > Signed-off-by: Andrew Melnychenko <[email protected]> > > Signed-off-by: Akihiko Odaki <[email protected]> > > --- > > Supersedes: <[email protected]> > > ("[PATCH v2] e1000e: Added ICR clearing by corresponding IMS bit.") > > > > hw/net/e1000e_core.c | 38 ++++++++++++++++++++++++++++++++------ > > hw/net/trace-events | 1 + > > 2 files changed, 33 insertions(+), 6 deletions(-) > > > > diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c > > index 9785ef279c..338bbbf4f4 100644 > > --- a/hw/net/e1000e_core.c > > +++ b/hw/net/e1000e_core.c > > @@ -2607,12 +2607,38 @@ e1000e_mac_icr_read(E1000ECore *core, int index) > > e1000e_lower_interrupts(core, ICR, 0xffffffff); > > } > > > > - if ((core->mac[ICR] & E1000_ICR_ASSERTED) && > > - (core->mac[CTRL_EXT] & E1000_CTRL_EXT_IAME)) { > > - trace_e1000e_irq_icr_clear_iame(); > > - e1000e_lower_interrupts(core, ICR, 0xffffffff); > > - trace_e1000e_irq_icr_process_iame(); > > - e1000e_lower_interrupts(core, IMS, core->mac[IAM]); > > + if (core->mac[ICR] & E1000_ICR_ASSERTED) { > > + if (core->mac[CTRL_EXT] & E1000_CTRL_EXT_IAME) { > > + trace_e1000e_irq_icr_clear_iame(); > > + e1000e_lower_interrupts(core, ICR, 0xffffffff); > > + trace_e1000e_irq_icr_process_iame(); > > + e1000e_lower_interrupts(core, IMS, core->mac[IAM]); > > + } > > + > > + /* > > + * The datasheet does not say what happens when interrupt was > > asserted > > + * (ICR.INT_ASSERT=1) and auto mask is *not* active. > > + * However, section of 13.3.27 the PCIe* GbE Controllers Open > > Source > > + * Software Developer’s Manual, which were written for older > > devices, > > + * namely 631xESB/632xESB, 82563EB/82564EB, 82571EB/82572EI & > > + * 82573E/82573V/82573L, does say: > > + * > If IMS = 0b, then the ICR register is always clear-on-read. > > If IMS > > + * > is not 0b, but some ICR bit is set where the corresponding > > IMS bit > > + * > is not set, then a read does not clear the ICR register. For > > + * > example, if IMS = 10101010b and ICR = 01010101b, then a read > > to the > > + * > ICR register does not clear it. If IMS = 10101010b and > > + * > ICR = 0101011b, then a read to the ICR register clears it > > entirely > > + * > (ICR.INT_ASSERTED = 1b). > > + * > > + * Linux does no longer activate auto mask since commit > > + * 0a8047ac68e50e4ccbadcfc6b6b070805b976885 and the real hardware > > + * clears ICR even in such a case so we also should do so. > > + */ > > + if (core->mac[ICR] & core->mac[IMS]) { > > + trace_e1000e_irq_icr_clear_icr_bit_ims(core->mac[ICR], > > + core->mac[IMS]); > > + e1000e_lower_interrupts(core, ICR, 0xffffffff); > > + } > > } > > > > return ret; > > diff --git a/hw/net/trace-events b/hw/net/trace-events > > index e97e9dc17b..9103488e17 100644 > > --- a/hw/net/trace-events > > +++ b/hw/net/trace-events > > @@ -217,6 +217,7 @@ e1000e_irq_read_ims(uint32_t ims) "Current IMS: 0x%x" > > e1000e_irq_icr_clear_nonmsix_icr_read(void) "Clearing ICR on read due to > > non MSI-X int" > > e1000e_irq_icr_clear_zero_ims(void) "Clearing ICR on read due to zero IMS" > > e1000e_irq_icr_clear_iame(void) "Clearing ICR on read due to IAME" > > +e1000e_irq_icr_clear_icr_bit_ims(uint32_t icr, uint32_t ims) "Clearing ICR > > on read due corresponding IMS bit: 0x%x & 0x%x" > > e1000e_irq_iam_clear_eiame(uint32_t iam, uint32_t cause) "Clearing IMS > > due to EIAME, IAM: 0x%X, cause: 0x%X" > > e1000e_irq_icr_clear_eiac(uint32_t icr, uint32_t eiac) "Clearing ICR bits > > due to EIAC, ICR: 0x%X, EIAC: 0x%X" > > e1000e_irq_ims_clear_set_imc(uint32_t val) "Clearing IMS bits due to IMC > > write 0x%x" > > Hi Jason, > > Can you have a look at this patch and > "[PATCH] igb: Remove obsolete workaround for Windows": > https://patchew.org/QEMU/[email protected]/ > > Regards, > Akihiko Odaki
I've queued both of the patches. Thanks >
