Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one

[Dave Frodin]

      Kevin,

Thanks.
The new patch that I've attached no longer alters the pci device struct.
I also no longer repeatedly read the file in from CBFS, but keep the
pointer to it around for the next time the function is called.

As far as the use case is concerned. We (or a client) will often have several of
the same mainboard (e.g. persimmon), but the graphics ID will vary depending on
which version of the cpu is installed. Previously, we would have to figure out
what the PCI ID of the graphics devices was, adjust the ID in coreboot's config,
build/flash the new rom image. This seabios change along with a change to
coreboot to set the ID and stuff the correct vendev mapping file into CBFS
will allow a single coreboot/seabios image to be used on any particular
mainboard independent of what cpu was installed. This change won't be useful
to someone who is using a single board who's graphics ID would never change.
For that case they wouldn't need to do anything. If the vendev mapping file
doesn't get added they would see no difference.

Thanks again,
Dave

----- Original Message -----
  From: "Kevin O'Connor"
<kevin(a)koconnor.net>
 To: "Dave Frodin" <dave.frodin(a)se-eng.com>
 Cc: "seabios" <seabios(a)seabios.org>
 Sent: Thursday, May 23, 2013 9:01:00 PM
 Subject: Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one
 
 On Thu, May 23, 2013 at 05:25:42PM -0500, Dave Frodin wrote:
  Kevin,
 I've attached a patch that I hope is more along the lines
 of where you wanted me to go with this change. In my testing
 of the patch I'm currently just stuffing the vendev-map.bin file
 into my coreboot image.
 
 Thanks.
 
 [...]
  +    for (i=0; i < filesize/(sizeof (u32));
i+=2) {
 +        if ( filedata[i] == ((pci->vendor << 16) | pci->device)) {
 +            dprintf(1, "Mapping PCI device 0x%8x to 0x%8x\n",
 +                ((pci->vendor << 16) | pci->device), filedata[i+1]);
 +            pci->vendor = filedata[i+1] >> 16;
 +            pci->device = filedata[i+1] & 0xFFFF;
 +            return;
 
 I don't understand why struct pci_device is being modified.  Maybe you
 could further explain the use case.  After modifying struct
 pci_device, where does the video rom actually come from?
 
 -Kevin

[PATCH] Seabios: allow mapping of multiple PCI option ROMs to one

[Kevin O'Connor]

      On Fri, May 24, 2013 at 09:06:54AM -0500, Dave Frodin wrote:
  Kevin,
 
 Thanks.
 The new patch that I've attached no longer alters the pci device struct.
 I also no longer repeatedly read the file in from CBFS, but keep the
 pointer to it around for the next time the function is called.
 
 As far as the use case is concerned. We (or a client) will often have several of
 the same mainboard (e.g. persimmon), but the graphics ID will vary depending on
 which version of the cpu is installed. Previously, we would have to figure out
 what the PCI ID of the graphics devices was, adjust the ID in coreboot's config,
 build/flash the new rom image. This seabios change along with a change to
 coreboot to set the ID and stuff the correct vendev mapping file into CBFS
 will allow a single coreboot/seabios image to be used on any particular
 mainboard independent of what cpu was installed. This change won't be useful
 to someone who is using a single board who's graphics ID would never change.
 For that case they wouldn't need to do anything. If the vendev mapping file
 doesn't get added they would see no difference.
Thanks.

Why not just place the rom in "vgaroms/" directory where it will
always be run?

Also, I think we could avoid the binary structure in CBFS.  Something
like the below (totally untested).

-Kevin


--- a/src/optionroms.c
+++ b/src/optionroms.c
@@ -178,10 +178,19 @@ deploy_romfile(struct romfile_s *file)
 static struct rom_header *
 lookup_hardcode(struct pci_device *pci)
 {
-    char fname[17];
-    snprintf(fname, sizeof(fname), "pci%04x,%04x.rom"
+    struct romfile_s *file;
+    char fname[19];
+    snprintf(fname, sizeof(fname), "alias%04x,%04x.rom"
              , pci->vendor, pci->device);
-    struct romfile_s *file = romfile_find(fname);
+    char *alias = romfile_loadfile(fname, NULL);
+    if (alias) {
+        file = romfile_find(alias);
+        free(alias);
+    } else {
+        snprintf(fname, sizeof(fname), "pci%04x,%04x.rom"
+                 , pci->vendor, pci->device);
+        file = romfile_find(fname);
+    }
     if (file)
         return deploy_romfile(file);
     return NULL;