On Mon, Nov 29, 2010 at 08:34:03PM -0500, Kevin O'Connor wrote:
On Sun, Nov 28, 2010 at 08:47:34PM +0200, Gleb Natapov wrote:
On Sun, Nov 28, 2010 at 12:15:44PM -0500, Kevin O'Connor wrote:
It's unclear to me how SeaBIOS is supposed to do that.
Suppose we have "/pci@i0cf8/scsi@3/disk@0,0" with boot index 5 in boot devices list and suppose pci device in slot 3 function 0 has optionrom. When Seabios load the option rom from device to memory it looks for string that matches "/pci@i0cf8/.*@3.*" if the string is found option rom gets boot index from it. In our case "/pci@i0cf8/scsi@3/disk@0,0" will match and optionrom will get boot index 5. In practice Seabios will know that device is SCSI by reading device class so it will be able to construct string "/pci@i0cf8/scsi@3" and use simple strstr to find matching device path.
I recognize that if we had a regex engine in seabios this would work, but I'm reluctant to add one. strstr doesn't work becuase "@3" could match some unrelated part of the path (eg, don't want to match "/pci@i0cf8/scsi@1/disk@3,0") - so, what you seem to want is "/pci@i0cf8/[^/]*@3(/|$)".
We do not need regex engine to part this very special and regular string. The patch below reads boot order list from qemu and creates rom2prio table for pci roms.
[...]
I'm still stuck on how seabios is supposed to know it's an ethernet card. Sure, seabios could hardcode translations from classid to strings, but that seems fragile. What happens when the user wants to boot from myranet, or fiberchannel, or whatnot?
This is not fragile since class to name translation is defined by the spec. But even that is not required if Seabios will be a little bit smarter and will implement fuzzy matching. i.e do not match "/pci@i0cf8/ethernet@4/ethernet-phy@0" exactly but match "/pci@i0cf8/.*@4.*" instead.
I think we're agreeing here that we don't want to put class to name translation in seabios. :-)
We did? I think it is not needed but I don't not see why it would be a problem.
Maybe we can compromise here - if the user selects booting from a device, and qemu sees there is a rom for that device, then qemu can specify two boot options:
/pci@i0cf8/ethernet@4/ethernet-phy@0 /pci@i0cf8/rom@4
This patch series implement device paths as defined by Openfirmware spec. /pci@i0cf8/rom@4 sould be out of spec. But I do not see why Seabios can't build later from the former. Also I do not see why it would be needed at all.
The name isn't important to me - call it something else if you want. It's value is that SeaBIOS doesn't then need to do fuzzy matching or parsing of the device names. That is, we turn the list from boot devices to boot methods which makes life easier for the firmware.
SeaBIOS will ignore the first entry, and act on the second entry. If at some later point seabios knows how to natively boot from the device (eg, scsi), then it will use the first entry and ignore the second.
If you let go to the idea of exact matching of string built by qemu in Seabios it will be easy to see that /pci@i0cf8/ethernet@4/ethernet-phy@0 provides everything that Seabios needs to know and even more. If you ignore all the noise it just says "boot from pci device slot 4 fn 0". Seabios may have native support for the card in the slot or it can use option rom on the card. Qemu does not care.
I'm having a hard time letting go of string matching. I understand all the info is there if SeaBIOS parses the string. However, I think parsing out openbios device strings is overkill in an x86 bios that just wants to order the boot objects it knows about.
Is there an issue with qemu generating two strings for devices with roms?
I just do not see how I can justify this addition to qemu maintainers given that the parsing code below is very simple.
diff --git a/src/boot.c b/src/boot.c index 021b8ac..c1ac9af 100644 --- a/src/boot.c +++ b/src/boot.c @@ -67,6 +67,47 @@ boot_setup(void) if (!(inb_cmos(CMOS_BIOS_BOOTFLAG1) & 1)) IPL.checkfloppysig = 1; } + + u32 file = romfile_find("bootorder"); + if (!file) + return; + + int filesize = romfile_size(file); + char *f = malloc_tmphigh(filesize); + dprintf(3, "bootorder file found (len %d)\n", filesize); + + if (!f) { + dprintf(1, "can't allocate memory for bootorder file\n"); + return; + } + + romfile_copy(file, f, filesize); + int i; + IPL.fw_bootorder_count = 1; + while(f[i]) { + if (f[i] == '\n') + IPL.fw_bootorder_count++; + i++; + } + IPL.fw_bootorder = malloc_tmphigh(IPL.fw_bootorder_count*sizeof(char*)); + if (!IPL.fw_bootorder) { + dprintf(1, "can't allocate memory for bootorder\n"); + free(f); + return; + } + + dprintf(3, "boot order:\n"); + i = 0; + do { + IPL.fw_bootorder[i] = f; + f = strchr(f, '\n'); + if (*f) { + *f = '\0'; + f++; + dprintf(3, "%d: %s\n", i, IPL.fw_bootorder[i]); + i++; + } + } while(f); }
// Add a BEV vector for a given pnp compatible option rom. @@ -506,3 +547,78 @@ handle_19(void) SET_EBDA(boot_sequence, 0); do_boot(0); } + +/* + * function returns string representing firts device path element in 'dp' + * and puts pointer to the reset of the device path into 'end' + */ +static char *dev_path_get_node(const char *dp, const char **end) +{ + int len; + char *node; + + dp += 1; /* skip '/' */ + + *end = strchr(dp, '/'); + + if (*end == NULL) { + len = strlen(dp); /* last path element */ + *end = dp + len; + } + else + len = *end - dp; + + if (len == 0) + return NULL; + + node = malloc_tmphigh(len + 1); + memcpy(node, dp, len); + node[len] = '\0'; + + return node; +} + +static int match_unit_address(const char *pe, const char *unit_address) +{ + char *s = strchr(pe, '@'); + + if (s == NULL) + return 0; + + return !strcmp(s, unit_address); +} + +#define FW_PCI_DOMAIN "/pci@i0cf8" + +int bootprio_match_pci_device(int dev, int fn) +{ + int i, l; + char pci[7]; + + if (!fn) + l = snprintf(pci, sizeof(pci), "@%x", dev); + else + l = snprintf(pci, sizeof(pci), "@%x,%x", dev, fn); + + for (i = 0; i < IPL.fw_bootorder_count; i++) { + char *node; + const char *next, *path = IPL.fw_bootorder[i]; + int r; + + /* is pci domain? */ + if (memcmp(path, FW_PCI_DOMAIN, strlen(FW_PCI_DOMAIN))) + continue; + + node = dev_path_get_node(path + strlen(FW_PCI_DOMAIN), &next); + if (node == NULL) + continue; + + r = match_unit_address(node, pci); + + free(node); + if (r) + return i; + } + + return -1; +} diff --git a/src/boot.h b/src/boot.h index db046e3..07467e6 100644 --- a/src/boot.h +++ b/src/boot.h @@ -20,6 +20,8 @@ struct ipl_s { int bevcount, bcvcount; u32 bootorder; int checkfloppysig; + char **fw_bootorder; + int fw_bootorder_count; };
#define IPL_TYPE_FLOPPY 0x01 @@ -44,5 +46,6 @@ void add_bcv(u16 seg, u16 ip, u16 desc); struct drive_s; void add_bcv_internal(struct drive_s *drive_g); void boot_prep(void); +int bootprio_match_pci_device(int dev, int fn);
#endif // __BOOT_H diff --git a/src/optionroms.c b/src/optionroms.c index 854c33f..5500c56 100644 --- a/src/optionroms.c +++ b/src/optionroms.c @@ -74,6 +74,13 @@ struct pnp_data { // The end of the last deployed rom. u32 RomEnd = BUILD_ROM_START;
+static struct rom_boot_prio { + u32 addr; + int prio; +} rom2prio[92]; + +static int rom2prio_index; +
/**************************************************************** * Helper functions @@ -342,7 +349,20 @@ init_pcirom(u16 bdf, int isvga) if (! rom) // No ROM present. return -1; - return init_optionrom(rom, bdf, isvga); + + int r = init_optionrom(rom, bdf, isvga); + + if (r || !is_valid_rom(rom)) + return r; + + rom2prio[rom2prio_index].addr = (u32)rom; + rom2prio[rom2prio_index].prio = + bootprio_match_pci_device(pci_bdf_to_dev(bdf), pci_bdf_to_fn(bdf)); + dprintf(3, "pci rom at memory address %x has boot prio %d\n", + rom2prio[rom2prio_index].addr, rom2prio[rom2prio_index].prio); + rom2prio_index++; + + return r; }
diff --git a/src/util.c b/src/util.c index 8e02d1e..cfb4add 100644 --- a/src/util.c +++ b/src/util.c @@ -253,6 +253,17 @@ strtcpy(char *dest, const char *src, size_t len) return dest; }
+// locate first occurance of character c in the string s +char * +strchr(const char *s, int c) +{ + int i = 0; + + while(s[i] && s[i] != c) + i++; + + return s[i] ? (char*)&s[i] : NULL; +}
/**************************************************************** * Keyboard calls diff --git a/src/util.h b/src/util.h index 18ab814..cd46d9c 100644 --- a/src/util.h +++ b/src/util.h @@ -208,6 +208,7 @@ void *memcpy(void *d1, const void *s1, size_t len); void iomemcpy(void *d, const void *s, u32 len); void *memmove(void *d, const void *s, size_t len); char *strtcpy(char *dest, const char *src, size_t len); +char *strchr(const char *s, int c); int get_keystroke(int msec);
// stacks.c -- Gleb.