[Qemu-devel] e1000 patch for osx

[Qemu-devel] e1000 patch for osx

[jacek burghardt]

[-- Attachment #1: Type: text/plain, Size: 256 bytes --]

Is there a patch for qemu git master that pre init e1000 so I can get rid
off unpluged network cable message ? I know there is patch but is is for
older version of qemu and it seeem that it no longer functions and does not
apply fully as code was changed.

[-- Attachment #2: Type: text/html, Size: 279 bytes --]

Re: [Qemu-devel] e1000 patch for osx

[Paolo Bonzini]

Il 25/10/2013 14:53, jacek burghardt ha scritto:
> Is there a patch for qemu git master that pre init e1000 so I can get
> rid off unpluged network cable message ? I know there is patch but is is
> for older version of qemu and it seeem that it no longer functions and
> does not apply fully as code was changed. 

Which patch was that?

Paolo

Re: [Qemu-devel] e1000 patch for osx

[jacek burghardt]

[-- Attachment #1: Type: text/plain, Size: 993 bytes --]

https://github.com/saucelabs/mac-osx-on-kvm/blob/master/e1000-mac-hacks.patch

-        } else
-            s->phy_reg[addr] = data;
+        } else {
+            /* some (reset) bits are self clearing, so better clear them */
+            switch (addr) {
+            case PHY_CTRL:
+                s->phy_reg[addr] = data & 0x7eff;
+                if (s->phy_reg[addr] != data)
+                        set_ics(s, 0, E1000_ICR_LSC);
+                break;
+            default:
+                s->phy_reg[addr] = data;
+            }
+        }



On Fri, Oct 25, 2013 at 8:21 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 25/10/2013 14:53, jacek burghardt ha scritto:
> > Is there a patch for qemu git master that pre init e1000 so I can get
> > rid off unpluged network cable message ? I know there is patch but is is
> > for older version of qemu and it seeem that it no longer functions and
> > does not apply fully as code was changed.
>
> Which patch was that?
>
> Paolo
>

[-- Attachment #2: Type: text/html, Size: 3633 bytes --]

Re: [Qemu-devel] e1000 patch for osx

[Stefan Hajnoczi]

On Fri, Oct 25, 2013 at 08:27:27AM -0600, jacek burghardt wrote:
> https://github.com/saucelabs/mac-osx-on-kvm/blob/master/e1000-mac-hacks.patch
> 
> -        } else
> -            s->phy_reg[addr] = data;
> +        } else {
> +            /* some (reset) bits are self clearing, so better clear them */
> +            switch (addr) {
> +            case PHY_CTRL:
> +                s->phy_reg[addr] = data & 0x7eff;
> +                if (s->phy_reg[addr] != data)
> +                        set_ics(s, 0, E1000_ICR_LSC);
> +                break;
> +            default:
> +                s->phy_reg[addr] = data;
> +            }
> +        }

Patches welcome!  Please see:

http://qemu-project.org/Contribute/SubmitAPatch

I suggest you contact the author of this patch and ask them to submit
it.

Stefan

Re: [Qemu-devel] e1000 patch for osx

[jacek burghardt]

[-- Attachment #1: Type: text/plain, Size: 98 bytes --]

Well I was hoping anyone could port the patch to qemu it seems the code
changed a lot in one year

[-- Attachment #2: Type: text/html, Size: 115 bytes --]

Re: [Qemu-devel] e1000 patch for osx

[Andreas Färber]

Am 30.10.2013 14:26, schrieb jacek burghardt:
> Well I was hoping anyone could port the patch to qemu it seems the code
> changed a lot in one year

Problem is that the patch you referenced does not have a Signed-off-by.
We can't just rebase and apply it without contacting the author.

Some minor nits in there btw, including missing braces and spaces around +.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] e1000 patch for osx

[Paolo Bonzini]

Il 25/10/2013 15:53, jacek burghardt ha scritto:
> Is there a patch for qemu git master that pre init e1000 so I can get
> rid off unpluged network cable message ? I know there is patch but is is
> for older version of qemu and it seeem that it no longer functions and
> does not apply fully as code was changed. 

Does this patch work?

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 70a59fd..a9ce08f 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -203,6 +203,11 @@ set_phy_ctrl(E1000State *s, int index, uint16_t val)
         DBGOUT(PHY, "Start link auto negotiation\n");
         timer_mod(s->autoneg_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 500);
     }
+
+    if (val & 0x8000) {
+        val &= 0x7fff;
+        set_ics(s, 0, E1000_ICR_LSC);
+    }
 }
 
 static void
@@ -445,8 +450,9 @@ set_mdic(E1000State *s, int index, uint32_t val)
         } else {
             if (addr < NPHYWRITEOPS && phyreg_writeops[addr]) {
                 phyreg_writeops[addr](s, index, data);
+            } else {
+                s->phy_reg[addr] = data;
             }
-            s->phy_reg[addr] = data;
         }
     }
     s->mac_reg[MDIC] = val | E1000_MDIC_READY;

Paolo

Re: [Qemu-devel] e1000 patch for osx

[Peter Maydell]

On 30 October 2013 16:52, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 25/10/2013 15:53, jacek burghardt ha scritto:
>> Is there a patch for qemu git master that pre init e1000 so I can get
>> rid off unpluged network cable message ? I know there is patch but is is
>> for older version of qemu and it seeem that it no longer functions and
>> does not apply fully as code was changed.
>
> Does this patch work?
>
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index 70a59fd..a9ce08f 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -203,6 +203,11 @@ set_phy_ctrl(E1000State *s, int index, uint16_t val)
>          DBGOUT(PHY, "Start link auto negotiation\n");
>          timer_mod(s->autoneg_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 500);
>      }
> +
> +    if (val & 0x8000) {
> +        val &= 0x7fff;
> +        set_ics(s, 0, E1000_ICR_LSC);
> +    }

This looks odd -- you seem to be modifying val but then not
using the modified value before we reach the end of the function.

>  }
>
>  static void
> @@ -445,8 +450,9 @@ set_mdic(E1000State *s, int index, uint32_t val)
>          } else {
>              if (addr < NPHYWRITEOPS && phyreg_writeops[addr]) {
>                  phyreg_writeops[addr](s, index, data);
> +            } else {
> +                s->phy_reg[addr] = data;
>              }
> -            s->phy_reg[addr] = data;
>          }

...and this part seems to remove the code which sets
phy_reg[PHY_CTRL], so it will now always read back as zero.



thanks
-- PMM

Re: [Qemu-devel] e1000 patch for osx

[jacek burghardt]

[-- Attachment #1: Type: text/plain, Size: 100 bytes --]

The function of this patch is initialize emulated network card and pass
data just like Mac firmware

[-- Attachment #2: Type: text/html, Size: 117 bytes --]

Re: [Qemu-devel] e1000 patch for osx

[Paolo Bonzini]

Il 30/10/2013 18:29, Peter Maydell ha scritto:
> This looks odd -- you seem to be modifying val but then not
> using the modified value before we reach the end of the function.
> 
>> >  }
>> >
>> >  static void
>> > @@ -445,8 +450,9 @@ set_mdic(E1000State *s, int index, uint32_t val)
>> >          } else {
>> >              if (addr < NPHYWRITEOPS && phyreg_writeops[addr]) {
>> >                  phyreg_writeops[addr](s, index, data);
>> > +            } else {
>> > +                s->phy_reg[addr] = data;
>> >              }
>> > -            s->phy_reg[addr] = data;
>> >          }
> ...and this part seems to remove the code which sets
> phy_reg[PHY_CTRL], so it will now always read back as zero.

