Skip to contentSkip to conversation

e1000: reset handling

PATCH In review Michael S. Tsirkin 2 participants 4 messages Sep 27, 2011

lore
Share & tools
lore
#1 lore raw

PATCH 1/2 e1000: minor reset code refactoring

In reply to Michael S. Tsirkin

Move reset callback to start of file to make it easier to reuse it elsewhere in code.

---
 hw/e1000.c |   92 ++++++++++++++++++++++++++++++------------------------------
 1 files changed, 46 insertions(+), 46 deletions(-)
hw/e1000.c +46 -46
index 6a3a941..87a1104 100644
@@ -151,6 +151,40 @@ static const char phy_regcap[0x20] = {
     [PHY_ID2] = PHY_R,		[M88E1000_PHY_SPEC_STATUS] = PHY_R
 };
 
+static const uint16_t e1000_eeprom_template[64] = {
+    0x0000, 0x0000, 0x0000, 0x0000,      0xffff, 0x0000,      0x0000, 0x0000,
+    0x3000, 0x1000, 0x6403, E1000_DEVID, 0x8086, E1000_DEVID, 0x8086, 0x3040,
+    0x0008, 0x2000, 0x7e14, 0x0048,      0x1000, 0x00d8,      0x0000, 0x2700,
+    0x6cc9, 0x3150, 0x0722, 0x040b,      0x0984, 0x0000,      0xc000, 0x0706,
+    0x1008, 0x0000, 0x0f04, 0x7fff,      0x4d01, 0xffff,      0xffff, 0xffff,
+    0xffff, 0xffff, 0xffff, 0xffff,      0xffff, 0xffff,      0xffff, 0xffff,
+    0x0100, 0x4000, 0x121c, 0xffff,      0xffff, 0xffff,      0xffff, 0xffff,
+    0xffff, 0xffff, 0xffff, 0xffff,      0xffff, 0xffff,      0xffff, 0x0000,
+};
+
+static const uint16_t phy_reg_init[] = {
+    [PHY_CTRL] = 0x1140,			[PHY_STATUS] = 0x796d, // link initially up
+    [PHY_ID1] = 0x141,				[PHY_ID2] = PHY_ID2_INIT,
+    [PHY_1000T_CTRL] = 0x0e00,			[M88E1000_PHY_SPEC_CTRL] = 0x360,
+    [M88E1000_EXT_PHY_SPEC_CTRL] = 0x0d60,	[PHY_AUTONEG_ADV] = 0xde1,
+    [PHY_LP_ABILITY] = 0x1e0,			[PHY_1000T_STATUS] = 0x3c00,
+    [M88E1000_PHY_SPEC_STATUS] = 0xac00,
+};
+
+static const uint32_t mac_reg_init[] = {
+    [PBA] =     0x00100030,
+    [LEDCTL] =  0x602,
+    [CTRL] =    E1000_CTRL_SWDPIN2 | E1000_CTRL_SWDPIN0 |
+                E1000_CTRL_SPD_1000 | E1000_CTRL_SLU,
+    [STATUS] =  0x80000000 | E1000_STATUS_GIO_MASTER_ENABLE |
+                E1000_STATUS_ASDV | E1000_STATUS_MTXCKOK |
+                E1000_STATUS_SPEED_1000 | E1000_STATUS_FD |
+                E1000_STATUS_LU,
+    [MANC] =    E1000_MANC_EN_MNG2HOST | E1000_MANC_RCV_TCO_EN |
+                E1000_MANC_ARP_EN | E1000_MANC_0298_EN |
+                E1000_MANC_RMCP_EN,
+};
+
 static void
 set_interrupt_cause(E1000State *s, int index, uint32_t val)
 {
@@ -192,6 +226,18 @@ rxbufsize(uint32_t v)
     return 2048;
 }
 
+static void e1000_reset(void *opaque)
+{
+    E1000State *d = opaque;
+
+    memset(d->phy_reg, 0, sizeof d->phy_reg);
+    memmove(d->phy_reg, phy_reg_init, sizeof phy_reg_init);
+    memset(d->mac_reg, 0, sizeof d->mac_reg);
+    memmove(d->mac_reg, mac_reg_init, sizeof mac_reg_init);
+    d->rxbuf_min_shift = 1;
+    memset(&d->tx, 0, sizeof d->tx);
+}
+
 static void
 set_ctrl(E1000State *s, int index, uint32_t val)
 {
@@ -1047,40 +1093,6 @@ static const VMStateDescription vmstate_e1000 = {
     }
 };
 
