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

2013-07-09 Thread Dave Frodin
Kevin,

I talked with Marc Jones about this and he sees value in the method I've 
proposed.
He also predicts that this will be an issue in the future on Intel motherboards.

The one major difference is that if you were building coreboot for a Family15 
mainboard
you would only need to add one file (vendev_map) rather than 20 alias mapping 
files to
CBFS. I agree that the parsing code is more than clunky (what I wouldn't give 
for a sscanf()),
perhaps my patch could use some changes rather than throwing it out.

I'll re-attach my patch in case anybody wants to review it.

Thanks again,
Dave


- Original Message -
> From: "Kevin O'Connor" 
> To: "Dave Frodin" 
> Cc: "seabios" 
> Sent: Tuesday, July 2, 2013 10:29:39 PM
> Subject: Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option 
> ROMs to one
> 
> On Tue, Jul 02, 2013 at 07:40:17AM -0500, Dave Frodin wrote:
> > Kevin,
> > 
> > Did you ever get a chance to look over this patch?
> 
> I don't see the gain in introducing a config file parser for this
> task.  What was wrong with the approach of creating "alias" files?
> 
> -Kevin
> 
From 907686ae6a41a0cb75d0ff2a565586b6f4e25615 Mon Sep 17 00:00:00 2001
From: Dave Frodin 
Date: Thu, 23 May 2013 16:07:17 -0600
Subject: [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one

This feature was added to allow mapping multiple different PCI
vendor/device IDs to a single ID. It allows a coreboot/seabios ROM
image to be used on a mainboard whose graphics ID will vary depending
on which cpu is installed. The intent is to have the coreboot mainboard
define its VGA_BIOS_ID as the ID that is present in the actual VGA BIOS.
The PCI ID of all possible graphics IDs would then be mapped to that ID.
It may have use for other PCI devices where a single option ROM can be
used with PCI devices with varying IDs.

Signed-off-by: Dave Frodin 
---
 src/optionroms.c | 87 +++-
 1 file changed, 86 insertions(+), 1 deletion(-)

diff --git a/src/optionroms.c b/src/optionroms.c
index ac92613..3b86244 100644
--- a/src/optionroms.c
+++ b/src/optionroms.c
@@ -177,13 +177,98 @@ deploy_romfile(struct romfile_s *file)
 return rom;
 }
 