Yeah, I forgot one line:

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 70a59fd..b7a1953 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -203,6 +203,12 @@ set_phy_ctrl(E1000State *s, int index, uint16_t val)
         DBGOUT(PHY, "Start link auto negotiation\n");
         timer_mod(s->autoneg_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 500);
     }
+
+    if (val & 0x8000) {
+        val &= 0x7fff;
+        set_ics(s, 0, E1000_ICR_LSC);
+    }
+    s->phy_reg[PHY_CTRL] = val;
 }
 
 static void
@@ -445,8 +451,9 @@ set_mdic(E1000State *s, int index, uint32_t val)
         } else {
             if (addr < NPHYWRITEOPS && phyreg_writeops[addr]) {
                 phyreg_writeops[addr](s, index, data);
+            } else {
+                s->phy_reg[addr] = data;
             }
-            s->phy_reg[addr] = data;
         }
     }
     s->mac_reg[MDIC] = val | E1000_MDIC_READY;

Paolo

Re: [Qemu-devel] e1000 patch for osx

[jacek burghardt]

[-- Attachment #1: Type: text/plain, Size: 1190 bytes --]

I wonder if anyone can post reworked patch to latest  qemu


On Fri, Oct 25, 2013 at 8:27 AM, jacek burghardt
<jaceksburghardt@gmail.com>wrote:

>
> https://github.com/saucelabs/mac-osx-on-kvm/blob/master/e1000-mac-hacks.patch
>
> -        } else
> -            s->phy_reg[addr] = data;
> +        } else {
> +            /* some (reset) bits are self clearing, so better clear them */
> +            switch (addr) {
> +            case PHY_CTRL:
> +                s->phy_reg[addr] = data & 0x7eff;
> +                if (s->phy_reg[addr] != data)
> +                        set_ics(s, 0, E1000_ICR_LSC);
> +                break;
> +            default:
> +                s->phy_reg[addr] = data;
> +            }
> +        }
>
>
>
> On Fri, Oct 25, 2013 at 8:21 AM, Paolo Bonzini <pbonzini@redhat.com>wrote:
>
>> Il 25/10/2013 14:53, jacek burghardt ha scritto:
>> > Is there a patch for qemu git master that pre init e1000 so I can get
>> > rid off unpluged network cable message ? I know there is patch but is is
>> > for older version of qemu and it seeem that it no longer functions and
>> > does not apply fully as code was changed.
>>
>> Which patch was that?
>>
>> Paolo
>>
>
>

[-- Attachment #2: Type: text/html, Size: 3743 bytes --]

Re: [Qemu-devel] e1000 patch for osx

[Paolo Bonzini]

Il 31/10/2013 00:54, jacek burghardt ha scritto:
> I wonder if anyone can post reworked patch to latest  qemu 

That's what I did 5 hours ago, though what I did was actually to look at
the bits affected by the patch and reimplement them based on the e1000
hardware spec.  Can you test the second patch I posted and see if it
works for you?

Paolo

Re: [Qemu-devel] e1000 patch for osx

[jacek burghardt]

[-- Attachment #1: Type: text/plain, Size: 3099 bytes --]

I am in process of recompiling qemu right now
I came up with this patch is this correct
diff -Naur qemu/hw/net/e1000.c qemu-a/hw/net/e1000.c
--- qemu/hw/net/e1000.c 2013-10-27 15:36:05.496526538 -0600
+++ qemu-a/hw/net/e1000.c       2013-10-30 18:09:03.101711694 -0600
@@ -203,6 +203,12 @@
         DBGOUT(PHY, "Start link auto negotiation\n");
         timer_mod(s->autoneg_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL)
+ 500);
     }
+
+     if (val & 0x8000) {
+         val &= 0x7fff;
+         set_ics(s, 0, E1000_ICR_LSC);
+     }
+     s->phy_reg[PHY_CTRL] = val;
 }

 static void
@@ -390,6 +396,8 @@
     d->rxbuf_min_shift = 1;
     memset(&d->tx, 0, sizeof d->tx);

+    d->mac_reg[RA+0] = (macaddr[3] << 24) | (macaddr[2] << 16) |
(macaddr[1] << 8) | macaddr[0];
+    d->mac_reg[RA+1] = E1000_RAH_AV | (macaddr[5] << 8) | macaddr[4];
     if (qemu_get_queue(d->nic)->link_down) {
         e1000_link_down(d);
     }
@@ -409,6 +417,8 @@
 {
     /* RST is self clearing */
     s->mac_reg[CTRL] = val & ~E1000_CTRL_RST;
+    if (val & E1000_CTRL_RST)
+       set_ics(s, 0, E1000_ICR_LSC);
 }

 static void
@@ -446,7 +456,15 @@
             if (addr < NPHYWRITEOPS && phyreg_writeops[addr]) {
                 phyreg_writeops[addr](s, index, data);
             }
-            s->phy_reg[addr] = data;
+            switch (addr) {
+           case PHY_CTRL:
+               s->phy_reg[addr] = data & 0x7eff;
+               if (s->phy_reg[addr] != data)
+                       set_ics(s, 0, E1000_ICR_LSC);
+               break;
+           default:
+               s->phy_reg[addr] = data;
+           }
         }
     }
     s->mac_reg[MDIC] = val | E1000_MDIC_READY;
@@ -1163,8 +1181,13 @@
 static void
 set_ims(E1000State *s, int index, uint32_t val)
 {
+    uint32_t old_val = s->mac_reg[IMS];
     s->mac_reg[IMS] |= val;
-    set_ics(s, 0, 0);
+    if ((val & E1000_ICR_LSC) && old_val == 0) {
+      set_ics(s, 0, E1000_ICR_LSC); /* inject Link Status for
uncooperative Darwin driver */
+    } else {
+      set_ics(s, 0, 0);
+    }
 }

 #define getreg(x)      [x] = mac_readreg
