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

[Dave Frodin]

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,
Dave

----- Original Message -----
> From: "Kevin O'Connor" <[e-mail]>
> To: "Dave Frodin" <[e-mail]>
> Cc: "seabios" <[e-mail]>
> Sent: Wednesday, May 22, 2013 6:45:34 AM
> Subject: Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one
>
> On Tue, May 21, 2013 at 02:47:18PM -0500, Dave Frodin wrote:
> > All,
> > This is a patch that reproduces the vendor ID mapping that is done in
> > coreboot in the various AMD northbridge's. The coreboot mapping is only
> > useful if coreboot is used to run the vga bios. If seabios is the payload
> > then most coreboot configs leave the vga bios init for it.
>
> Thanks.  See my comments below.
>
> [...]
> > --- a/src/optionroms.c
> > +++ b/src/optionroms.c
> > @@ -154,7 +154,6 @@ getRomPriority(u64 *sources, struct rom_header *rom,
> > int instance)
> > return bootprio_find_named_rom(file->name, instance);
> > }
>
> Looks like whitespace got corrupted in your email.
>
> [...]
> > +static u32
> > +map_oprom_vendev(u32 vendev)
> > +{
> > + u32 new_vendev = vendev;
> > +
> > + switch (vendev) {
> > + case 0x10029803: // Family14
> > + case 0x10029804:
>
> If we're going to have a mapping, I think it has to be read from
> CBFS.  I don't think we should hardcode a list into seabios.
>
> [...]
> > static struct rom_header *
> > lookup_hardcode(struct pci_device *pci)
> > {
> > char fname[17];
> > + u32 vendev_mapped;
> > +
> > + vendev_mapped = map_oprom_vendev((pci->vendor << 16) | pci->device);
> > + pci->vendor = vendev_mapped >> 16;
> > + pci->device = vendev_mapped & 0xFFFF;
>
> Modifying struct pci_device doesn't look right.
>
> -Kevin

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

[Kevin O'Connor]

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