* [Qemu-devel] e1000 patch for osx @ 2013-10-25 13:53 jacek burghardt 2013-10-25 14:21 ` Paolo Bonzini 2013-10-30 16:52 ` [Qemu-devel] e1000 patch for osx Paolo Bonzini 0 siblings, 2 replies; 27+ messages in thread From: jacek burghardt @ 2013-10-25 13:53 UTC (permalink / raw) To: qemu-devel, qemu-discuss [-- 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 --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] e1000 patch for osx 2013-10-25 13:53 [Qemu-devel] e1000 patch for osx jacek burghardt @ 2013-10-25 14:21 ` Paolo Bonzini 2013-10-25 14:27 ` jacek burghardt 2013-10-30 16:52 ` [Qemu-devel] e1000 patch for osx Paolo Bonzini 1 sibling, 1 reply; 27+ messages in thread From: Paolo Bonzini @ 2013-10-25 14:21 UTC (permalink / raw) To: jacek burghardt; +Cc: qemu-devel, qemu-discuss 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] e1000 patch for osx 2013-10-25 14:21 ` Paolo Bonzini @ 2013-10-25 14:27 ` jacek burghardt 2013-10-30 9:41 ` Stefan Hajnoczi 2013-10-30 23:54 ` jacek burghardt 0 siblings, 2 replies; 27+ messages in thread From: jacek burghardt @ 2013-10-25 14:27 UTC (permalink / raw) To: Paolo Bonzini; +Cc: qemu-devel, qemu-discuss [-- 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 --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] e1000 patch for osx 2013-10-25 14:27 ` jacek burghardt @ 2013-10-30 9:41 ` Stefan Hajnoczi 2013-10-30 13:26 ` jacek burghardt 2013-10-30 23:54 ` jacek burghardt 1 sibling, 1 reply; 27+ messages in thread From: Stefan Hajnoczi @ 2013-10-30 9:41 UTC (permalink / raw) To: jacek burghardt; +Cc: Paolo Bonzini, qemu-devel, qemu-discuss 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] e1000 patch for osx 2013-10-30 9:41 ` Stefan Hajnoczi @ 2013-10-30 13:26 ` jacek burghardt 2013-10-30 13:35 ` Andreas Färber 0 siblings, 1 reply; 27+ messages in thread From: jacek burghardt @ 2013-10-30 13:26 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: Paolo Bonzini, qemu-devel, qemu-discuss [-- 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 --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] e1000 patch for osx 2013-10-30 13:26 ` jacek burghardt @ 2013-10-30 13:35 ` Andreas Färber 0 siblings, 0 replies; 27+ messages in thread From: Andreas Färber @ 2013-10-30 13:35 UTC (permalink / raw) To: jacek burghardt; +Cc: Stefan Hajnoczi, qemu-discuss, qemu-devel, Paolo Bonzini 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] e1000 patch for osx 2013-10-25 14:27 ` jacek burghardt 2013-10-30 9:41 ` Stefan Hajnoczi @ 2013-10-30 23:54 ` jacek burghardt 2013-10-31 0:00 ` Paolo Bonzini 1 sibling, 1 reply; 27+ messages in thread From: jacek burghardt @ 2013-10-30 23:54 UTC (permalink / raw) To: Paolo Bonzini; +Cc: qemu-devel, qemu-discuss [-- 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 --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] e1000 patch for osx 2013-10-30 23:54 ` jacek burghardt @ 2013-10-31 0:00 ` Paolo Bonzini 2013-10-31 0:21 ` jacek burghardt 0 siblings, 1 reply; 27+ messages in thread From: Paolo Bonzini @ 2013-10-31 0:00 UTC (permalink / raw) To: jacek burghardt; +Cc: qemu-devel, qemu-discuss 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] e1000 patch for osx 2013-10-31 0:00 ` Paolo Bonzini @ 2013-10-31 0:21 ` jacek burghardt 2013-10-31 0:24 ` jacek burghardt 2013-10-31 0:27 ` Paolo Bonzini 0 siblings, 2 replies; 27+ messages in thread From: jacek burghardt @ 2013-10-31 0:21 UTC (permalink / raw) To: Paolo Bonzini; +Cc: qemu-devel, qemu-discuss [-- 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 --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] e1000 patch for osx 2013-10-31 0:21 ` jacek burghardt @ 2013-10-31 0:24 ` jacek burghardt 2013-10-31 0:27 ` Paolo Bonzini 1 sibling, 0 replies; 27+ messages in thread From: jacek burghardt @ 2013-10-31 0:24 UTC (permalink / raw) To: Paolo Bonzini; +Cc: qemu-devel, qemu-discuss [-- 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 --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] e1000 patch for osx 2013-10-31 0:21 ` jacek burghardt 2013-10-31 0:24 ` jacek burghardt @ 2013-10-31 0:27 ` Paolo Bonzini 2013-10-31 0:31 ` jacek burghardt 1 sibling, 1 reply; 27+ messages in thread From: Paolo Bonzini @ 2013-10-31 0:27 UTC (permalink / raw) To: jacek burghardt; +Cc: qemu-devel, qemu-discuss 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] e1000 patch for osx 2013-10-31 0:27 ` Paolo Bonzini @ 2013-10-31 0:31 ` jacek burghardt 2013-10-31 0:51 ` jacek burghardt 0 siblings, 1 reply; 27+ messages in thread From: jacek burghardt @ 2013-10-31 0:31 UTC (permalink / raw) To: Paolo Bonzini; +Cc: qemu-devel, qemu-discuss [-- 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 --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] e1000 patch for osx 2013-10-31 0:31 ` jacek burghardt @ 2013-10-31 0:51 ` jacek burghardt 2013-10-31 15:06 ` Gabriel L. Somlo 0 siblings, 1 reply; 27+ messages in thread From: jacek burghardt @ 2013-10-31 0:51 UTC (permalink / raw) To: Paolo Bonzini; +Cc: qemu-devel, qemu-discuss [-- 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 --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] e1000 patch for osx 2013-10-31 0:51 ` jacek burghardt @ 2013-10-31 15:06 ` Gabriel L. Somlo 2013-10-31 21:38 ` jacek burghardt 2013-11-07 18:04 ` [Qemu-devel] [PATCH v1] e1000: initial link negotiation on mac osx Gabriel L. Somlo 0 siblings, 2 replies; 27+ messages in thread From: Gabriel L. Somlo @ 2013-10-31 15:06 UTC (permalink / raw) To: qemu-devel; +Cc: pbonzini, jaceksburghardt 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; ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] e1000 patch for osx 2013-10-31 15:06 ` Gabriel L. Somlo @ 2013-10-31 21:38 ` jacek burghardt 2013-11-07 18:04 ` [Qemu-devel] [PATCH v1] e1000: initial link negotiation on mac osx Gabriel L. Somlo 1 sibling, 0 replies; 27+ messages in thread From: jacek burghardt @ 2013-10-31 21:38 UTC (permalink / raw) To: Gabriel L. Somlo; +Cc: Paolo Bonzini, qemu-devel [-- Attachment #1: Type: text/plain, Size: 29 bytes --] I will test this with 10.8.3 [-- Attachment #2: Type: text/html, Size: 46 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v1] e1000: initial link negotiation on mac osx 2013-10-31 15:06 ` Gabriel L. Somlo 2013-10-31 21:38 ` jacek burghardt @ 2013-11-07 18:04 ` Gabriel L. Somlo 2013-11-07 19:28 ` Paolo Bonzini 1 sibling, 1 reply; 27+ messages in thread From: Gabriel L. Somlo @ 2013-11-07 18:04 UTC (permalink / raw) To: qemu-devel; +Cc: pbonzini, jaceksburghardt, stefanha 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v1] e1000: initial link negotiation on mac osx 2013-11-07 18:04 ` [Qemu-devel] [PATCH v1] e1000: initial link negotiation on mac osx Gabriel L. Somlo @ 2013-11-07 19:28 ` Paolo Bonzini 2013-11-07 19:44 ` jacek burghardt 2013-11-07 20:28 ` [Qemu-devel] [PATCH v2] " Gabriel L. Somlo 0 siblings, 2 replies; 27+ messages in thread From: Paolo Bonzini @ 2013-11-07 19:28 UTC (permalink / raw) To: Gabriel L. Somlo; +Cc: stefanha, qemu-devel, jaceksburghardt 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v1] e1000: initial link negotiation on mac osx 2013-11-07 19:28 ` Paolo Bonzini @ 2013-11-07 19:44 ` jacek burghardt 2013-11-07 20:28 ` [Qemu-devel] [PATCH v2] " Gabriel L. Somlo 1 sibling, 0 replies; 27+ messages in thread From: jacek burghardt @ 2013-11-07 19:44 UTC (permalink / raw) To: Paolo Bonzini; +Cc: stefanha, Gabriel L. Somlo, qemu-devel [-- Attachment #1: Type: text/plain, Size: 37 bytes --] I know there are patches for seabios [-- Attachment #2: Type: text/html, Size: 55 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v2] e1000: initial link negotiation on mac osx 2013-11-07 19:28 ` Paolo Bonzini 2013-11-07 19:44 ` jacek burghardt @ 2013-11-07 20:28 ` Gabriel L. Somlo 2013-11-07 23:12 ` Alexander Graf 1 sibling, 1 reply; 27+ messages in thread From: Gabriel L. Somlo @ 2013-11-07 20:28 UTC (permalink / raw) To: qemu-devel; +Cc: pbonzini, jaceksburghardt, stefanha 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v2] e1000: initial link negotiation on mac osx 2013-11-07 20:28 ` [Qemu-devel] [PATCH v2] " Gabriel L. Somlo @ 2013-11-07 23:12 ` Alexander Graf 2013-11-08 13:39 ` Stefan Hajnoczi 0 siblings, 1 reply; 27+ messages in thread From: Alexander Graf @ 2013-11-07 23:12 UTC (permalink / raw) To: Gabriel L. Somlo Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, jaceksburghardt@gmail.com, stefanha@gmail.com 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 > ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v2] e1000: initial link negotiation on mac osx 2013-11-07 23:12 ` Alexander Graf @ 2013-11-08 13:39 ` Stefan Hajnoczi 2013-11-08 15:52 ` Gabriel L. Somlo 0 siblings, 1 reply; 27+ messages in thread From: Stefan Hajnoczi @ 2013-11-08 13:39 UTC (permalink / raw) To: Alexander Graf Cc: pbonzini@redhat.com, Gabriel L. Somlo, qemu-devel@nongnu.org, jaceksburghardt@gmail.com 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v2] e1000: initial link negotiation on mac osx 2013-11-08 13:39 ` Stefan Hajnoczi @ 2013-11-08 15:52 ` Gabriel L. Somlo 2013-11-11 9:48 ` Stefan Hajnoczi 0 siblings, 1 reply; 27+ messages in thread From: Gabriel L. Somlo @ 2013-11-08 15:52 UTC (permalink / raw) To: Stefan Hajnoczi Cc: pbonzini@redhat.com, Alexander Graf, jaceksburghardt@gmail.com, qemu-devel@nongnu.org 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v2] e1000: initial link negotiation on mac osx 2013-11-08 15:52 ` Gabriel L. Somlo @ 2013-11-11 9:48 ` Stefan Hajnoczi 0 siblings, 0 replies; 27+ messages in thread From: Stefan Hajnoczi @ 2013-11-11 9:48 UTC (permalink / raw) To: Gabriel L. Somlo Cc: pbonzini@redhat.com, Alexander Graf, jaceksburghardt@gmail.com, qemu-devel@nongnu.org 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] e1000 patch for osx 2013-10-25 13:53 [Qemu-devel] e1000 patch for osx jacek burghardt 2013-10-25 14:21 ` Paolo Bonzini @ 2013-10-30 16:52 ` Paolo Bonzini 2013-10-30 17:29 ` Peter Maydell 1 sibling, 1 reply; 27+ messages in thread From: Paolo Bonzini @ 2013-10-30 16:52 UTC (permalink / raw) To: jacek burghardt; +Cc: qemu-devel, qemu-discuss 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] e1000 patch for osx 2013-10-30 16:52 ` [Qemu-devel] e1000 patch for osx Paolo Bonzini @ 2013-10-30 17:29 ` Peter Maydell 2013-10-30 17:50 ` jacek burghardt 2013-10-30 18:00 ` Paolo Bonzini 0 siblings, 2 replies; 27+ messages in thread From: Peter Maydell @ 2013-10-30 17:29 UTC (permalink / raw) To: Paolo Bonzini; +Cc: QEMU Developers, jacek burghardt, qemu-discuss 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] e1000 patch for osx 2013-10-30 17:29 ` Peter Maydell @ 2013-10-30 17:50 ` jacek burghardt 2013-10-30 18:00 ` Paolo Bonzini 1 sibling, 0 replies; 27+ messages in thread From: jacek burghardt @ 2013-10-30 17:50 UTC (permalink / raw) To: Peter Maydell; +Cc: Paolo Bonzini, qemu-devel, qemu-discuss [-- 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 --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] e1000 patch for osx 2013-10-30 17:29 ` Peter Maydell 2013-10-30 17:50 ` jacek burghardt @ 2013-10-30 18:00 ` Paolo Bonzini 1 sibling, 0 replies; 27+ messages in thread From: Paolo Bonzini @ 2013-10-30 18:00 UTC (permalink / raw) To: Peter Maydell; +Cc: QEMU Developers, jacek burghardt, qemu-discuss 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
end of thread, other threads:[~2013-11-11 9:48 UTC | newest] Thread overview: 27+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-10-25 13:53 [Qemu-devel] e1000 patch for osx jacek burghardt 2013-10-25 14:21 ` Paolo Bonzini 2013-10-25 14:27 ` jacek burghardt 2013-10-30 9:41 ` Stefan Hajnoczi 2013-10-30 13:26 ` jacek burghardt 2013-10-30 13:35 ` Andreas Färber 2013-10-30 23:54 ` jacek burghardt 2013-10-31 0:00 ` Paolo Bonzini 2013-10-31 0:21 ` jacek burghardt 2013-10-31 0:24 ` jacek burghardt 2013-10-31 0:27 ` Paolo Bonzini 2013-10-31 0:31 ` jacek burghardt 2013-10-31 0:51 ` jacek burghardt 2013-10-31 15:06 ` Gabriel L. Somlo 2013-10-31 21:38 ` jacek burghardt 2013-11-07 18:04 ` [Qemu-devel] [PATCH v1] e1000: initial link negotiation on mac osx Gabriel L. Somlo 2013-11-07 19:28 ` Paolo Bonzini 2013-11-07 19:44 ` jacek burghardt 2013-11-07 20:28 ` [Qemu-devel] [PATCH v2] " Gabriel L. Somlo 2013-11-07 23:12 ` Alexander Graf 2013-11-08 13:39 ` Stefan Hajnoczi 2013-11-08 15:52 ` Gabriel L. Somlo 2013-11-11 9:48 ` Stefan Hajnoczi 2013-10-30 16:52 ` [Qemu-devel] e1000 patch for osx Paolo Bonzini 2013-10-30 17:29 ` Peter Maydell 2013-10-30 17:50 ` jacek burghardt 2013-10-30 18:00 ` Paolo Bonzini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).