-static const uint16_t e1000_eeprom_template[64] = {
-    0x0000, 0x0000, 0x0000, 0x0000,      0xffff, 0x0000,      0x0000, 0x0000,
-    0x3000, 0x1000, 0x6403, E1000_DEVID, 0x8086, E1000_DEVID, 0x8086, 0x3040,
-    0x0008, 0x2000, 0x7e14, 0x0048,      0x1000, 0x00d8,      0x0000, 0x2700,
-    0x6cc9, 0x3150, 0x0722, 0x040b,      0x0984, 0x0000,      0xc000, 0x0706,
-    0x1008, 0x0000, 0x0f04, 0x7fff,      0x4d01, 0xffff,      0xffff, 0xffff,
-    0xffff, 0xffff, 0xffff, 0xffff,      0xffff, 0xffff,      0xffff, 0xffff,
-    0x0100, 0x4000, 0x121c, 0xffff,      0xffff, 0xffff,      0xffff, 0xffff,
-    0xffff, 0xffff, 0xffff, 0xffff,      0xffff, 0xffff,      0xffff, 0x0000,
-};
-
-static const uint16_t phy_reg_init[] = {
-    [PHY_CTRL] = 0x1140,			[PHY_STATUS] = 0x796d, // link initially up
-    [PHY_ID1] = 0x141,				[PHY_ID2] = PHY_ID2_INIT,
-    [PHY_1000T_CTRL] = 0x0e00,			[M88E1000_PHY_SPEC_CTRL] = 0x360,
-    [M88E1000_EXT_PHY_SPEC_CTRL] = 0x0d60,	[PHY_AUTONEG_ADV] = 0xde1,
-    [PHY_LP_ABILITY] = 0x1e0,			[PHY_1000T_STATUS] = 0x3c00,
-    [M88E1000_PHY_SPEC_STATUS] = 0xac00,
-};
-
-static const uint32_t mac_reg_init[] = {
-    [PBA] =     0x00100030,
-    [LEDCTL] =  0x602,
-    [CTRL] =    E1000_CTRL_SWDPIN2 | E1000_CTRL_SWDPIN0 |
-                E1000_CTRL_SPD_1000 | E1000_CTRL_SLU,
-    [STATUS] =  0x80000000 | E1000_STATUS_GIO_MASTER_ENABLE |
-                E1000_STATUS_ASDV | E1000_STATUS_MTXCKOK |
-                E1000_STATUS_SPEED_1000 | E1000_STATUS_FD |
-                E1000_STATUS_LU,
-    [MANC] =    E1000_MANC_EN_MNG2HOST | E1000_MANC_RCV_TCO_EN |
-                E1000_MANC_ARP_EN | E1000_MANC_0298_EN |
-                E1000_MANC_RMCP_EN,
-};
-
 /* PCI interface */
 
 static void
@@ -1120,18 +1132,6 @@ pci_e1000_uninit(PCIDevice *dev)
     return 0;
 }
 
-static void e1000_reset(void *opaque)
-{
-    E1000State *d = opaque;
-
-    memset(d->phy_reg, 0, sizeof d->phy_reg);
-    memmove(d->phy_reg, phy_reg_init, sizeof phy_reg_init);
-    memset(d->mac_reg, 0, sizeof d->mac_reg);
-    memmove(d->mac_reg, mac_reg_init, sizeof mac_reg_init);
-    d->rxbuf_min_shift = 1;
-    memset(&d->tx, 0, sizeof d->tx);
-}
-
 static NetClientInfo net_e1000_info = {
     .type = NET_CLIENT_TYPE_NIC,
     .size = sizeof(NICState),
Signature · 1 line
1.7.5.53.gc233e
#0 lore raw

cover e1000: reset handling

Here's a fix for e1000 reset as suggested by Andy Gospodarek and Anthony PERARD, written in a way that doesn't require forward declarations, and with reset clearing interrupts.

Changes from v1:
- value written was stored in CTRL register so it will be non 0 after reset.

Notes: some internal state:

    uint32_t rxbuf_size;
    int check_rxov;

isn't cleared in reset need to check that it's ok.

Signature · 1 line
MST
#2 lore raw

PATCH 2/2 e1000: CTRL.RST emulation

e1000 spec says CTRL.RST write should have the same effect as bus reset, except that is preserves PCI Config. Reset device registers and interrupts.

Fix suggested by Andy Gospodarek <[email protected]> Similar fix proposed by Anthony PERARD <[email protected]>

---
 hw/e1000.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)
hw/e1000.c +7 -2
index 87a1104..b51e089 100644
@@ -241,8 +241,13 @@ static void e1000_reset(void *opaque)
 static void
 set_ctrl(E1000State *s, int index, uint32_t val)
 {
-    /* RST is self clearing */
-    s->mac_reg[CTRL] = val & ~E1000_CTRL_RST;
+    if (val & E1000_CTRL_RST) {
+        e1000_reset(s);
+        qemu_set_irq(s->dev.irq[0], 0);
+        return;
+    }
+
+    s->mac_reg[CTRL] = val;
 }
 
 static void
Signature · 1 line
1.7.5.53.gc233e
#3 lore raw

Re: [Qemu-devel] [PATCH 2/2] e1000: CTRL.RST emulation

Quoted 31 lines · Michael S. Tsirkin

e1000 spec says CTRL.RST write should have the same effect as bus reset, except that is preserves PCI Config. Reset device registers and interrupts.

Fix suggested by Andy Gospodarek <[email protected]> Similar fix proposed by Anthony PERARD <[email protected]>

---
 hw/e1000.c |    9 +++++++--

 1 files changed, 7 insertions(+), 2 deletions(-)
hw/e1000.c +0 -0
index 87a1104..b51e089 100644
@@ -241,8 +241,13 @@ static void e1000_reset(void *opaque)

 static void
 set_ctrl(E1000State *s, int index, uint32_t val) {
-    /* RST is self clearing */
-    s->mac_reg[CTRL] = val & ~E1000_CTRL_RST;
+    if (val & E1000_CTRL_RST) {
+        e1000_reset(s);
+        qemu_set_irq(s->dev.irq[0], 0);
+        return;
+    }
+
+    s->mac_reg[CTRL] = val;
 }

Quoted 4 lines

 static void
--
1.7.5.53.gc233e

Looks good to me. Thanks for following up with this, Michael.