All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/2] e1000: reset handling
@ 2011-09-27 12:58 Michael S. Tsirkin
  2011-09-27 12:58 ` [Qemu-devel] [PATCH 1/2] e1000: minor reset code refactoring Michael S. Tsirkin
  2011-09-27 12:58 ` [Qemu-devel] [PATCH 2/2] e1000: CTRL.RST emulation Michael S. Tsirkin
  0 siblings, 2 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2011-09-27 12:58 UTC (permalink / raw)
  To: qemu-devel, Andy Gospodarek, Dean Nelson, Jesse Brandeburg, Jeff Kirsher
  Cc: peter.maydell, Anthony Liguori, Michael S. Tsirkin,
	Alex Williamson, Anthony PERARD, Kevin Wolf, Aurelien Jarno

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.

-- 
MST

^ permalink raw reply	[flat|nested] 4+ messages in thread
* [Qemu-devel] [PATCH 1/2] e1000: minor reset code refactoring
  2011-09-27 12:58 [Qemu-devel] [PATCH 0/2] e1000: reset handling Michael S. Tsirkin
@ 2011-09-27 12:58 ` Michael S. Tsirkin
  2011-09-27 12:58 ` [Qemu-devel] [PATCH 2/2] e1000: CTRL.RST emulation Michael S. Tsirkin
  1 sibling, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2011-09-27 12:58 UTC (permalink / raw)
  To: qemu-devel, Andy Gospodarek, Dean Nelson, Jesse Brandeburg, Jeff Kirsher
  Cc: peter.maydell, Anthony Liguori, Michael S. Tsirkin,
	Alex Williamson, Anthony PERARD, Kevin Wolf, Aurelien Jarno

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

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/e1000.c |   92 ++++++++++++++++++++++++++++++------------------------------
 1 files changed, 46 insertions(+), 46 deletions(-)

diff --git a/hw/e1000.c b/hw/e1000.c
index 6a3a941..87a1104 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -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),
-- 
1.7.5.53.gc233e

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [Qemu-devel] [PATCH 2/2] e1000: CTRL.RST emulation
  2011-09-27 12:58 [Qemu-devel] [PATCH 0/2] e1000: reset handling Michael S. Tsirkin
  2011-09-27 12:58 ` [Qemu-devel] [PATCH 1/2] e1000: minor reset code refactoring Michael S. Tsirkin
@ 2011-09-27 12:58 ` Michael S. Tsirkin
  2011-09-27 15:33   ` Andy Gospodarek
  1 sibling, 1 reply; 4+ messages in thread
From: Michael S. Tsirkin @ 2011-09-27 12:58 UTC (permalink / raw)
  To: qemu-devel, Andy Gospodarek, Dean Nelson, Jesse Brandeburg, Jeff Kirsher
  Cc: peter.maydell, Anthony Liguori, Michael S. Tsirkin,
	Alex Williamson, Anthony PERARD, Kevin Wolf, Aurelien Jarno

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 <andy@greyhouse.net>
Similar fix proposed by Anthony PERARD <anthony.perard@citrix.com>

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/e1000.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/e1000.c b/hw/e1000.c
index 87a1104..b51e089 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -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
-- 
1.7.5.53.gc233e

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] e1000: CTRL.RST emulation
  2011-09-27 12:58 ` [Qemu-devel] [PATCH 2/2] e1000: CTRL.RST emulation Michael S. Tsirkin
@ 2011-09-27 15:33   ` Andy Gospodarek
  0 siblings, 0 replies; 4+ messages in thread
From: Andy Gospodarek @ 2011-09-27 15:33 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: peter.maydell, Anthony Liguori, Dean Nelson, qemu-devel,
	Jesse Brandeburg, Alex Williamson, Jeff Kirsher, Anthony PERARD,
	Kevin Wolf, Aurelien Jarno

On Tue, Sep 27, 2011 at 8:58 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> 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 <andy@greyhouse.net>
> Similar fix proposed by Anthony PERARD <anthony.perard@citrix.com>
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/e1000.c |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/hw/e1000.c b/hw/e1000.c
> index 87a1104..b51e089 100644
> --- a/hw/e1000.c
> +++ b/hw/e1000.c
> @@ -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
> --
> 1.7.5.53.gc233e
>

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

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-09-27 15:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-27 12:58 [Qemu-devel] [PATCH 0/2] e1000: reset handling Michael S. Tsirkin
2011-09-27 12:58 ` [Qemu-devel] [PATCH 1/2] e1000: minor reset code refactoring Michael S. Tsirkin
2011-09-27 12:58 ` [Qemu-devel] [PATCH 2/2] e1000: CTRL.RST emulation Michael S. Tsirkin
2011-09-27 15:33   ` Andy Gospodarek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.