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" 
> To: "Dave Frodin" 
> Cc: "seabios" 
> 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;