@@ -1192,7 +1215,7 @@
 static void (*macreg_writeops[])(E1000State *, int, uint32_t) = {
     putreg(PBA),       putreg(EERD),   putreg(SWSM),   putreg(WUFC),
     putreg(TDBAL),     putreg(TDBAH),  putreg(TXDCTL), putreg(RDBAH),
-    putreg(RDBAL),     putreg(LEDCTL), putreg(VET),
+    putreg(RDBAL),     putreg(LEDCTL), putreg(VET),    putreg(MANC),
     [TDLEN] = set_dlen,        [RDLEN] = set_dlen,     [TCTL] = set_tctl,
     [TDT] = set_tctl,  [MDIC] = set_mdic,      [ICS] = set_ics,
     [TDH] = set_16bit, [RDH] = set_16bit,      [RDT] = set_rdt,



On Wed, Oct 30, 2013 at 6:00 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 31/10/2013 00:54, jacek burghardt ha scritto:
> > I wonder if anyone can post reworked patch to latest  qemu
>
> That's what I did 5 hours ago, though what I did was actually to look at
> the bits affected by the patch and reimplement them based on the e1000
> hardware spec.  Can you test the second patch I posted and see if it
> works for you?
>
> Paolo
>

[-- Attachment #2: Type: text/html, Size: 4497 bytes --]

Re: [Qemu-devel] e1000 patch for osx

[jacek burghardt]

[-- Attachment #1: Type: text/plain, Size: 4175 bytes --]

i got this error
hw/net/e1000.c: In function 'set_phy_ctrl':
hw/net/e1000.c:209:10: warning: implicit declaration of function 'set_ics'
[-Wimplicit-function-declaration]
          set_ics(s, 0, E1000_ICR_LSC);
          ^
hw/net/e1000.c:209:10: warning: nested extern declaration of 'set_ics'
[-Wnested-externs]
hw/net/e1000.c: At top level:
hw/net/e1000.c:351:1: warning: conflicting types for 'set_ics' [enabled by
default]
 set_ics(E1000State *s, int index, uint32_t val)
 ^
hw/net/e1000.c:351:1: error: static declaration of 'set_ics' follows
non-static declaration
hw/net/e1000.c:209:10: note: previous implicit declaration of 'set_ics' was
here
          set_ics(s, 0, E1000_ICR_LSC);
          ^
/data/dev/xen/xen-git/src/xen-build/tools/qemu-xen-dir/rules.mak:25: recipe
for target 'hw/net/e1000.o' failed



On Wed, Oct 30, 2013 at 6:21 PM, jacek burghardt
<jaceksburghardt@gmail.com>wrote:

> I am in process of recompiling qemu right now
> I came up with this patch is this correct
> diff -Naur qemu/hw/net/e1000.c qemu-a/hw/net/e1000.c
> --- qemu/hw/net/e1000.c 2013-10-27 15:36:05.496526538 -0600
> +++ qemu-a/hw/net/e1000.c       2013-10-30 18:09:03.101711694 -0600
> @@ -203,6 +203,12 @@
>          DBGOUT(PHY, "Start link auto negotiation\n");
>          timer_mod(s->autoneg_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL)
> + 500);
>      }
> +
> +     if (val & 0x8000) {
> +         val &= 0x7fff;
> +         set_ics(s, 0, E1000_ICR_LSC);
> +     }
> +     s->phy_reg[PHY_CTRL] = val;
>  }
>
>  static void
> @@ -390,6 +396,8 @@
>      d->rxbuf_min_shift = 1;
>      memset(&d->tx, 0, sizeof d->tx);
>
> +    d->mac_reg[RA+0] = (macaddr[3] << 24) | (macaddr[2] << 16) |
> (macaddr[1] << 8) | macaddr[0];
> +    d->mac_reg[RA+1] = E1000_RAH_AV | (macaddr[5] << 8) | macaddr[4];
>      if (qemu_get_queue(d->nic)->link_down) {
>          e1000_link_down(d);
>      }
> @@ -409,6 +417,8 @@
>  {
>      /* RST is self clearing */
>      s->mac_reg[CTRL] = val & ~E1000_CTRL_RST;
> +    if (val & E1000_CTRL_RST)
> +       set_ics(s, 0, E1000_ICR_LSC);
>  }
>
>  static void
> @@ -446,7 +456,15 @@
>              if (addr < NPHYWRITEOPS && phyreg_writeops[addr]) {
>                  phyreg_writeops[addr](s, index, data);
>              }
> -            s->phy_reg[addr] = data;
> +            switch (addr) {
> +           case PHY_CTRL:
> +               s->phy_reg[addr] = data & 0x7eff;
> +               if (s->phy_reg[addr] != data)
> +                       set_ics(s, 0, E1000_ICR_LSC);
> +               break;
> +           default:
> +               s->phy_reg[addr] = data;
> +           }
>          }
>      }
>      s->mac_reg[MDIC] = val | E1000_MDIC_READY;
> @@ -1163,8 +1181,13 @@
>  static void
>  set_ims(E1000State *s, int index, uint32_t val)
>  {
> +    uint32_t old_val = s->mac_reg[IMS];
>      s->mac_reg[IMS] |= val;
> -    set_ics(s, 0, 0);
> +    if ((val & E1000_ICR_LSC) && old_val == 0) {
> +      set_ics(s, 0, E1000_ICR_LSC); /* inject Link Status for
> uncooperative Darwin driver */
> +    } else {
> +      set_ics(s, 0, 0);
> +    }
>  }
>
>  #define getreg(x)      [x] = mac_readreg
> @@ -1192,7 +1215,7 @@
>  static void (*macreg_writeops[])(E1000State *, int, uint32_t) = {
>      putreg(PBA),       putreg(EERD),   putreg(SWSM),   putreg(WUFC),
>      putreg(TDBAL),     putreg(TDBAH),  putreg(TXDCTL), putreg(RDBAH),
> -    putreg(RDBAL),     putreg(LEDCTL), putreg(VET),
> +    putreg(RDBAL),     putreg(LEDCTL), putreg(VET),    putreg(MANC),
>      [TDLEN] = set_dlen,        [RDLEN] = set_dlen,     [TCTL] = set_tctl,
>      [TDT] = set_tctl,  [MDIC] = set_mdic,      [ICS] = set_ics,
>      [TDH] = set_16bit, [RDH] = set_16bit,      [RDT] = set_rdt,
>
>
>
> On Wed, Oct 30, 2013 at 6:00 PM, Paolo Bonzini <pbonzini@redhat.com>wrote:
>
>> Il 31/10/2013 00:54, jacek burghardt ha scritto:
>> > I wonder if anyone can post reworked patch to latest  qemu
>>
>> That's what I did 5 hours ago, though what I did was actually to look at
>> the bits affected by the patch and reimplement them based on the e1000
>> hardware spec.  Can you test the second patch I posted and see if it
>> works for you?
>>
>> Paolo
>>
>
>

[-- Attachment #2: Type: text/html, Size: 6142 bytes --]

Re: [Qemu-devel] e1000 patch for osx

[Paolo Bonzini]

Il 31/10/2013 01:21, jacek burghardt ha scritto:
> I am in process of recompiling qemu right now 
> I came up with this patch is this correct
> diff -Naur qemu/hw/net/e1000.c qemu-a/hw/net/e1000.c
> --- qemu/hw/net/e1000.c 2013-10-27 15:36:05.496526538 -0600
> +++ qemu-a/hw/net/e1000.c       2013-10-30 18:09:03.101711694 -0600
> @@ -203,6 +203,12 @@
>          DBGOUT(PHY, "Start link auto negotiation\n");
>          timer_mod(s->autoneg_timer,
> qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 500);
>      }
> +
> +     if (val & 0x8000) {
> +         val &= 0x7fff;
> +         set_ics(s, 0, E1000_ICR_LSC);
> +     }
> +     s->phy_reg[PHY_CTRL] = val;
>  }
> 
>  static void
> @@ -390,6 +396,8 @@
>      d->rxbuf_min_shift = 1;
>      memset(&d->tx, 0, sizeof d->tx);
> 
> +    d->mac_reg[RA+0] = (macaddr[3] << 24) | (macaddr[2] << 16) |
> (macaddr[1] << 8) | macaddr[0];
> +    d->mac_reg[RA+1] = E1000_RAH_AV | (macaddr[5] << 8) | macaddr[4];
>      if (qemu_get_queue(d->nic)->link_down) {
>          e1000_link_down(d);
>      }
> @@ -409,6 +417,8 @@
>  {
>      /* RST is self clearing */
>      s->mac_reg[CTRL] = val & ~E1000_CTRL_RST;
> +    if (val & E1000_CTRL_RST)
> +       set_ics(s, 0, E1000_ICR_LSC);

Are these two lines necessary?

>  }
> 
>  static void
> @@ -446,7 +456,15 @@
>              if (addr < NPHYWRITEOPS && phyreg_writeops[addr]) {
>                  phyreg_writeops[addr](s, index, data);
>              }
> -            s->phy_reg[addr] = data;
> +            switch (addr) {
> +           case PHY_CTRL:
> +               s->phy_reg[addr] = data & 0x7eff;

Does it work if you put 0x7fff here?

Paolo

Re: [Qemu-devel] e1000 patch for osx

[jacek burghardt]

[-- Attachment #1: Type: text/plain, Size: 4341 bytes --]

I will try that the original patch:
--- qemu-kvm-0.12.5/hw/e1000.c.vanilla 2011-02-19 12:28:38.000000000 +0000

+++ qemu-kvm-0.12.5/hw/e1000.c	2011-02-21 14:11:33.000000000 +0000
@@ -199,6 +202,8 @@
 {
     /* RST is self clearing */
     s->mac_reg[CTRL] = val & ~E1000_CTRL_RST;
+    if (val & E1000_CTRL_RST)
+	set_ics(s, 0, E1000_ICR_LSC);
 }

 static void
@@ -231,8 +236,18 @@
         if (!(phy_regcap[addr] & PHY_W)) {
             DBGOUT(MDIC, "MDIC write reg %x unhandled\n", addr);
             val |= E1000_MDIC_ERROR;
-        } else
-            s->phy_reg[addr] = data;
+        } else {
+	    /* some (reset) bits are self clearing, so better clear them */
+	    switch (addr) {
+	    case PHY_CTRL:
+		s->phy_reg[addr] = data & 0x7eff;
+		if (s->phy_reg[addr] != data)
+			set_ics(s, 0, E1000_ICR_LSC);
+		break;
+	    default:
+		s->phy_reg[addr] = data;
+	    }
+	}
     }
     s->mac_reg[MDIC] = val | E1000_MDIC_READY;
 DBGOUT(MDIC, "mac_reg[MDIC] = %x\n", s->mac_reg[MDIC]);
