Re: [Qemu-devel] [RFC PATCH v1 2/2] e1000: adjust initial autoneg timing (for piix/osx)

[Gabriel L. Somlo]

On Wed, Jul 02, 2014 at 12:33:16PM +0300, Michael S. Tsirkin wrote:
> Just poking around the spec I find more things
> we don't implement correctly wrt to auto-negotiation.
> For example, MII_SR_AUTONEG_CAPS isn't set, is it?
> Maybe that's why your guest doesn't work:
> it doesn't expect to get autonegotation at all?
> 
> So I have a question: does your patch actually help any guests?
> If not, maybe we should defer it to after release,
> and try to clean up autonegotiation more thouroughly for 2.2?

I'll re-submit after 2.1 is officially out. But, since we're talking
about MII_SR_AUTONEG_CAPS:  PHY_STATUS is initialized to 0x794d, which
includes setting the MII_SR_AUTONEG_CAPS bit (|= 0x8).

Did you mean: we should check for MII_SR_AUTONEG_CAPS in have_autoneg() ?
(i.e., on the chance it gets turned off by a guest-side write to
PHY_STATUS) ?

Thx,
--G

PS. Maybe also spell out the individual bits in phy_reg_init[] ? Like,
instead of:

    [PHY_STATUS] = 0x794d,

do this:

    [PHY_STATUS] = MII_SR_EXTENDED_CAPS |
                   MII_SR_LINK_STATUS |
                   MII_SR_AUTONEG_CAPS | 
                   MII_SR_PREAMBLE_SUPPRESS | 
                   MII_SR_EXTENDED_STATUS | 
                   MII_SR_10T_HD_CAPS | 
                   MII_SR_10T_FD_CAPS | 
                   MII_SR_100X_HD_CAPS | 
                   MII_SR_100X_FD_CAPS,

... for all registers ? Much more verbose, but IMHO that'd be a good
thing :)