From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36996) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X2Hos-0006DR-SP for qemu-devel@nongnu.org; Wed, 02 Jul 2014 06:31:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X2Hon-0001Dq-4B for qemu-devel@nongnu.org; Wed, 02 Jul 2014 06:31:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37140) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X2Hom-0001De-S7 for qemu-devel@nongnu.org; Wed, 02 Jul 2014 06:31:13 -0400 Date: Wed, 2 Jul 2014 12:33:16 +0300 From: "Michael S. Tsirkin" Message-ID: <20140702093316.GA3692@redhat.com> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140702090207.GA2945@crash.ini.cmu.edu> 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: "Gabriel L. Somlo" Cc: "pbonzini@redhat.com" , Alexander Graf , "stefanha@redhat.com" , "qemu-devel@nongnu.org" 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? -- MST