From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57254) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YQDlg-0005xP-D0 for qemu-devel@nongnu.org; Tue, 24 Feb 2015 06:35:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YQDld-0007x4-5s for qemu-devel@nongnu.org; Tue, 24 Feb 2015 06:35:12 -0500 Received: from mail-we0-x22c.google.com ([2a00:1450:400c:c03::22c]:33056) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YQDlc-0007wJ-S8 for qemu-devel@nongnu.org; Tue, 24 Feb 2015 06:35:09 -0500 Received: by wevk48 with SMTP id k48so24597742wev.0 for ; Tue, 24 Feb 2015 03:35:08 -0800 (PST) Date: Tue, 24 Feb 2015 11:35:05 +0000 From: Stefan Hajnoczi Message-ID: <20150224113505.GB29466@stefanha-thinkpad.redhat.com> References: <1424373859-2019-1-git-send-email-rkrcmar@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="24zk1gE8NUlDmwG9" Content-Disposition: inline In-Reply-To: <1424373859-2019-1-git-send-email-rkrcmar@redhat.com> Subject: Re: [Qemu-devel] [PATCH] e1000: work around win 8.0 boot hang List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Radim =?utf-8?B?S3LEjW3DocWZ?= Cc: alexander.h.duyck@intel.com, qemu-devel@nongnu.org, Wei Huang --24zk1gE8NUlDmwG9 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Feb 19, 2015 at 08:24:19PM +0100, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: > Window 8.0 driver has a particular behavior for a small time frame after > it enables rx interrupts: the interrupt handler never clears > E1000_ICR_RXT0. The handler does this something like this: > set_imc(-1) (1) disable all interrupts > val =3D read_icr() (2) clear ICR > handled =3D magic(val) (3) do nothing to E1000_ICR_RXT0 > set_ics(val & ~handled) (4) set unhandled interrupts back to ICR > set_ims(157) (5) enable some interrupts >=20 > so if we started with RXT0, then every time the handler re-enables e1000 > interrupts, it receives one. This likely wouldn't matter in real > hardware, because it is slow enough to make some progress between > interrupts, but KVM instantly interrupts it, and boot hangs. > (If we have multiple VCPUs, the interrupt gets load-balanced and > everything is fine.) >=20 > I haven't found any problem in earlier phase of initialization and > windows writes 0 to RADV and RDTR, so some workaround looks like the > only way if we want to support win8.0 on uniprocessors. (I vote NO.) >=20 > This workaround uses the fact that a constant is cleared from ICR and > later set back to it. After detecting this situation, we reuse the > mitigation framework to inject an interrupt 10 microseconds later. > (It's not exactly 10 microseconds, to keep the existing logic intact.) >=20 > The detection is done by checking at (1), (2), and (5). (2) and (5) > require that the only bit in ICR is RXT0. We could also check at (4), > and on writes to any other register, but it would most likely only add > more useless code, because normal operations shouldn't behave like that > anyway. (An OS that deliberately keeps bits in ICR to notify itself > that there are more packets, or for more creative reasons, is nothing we > should care about.) >=20 > Signed-off-by: Radim Kr=C4=8Dm=C3=A1=C5=99 > --- > The patch is still untested -- it only approximates the behavior of RHEL > patches that worked, I'll try to get a reproducer ... >=20 > hw/net/e1000.c | 29 ++++++++++++++++++++++------- > 1 file changed, 22 insertions(+), 7 deletions(-) Hi Alex, I've CCed you in case you have any advice regarding QEMU's e1000 emulation. It seems Windows 8 gets itself into a kind of interrupt storm and a workaround in QEMU will be necessary. Any thoughts? Thanks, Stefan > diff --git a/hw/net/e1000.c b/hw/net/e1000.c > index a207e21bcf77..773aac47f0b2 100644 > --- a/hw/net/e1000.c > +++ b/hw/net/e1000.c > @@ -138,6 +138,10 @@ typedef struct E1000State_st { > #define E1000_FLAG_AUTONEG (1 << E1000_FLAG_AUTONEG_BIT) > #define E1000_FLAG_MIT (1 << E1000_FLAG_MIT_BIT) > uint32_t compat_flags; > + > +#define E1000_WIN8_WORKAROUND_ICR E1000_ICR_RXT0 > +#define E1000_WIN8_WORKAROUND_DELAY_US 10 > + bool win8_workaround_needed; > } E1000State; > =20 > typedef struct E1000BaseClass { > @@ -288,7 +292,7 @@ set_interrupt_cause(E1000State *s, int index, uint32_= t val) > { > PCIDevice *d =3D PCI_DEVICE(s); > uint32_t pending_ints; > - uint32_t mit_delay; > + uint32_t mit_delay =3D 0; > =20 > s->mac_reg[ICR] =3D val; > =20 > @@ -316,13 +320,17 @@ set_interrupt_cause(E1000State *s, int index, uint3= 2_t val) > if (s->mit_timer_on) { > return; > } > + > + if (s->win8_workround_needed) { > + mit_delay =3D E1000_WIN8_WORKAROUND_DELAY_US * 4; > + } > + > if (s->compat_flags & E1000_FLAG_MIT) { > /* Compute the next mitigation delay according to pending > * interrupts and the current values of RADV (provided > * RDTR!=3D0), TADV and ITR. > * Then rearm the timer. > */ > - mit_delay =3D 0; > if (s->mit_ide && > (pending_ints & (E1000_ICR_TXQE | E1000_ICR_TXDW))) { > mit_update_delay(&mit_delay, s->mac_reg[TADV] * 4); > @@ -332,13 +340,14 @@ set_interrupt_cause(E1000State *s, int index, uint3= 2_t val) > } > mit_update_delay(&mit_delay, s->mac_reg[ITR]); > =20 > - if (mit_delay) { > - s->mit_timer_on =3D 1; > - timer_mod(s->mit_timer, qemu_clock_get_ns(QEMU_CLOCK_VIR= TUAL) + > - mit_delay * 256); > - } > s->mit_ide =3D 0; > } > + > + if (mit_delay) { > + s->mit_timer_on =3D 1; > + timer_mod(s->mit_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL= ) + > + mit_delay * 256); > + } > } > =20 > s->mit_irq_level =3D (pending_ints !=3D 0); > @@ -411,6 +420,7 @@ static void e1000_reset(void *opaque) > d->mit_timer_on =3D 0; > d->mit_irq_level =3D 0; > d->mit_ide =3D 0; > + d->win8_workaround_needed =3D false; > memset(d->phy_reg, 0, sizeof d->phy_reg); > memmove(d->phy_reg, phy_reg_init, sizeof phy_reg_init); > d->phy_reg[PHY_ID2] =3D edc->phy_id2; > @@ -1114,6 +1124,8 @@ mac_icr_read(E1000State *s, int index) > { > uint32_t ret =3D s->mac_reg[ICR]; > =20 > + s->win8_workaround_needed &=3D ret =3D=3D E1000_WIN8_WORKAROUND_ICR; > + > DBGOUT(INTERRUPT, "ICR read: %x\n", ret); > set_interrupt_cause(s, 0, 0); > return ret; > @@ -1192,6 +1204,7 @@ static void > set_imc(E1000State *s, int index, uint32_t val) > { > s->mac_reg[IMS] &=3D ~val; > + s->win8_workaround_needed =3D ~val =3D=3D 0; > set_ics(s, 0, 0); > } > =20 > @@ -1199,7 +1212,9 @@ static void > set_ims(E1000State *s, int index, uint32_t val) > { > s->mac_reg[IMS] |=3D val; > + s->win8_workaround_needed &=3D s->mac_reg[ICR] =3D=3D E1000_WIN8_WOR= KAROUND_ICR; > set_ics(s, 0, 0); > + s->win8_workaround_needed =3D false; > } > =20 > #define getreg(x) [x] =3D mac_readreg > --=20 > 2.3.0 >=20 >=20 --24zk1gE8NUlDmwG9 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJU7GHpAAoJEJykq7OBq3PIoawH/jSgjUIQYRwFHmA2vKOKIkmG 01UvH8TdIb7iTfyiFYPfxMtZ9nOUS/70b1mrs4iHfOOXvhCrgtQUdb1eQEp6M7YU bGuqebWKjQpNEUiaToxLcxBkvq0OdY42sBvWA2/pcPSK9H5RxJT1J/F0b1aeQVNY iTN5lxNUjmuyDDDKRzj3SD21hosCvYg/SGRCJlZx10tTwYE8BdnuTOzEQuB4pQTS 0aThkFEZ98AHOJXNnXqUW9B1fhj8IMWpornrpkEdrw7a6YlFCAUQcM6pWNLPliHm j8370dpnHkWov6yo8g9CcyO+afdboQGyk7XafTPo/KjRzKRN6rvuhmfCRcJeE60= =ax7s -----END PGP SIGNATURE----- --24zk1gE8NUlDmwG9--