[Qemu-devel] Re: [PATCHv6 00/16] boot order specification

[Gleb Natapov]

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 "/address@hidden/address@hidden/address@hidden,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 "/address@hidden/address@hidden" if the string is 
> > found option
> > rom gets boot index from it. In our case 
> > "/address@hidden/address@hidden/address@hidden,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 "/address@hidden/address@hidden" 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
> "/address@hidden/address@hidden/address@hidden,0") - so, what you seem to 
> want is
> "/address@hidden/[^/address@hidden(/|$)".
> 
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 "/address@hidden/address@hidden/address@hidden" exactly but match
> > "/address@hidden/address@hidden" 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:
> > > 
> > > /address@hidden/address@hidden/address@hidden
> > > /address@hidden/address@hidden
> > > 
> > This patch series implement device paths as defined by Openfirmware
> > spec. /address@hidden/address@hidden 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 
> > /address@hidden/address@hidden/address@hidden
> > 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 "/address@hidden"
+
+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.