From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60232) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X2JIW-0001rW-TW for qemu-devel@nongnu.org; Wed, 02 Jul 2014 08:06:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X2JIQ-0008I8-Of for qemu-devel@nongnu.org; Wed, 02 Jul 2014 08:06:00 -0400 Received: from mail-qc0-x232.google.com ([2607:f8b0:400d:c01::232]:49109) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X2JIQ-0008Hu-KI for qemu-devel@nongnu.org; Wed, 02 Jul 2014 08:05:54 -0400 Received: by mail-qc0-f178.google.com with SMTP id c9so9834894qcz.9 for ; Wed, 02 Jul 2014 05:05:54 -0700 (PDT) Date: Wed, 2 Jul 2014 08:05:46 -0400 From: "Gabriel L. Somlo" Message-ID: <20140702120545.GA3470@crash.ini.cmu.edu> References: <1404147350-28904-1-git-send-email-somlo@cmu.edu> <1404147350-28904-3-git-send-email-somlo@cmu.edu> <20140630175554.GA2871@redhat.com> <20140702090207.GA2945@crash.ini.cmu.edu> <20140702093316.GA3692@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140702093316.GA3692@redhat.com> Subject: Re: [Qemu-devel] [RFC PATCH v1 2/2] e1000: adjust initial autoneg timing (for piix/osx) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: "pbonzini@redhat.com" , Alexander Graf , "stefanha@redhat.com" , "qemu-devel@nongnu.org" On Wed, Jul 02, 2014 at 12:33:16PM +0300, Michael S. Tsirkin wrote: > On Wed, Jul 02, 2014 at 05:02:07AM -0400, Gabriel L. Somlo wrote: > > On Mon, Jun 30, 2014 at 08:21:43PM +0200, Alexander Graf wrote: > > > > > > > > > > Am 30.06.2014 um 19:55 schrieb "Michael S. Tsirkin" : > > > > > > > >> On Mon, Jun 30, 2014 at 12:55:50PM -0400, Gabriel L. Somlo wrote: > > > >> When running on PIIX (as opposed to q35), the stock OS X e1000 > > > >> driver (AppleIntel8254XEthernet.kext) takes longer to load and > > > >> activete, and will "miss" the link status change interrupt > > > >> injected when the emulated "hardware" autonegotiation completes > > > >> (see commit 39bb8ee737595e9b264d075dfcd7d86f4d3f1133). > > > >> > > > >> This patch extends the delay of the autonetotiation timer set up > > > >> during set_phy_ctrl() to a value just large enough to work with > > > >> the OS X driver. > > > >> > > > >> Signed-off-by: Gabriel Somlo > > > >> --- > > > >> > > > >> So, the loading OS X driver must take longer between its last > > > >> write to the PHY_CTRL register and the time it starts looking > > > >> for LSC interrupts, because at delay==500 it obviously misses > > > >> the relevant interrupt. Making this 5500 (actually anything > > > >> larger than 5300, but there's a bit of variation across OS X > > > >> versions, so I rounded up a bit) has the timer fire after > > > >> enough time has passed that the driver knows what to do when > > > >> the interrupt from the network card fires... > > > >> > > > >> Thanks, > > > >> Gabriel > > > >> > > > >> hw/net/e1000.c | 2 +- > > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > > >> > > > >> diff --git a/hw/net/e1000.c b/hw/net/e1000.c > > > >> index 2376910..2300477 100644 > > > >> --- a/hw/net/e1000.c > > > >> +++ b/hw/net/e1000.c > > > >> @@ -209,7 +209,7 @@ set_phy_ctrl(E1000State *s, int index, uint16_t val) > > > >> e1000_link_down(s); > > > >> DBGOUT(PHY, "Start link auto negotiation\n"); > > > >> timer_mod(s->autoneg_timer, > > > >> - qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 500); > > > >> + qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 5500); > > > >> } > > > >> } > > > > > > > > > > > > Besides being a bit hacky, it actually has a decent chance > > > > to delay boot for guests. 500ms is probably the max we > > > > can reasonably tolerate, even that is a bit high. > > > > > > Are you sure there's not just simply some irq unmasking event after 5500ms we don't handle properly? > > > > I poked around a bit, and the e1000 interrupt mask register is NOT the > > problem (the LSC mask bit is clear at all times). If anything, maybe > > the PIIX southbridge (or something further up "north") is masking PCI > > interrupts (at least from e1000) until roughly 5500 ms into the boot > > process ? Any ideas on how I could go about verifying this (without > > access to the guest source, obviously :) ) would be very helpful... > > > > Thanks, > > --Gabriel > > > Just poking around the spec I find more things > we don't implement correctly wrt to auto-negotiation. > For example, MII_SR_AUTONEG_CAPS isn't set, is it? > Maybe that's why your guest doesn't work: > it doesn't expect to get autonegotation at all? > > So I have a question: does your patch actually help any guests? > If not, maybe we should defer it to after release, > and try to clean up autonegotiation more thouroughly for 2.2? The patch you just accepted (handling self clearing + reserved bits for phy_ctrl) was just "collateral cleanup" in my attempts to fix the suspected interrupt masking issue on PIIX. As such, either now or waiting for 2.2 is fine with me. Not sure I'll be able to figure out the real/main issue while 2.1 bugfixes are still being accepted, but *that* would actually be really nice :) Thanks, --Gabriel