PATCH: search PCI option ROMs on CBFS in case insensitive way

[Leah Rowe]

Hi,

Attached is a patch for SeaBIOS's searching of CBFS, specifically for
PCI option ROMs as per PCI vendor/device ID detected in each PCI device
scanned.

At present, SeaBIOS assumes that the hexademical characters in
vendor/device ID are all-lowercase, which is not necessarily a problem,
but it may not be obvious to users.

Sure enough, I had a user earlier who couldn't get their Nvidia card
working properly even though they had the correct VGA ROM. After much
trial and error, the user realised that they needed the PCI ROM to have
an all-lowercase file name in CBFS.

I suspect that a lot of users will make the same mistake, and not come
to the same realisation as the other user. To prevent such headaches,
the attached patch modifies SeaBIOS's handling such that PCI option
ROM file names are treated as case insensitive, when scanning CBFS for
files in the notation:

pcivvvv,dddd.rom

[Kevin O'Connor]

On Tue, Jun 06, 2023 at 02:51:34AM +0100, Leah Rowe via SeaBIOS wrote:
> Hi,
>
> Attached is a patch for SeaBIOS's searching of CBFS, specifically for
> PCI option ROMs as per PCI vendor/device ID detected in each PCI device
> scanned.
>
> At present, SeaBIOS assumes that the hexademical characters in
> vendor/device ID are all-lowercase, which is not necessarily a problem,
> but it may not be obvious to users.
>
> Sure enough, I had a user earlier who couldn't get their Nvidia card
> working properly even though they had the correct VGA ROM. After much
> trial and error, the user realised that they needed the PCI ROM to have
> an all-lowercase file name in CBFS.
>
> I suspect that a lot of users will make the same mistake, and not come
> to the same realisation as the other user. To prevent such headaches,
> the attached patch modifies SeaBIOS's handling such that PCI option
> ROM file names are treated as case insensitive, when scanning CBFS for
> files in the notation:
>
> pcivvvv,dddd.rom

Thanks, however this seems to add some code complexity.  I wonder if
updating the documentation would be a simpler approach.

Cheers,
-Kevin
>
> From c4d25a6a3d8add4d8a797bef08f03c031724bec9 Mon Sep 17 00:00:00 2001
> From: Leah Rowe 
> Date: Tue, 6 Jun 2023 02:15:17 +0100
> Subject: [PATCH 2/2] optionroms: case-insensitive PCI ROM names in CBFS
>
> I encountered a user today who couldn't get their GPU working
> and the only thing they did wrong was name their PCI ROM with
> all-uppercase for the hexadecimal characters. No doubt, other
> users inserting VGA ROMs will make the same mistake.
>
> SeaBIOS assumes that these characters are all lowercase. For
> ease of use, let's treat it as case insensitive.
> Signed-off-by: Leah Rowe
>
> src/optionroms.c |  2 +-
>  src/romfile.c    | 31 ++++++++++++++++++++++++++-----
>  src/romfile.h    |  4 ++++
>  src/string.c     | 27 +++++++++++++++++++++++++++
>  src/string.h     |  2 ++
>  5 files changed, 60 insertions(+), 6 deletions(-)
>
> diff --git a/src/optionroms.c b/src/optionroms.c
> index e906ab9..e3adcf7 100644
> --- a/src/optionroms.c
> +++ b/src/optionroms.c
> @@ -332,7 +332,7 @@ init_pcirom(struct pci_device *pci, int isvga, u64 *sources)
>      char fname[17];
>      snprintf(fname, sizeof(fname), "pci%04x,%04x.rom"
>               , pci->vendor, pci->device);
> struct romfile_s *file = romfile_find(fname);
> struct romfile_s *file = pcirom_find(fname);
> struct rom_header *rom = NULL;
> if (file)
>     rom = deploy_romfile(file);
>
> diff --git a/src/romfile.c b/src/romfile.c
> index b598274..a6fa2ec 100644
> --- a/src/romfile.c
> +++ b/src/romfile.c
> @@ -22,14 +22,25 @@ romfile_add(struct romfile_s *file)
>
> // Search for the specified file.
>  static struct romfile_s *
> -__romfile_findprefix(const char *prefix, int prefixlen, struct romfile_s *prev)
> +__romfile_findprefix(const char *prefix, int prefixlen, struct romfile_s *prev, int case_sensitive)
>  {
>      struct romfile_s *cur = RomfileRoot;
>      if (prev)
>          cur = prev->next;
>      while (cur) {
>    if (memcmp(prefix, cur->name, prefixlen) == 0)
>        return cur;
>    if (case_sensitive) {
>        if (memcmp(prefix, cur->name, prefixlen) == 0)
>            return cur;
>    } else {
>        /*
>         * scanning pci rom pciVVVV,DDDD.rom, treat case
>         * insensitive so that e.g. pci2AAA,BBBB.rom and
>         * pci2aaa,bbbb.rom are both detected e.g. in CBFS in
>         * case the user didn't use an all-lowercase name
>         */
>        if (memcmp_pcirom(prefix, cur->name, prefixlen) == 0)
>            return cur;
>    }
>    cur = cur->next;
> }
>    return NULL;
>
> @@ -38,13 +49,23 @@ __romfile_findprefix(const char *prefix, int prefixlen, struct romfile_s *prev)
>  struct romfile_s *
>  romfile_findprefix(const char *prefix, struct romfile_s *prev)
>  {
> return __romfile_findprefix(prefix, strlen(prefix), prev);
> return __romfile_findprefix(prefix, strlen(prefix), prev,
>  CASE_SENSITIVE);
>
> }
>
> struct romfile_s *
>  romfile_find(const char *name)
>  {
> return __romfile_findprefix(name, strlen(name) + 1, NULL);
> return __romfile_findprefix(name, strlen(name) + 1, NULL,
>  CASE_SENSITIVE);
>
> +}
>
> +/* specifically search for PCI ROM, for case insensitive search */
> +struct romfile_s *
> +pcirom_find(const char *name)
> +{
> return __romfile_findprefix(name, strlen(name) + 1, NULL,
>  CASE_INSENSITIVE);
>
> }
>
> // Helper function to find, malloc_tmphigh, and copy a romfile.  This
> diff --git a/src/romfile.h b/src/romfile.h
> index 3e0f820..e6f8616 100644
> --- a/src/romfile.h
> +++ b/src/romfile.h
> @@ -3,6 +3,9 @@
>
> #include "types.h" // u32
>
> +#define CASE_SENSITIVE 1
> +#define CASE_INSENSITIVE 0
>
> // romfile.c
>  struct romfile_s {
>      struct romfile_s *next;
> @@ -13,6 +16,7 @@ struct romfile_s {
>  void romfile_add(struct romfile_s *file);
>  struct romfile_s *romfile_findprefix(const char *prefix, struct romfile_s *prev);
>  struct romfile_s *romfile_find(const char *name);
> +struct romfile_s *pcirom_find(const char *name);
>  void *romfile_loadfile(const char *name, int *psize);
>  u64 romfile_loadint(const char *name, u64 defval);
>
> diff --git a/src/string.c b/src/string.c
> index b45565a..d85d1c1 100644
> --- a/src/string.c
> +++ b/src/string.c
> @@ -69,6 +69,33 @@ memcmp(const void *s1, const void *s2, size_t n)
>      return 0;
>  }
>
> +// Compare two areas of memory.
> +// Assume strings are being checked, case insensitive
> +// For scanning PCI option ROMs
> +int
> +memcmp_pcirom(const void *s1, const void *s2, size_t n)
> +{
> u8 ch1, ch2;
> while (n) {
>    ch1 = ch_lowercase(*(u8*)s1);
>    ch2 = ch_lowercase(*(u8*)s2);
>    if (ch1 != ch2)
>        return ch1 < ch2 ? -1 : 1;
>    s1++;
>    s2++;
>    n--;
> }
> return 0;
>
> +}
>
> +u8
> +ch_lowercase(u8 ch)
> +{
> if (ch >= 'A' && ch <= 'Z')
>    return (ch + 32);
> return ch;
>
> +}
>
> // Compare two strings.
>  int
>  strcmp(const char *s1, const char *s2)
> diff --git a/src/string.h b/src/string.h
> index d069989..6c62741 100644
> --- a/src/string.h
> +++ b/src/string.h
> @@ -10,6 +10,8 @@ u8 checksum(void *buf, u32 len);
>  size_t strlen(const char *s);
>  int memcmp_far(u16 s1seg, const void *s1, u16 s2seg, const void *s2, size_t n);
>  int memcmp(const void *s1, const void *s2, size_t n);
> +int memcmp_pcirom(const void *s1, const void *s2, size_t n);
> +u8 ch_lowercase(u8 ch);
>  int strcmp(const char *s1, const char *s2);
>  void memset_far(u16 d_seg, void *d_far, u8 c, size_t len);
>  void memset16_far(u16 d_seg, void *d_far, u16 c, size_t len);
> -- 
> 2.40.1
> SeaBIOS mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]