Rom images for e1000 and ne2k missing vendor and device id

Bug #948323 reported by Stefan Bader
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
ipxe (Ubuntu)
Fix Released
Low
Stefan Bader

Bug Description

The build system depends on a certain naming scheme of the rom images. The basename of the rom file (e.g. rtl8139.rom -> rtl8139) must be defined in the drivers PCI_ROM or ISA_ROM makro. For the rtl8139 driver this is correctly done:

PCI_ROM(0x10ec, 0x8139, "rtl8139", "Realtek 8139", 0)

However at least the ne2k_isa and the e1000_82540 drivers are not behaving well:

PCI_ROM(0x8086, 0x100E, "E1000_DEV_ID_82540EM", "E1000_DEV_ID_82540EM", e1000_82540),

ISA_DRIVER ( ne_driver, ne_probe_addrs, ne_probe1,GENERIC_ISAPNP_VENDOR, 0x0600 );
DRIVER ( "ne", nic_driver, isapnp_driver, ne_driver, ne_probe, ne_disable );
ISA_ROM("ne","NE1000/2000 and clones");

The KVM roms to be build use the name of the driver's C file. This does unfortunately end in a successful build but with vendor and device id of 0 coded into the rom images.

This seems to be no problem for KVM (so for ipxe it might be lower priority) but the Xen hvmloader is build in such a way that the roms from the kvm-ipxe binary package are used and then included in a way that on boot a matching rom is search by pci id of the card. This fails for e1000 because of this bug.

ProblemType: Bug
DistroRelease: Ubuntu 12.04
Package: kvm-ipxe 1.0.0+git-3.55f6c88-0ubuntu1
ProcVersionSignature: Ubuntu 3.2.0-18.28-generic 3.2.9
Uname: Linux 3.2.0-18-generic x86_64
ApportVersion: 1.94-0ubuntu1
Architecture: amd64
Date: Tue Mar 6 20:04:45 2012
Dependencies:

EcryptfsInUse: Yes
InstallationMedia: Ubuntu 11.04 "Natty Narwhal" - Release amd64 (20110426)
PackageArchitecture: all
SourcePackage: ipxe
UpgradeStatus: Upgraded to precise on 2012-02-24 (11 days ago)

Related branches

Revision history for this message
Stefan Bader (smb) wrote :
Revision history for this message
Serge Hallyn (serge-hallyn) wrote :

@Stefan,

I'm not sure I understand the bug or the fix.

Would it suffice to simply keep the roms by their original names in the ipxe package, while the renamed roms are in the kvm-ipxe package? If so I'll just s/mv/cp/ debian/rules.

Revision history for this message
Stefan Bader (smb) wrote :

The bug, if to say so, is that for kvm there have been additional rom image targets added. By the way the build system for ipxe works, I believe all the required rom images have been build already by the makeall (whatever it is called again). Just the names are different for some of them (for above reasons).
So a fix may indeed be:
1. remove the additional build target for allqemu (not 100% sure about virtio-net) or
    at least all beside of virtio-net
2. just copy them to different names (ne->ne2k_isa, 8086100e->e1000_82540).
The other two roms should be already present under their name (pcnet32 and rtl8139).

On the other hand I checked and it seems that kvm does not need the rom header to include the correct vendor and device id. So the change is not strictly required. For Xen we now have a patch to not include the kvm rom name but the auto build one. So it could just be a build optimization (not to build those two twice, probably just a linking step)

Revision history for this message
Stefan Bader (smb) wrote :

Or actually modify the QEMUS line instead of removing things:
QEMUS = 8086100e ne pcnet32 rtl8139 virtio-net

Changed in ipxe (Ubuntu):
status: New → Confirmed
assignee: nobody → Stefan Bader (stefan-bader-canonical)
Revision history for this message
Stefan Bader (smb) wrote :

This is how I would propose to do the fix. It keeps the target names for kvm-ipxe but modifies the file names in the makefile and target name for the links. I did it that way because to fix the Xen issue I modified the xen package to look for the 8086100e.rom.

Before:
/usr/share/qemu/pxe-ne2k_isa.rom -> /usr/lib/ipxe/ne2k_isa.rom (header with 0 ids)
/usr/share/qemu/pxe-e1000.rom -> /usr/lib/ipxe/e1000_82540.rom (header with 0 ids)
and
/usr/lib/ipxe/ne.rom (same as ne2k_isa.rom but with ids in header)
/usr/lib/ipxe/8086100e.rom (same as e1000_82540.rom but with ids in header)

After:
/usr/share/qemu/pxe-ne2k_isa.rom -> /usr/lib/ipxe/ne.rom
/usr/share/qemu/pxe-e1000.rom -> /usr/lib/ipxe/8086100e.rom

summary: - Rom images for e1000 and ne2k missign vendor and device id
+ Rom images for e1000 and ne2k missing vendor and device id
tags: added: patch
Revision history for this message
Serge Hallyn (serge-hallyn) wrote :

Thanks, Stefan. Do you still plan to send this upstream and to debian? Do you need this urgently - i.e. do you need me to apply it to ubuntu now, or can we wait for the patch to trickle down?

Revision history for this message
Stefan Bader (smb) wrote :

Serge, I had not directly thought of it because our discussion went probably into some confusion when also talking about (bug 949028). But it would make sense. It is not critical to get it into Ubuntu right now as the Xen build has been fixed to use the alternative file name which works.

Revision history for this message
Stefan Bader (smb) wrote :

Hm, thinking of this again, it does seem to me that the whole "allqemu" target and special kvm-ipxe may be Ubuntu specific and neither in upstream or Debian. Checking the latest Debian source to confirm this...

Revision history for this message
Stefan Bader (smb) wrote :

Ok, I think I confirmed this to be only in the Ubuntu version. Marc, I believe you have been working on this. Maybe we could get the change reviewed and loosely queued (without upload) to be picked up whenever something serious (or the next release) come up?

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ipxe - 1.0.0+git-3.55f6c88-0ubuntu3

---------------
ipxe (1.0.0+git-3.55f6c88-0ubuntu3) quantal; urgency=low

  [ Serge Hallyn ]
  * debian/patches/rom-set-banner-timeout-0.diff: set rom banner timeout
    to 0. (LP: #921230)

  [ Stefan Bader ]
  * Modify the ROM names in of the allqemu target to use 8086100e instead
    of e1000_82540 and ne instead of ne2k_isa (LP: #948323)
 -- Serge Hallyn <email address hidden> Mon, 28 May 2012 11:57:48 -0500

Changed in ipxe (Ubuntu):
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.