Re: [Qemu-devel] [PATCH 1/2] e1000: no need auto-negotiation if link was down

[Jason Wang <jasowang@redhat.com>]

On 01/03/2013 08:20 PM, Stefan Hajnoczi wrote:
> On Fri, Dec 28, 2012 at 05:29:10PM +0800, Amos Kong wrote:
>> Commit b9d03e352cb6b31a66545763f6a1e20c9abf0c2c added link
>> auto-negotiation emulation, it would always set link up by
>> callback function. Problem exists if original link status
>> was down, link status should not be changed in auto-negotiation.
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> Signed-off-by: Amos Kong <akong@redhat.com>
>> ---
>>  hw/e1000.c |    5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/hw/e1000.c b/hw/e1000.c
>> index 92fb00a..eebcd1d 100644
>> --- a/hw/e1000.c
>> +++ b/hw/e1000.c
>> @@ -164,6 +164,11 @@ 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;
> Do we need set_ics(s, 0, E1000_ICR_LSC) when autonegotiation completes?
> The code doesn't but I wonder if we should.

Not in this case I think. The hack of the auto-negotiation was used to
prevent the irq to be injected before the handler is registered in
windows guest. So an irq would be raised here if we do this which breaks
the hack.
>
> Stefan