Re: [Qemu-stable] [PATCH for-1.4] Revert "e1000: no need auto-negotiation if link was down"

[Michael S. Tsirkin]

On Thu, Jan 31, 2013 at 05:43:51PM -0600, Michael Roth wrote:
> This reverts commit 84dd2120247a7d25ff1bb337de21c0e76816ad2d.
> 
> I'm not sure what issue the original commit was meant to fix, or if
> the logic is actually wrong, but it causes e1000 to stop working
> after a guest issues a reset.
> 
> >From what I can tell a guest with an e1000 nic has no way of changing
> the link status, as far as it's NetClient peer is concerned, except
> in the auto-negotiation path, so with this patch in place there's no
> recovery after a reset, since the link goes down and stays that way.
> 
> Revert this patch now to fix the bigger problem, and handle any
> lingering issues with a follow-up.
> 
> Reproduced/tested with qemu-jeos and Ubuntu 12.10.
> 
> Signed-off-by: Michael Roth <address@hidden>

So from discussion on list, it seems clear the proper
fix will involve adding more state, which likely will
need to be migrated as well.
Seems like revert is the best we can do for 1.4.

Acked-by: Michael S. Tsirkin <address@hidden>


> ---
>  hw/e1000.c |    5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/hw/e1000.c b/hw/e1000.c
> index ef06ca1..563a58f 100644
> --- a/hw/e1000.c
> +++ b/hw/e1000.c
> @@ -166,11 +166,6 @@ static void
>  set_phy_ctrl(E1000State *s, int index, uint16_t val)
>  {
>      if ((val & MII_CR_AUTO_NEG_EN) && (val & MII_CR_RESTART_AUTO_NEG)) {
> -        /* no need auto-negotiation if link was down */
> -        if (s->nic->nc.link_down) {
> -            s->phy_reg[PHY_STATUS] |= MII_SR_AUTONEG_COMPLETE;
> -            return;
> -        }
>          s->nic->nc.link_down = true;
>          e1000_link_down(s);
>          s->phy_reg[PHY_STATUS] &= ~MII_SR_AUTONEG_COMPLETE;
> -- 
> 1.7.9.5
>