From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:60826) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QvXPq-0008GA-4c for qemu-devel@nongnu.org; Mon, 22 Aug 2011 12:31:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QvXPo-0003Aq-Rt for qemu-devel@nongnu.org; Mon, 22 Aug 2011 12:31:58 -0400 Received: from mail-yi0-f45.google.com ([209.85.218.45]:55755) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QvXPo-00039f-Gu for qemu-devel@nongnu.org; Mon, 22 Aug 2011 12:31:56 -0400 Received: by yih10 with SMTP id 10so4383175yih.4 for ; Mon, 22 Aug 2011 09:31:56 -0700 (PDT) Message-ID: <4E52847A.1090701@codemonkey.ws> Date: Mon, 22 Aug 2011 11:31:54 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1313571794-9087-1-git-send-email-bjorn@mork.no> In-Reply-To: <1313571794-9087-1-git-send-email-bjorn@mork.no> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH] e1000: use MII status register for link up/down List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QmrDuHJuIE1vcms=?= Cc: qemu-devel@nongnu.org On 08/17/2011 04:03 AM, Bjørn Mork wrote: > Some guests will use the standard MII status register > to verify link state. They will not notice link changes > unless this register is updated. > > Verified with Linux 3.0 and Windows XP guests. > > Without this patch, ethtool will report speed and duplex as > unknown when the link is down, but still report the link as > up. This is because the Linux e1000 driver checks the > mac_reg[STATUS] register link state before it checks speed > and duplex, but uses the phy_reg[PHY_STATUS] register for > the actual link state check. Fix by updating both registers > on link state changes. > > Linux guest before: > > (qemu) set_link e1000.0 off > > kvm-sid:~# ethtool eth0 > Settings for eth0: > Supported ports: [ TP ] > Supported link modes: 10baseT/Half 10baseT/Full > 100baseT/Half 100baseT/Full > 1000baseT/Full > Supports auto-negotiation: Yes > Advertised link modes: 10baseT/Half 10baseT/Full > 100baseT/Half 100baseT/Full > 1000baseT/Full > Advertised pause frame use: No > Advertised auto-negotiation: Yes > Speed: Unknown! > Duplex: Unknown! (255) > Port: Twisted Pair > PHYAD: 0 > Transceiver: internal > Auto-negotiation: on > MDI-X: Unknown > Supports Wake-on: umbg > Wake-on: d > Current message level: 0x00000007 (7) > drv probe link > Link detected: yes > > (qemu) set_link e1000.0 on > > Linux guest after: > > (qemu) set_link e1000.0 off > [ 63.384221] e1000: eth0 NIC Link is Down > > kvm-sid:~# ethtool eth0 > Settings for eth0: > Supported ports: [ TP ] > Supported link modes: 10baseT/Half 10baseT/Full > 100baseT/Half 100baseT/Full > 1000baseT/Full > Supports auto-negotiation: Yes > Advertised link modes: 10baseT/Half 10baseT/Full > 100baseT/Half 100baseT/Full > 1000baseT/Full > Advertised pause frame use: No > Advertised auto-negotiation: Yes > Speed: Unknown! > Duplex: Unknown! (255) > Port: Twisted Pair > PHYAD: 0 > Transceiver: internal > Auto-negotiation: on > MDI-X: Unknown > Supports Wake-on: umbg > Wake-on: d > Current message level: 0x00000007 (7) > drv probe link > Link detected: no > > (qemu) set_link e1000.0 on > [ 84.304582] e1000: eth0 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX > > Signed-off-by: Bjørn Mork Applied. Thanks. Regards, Anthony Liguori > --- > hw/e1000.c | 7 +++++-- > hw/e1000_hw.h | 17 +++++++++++++++++ > 2 files changed, 22 insertions(+), 2 deletions(-) > > diff --git a/hw/e1000.c b/hw/e1000.c > index 29b453f..a6d12c5 100644 > --- a/hw/e1000.c > +++ b/hw/e1000.c > @@ -617,10 +617,13 @@ e1000_set_link_status(VLANClientState *nc) > E1000State *s = DO_UPCAST(NICState, nc, nc)->opaque; > uint32_t old_status = s->mac_reg[STATUS]; > > - if (nc->link_down) > + if (nc->link_down) { > s->mac_reg[STATUS]&= ~E1000_STATUS_LU; > - else > + s->phy_reg[PHY_STATUS]&= ~MII_SR_LINK_STATUS; > + } else { > s->mac_reg[STATUS] |= E1000_STATUS_LU; > + s->phy_reg[PHY_STATUS] |= MII_SR_LINK_STATUS; > + } > > if (s->mac_reg[STATUS] != old_status) > set_ics(s, 0, E1000_ICR_LSC); > diff --git a/hw/e1000_hw.h b/hw/e1000_hw.h > index 9bd8a4b..2e341ac 100644 > --- a/hw/e1000_hw.h > +++ b/hw/e1000_hw.h > @@ -349,6 +349,23 @@ > #define M88E1000_PHY_VCO_REG_BIT8 0x100 /* Bits 8& 11 are adjusted for */ > #define M88E1000_PHY_VCO_REG_BIT11 0x800 /* improved BER performance */ > > +/* PHY Status Register */ > +#define MII_SR_EXTENDED_CAPS 0x0001 /* Extended register capabilities */ > +#define MII_SR_JABBER_DETECT 0x0002 /* Jabber Detected */ > +#define MII_SR_LINK_STATUS 0x0004 /* Link Status 1 = link */ > +#define MII_SR_AUTONEG_CAPS 0x0008 /* Auto Neg Capable */ > +#define MII_SR_REMOTE_FAULT 0x0010 /* Remote Fault Detect */ > +#define MII_SR_AUTONEG_COMPLETE 0x0020 /* Auto Neg Complete */ > +#define MII_SR_PREAMBLE_SUPPRESS 0x0040 /* Preamble may be suppressed */ > +#define MII_SR_EXTENDED_STATUS 0x0100 /* Ext. status info in Reg 0x0F */ > +#define MII_SR_100T2_HD_CAPS 0x0200 /* 100T2 Half Duplex Capable */ > +#define MII_SR_100T2_FD_CAPS 0x0400 /* 100T2 Full Duplex Capable */ > +#define MII_SR_10T_HD_CAPS 0x0800 /* 10T Half Duplex Capable */ > +#define MII_SR_10T_FD_CAPS 0x1000 /* 10T Full Duplex Capable */ > +#define MII_SR_100X_HD_CAPS 0x2000 /* 100X Half Duplex Capable */ > +#define MII_SR_100X_FD_CAPS 0x4000 /* 100X Full Duplex Capable */ > +#define MII_SR_100T4_CAPS 0x8000 /* 100T4 Capable */ > + > /* Interrupt Cause Read */ > #define E1000_ICR_TXDW 0x00000001 /* Transmit desc written back */ > #define E1000_ICR_TXQE 0x00000002 /* Transmit Queue empty */