@@ -802,8 +807,13 @@
 static void
 set_ims(E1000State *s, int index, uint32_t val)
 {
+    uint32_t old_val = s->mac_reg[IMS];
     s->mac_reg[IMS] |= val;
-    set_ics(s, 0, 0);
+    if ((val & E1000_ICR_LSC) && old_val == 0) {
+      set_ics(s, 0, E1000_ICR_LSC); /* inject Link Status for
uncooperative Darwin driver */
+    } else {
+      set_ics(s, 0, 0);
+    }
 }

 #define getreg(x)	[x] = mac_readreg
@@ -810,7 +825,7 @@
 static void (*macreg_writeops[])(E1000State *, int, uint32_t) = {
     putreg(PBA),	putreg(EERD),	putreg(SWSM),	putreg(WUFC),
     putreg(TDBAL),	putreg(TDBAH),	putreg(TXDCTL),	putreg(RDBAH),
-    putreg(RDBAL),	putreg(LEDCTL), putreg(VET),
+    putreg(RDBAL),	putreg(LEDCTL), putreg(VET),	putreg(MANC),
     [TDLEN] = set_dlen,	[RDLEN] = set_dlen,	[TCTL] = set_tctl,
     [TDT] = set_tctl,	[MDIC] = set_mdic,	[ICS] = set_ics,
     [TDH] = set_16bit,	[RDH] = set_16bit,	[RDT] = set_rdt,
@@ -1059,6 +1059,7 @@
 static void e1000_reset(void *opaque)
 {
     E1000State *d = opaque;
+    uint8_t *macaddr = d->eeprom_data;

     memset(d->phy_reg, 0, sizeof d->phy_reg);
     memmove(d->phy_reg, phy_reg_init, sizeof phy_reg_init);
@@ -1066,6 +1067,9 @@
     memmove(d->mac_reg, mac_reg_init, sizeof mac_reg_init);
     d->rxbuf_min_shift = 1;
     memset(&d->tx, 0, sizeof d->tx);
+
+    d->mac_reg[RA+0] = (macaddr[3] << 24) | (macaddr[2] << 16) |
(macaddr[1] << 8) | macaddr[0];
+    d->mac_reg[RA+1] = E1000_RAH_AV | (macaddr[5] << 8) | macaddr[4];
 }

 static NetClientInfo net_e1000_info = {



On Wed, Oct 30, 2013 at 6:27 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 31/10/2013 01:21, jacek burghardt ha scritto:
> > I am in process of recompiling qemu right now
> > I came up with this patch is this correct
> > diff -Naur qemu/hw/net/e1000.c qemu-a/hw/net/e1000.c
> > --- qemu/hw/net/e1000.c 2013-10-27 15:36:05.496526538 -0600
> > +++ qemu-a/hw/net/e1000.c       2013-10-30 18:09:03.101711694 -0600
> > @@ -203,6 +203,12 @@
> >          DBGOUT(PHY, "Start link auto negotiation\n");
> >          timer_mod(s->autoneg_timer,
> > qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 500);
> >      }
> > +
> > +     if (val & 0x8000) {
> > +         val &= 0x7fff;
> > +         set_ics(s, 0, E1000_ICR_LSC);
> > +     }
> > +     s->phy_reg[PHY_CTRL] = val;
> >  }
> >
> >  static void
> > @@ -390,6 +396,8 @@
> >      d->rxbuf_min_shift = 1;
> >      memset(&d->tx, 0, sizeof d->tx);
> >
> > +    d->mac_reg[RA+0] = (macaddr[3] << 24) | (macaddr[2] << 16) |
> > (macaddr[1] << 8) | macaddr[0];
> > +    d->mac_reg[RA+1] = E1000_RAH_AV | (macaddr[5] << 8) | macaddr[4];
> >      if (qemu_get_queue(d->nic)->link_down) {
> >          e1000_link_down(d);
> >      }
> > @@ -409,6 +417,8 @@
> >  {
> >      /* RST is self clearing */
> >      s->mac_reg[CTRL] = val & ~E1000_CTRL_RST;
> > +    if (val & E1000_CTRL_RST)
> > +       set_ics(s, 0, E1000_ICR_LSC);
>
> Are these two lines necessary?
>
> >  }
> >
> >  static void
> > @@ -446,7 +456,15 @@
> >              if (addr < NPHYWRITEOPS && phyreg_writeops[addr]) {
> >                  phyreg_writeops[addr](s, index, data);
> >              }
> > -            s->phy_reg[addr] = data;
> > +            switch (addr) {
> > +           case PHY_CTRL:
> > +               s->phy_reg[addr] = data & 0x7eff;
>
> Does it work if you put 0x7fff here?
>
> Paolo
>
>

[-- Attachment #2: Type: text/html, Size: 5519 bytes --]

Re: [Qemu-devel] e1000 patch for osx

[jacek burghardt]

[-- Attachment #1: Type: text/plain, Size: 5329 bytes --]

I got this error
hw/net/e1000.c: In function 'set_phy_ctrl':
hw/net/e1000.c:209:10: warning: implicit declaration of function 'set_ics'
[-Wimplicit-function-declaration]
          set_ics(s, 0, E1000_ICR_LSC);
          ^
hw/net/e1000.c:209:10: warning: nested extern declaration of 'set_ics'
[-Wnested-externs]
hw/net/e1000.c: At top level:
hw/net/e1000.c:351:1: warning: conflicting types for 'set_ics' [enabled by
default]
 set_ics(E1000State *s, int index, uint32_t val)
 ^
hw/net/e1000.c:351:1: error: static declaration of 'set_ics' follows
non-static declaration
hw/net/e1000.c:209:10: note: previous implicit declaration of 'set_ics' was
here
          set_ics(s, 0, E1000_ICR_LSC);



On Wed, Oct 30, 2013 at 6:31 PM, jacek burghardt
<jaceksburghardt@gmail.com>wrote:

> I will try that the original patch:
> --- qemu-kvm-0.12.5/hw/e1000.c.vanilla 2011-02-19 12:28:38.000000000 +0000
>
> +++ qemu-kvm-0.12.5/hw/e1000.c	2011-02-21 14:11:33.000000000 +0000
> @@ -199,6 +202,8 @@
>  {
>      /* RST is self clearing */
>      s->mac_reg[CTRL] = val & ~E1000_CTRL_RST;
> +    if (val & E1000_CTRL_RST)
> +	set_ics(s, 0, E1000_ICR_LSC);
>  }
>
>  static void
> @@ -231,8 +236,18 @@
>          if (!(phy_regcap[addr] & PHY_W)) {
>              DBGOUT(MDIC, "MDIC write reg %x unhandled\n", addr);
>              val |= E1000_MDIC_ERROR;
> -        } else
> -            s->phy_reg[addr] = data;
> +        } else {
> +	    /* some (reset) bits are self clearing, so better clear them */
> +	    switch (addr) {
> +	    case PHY_CTRL:
> +		s->phy_reg[addr] = data & 0x7eff;
> +		if (s->phy_reg[addr] != data)
> +			set_ics(s, 0, E1000_ICR_LSC);
> +		break;
> +	    default:
> +		s->phy_reg[addr] = data;
> +	    }
> +	}
>      }
>      s->mac_reg[MDIC] = val | E1000_MDIC_READY;
>  DBGOUT(MDIC, "mac_reg[MDIC] = %x\n", s->mac_reg[MDIC]);
> @@ -802,8 +807,13 @@
>  static void
>  set_ims(E1000State *s, int index, uint32_t val)
>  {
> +    uint32_t old_val = s->mac_reg[IMS];
>      s->mac_reg[IMS] |= val;
> -    set_ics(s, 0, 0);
> +    if ((val & E1000_ICR_LSC) && old_val == 0) {
> +      set_ics(s, 0, E1000_ICR_LSC); /* inject Link Status for uncooperative Darwin driver */
> +    } else {
> +      set_ics(s, 0, 0);
> +    }
>  }
>
>  #define getreg(x)	[x] = mac_readreg
> @@ -810,7 +825,7 @@
>  static void (*macreg_writeops[])(E1000State *, int, uint32_t) = {
>      putreg(PBA),	putreg(EERD),	putreg(SWSM),	putreg(WUFC),
>      putreg(TDBAL),	putreg(TDBAH),	putreg(TXDCTL),	putreg(RDBAH),
> -    putreg(RDBAL),	putreg(LEDCTL), putreg(VET),
> +    putreg(RDBAL),	putreg(LEDCTL), putreg(VET),	putreg(MANC),
>      [TDLEN] = set_dlen,	[RDLEN] = set_dlen,	[TCTL] = set_tctl,
>      [TDT] = set_tctl,	[MDIC] = set_mdic,	[ICS] = set_ics,
>      [TDH] = set_16bit,	[RDH] = set_16bit,	[RDT] = set_rdt,
> @@ -1059,6 +1059,7 @@
>  static void e1000_reset(void *opaque)
>  {
>      E1000State *d = opaque;
> +    uint8_t *macaddr = d->eeprom_data;
>
>      memset(d->phy_reg, 0, sizeof d->phy_reg);
>      memmove(d->phy_reg, phy_reg_init, sizeof phy_reg_init);
> @@ -1066,6 +1067,9 @@
>      memmove(d->mac_reg, mac_reg_init, sizeof mac_reg_init);
>      d->rxbuf_min_shift = 1;
>      memset(&d->tx, 0, sizeof d->tx);
> +
> +    d->mac_reg[RA+0] = (macaddr[3] << 24) | (macaddr[2] << 16) | (macaddr[1] << 8) | macaddr[0];
> +    d->mac_reg[RA+1] = E1000_RAH_AV | (macaddr[5] << 8) | macaddr[4];
>  }
>
>  static NetClientInfo net_e1000_info = {
>
>
>
> On Wed, Oct 30, 2013 at 6:27 PM, Paolo Bonzini <pbonzini@redhat.com>wrote:
>
>> Il 31/10/2013 01:21, jacek burghardt ha scritto:
>> > I am in process of recompiling qemu right now
>> > I came up with this patch is this correct
>> > diff -Naur qemu/hw/net/e1000.c qemu-a/hw/net/e1000.c
>> > --- qemu/hw/net/e1000.c 2013-10-27 15:36:05.496526538 -0600
>> > +++ qemu-a/hw/net/e1000.c       2013-10-30 18:09:03.101711694 -0600
>> > @@ -203,6 +203,12 @@
>> >          DBGOUT(PHY, "Start link auto negotiation\n");
>> >          timer_mod(s->autoneg_timer,
>> > qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 500);
>> >      }
>> > +
>> > +     if (val & 0x8000) {
>> > +         val &= 0x7fff;
>> > +         set_ics(s, 0, E1000_ICR_LSC);
>> > +     }
>> > +     s->phy_reg[PHY_CTRL] = val;
>> >  }
>> >
>> >  static void
>> > @@ -390,6 +396,8 @@
>> >      d->rxbuf_min_shift = 1;
>> >      memset(&d->tx, 0, sizeof d->tx);
>> >
>> > +    d->mac_reg[RA+0] = (macaddr[3] << 24) | (macaddr[2] << 16) |
>> > (macaddr[1] << 8) | macaddr[0];
>> > +    d->mac_reg[RA+1] = E1000_RAH_AV | (macaddr[5] << 8) | macaddr[4];
>> >      if (qemu_get_queue(d->nic)->link_down) {
>> >          e1000_link_down(d);
>> >      }
>> > @@ -409,6 +417,8 @@
>> >  {
>> >      /* RST is self clearing */
>> >      s->mac_reg[CTRL] = val & ~E1000_CTRL_RST;
>> > +    if (val & E1000_CTRL_RST)
>> > +       set_ics(s, 0, E1000_ICR_LSC);
>>
>> Are these two lines necessary?
>>
>> >  }
>> >
>> >  static void
>> > @@ -446,7 +456,15 @@
>> >              if (addr < NPHYWRITEOPS && phyreg_writeops[addr]) {
>> >                  phyreg_writeops[addr](s, index, data);
>> >              }
>> > -            s->phy_reg[addr] = data;
>> > +            switch (addr) {
>> > +           case PHY_CTRL:
>> > +               s->phy_reg[addr] = data & 0x7eff;
>>
>> Does it work if you put 0x7fff here?
>>
>> Paolo
>>
>>
>

[-- Attachment #2: Type: text/html, Size: 6784 bytes --]

Re: [Qemu-devel] e1000 patch for osx

[Gabriel L. Somlo]

On Wed, Oct 30, 2013 at 6:31 PM, jacek burghardt wrote:
> I got this error
> hw/net/e1000.c: In function 'set_phy_ctrl':
> hw/net/e1000.c:209:10: warning: implicit declaration of function 'set_ics'
> [-Wimplicit-function-declaration]

That's because set_ics needs to be declared before being used in
set_phy_ctrl(). A full patch which builds on the current qemu git
master would look like below.

That being said, now it builds, but doesn't actually make osx
(10.6 in my case) start with a working network interface. I still
need to bounce the link (via "set_link foo off; set_link foo on")
from the qemu monitor command line in order to get network access
from osx.

--Gabriel

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 70a59fd..7c2dd9f 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -186,6 +186,9 @@ e1000_link_up(E1000State *s)
     s->phy_reg[PHY_STATUS] |= MII_SR_LINK_STATUS;
 }
 
+//FIXME: maybe move set_phy_ctrl *after* set_ics instead ?
+static void set_ics(E1000State *s, int index, uint32_t val);
+
 static void
 set_phy_ctrl(E1000State *s, int index, uint16_t val)
 {
@@ -203,6 +206,12 @@ set_phy_ctrl(E1000State *s, int index, uint16_t val)
         DBGOUT(PHY, "Start link auto negotiation\n");
         timer_mod(s->autoneg_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 500);
     }
+
+    if (val & 0x8000) {
+        val &= 0x7fff;
+        set_ics(s, 0, E1000_ICR_LSC);
+    }
+    s->phy_reg[PHY_CTRL] = val;
 }
 
 static void
@@ -445,8 +454,9 @@ set_mdic(E1000State *s, int index, uint32_t val)
         } else {
             if (addr < NPHYWRITEOPS && phyreg_writeops[addr]) {
                 phyreg_writeops[addr](s, index, data);
+            } else {
+                s->phy_reg[addr] = data;
             }
-            s->phy_reg[addr] = data;
         }
     }
     s->mac_reg[MDIC] = val | E1000_MDIC_READY;