+/* Read up to 8 ascii chars plus one non-convertible character from a
+ * string and convert to binary.
+ */
+static u32
+text_to_binary( char **ptr )
+{
+u32 data = 0;
+u8 loop = 0;
+if (*ptr == NULL)
+return data;
+for ( ; ; (*ptr)++ ) {
+if ((**ptr >= '0') && ( **ptr <= '9'))
+data = (data * 0x10) + (**ptr - '0');
+else if ((**ptr >= 'a') && (**ptr <= 'f'))
+data = (data * 0x10) + (**ptr - 'a' + 0x0A);
+else if ((**ptr >= 'A') && (**ptr <= 'F'))
+data = (data * 0x10) + (**ptr - 'A' + 0x0A);
+else
+break;
+if (++loop >= 9)
+break;
+}
+(*ptr)++; /* point to the char after the non-convertible char */
+return data;
+}
+
+/* Allow mapping of multiple different PCI IDs to a single ID. A single AMD
+ * VGA BIOS will quite often be used on hardware that reports different
+ * PCI graphics IDs. This allows a mainboard to have a single definition
+ * (which would match the ID in the option ROM) yet would support multiple
+ * CPU IDs.
+ * Example vendev-map format for AMD Family14 (ID=1002) vgabios.
+ *10029803-10029802
+ *10029804-10029802
+ *10029805-10029802
+ *10029806-10029802
+ *10029807-10029802
+ *10029808-10029802
+ *10029809-10029802
+ */
+#define MAP_FILE_STATUS_UNINITED 0
+#define MAP_FILE_STATUS_GOOD 1
+#define MAP_FILE_STATUS_NOEXIST  2
+#define MAP_FILE_STATUS_USED 3
+#define MAPPER_BYTES_PER_LINE18 /* U32 src, '-', U32 dst, 0x0A */
+
+static u32
+map_oprom_vendev(u32 vendev)
+{
+static char *filedata = NULL;
+static int filesize = 0, map_status = MAP_FILE_STATUS_UNINITED;
+u32 vendev_src = 0, vendev_dst = 0;
+char *tmp_ptr;
+int i;
+
+if ((map_status == MAP_FILE_STATUS_NOEXIST) ||
+(map_status == MAP_FILE_STATUS_USED))
+return vendev;
+
+if (map_status == MAP_FILE_STATUS_UNINITED) {
+filedata = romfile_loadfile("vendev-map", &filesize);
+if (!filedata) {
+map_status = MAP_FILE_STATUS_NOEXIST;
+return vendev;
+}
+else
+map_status = MAP_FILE_STATUS_GOOD;
+}
+
+tmp_ptr = filedata;
+for (i=0; i < filesize/MAPPER_BYTES_PER_LINE; i++) {
+vendev_src = text_to_binary( &tmp_ptr );
+vendev_dst = text_to_binary( &tmp_ptr );
+dprintf(5, "map %d: src=%x dst=%x\n", i, vendev_src, vendev_dst);
+if ( vendev

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

2013-07-02 Thread Kevin O'Connor
On Tue, Jul 02, 2013 at 07:40:17AM -0500, Dave Frodin wrote:
> Kevin,
> 
> Did you ever get a chance to look over this patch?

I don't see the gain in introducing a config file parser for this
task.  What was wrong with the approach of creating "alias" files?

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


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

2013-07-02 Thread Dave Frodin
Kevin,

Did you ever get a chance to look over this patch?

Thanks,
Dave

- Original Message -
> From: "Dave Frodin" 
> To: "Kevin O'Connor" 
> Cc: "seabios" 
> Sent: Wednesday, June 5, 2013 8:45:11 AM
> Subject: Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option 
> ROMs to one
> 
> Kevin,
> 
> Attached is the new option rom mapping patch.
> I was hunting around for an existing SeaBIOS ftn that would convert
> ascii strings to a binary value (i.e. sscanf) but couldn't find one.
> If there's already one in the code I can replace the text_to_binary()
> with it.
> 
> I tested this on a AMD family14 platform. The vendev-map file I added
> to CBFS looks like this...
> 
> 10029803-10029802
> 10029804-10029802
> 10029805-10029802
> 10029806-10029802
> 10029807-10029802
> 10029808-10029802
> 10029809-10029802
> 
> Each line ends with a NEWLINE character (0x0A).
> 
> On my test platform the actual graphics ID of the part is 10029804.
> The vgabios gets added to CBFS with the name "pci1002,9802.rom".
> 
> Thanks,
> Dave
> ___
> SeaBIOS mailing list
> SeaBIOS@seabios.org
> http://www.seabios.org/mailman/listinfo/seabios
> 

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


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

2013-06-05 Thread Dave Frodin
Kevin,

Attached is the new option rom mapping patch.
I was hunting around for an existing SeaBIOS ftn that would convert
ascii strings to a binary value (i.e. sscanf) but couldn't find one.
If there's already one in the code I can replace the text_to_binary()
with it.

I tested this on a AMD family14 platform. The vendev-map file I added
to CBFS looks like this...

10029803-10029802
10029804-10029802
10029805-10029802
10029806-10029802
10029807-10029802
10029808-10029802
10029809-10029802

Each line ends with a NEWLINE character (0x0A).

On my test platform the actual graphics ID of the part is 10029804.
The vgabios gets added to CBFS with the name "pci1002,9802.rom".

Thanks,
DaveFrom 907686ae6a41a0cb75d0ff2a565586b6f4e25615 Mon Sep 17 00:00:00 2001
From: Dave Frodin 
Date: Thu, 23 May 2013 16:07:17 -0600
Subject: [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one

This feature was added to allow mapping multiple different PCI
vendor/device IDs to a single ID. It allows a coreboot/seabios ROM
image to be used on a mainboard whose graphics ID will vary depending
on which cpu is installed. The intent is to have the coreboot mainboard
define its VGA_BIOS_ID as the ID that is present in the actual VGA BIOS.
The PCI ID of all possible graphics IDs would then be mapped to that ID.
It may have use for other PCI devices where a single option ROM can be
used with PCI devices with varying IDs.

Signed-off-by: Dave Frodin 
---
 src/optionroms.c | 87 +++-
 1 file changed, 86 insertions(+), 1 deletion(-)

diff --git a/src/optionroms.c b/src/optionroms.c
index ac92613..3b86244 100644
--- a/src/optionroms.c
+++ b/src/optionroms.c
@@ -177,13 +177,98 @@ deploy_romfile(struct romfile_s *file)
 return rom;
 }
 
+/* Read up to 8 ascii chars plus one non-convertible character from a
+ * string and convert to binary.
+ */
+static u32
+text_to_binary( char **ptr )
+{
+u32 data = 0;
+u8 loop = 0;
+if (*ptr == NULL)
+return data;
+for ( ; ; (*ptr)++ ) {
+if ((**ptr >= '0') && ( **ptr <= '9'))
+data = (data * 0x10) + (**ptr - '0');
+else if ((**ptr >= 'a') && (**ptr <= 'f'))
+data = (data * 0x10) + (**ptr - 'a' + 0x0A);
+else if ((**ptr >= 'A') && (**ptr <= 'F'))
+data = (data * 0x10) + (**ptr - 'A' + 0x0A);
+else
+break;
+if (++loop >= 9)
+break;
+}
+(*ptr)++; /* point to the char after the non-convertible char */
+return data;
+}
+
+/* Allow mapping of multiple different PCI IDs to a single ID. A single AMD
+ * VGA BIOS will quite often be used on hardware that reports different
+ * PCI graphics IDs. This allows a mainboard to have a single definition
+ * (which would match the ID in the option ROM) yet would support multiple
+ * CPU IDs.
+ * Example vendev-map format for AMD Family14 (ID=1002) vgabios.
+ *10029803-10029802
+ *10029804-10029802
+ *10029805-10029802
+ *10029806-10029802
+ *10029807-10029802
+ *10029808-10029802
+ *10029809-10029802
+ */
+#define MAP_FILE_STATUS_UNINITED 0
+#define MAP_FILE_STATUS_GOOD 1
+#define MAP_FILE_STATUS_NOEXIST  2
+#define MAP_FILE_STATUS_USED 3
+#define MAPPER_BYTES_PER_LINE18 /* U32 src, '-', U32 dst, 0x0A */
+
+static u32
+map_oprom_vendev(u32 vendev)
+{
+static char *filedata = NULL;
+static int filesize = 0, map_status = MAP_FILE_STATUS_UNINITED;
+u32 vendev_src = 0, vendev_dst = 0;
+char *tmp_ptr;
+int i;
+
+if ((map_status == MAP_FILE_STATUS_NOEXIST) ||
+(map_status == MAP_FILE_STATUS_USED))
+return vendev;
+
+if (map_status == MAP_FILE_STATUS_UNINITED) {
+filedata = romfile_loadfile("vendev-map", &filesize);
+if (!filedata) {
+map_status = MAP_FILE_STATUS_NOEXIST;
+return vendev;
+}
+else
+map_status = MAP_FILE_STATUS_GOOD;
+}
+
+tmp_ptr = filedata;
+for (i=0; i < filesize/MAPPER_BYTES_PER_LINE; i++) {
+vendev_src = text_to_binary( &tmp_ptr );
+vendev_dst = text_to_binary( &tmp_ptr );
+dprintf(5, "map %d: src=%x dst=%x\n", i, vendev_src, vendev_dst);
+if ( vendev_src == vendev) {
+dprintf(1, "Mapping PCI device %8x to %8x\n",vendev, vendev_dst);
+map_status = MAP_FILE_STATUS_USED;
+free(filedata);
+return vendev_dst;
+}
+}
+return vendev;
+}
+
 // Check if an option rom is at a hardcoded location or in CBFS.
 static struct rom_header *
 lookup_hardcode(struct pci_device *pci)
 {
 char fname[17];
+u32 vendev = map_oprom_vendev((pci->vendor << 16) | pci->device);
 snprintf(fname, sizeof(fname), "pci%04x,%04x.rom"
- , pci->vendor, pci->device);
+ , vendev >> 16, vendev & 0x);
 struct romfile_s *file = romfile_find(fname);
 if (file)
 return deploy_romfile(file);
-- 
1.8

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

2013-06-01 Thread Dave Frodin


- Original Message -
> From: "Kevin O'Connor" 
> To: "Dave Frodin" 
> Cc: "seabios" 
> Sent: Friday, May 31, 2013 9:18:21 PM
> Subject: Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option 
> ROMs to one
> 
> On Fri, May 31, 2013 at 08:32:16AM -0500, Dave Frodin wrote:
> > I see now, that's pretty slick. The problem I see with that approach is
> > that coreboot would need to add all of these files for a family14
> > mainboard.
> [...]
> > And all of these files for a family15 mainboard
> [...]
> > 
> > Since at build time we only know what cpu family the rom is being
> > built for and not (necessarily) what the exact graphics ID is.
> > 
> > The method I proposed would have one family specific "vendev-map.bin" file.
> 
> Yes, but it would be quite difficult to inspect vendev-map.bin.  I
> think an ascii interface is preferable.
> 
> One could go a level up and make a "symbolic link" type in cbfs (this
> is effectively what my patch did).  The advantage would be that
> cbfstool could display it in a nicer format.
> 
> -Kevin

I agree with that, and marc made the same suggestion. I'll put together a
new patch next week.

Dave

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


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

2013-05-31 Thread Kevin O'Connor
On Fri, May 31, 2013 at 08:32:16AM -0500, Dave Frodin wrote:
> I see now, that's pretty slick. The problem I see with that approach is
> that coreboot would need to add all of these files for a family14 mainboard.
[...]
> And all of these files for a family15 mainboard
[...]
> 
> Since at build time we only know what cpu family the rom is being
> built for and not (necessarily) what the exact graphics ID is.
> 
> The method I proposed would have one family specific "vendev-map.bin" file.

Yes, but it would be quite difficult to inspect vendev-map.bin.  I
think an ascii interface is preferable.

One could go a level up and make a "symbolic link" type in cbfs (this
is effectively what my patch did).  The advantage would be that
cbfstool could display it in a nicer format.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


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

2013-05-31 Thread Dave Frodin


- Original Message -
> From: "Kevin O'Connor" 
> To: "Dave Frodin" 
> Cc: "seabios" 
> Sent: Thursday, May 30, 2013 7:45:13 PM
> Subject: Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option 
> ROMs to one
> 
> On Thu, May 30, 2013 at 07:49:36AM -0500, Dave Frodin wrote:
> > > From: "Kevin O'Connor" 
> > > --- 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;
> > >
> > 
> > In your sample code above, I don't see where any pci ID translation
> > (mapping) occurs.
> > As an example we could have a family14 mainboard with a coreboot
> > .config that generates a vga option rom with the name
> > "pci1002,9802.rom". The actual mainboard could have a graphics chip
> > with the id of 1002,9804 (which is 1 of 8 possible IDs). So there
> > needs to be some sort of mapping function (in SeaBIOS) that maps the
> > ID of 1002,9804 to 1002,9802.
> 
> With the above patch (assuming it works) one should be able to create
> a CBFS file named "alias1002,9804.rom" that contains the text
> "pci1002,9802.rom".
> 
> -Kevin

I see now, that's pretty slick. The problem I see with that approach is
that coreboot would need to add all of these files for a family14 mainboard.
alias1002,9803.rom
alias1002,9804.rom
alias1002,9805.rom
alias1002,9806.rom
alias1002,9807.rom
alias1002,9808.rom
alias1002,9809.rom

And all of these files for a family15 mainboard
alias1002,9901.rom
alias1002,9903.rom
alias1002,9904.rom
alias1002,9906.rom
alias1002,9907.rom
alias1002,9908.rom
alias1002,990a.rom
alias1002,9910.rom
alias1002,9913.rom
alias1002,9917.rom
alias1002,9918.rom
alias1002,9919.rom
alias1002,9990.rom
alias1002,9991.rom
alias1002,9992.rom
alias1002,9993.rom
alias1002,9994.rom
alias1002,99a0.rom
alias1002,99a2.rom
alias1002,99a4.rom

Since at build time we only know what cpu family the rom is being
built for and not (necessarily) what the exact graphics ID is.

The method I proposed would have one family specific "vendev-map.bin" file.

Someone who's building a rom for a mainboard with a single known graphics
ID wouldn't need to add any files with either method.

Thanks, Dave

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


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

2013-05-30 Thread Kevin O'Connor
On Thu, May 30, 2013 at 07:49:36AM -0500, Dave Frodin wrote:
> > From: "Kevin O'Connor" 
> > --- 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;
> >
> 
> In your sample code above, I don't see where any pci ID translation
> (mapping) occurs.
> As an example we could have a family14 mainboard with a coreboot
> .config that generates a vga option rom with the name
> "pci1002,9802.rom". The actual mainboard could have a graphics chip
> with the id of 1002,9804 (which is 1 of 8 possible IDs). So there
> needs to be some sort of mapping function (in SeaBIOS) that maps the
> ID of 1002,9804 to 1002,9802.

With the above patch (assuming it works) one should be able to create
a CBFS file named "alias1002,9804.rom" that contains the text
"pci1002,9802.rom".

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


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

2013-05-30 Thread Dave Frodin


- Original Message -
> From: "Kevin O'Connor" 
> To: "Dave Frodin" 
> Cc: "seabios" 
> Sent: Saturday, May 25, 2013 10:47:42 AM
> Subject: Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option 
> ROMs to one
> 
> 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?

That is an option but I was hoping to reduce the number of configuration
differences between having coreboot vs seabios load/run the vga option rom.
Currently, coreboot has vendor/device ID mapping functions for the family14,
family15tn northbridges.

> 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;
>

In your sample code above, I don't see where any pci ID translation (mapping) 
occurs.

As an example we could have a family14 mainboard with a coreboot .config that 
generates
a vga option rom with the name "pci1002,9802.rom". The actual mainboard could 
have a
graphics chip with the id of 1002,9804 (which is 1 of 8 possible IDs). So there 
needs
to be some sort of mapping function (in SeaBIOS) that maps the ID of 1002,9804 
to 1002,9802.

Thanks again,
Dave

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


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

2013-05-25 Thread 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;

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


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

2013-05-24 Thread 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] & 0x;
> > +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
> 
From f54a6a23e12cb8f76c46248d8effe7b07abfdaf3 Mon Sep 17 00:00:00 2001
From: Dave Frodin 
Date: Thu, 23 May 2013 16:07:17 -0600
Subject: [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one

This feature was added to allow mapping multiple different PCI
vendor/device IDs to a single ID. It allows a coreboot/seabios ROM
image to be used on a mainboard whose graphics ID will vary depending
on which cpu is installed. The intent is to have the coreboot mainboard
define its VGA_BIOS_ID as the ID that is present in the actual VGA BIOS.
The PCI ID of all possible graphics IDs would then be mapped to that ID.
It may have use for other PCI devices where a single option ROM can be
used with PCI devices with varying IDs.

Signed-off-by: Dave Frodin 
---
 src/optionroms.c |   48 +++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/src/optionroms.c b/src/optionroms.c
index ac92613..4f37560 100644
--- a/src/optionroms.c
+++ b/src/optionroms.c
@@ -177,13 +177,59 @@ deploy_romfile(struct romfile_s *file)
 return rom;
 }
 
+/* Allow mapping of multiple different PCI IDs to a single ID. A single AMD
+ * VGA BIOS will quite often be used on hardware that reports different
+ * PCI graphics IDs. This allows a mainboard to have a single definition
+ * (which would match the ID in the option ROM) yet would support multiple
+ * CPU IDs.
+ * Example vendev-map.bin format (shown as text) of two AMD (ID=1002) vgabios
+ * getting mapped from 9803/9804 to  9802
+ *   From  To
+ * 10029803 10029802
+ * 10029804 10029802
+ */
+#define MAP_FILE_STATUS_UNINITED 0
+#define MAP_FILE_STATUS_GOOD 1
+#define MAP_FILE_STATUS_BAD  2
+
+static u32
+map_oprom_vendev(u32 vendev)
+{
+static u32 *filedata = NULL;
+static int filesize = 0, map_status = MAP_FILE_STATUS_UNINITED;
+u32 new_vendev = vendev;
+int i;
+
+// On first call see if the file exists and cache it
+if (map_status == MAP_FILE_STATUS_UNINITED) {
+filedata = romfile_loadfile("vendev-map.bin", &filesize);
+if (!filedata)
+map_status = MAP_FILE_STATUS_BAD;
+else
+map_status = MAP_FILE_STATUS_GOOD;
+}
+
+if (map_status != MAP_FILE_STATUS_GOOD)
+return vendev;
+
+for (i=0; i < filesize/(sizeof (u32)); i+=2) {
+   if ( filedata[i] == vendev) {
+dprintf(1, "Mapping PCI device 0x%8x to 0x%8x\n",vendev, filedata[

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

2013-05-23 Thread 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] & 0x;
> +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

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


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

2013-05-23 Thread 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" 
> To: "Dave Frodin" 
> Cc: "seabios" 
> 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 & 0x;
> 
> Modifying struct pci_device doesn't look right.
> 
> -Kevin
> 
From 7538a044131c12eda885614ad8a0b1fcdd9b9364 Mon Sep 17 00:00:00 2001
From: Dave Frodin 
Date: Thu, 23 May 2013 16:07:17 -0600
Subject: [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one

This feature was added to allow mapping multiple different
PCI graphics vendor/device IDs to a single ID. The intent is
to have the coreboot mainboard define its VGA_BIOS_ID as the
ID that is present in the actual VGA BIOS. The PCI ID of the
graphics device would then be mapped to that ID.

Signed-off-by: Dave Frodin 
---
 src/optionroms.c |   33 +
 1 file changed, 33 insertions(+)

diff --git a/src/optionroms.c b/src/optionroms.c
index ac92613..6361217 100644
--- a/src/optionroms.c
+++ b/src/optionroms.c
@@ -177,11 +177,44 @@ deploy_romfile(struct romfile_s *file)
 return rom;
 }
 
+/* Allow mapping of multiple different PCI IDs to a single ID. A single AMD
+ * VGA BIOS will quite often be used on hardware that reports different
+ * PCI graphics IDs. This allows a mainboard to have a single definition
+ * (which would match the ID in the VGA BIOS) yet would support multiple
+ * CPU IDs.
+ * Example vendev-map.bin format (shown as text) of two AMD (1002) vgabios
+ * getting mapped from 9803/9804 to  9802
+ *   From  To
+ * 10029803 10029802
+ * 10029804 10029802
+ */
+static void
+map_oprom_vendev(struct pci_device *pci)
+{
+int filesize, i;
+
+u32 *filedata = romfile_loadfile("vendev-map.bin", &filesize);
+if (!filedata)
+return;
+
+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] & 0x;
+return;
+}
+}
+return;
+}
+
 // Check if an option rom is at a hardcoded location or in CBFS.
 static struct rom_header *
 lookup_hardcode(struct pci_device *pci)
 {
 char fname[17];
+map_oprom_vendev(pci);
 snprintf(fname, sizeof(fname), "pci%04x,%04x.rom"
  , pci->vendor, pci->device);
 struct romfile_s *file = romfile_find(fname);
-- 
1.7.9.5

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


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

2013-05-22 Thread Kevin O'Connor
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 & 0x; 

Modifying struct pci_device doesn't look right.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


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

2013-05-21 Thread Dave Frodin
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. 



>From 5b7f2ba9f43fbc67a81a2449d8bbd3d2f6e530aa Mon Sep 17 00:00:00 2001 
From: Dave Frodin  
Date: Tue, 7 May 2013 13:51:56 -0600 
Subject: [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one 

This feature was added to allow mapping multiple different 
PCI graphics vendor/device IDs to a single ID. The intent is 
to have the coreboot mainboard define its VGA_BIOS_ID as the 
ID that is present in the actual VGA BIOS. The PCI ID of the 
graphics device would then be mapped to that ID. 

Change-Id: Id06a1c9730546070146932a4dc8ab8229c4a59b9 
Signed-off-by: Dave Frodin  
--- 
src/optionroms.c | 69 +- 
1 files changed, 68 insertions(+), 1 deletions(-) 

diff --git a/src/optionroms.c b/src/optionroms.c 
index 00697b2..bf1e977 100644 
--- 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); 
} 

- 
/ 
* Roms in CBFS 
/ 
@@ -174,11 +173,79 @@ deploy_romfile(struct romfile_s *file) 
return rom; 
} 

+/* Allow mapping of multiple different PCI IDs to a single ID. A single AMD 
+ * VGA BIOS will quite often be used on hardware that reports different 
+ * PCI graphics IDs. This allows a mainboard to have a single definition 
+ * (which would match the ID in the VGA BIOS) yet would support multiple 
+ * CPU IDs. 
+ */ 
+static u32 
+map_oprom_vendev(u32 vendev) 
+{ 
+ u32 new_vendev = vendev; 
+ 
+ switch (vendev) { 
+ case 0x10029803: // Family14 
+ case 0x10029804: 
+ case 0x10029805: 
+ case 0x10029806: 
+ case 0x10029807: 
+ case 0x10029808: 
+ case 0x10029809: 
+ new_vendev = 0x10029802; 
+ break; 
+ case 0x10029901: // Family15tn 
+ case 0x10029903: 
+ case 0x10029904: 
+ case 0x10029906: 
+ case 0x10029907: 
+ case 0x10029908: 
+ case 0x1002990A: 
+ case 0x10029910: 
+ case 0x10029913: 
+ case 0x10029917: 
+ case 0x10029918: 
+ case 0x10029919: 
+ case 0x10029990: 
+ case 0x10029991: 
+ case 0x10029992: 
+ case 0x10029993: 
+ case 0x10029994: 
+ case 0x100299A0: 
+ case 0x100299A2: 
+ case 0x100299A4: 
+ new_vendev = 0x10029900; 
+ break; 
+ case 0x10029831: // Family16kb 
+ case 0x10029832: 
+ case 0x10029833: 
+ case 0x10029834: 
+ case 0x10029835: 
+ case 0x10029836: 
+ case 0x10029837: 
+ case 0x10029839: 
+ case 0x1002983D: 
+ new_vendev = 0x10029830; 
+ break; 
+ default: 
+ break; 
+ } 
+ if (vendev != new_vendev) 
+ dprintf(1, "Mapping PCI device %8x to %8x\n",vendev, new_vendev); 
+ return new_vendev; 
+} 
+ 
// Check if an option rom is at a hardcoded location or in CBFS. 
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 & 0x; 
+ 
snprintf(fname, sizeof(fname), "pci%04x,%04x.rom" 
, pci->vendor, pci->device); 
struct romfile_s *file = romfile_find(fname); 
-- 
1.7.9 

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios