diff mbox

[v2] e1000: initial link negotiation on mac osx

Message ID 20131107202842.GD13775@HEDWIG.INI.CMU.EDU
State New
Headers show

Commit Message

Gabriel L. Somlo Nov. 7, 2013, 8:28 p.m. UTC
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(-)

Comments

Alexander Graf Nov. 7, 2013, 11:12 p.m. UTC | #1
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
>
Stefan Hajnoczi Nov. 8, 2013, 1:39 p.m. UTC | #2
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
Gabriel L. Somlo Nov. 8, 2013, 3:52 p.m. UTC | #3
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
Stefan Hajnoczi Nov. 11, 2013, 9:48 a.m. UTC | #4
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
diff mbox

Patch

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