Re: [Qemu-devel] e1000 patch for osx

[jacek burghardt]

[-- Attachment #1: Type: text/plain, Size: 29 bytes --]

I will test this with 10.8.3

[-- Attachment #2: Type: text/html, Size: 46 bytes --]

[Qemu-devel] [PATCH v1] e1000: initial link negotiation on mac osx

[Gabriel L. Somlo]

Some guest operating systems' drivers (particularly Mac OS X)
expect the link state to be pre-initialized by an earlier
component such as a proprietary BIOS. This patch injects an
additional LSC event upon PHY reset, allowing the OS X driver
to successfully complete initial link negotiation. This is a
follow-up to commit 372254c6e5c078fb13b236bb648d2b9b2b0c70f1,
which works around the OS X driver's failure to properly set
up the MAC address.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---

I studied the Intel 8254xxx manual and the various earlier
suggestions, and came up with the following, which both works
(at least for me, using SnowLeopard) and also appears to make
sense from a "code flow" point of view. Please let me know
what you all think.

Thanks,
  Gabriel

 hw/net/e1000.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index ec8ecd7..2f2fc3a 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -186,6 +186,9 @@ e1000_link_up(E1000State *s)
     s->phy_reg[PHY_STATUS] |= MII_SR_LINK_STATUS;
 }
 
+/* Forward decl. for use in set_phy_ctrl() (OS X link nego. workaround) */
+static void set_ics(E1000State *s, int index, uint32_t val);
+
 static void
 set_phy_ctrl(E1000State *s, int index, uint16_t val)
 {
@@ -197,6 +200,15 @@ set_phy_ctrl(E1000State *s, int index, uint16_t val)
     if (!(s->compat_flags & E1000_FLAG_AUTONEG)) {
         return;
     }
+    /*
+     * The Mac OS X driver expects a pre-initialized network card; injecting
+     * an extra LSC event here allows initial link negotiation to succeed in
+     * the absence of the Apple EFI BIOS.
+     */
+    if ((val & MII_CR_RESET)) {
+        set_ics(s, 0, E1000_ICR_LSC);
+        return;
+    }
     if ((val & MII_CR_AUTO_NEG_EN) && (val & MII_CR_RESTART_AUTO_NEG)) {
         e1000_link_down(s);
         s->phy_reg[PHY_STATUS] &= ~MII_SR_AUTONEG_COMPLETE;
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v1] e1000: initial link negotiation on mac osx

[Paolo Bonzini]

Il 07/11/2013 19:04, Gabriel L. Somlo ha scritto:
> Some guest operating systems' drivers (particularly Mac OS X)
> expect the link state to be pre-initialized by an earlier
> component such as a proprietary BIOS. This patch injects an
> additional LSC event upon PHY reset, allowing the OS X driver
> to successfully complete initial link negotiation. This is a
> follow-up to commit 372254c6e5c078fb13b236bb648d2b9b2b0c70f1,
> which works around the OS X driver's failure to properly set
> up the MAC address.
> 
> Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
> ---
> 
> I studied the Intel 8254xxx manual and the various earlier
> suggestions, and came up with the following, which both works
> (at least for me, using SnowLeopard) and also appears to make
> sense from a "code flow" point of view. Please let me know
> what you all think.
> 
> Thanks,
>   Gabriel
> 
>  hw/net/e1000.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index ec8ecd7..2f2fc3a 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -186,6 +186,9 @@ e1000_link_up(E1000State *s)
>      s->phy_reg[PHY_STATUS] |= MII_SR_LINK_STATUS;
>  }
>  
> +/* Forward decl. for use in set_phy_ctrl() (OS X link nego. workaround) */
> +static void set_ics(E1000State *s, int index, uint32_t val);
> +
>  static void
>  set_phy_ctrl(E1000State *s, int index, uint16_t val)
>  {
> @@ -197,6 +200,15 @@ set_phy_ctrl(E1000State *s, int index, uint16_t val)
>      if (!(s->compat_flags & E1000_FLAG_AUTONEG)) {
>          return;
>      }
> +    /*
> +     * The Mac OS X driver expects a pre-initialized network card; injecting
> +     * an extra LSC event here allows initial link negotiation to succeed in
> +     * the absence of the Apple EFI BIOS.
> +     */
> +    if ((val & MII_CR_RESET)) {
> +        set_ics(s, 0, E1000_ICR_LSC);
> +        return;
> +    }
>      if ((val & MII_CR_AUTO_NEG_EN) && (val & MII_CR_RESTART_AUTO_NEG)) {
>          e1000_link_down(s);
>          s->phy_reg[PHY_STATUS] &= ~MII_SR_AUTONEG_COMPLETE;
> 

Is there any way to work around this in the guest?  Such as using a UEFI
driver for e1000 or something like that.

Paolo

Re: [Qemu-devel] [PATCH v1] e1000: initial link negotiation on mac osx

[jacek burghardt]

[-- Attachment #1: Type: text/plain, Size: 37 bytes --]

I know there are patches for seabios

[-- Attachment #2: Type: text/html, Size: 55 bytes --]

[Qemu-devel] [PATCH v2] e1000: initial link negotiation on mac osx

[Gabriel L. Somlo]

Some guest operating systems' drivers (particularly Mac OS X)
expect the link state to be pre-initialized by an earlier
component such as a proprietary BIOS. This patch injects
additional LSC events upon PHY reset, allowing the OS X driver
to successfully complete initial link negotiation. This is a
follow-up to commit 372254c6e5c078fb13b236bb648d2b9b2b0c70f1,
which works around the OS X driver's failure to properly set
up the MAC address.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---

On Thu, Nov 07, 2013 at 08:28:47PM +0100, Paolo Bonzini wrote:
> Is there any way to work around this in the guest?  Such as using a
> UEFI driver for e1000 or something like that.

Currently OS X boots on top of SeaBIOS and Chameleon, neither of which
know anything about the e1000 hardware. On real hardware, the XNU e1000
driver expects the proprietary BIOS to set things up "just right", and
doesn't have to bother jumping through all the hoops to properly
initialize the hardware from scratch (after all, the XNU driver
developers only have to care about a limited range of carefully
controlled hardware).

In the VM/guest scenario, QEMU is the only piece that has any knowledge
of the e1000 hardware, so having it prep things for the guest would be
the path of least resistance. Using a completely different alternative
to SeaBIOS (one that would/could assume e1000 is present and would know
enough about it to configure it just right) sounds a lot less feasible.

Oh, and it gets worse :) My v1 patch does enough cheating to get OS X
to link up successfully on a e1000-equipped *Q35* VM. If Q35 is not
specified, then another bit of trickery is needed (i.e. inject another
LSC when the driver initially unmasks LSC in the IMS register...

Anyhow, this second version seems to work both with Q35 and without,
on my vanilla SnowLeopard image.

Any more thoughts and ideas much appreciated!

Thanks,
  Gabriel

 hw/net/e1000.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index ae63591..fe0f34e 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -186,6 +186,9 @@ e1000_link_up(E1000State *s)
     s->phy_reg[PHY_STATUS] |= MII_SR_LINK_STATUS;
 }
 
+/* Forward decl. for use in set_phy_ctrl() (OS X link nego. workaround) */
+static void set_ics(E1000State *s, int index, uint32_t val);
+
 static void
 set_phy_ctrl(E1000State *s, int index, uint16_t val)
 {
@@ -197,6 +200,15 @@ set_phy_ctrl(E1000State *s, int index, uint16_t val)
     if (!(s->compat_flags & E1000_FLAG_AUTONEG)) {
         return;
     }
+    /*
+     * The Mac OS X driver expects a pre-initialized network card; injecting
+     * an extra LSC event here allows initial link negotiation to succeed in
+     * the absence of the Apple EFI BIOS.
+     */
+    if ((val & MII_CR_RESET)) {
+        set_ics(s, 0, E1000_ICR_LSC);
+        return;
+    }
     if ((val & MII_CR_AUTO_NEG_EN) && (val & MII_CR_RESTART_AUTO_NEG)) {
         e1000_link_down(s);
         s->phy_reg[PHY_STATUS] &= ~MII_SR_AUTONEG_COMPLETE;
@@ -1159,8 +1171,14 @@ set_imc(E1000State *s, int index, uint32_t val)
 static void
 set_ims(E1000State *s, int index, uint32_t val)
 {
+    uint32_t ics_val = 0;
+
+    /* When Mac OS X initially unmasks LSC, it expects to see it set in ICS */
+    if (s->mac_reg[IMS] == 0 && (val & E1000_IMS_LSC))
+        ics_val |= E1000_ICR_LSC;
+
     s->mac_reg[IMS] |= val;
-    set_ics(s, 0, 0);
+    set_ics(s, 0, ics_val);
 }
 
 #define getreg(x)	[x] = mac_readreg
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v2] e1000: initial link negotiation on mac osx

[Alexander Graf]

Am 07.11.2013 um 21:28 schrieb "Gabriel L. Somlo" <gsomlo@gmail.com>:

> Some guest operating systems' drivers (particularly Mac OS X)
> expect the link state to be pre-initialized by an earlier
> component such as a proprietary BIOS. This patch injects
> additional LSC events upon PHY reset, allowing the OS X driver
> to successfully complete initial link negotiation. This is a
> follow-up to commit 372254c6e5c078fb13b236bb648d2b9b2b0c70f1,
> which works around the OS X driver's failure to properly set
> up the MAC address.
> 
> Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
> ---
> 
> On Thu, Nov 07, 2013 at 08:28:47PM +0100, Paolo Bonzini wrote:
>> Is there any way to work around this in the guest?  Such as using a
>> UEFI driver for e1000 or something like that.
> 
> Currently OS X boots on top of SeaBIOS and Chameleon, neither of which
> know anything about the e1000 hardware. On real hardware, the XNU e1000
> driver expects the proprietary BIOS to set things up "just right", and
> doesn't have to bother jumping through all the hoops to properly
> initialize the hardware from scratch (after all, the XNU driver
> developers only have to care about a limited range of carefully
> controlled hardware).
> 
> In the VM/guest scenario, QEMU is the only piece that has any knowledge
> of the e1000 hardware, so having it prep things for the guest would be
> the path of least resistance. Using a completely different alternative
> to SeaBIOS (one that would/could assume e1000 is present and would know
> enough about it to configure it just right) sounds a lot less feasible.

I'm not sure I agree. We can easily modify SeaBIOS to just loop through all PCI devices, look for an e1000 and initialize it far enough for XNU, no?

After all, it sounds like that's closer to the way a real Mac works.

Alex

> 
> Oh, and it gets worse :) My v1 patch does enough cheating to get OS X
> to link up successfully on a e1000-equipped *Q35* VM. If Q35 is not
> specified, then another bit of trickery is needed (i.e. inject another
> LSC when the driver initially unmasks LSC in the IMS register...
> 
> Anyhow, this second version seems to work both with Q35 and without,
> on my vanilla SnowLeopard image.
> 
> Any more thoughts and ideas much appreciated!
> 
> Thanks,
>  Gabriel
> 
> hw/net/e1000.c | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index ae63591..fe0f34e 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -186,6 +186,9 @@ e1000_link_up(E1000State *s)
>     s->phy_reg[PHY_STATUS] |= MII_SR_LINK_STATUS;
> }
> 
> +/* Forward decl. for use in set_phy_ctrl() (OS X link nego. workaround) */
> +static void set_ics(E1000State *s, int index, uint32_t val);
> +
> static void
> set_phy_ctrl(E1000State *s, int index, uint16_t val)
> {
> @@ -197,6 +200,15 @@ set_phy_ctrl(E1000State *s, int index, uint16_t val)
>     if (!(s->compat_flags & E1000_FLAG_AUTONEG)) {
>         return;
>     }
> +    /*
> +     * The Mac OS X driver expects a pre-initialized network card; injecting
> +     * an extra LSC event here allows initial link negotiation to succeed in
> +     * the absence of the Apple EFI BIOS.
> +     */
> +    if ((val & MII_CR_RESET)) {
> +        set_ics(s, 0, E1000_ICR_LSC);
> +        return;
> +    }
>     if ((val & MII_CR_AUTO_NEG_EN) && (val & MII_CR_RESTART_AUTO_NEG)) {
>         e1000_link_down(s);
>         s->phy_reg[PHY_STATUS] &= ~MII_SR_AUTONEG_COMPLETE;
> @@ -1159,8 +1171,14 @@ set_imc(E1000State *s, int index, uint32_t val)
> static void
> set_ims(E1000State *s, int index, uint32_t val)
> {
> +    uint32_t ics_val = 0;
> +
> +    /* When Mac OS X initially unmasks LSC, it expects to see it set in ICS */
> +    if (s->mac_reg[IMS] == 0 && (val & E1000_IMS_LSC))
> +        ics_val |= E1000_ICR_LSC;
> +
>     s->mac_reg[IMS] |= val;
> -    set_ics(s, 0, 0);
> +    set_ics(s, 0, ics_val);
> }
> 
> #define getreg(x)    [x] = mac_readreg
> -- 
> 1.8.1.4
> 

Re: [Qemu-devel] [PATCH v2] e1000: initial link negotiation on mac osx

[Stefan Hajnoczi]

On Fri, Nov 08, 2013 at 12:12:52AM +0100, Alexander Graf wrote:
> Am 07.11.2013 um 21:28 schrieb "Gabriel L. Somlo" <gsomlo@gmail.com>:
> 
> > Some guest operating systems' drivers (particularly Mac OS X)
> > expect the link state to be pre-initialized by an earlier
> > component such as a proprietary BIOS. This patch injects
> > additional LSC events upon PHY reset, allowing the OS X driver
> > to successfully complete initial link negotiation. This is a
> > follow-up to commit 372254c6e5c078fb13b236bb648d2b9b2b0c70f1,
> > which works around the OS X driver's failure to properly set
> > up the MAC address.
> > 
> > Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
> > ---
> > 
> > On Thu, Nov 07, 2013 at 08:28:47PM +0100, Paolo Bonzini wrote:
> >> Is there any way to work around this in the guest?  Such as using a
> >> UEFI driver for e1000 or something like that.
> > 
> > Currently OS X boots on top of SeaBIOS and Chameleon, neither of which
> > know anything about the e1000 hardware. On real hardware, the XNU e1000
> > driver expects the proprietary BIOS to set things up "just right", and
> > doesn't have to bother jumping through all the hoops to properly
> > initialize the hardware from scratch (after all, the XNU driver
> > developers only have to care about a limited range of carefully
> > controlled hardware).
> > 
> > In the VM/guest scenario, QEMU is the only piece that has any knowledge
> > of the e1000 hardware, so having it prep things for the guest would be
> > the path of least resistance. Using a completely different alternative
> > to SeaBIOS (one that would/could assume e1000 is present and would know
> > enough about it to configure it just right) sounds a lot less feasible.
> 
> I'm not sure I agree. We can easily modify SeaBIOS to just loop through all PCI devices, look for an e1000 and initialize it far enough for XNU, no?
> 
> After all, it sounds like that's closer to the way a real Mac works.

I'd much prefer Alex's suggestion so we avoid putting guest-specific
hacks into QEMU.

If there is really no better solution, please make an "extra" behavior
disabled by default and accessible through a device property.  For
example -device e1000,xnu-preinit-hack=on.

Stefan

Re: [Qemu-devel] [PATCH v2] e1000: initial link negotiation on mac osx

[Gabriel L. Somlo]

On Fri, Nov 08, 2013 at 02:39:25PM +0100, Stefan Hajnoczi wrote:
> On Fri, Nov 08, 2013 at 12:12:52AM +0100, Alexander Graf wrote:
>> We can easily modify SeaBIOS to just loop through all PCI devices,
>> look for an e1000 and initialize it far enough for XNU, no?
>> After all, it sounds like that's closer to the way a real Mac works.
> 
> I'd much prefer Alex's suggestion so we avoid putting guest-specific
> hacks into QEMU.
> 
> If there is really no better solution, please make an "extra" behavior
> disabled by default and accessible through a device property.  For
> example -device e1000,xnu-preinit-hack=on.

I agree too, in principle. OTOH I'm a bit worried that teaching SeaBIOS
about e1000, and then getting that change upstreamed there might be
a whole different size of problem to solve :)

I will however give that a shot first, and fall back to
"xnu-preinit-hack=on" only if that doesn't work out...

Thanks,
--Gabriel

Re: [Qemu-devel] [PATCH v2] e1000: initial link negotiation on mac osx

[Stefan Hajnoczi]

On Fri, Nov 08, 2013 at 10:52:09AM -0500, Gabriel L. Somlo wrote:
> On Fri, Nov 08, 2013 at 02:39:25PM +0100, Stefan Hajnoczi wrote:
> > On Fri, Nov 08, 2013 at 12:12:52AM +0100, Alexander Graf wrote:
> >> We can easily modify SeaBIOS to just loop through all PCI devices,
> >> look for an e1000 and initialize it far enough for XNU, no?
> >> After all, it sounds like that's closer to the way a real Mac works.
> > 
> > I'd much prefer Alex's suggestion so we avoid putting guest-specific
> > hacks into QEMU.
> > 
> > If there is really no better solution, please make an "extra" behavior
> > disabled by default and accessible through a device property.  For
> > example -device e1000,xnu-preinit-hack=on.
> 
> I agree too, in principle. OTOH I'm a bit worried that teaching SeaBIOS
> about e1000, and then getting that change upstreamed there might be
> a whole different size of problem to solve :)
> 
> I will however give that a shot first, and fall back to
> "xnu-preinit-hack=on" only if that doesn't work out...

The other approach is to look at iPXE, the PXE boot ROM that QEMU ships
for the e1000 NIC.

It has an e1000 driver and you might find a hack to get things working:

Either see if you can chainload the bootloader on the harddisk after
having initialized the e1000 in iPXE.  (Start a network boot but then
use the 'boot' or 'chain' commands in iPXE.)

Or consider adding code to pre-initialize the e1000 to iPXE.  Whether
that hack will be accepted by the iPXE community is a different question
but this still pushes the hack into the guest firmware - closer to where
it lives on the real hardware.

Stefan