src/fw/csm.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-)
For CSM, the highest priority is zero. In SeaBIOS that means "don't", and
the highest priority is 1.
So we end up with the fun outcome that booting from NVMe worked only
when it *wasn't* selected as the primary boot target, because we don't
actually run the nvme_controller_setup() thread for an NVMe controller
if its boot prio is zero.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
---
src/fw/csm.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/src/fw/csm.c b/src/fw/csm.c
index 3fcc252..7663d31 100644
--- a/src/fw/csm.c
+++ b/src/fw/csm.c
@@ -319,6 +319,20 @@ handle_csm(struct bregs *regs)
csm_return(regs);
}
+static int csm_prio_to_seabios(u16 csm_prio)
+{
+ switch (csm_prio) {
+ case BBS_DO_NOT_BOOT_FROM:
+ case BBS_IGNORE_ENTRY:
+ return -1;
+
+ case BBS_LOWEST_PRIORITY:
+ case BBS_UNPRIORITIZED_ENTRY:
+ default:
+ return csm_prio + 1;
+ }
+}
+
int csm_bootprio_ata(struct pci_device *pci, int chanid, int slave)
{
if (!csm_boot_table)
@@ -327,7 +341,7 @@ int csm_bootprio_ata(struct pci_device *pci, int chanid, int slave)
int index = 1 + (chanid * 2) + slave;
dprintf(3, "CSM bootprio for ATA%d,%d (index %d) is %d\n", chanid, slave,
index, bbs[index].BootPriority);
- return bbs[index].BootPriority;
+ return csm_prio_to_seabios(bbs[index].BootPriority);
}
int csm_bootprio_fdc(struct pci_device *pci, int port, int fdid)
@@ -336,7 +350,7 @@ int csm_bootprio_fdc(struct pci_device *pci, int port, int fdid)
return -1;
BBS_TABLE *bbs = (void *)csm_boot_table->BbsTable;
dprintf(3, "CSM bootprio for FDC is %d\n", bbs[0].BootPriority);
- return bbs[0].BootPriority;
+ return csm_prio_to_seabios(bbs[0].BootPriority);
}
int csm_bootprio_pci(struct pci_device *pci)
@@ -350,7 +364,7 @@ int csm_bootprio_pci(struct pci_device *pci)
if (pci->bdf == pci_to_bdf(bbs[i].Bus, bbs[i].Device, bbs[i].Function)) {
dprintf(3, "CSM bootprio for PCI(%d,%d,%d) is %d\n", bbs[i].Bus,
bbs[i].Device, bbs[i].Function, bbs[i].BootPriority);
- return bbs[i].BootPriority;
+ return csm_prio_to_seabios(bbs[i].BootPriority);
}
}
return -1;
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
On Wed, 2019-06-19 at 12:27 +0100, David Woodhouse wrote: > For CSM, the highest priority is zero. In SeaBIOS that means "don't", and > the highest priority is 1. > > So we end up with the fun outcome that booting from NVMe worked only > when it *wasn't* selected as the primary boot target, because we don't > actually run the nvme_controller_setup() thread for an NVMe controller > if its boot prio is zero. > > Signed-off-by: David Woodhouse <dwmw2@infradead.org> Hm, turns out the NVMe hack is something that's only in our tree so for upstream that second paragraph is a lie and can be dropped. It's still a correct change to reflect the fact that SeaBIOS doesn't use zero for the highest priority, and correctly handle BBS_DO_NOT_BOOT_FROM and BBS_IGNORE_ENTRY values. _______________________________________________ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-leave@seabios.org
For CSM, the highest priority for a boot entry is zero. SeaBIOS doesn't
use zero, and the highest priority is 1.
Make the results of csm_bootprio_*() conform to that convention. Also
explicitly handle the BBS_DO_NOT_BOOT_FROM and BBS_IGNORE_ENTRY values.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
---
v2: No code change, just correct the commit message.
src/fw/csm.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/src/fw/csm.c b/src/fw/csm.c
index 3fcc252..7663d31 100644
--- a/src/fw/csm.c
+++ b/src/fw/csm.c
@@ -319,6 +319,20 @@ handle_csm(struct bregs *regs)
csm_return(regs);
}
+static int csm_prio_to_seabios(u16 csm_prio)
+{
+ switch (csm_prio) {
+ case BBS_DO_NOT_BOOT_FROM:
+ case BBS_IGNORE_ENTRY:
+ return -1;
+
+ case BBS_LOWEST_PRIORITY:
+ case BBS_UNPRIORITIZED_ENTRY:
+ default:
+ return csm_prio + 1;
+ }
+}
+
int csm_bootprio_ata(struct pci_device *pci, int chanid, int slave)
{
if (!csm_boot_table)
@@ -327,7 +341,7 @@ int csm_bootprio_ata(struct pci_device *pci, int chanid, int slave)
int index = 1 + (chanid * 2) + slave;
dprintf(3, "CSM bootprio for ATA%d,%d (index %d) is %d\n", chanid, slave,
index, bbs[index].BootPriority);
- return bbs[index].BootPriority;
+ return csm_prio_to_seabios(bbs[index].BootPriority);
}
int csm_bootprio_fdc(struct pci_device *pci, int port, int fdid)
@@ -336,7 +350,7 @@ int csm_bootprio_fdc(struct pci_device *pci, int port, int fdid)
return -1;
BBS_TABLE *bbs = (void *)csm_boot_table->BbsTable;
dprintf(3, "CSM bootprio for FDC is %d\n", bbs[0].BootPriority);
- return bbs[0].BootPriority;
+ return csm_prio_to_seabios(bbs[0].BootPriority);
}
int csm_bootprio_pci(struct pci_device *pci)
@@ -350,7 +364,7 @@ int csm_bootprio_pci(struct pci_device *pci)
if (pci->bdf == pci_to_bdf(bbs[i].Bus, bbs[i].Device, bbs[i].Function)) {
dprintf(3, "CSM bootprio for PCI(%d,%d,%d) is %d\n", bbs[i].Bus,
bbs[i].Device, bbs[i].Function, bbs[i].BootPriority);
- return bbs[i].BootPriority;
+ return csm_prio_to_seabios(bbs[i].BootPriority);
}
}
return -1;
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
On Thu, Jun 20, 2019 at 01:07:45PM +0100, David Woodhouse wrote: > For CSM, the highest priority for a boot entry is zero. SeaBIOS doesn't > use zero, and the highest priority is 1. FYI, SeaBIOS does treat zero as the highest priority. And a negative priority means "use default priority". I'm fine with the change, though. -Kevin _______________________________________________ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-leave@seabios.org
On Thu, 2019-06-20 at 09:43 -0400, Kevin O'Connor wrote:
> On Thu, Jun 20, 2019 at 01:07:45PM +0100, David Woodhouse wrote:
> > For CSM, the highest priority for a boot entry is zero. SeaBIOS doesn't
> > use zero, and the highest priority is 1.
>
> FYI, SeaBIOS does treat zero as the highest priority. And a negative
> priority means "use default priority".
>
> I'm fine with the change, though.
I don't think find_prio ever returns zero.
dprintf(1, "Searching bootorder for: %s\n", glob);
int i;
for (i = 0; i < BootorderCount; i++)
if (glob_prefix(glob, Bootorder[i]))
return i+1;
return -1;
Our downstream patch for not initialising NVMe controllers if we aren't
going to boot from them, makes its decision based on the priority.
Originally it had 'if (bootprio_find_pci_device(pci) == 1)' to start
only the top priority boot device; nowadays it has '> 0' to match all
bootable drives.
Now, I can fix that patch fairly easily to be '>= 0' now that it no
longer wants to match precisely only the first. But in general, it
seems that '1' is the top priority so making the CSM values match that
seems sensible.
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
On Thu, Jun 20, 2019 at 03:12:33PM +0100, David Woodhouse wrote: > On Thu, 2019-06-20 at 09:43 -0400, Kevin O'Connor wrote: > > On Thu, Jun 20, 2019 at 01:07:45PM +0100, David Woodhouse wrote: > > > For CSM, the highest priority for a boot entry is zero. SeaBIOS doesn't > > > use zero, and the highest priority is 1. > > > > FYI, SeaBIOS does treat zero as the highest priority. And a negative > > priority means "use default priority". > > > > I'm fine with the change, though. > > I don't think find_prio ever returns zero. > > dprintf(1, "Searching bootorder for: %s\n", glob); > int i; > for (i = 0; i < BootorderCount; i++) > if (glob_prefix(glob, Bootorder[i])) > return i+1; > return -1; True, but interactive_bootmenu() sets the priority to zero if the device is selected. (I'm guessing find_prio() returned 1+ just to avoid confusion with interactive_bootmenu().) In any case, I'm fine with your change. -Kevin _______________________________________________ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-leave@seabios.org
Hi, > Our downstream patch for not initialising NVMe controllers if we aren't > going to boot from them, makes its decision based on the priority. What is the state of that patch btw? I remember it being posted a while back, suggestion was to make it generic (skip init for everything which has a priority higher than "HALT", not only nvm), but the discussion went silent quickly and IIRC there never was a v2 of that patch ... cheers, Gerd _______________________________________________ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-leave@seabios.org
On Fri, 2019-06-21 at 10:40 +0200, Gerd Hoffmann wrote: > Hi, > > > Our downstream patch for not initialising NVMe controllers if we aren't > > going to boot from them, makes its decision based on the priority. > > What is the state of that patch btw? It's on my "technical debt that I want to kill so we're using a pristine upstream again" list. I'll take a look after I've fixed the final quirks and got CSM boots working seamlessly again. > I remember it being posted a while back, suggestion was to make it > generic (skip init for everything which has a priority higher than > "HALT", not only nvm), but the discussion went silent quickly and IIRC > there never was a v2 of that patch ... If we do that, then those drives won't be available to INT13h at all. We'd be saying that just because we don't want to *boot* from them, DOS can't see them at all. Would we assign INT13h drive numbers to them and initialise them on demand? I don't think that can work because we don't even know if an ATA drive is present, don't know how many NVMe namespaces are present, so couldn't assign numbers accordingly. Perhaps the first INT13h access to a drive that doesn't exist (0x81 normally?) would trigger *all* pending drive probes to happen? Can we even do them that late? Or do we make it an option — at build time or through fw_cfg or something else, to just *not* initialise those drives at all because we know we're not booting DOS today? _______________________________________________ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-leave@seabios.org
On Fri, Jun 21, 2019 at 11:33:08AM +0100, David Woodhouse wrote: > On Fri, 2019-06-21 at 10:40 +0200, Gerd Hoffmann wrote: > > Hi, > > > > > Our downstream patch for not initialising NVMe controllers if we aren't > > > going to boot from them, makes its decision based on the priority. > > > > What is the state of that patch btw? > > It's on my "technical debt that I want to kill so we're using a > pristine upstream again" list. Nice. > > I remember it being posted a while back, suggestion was to make it > > generic (skip init for everything which has a priority higher than > > "HALT", not only nvm), but the discussion went silent quickly and IIRC > > there never was a v2 of that patch ... > > If we do that, then those drives won't be available to INT13h at all. > We'd be saying that just because we don't want to *boot* from them, DOS > can't see them at all. > > Would we assign INT13h drive numbers to them and initialise them on > demand? I don't think that can work because we don't even know if an > ATA drive is present, don't know how many NVMe namespaces are present, > so couldn't assign numbers accordingly. > > Perhaps the first INT13h access to a drive that doesn't exist (0x81 > normally?) would trigger *all* pending drive probes to happen? Can we > even do them that late? > > Or do we make it an option — at build time or through fw_cfg or > something else, to just *not* initialise those drives at all because we > know we're not booting DOS today? It already is configurable. The "HALT" boot priority entry is only added to the list by qemu in case the user specifies "-boot strict=on" on the command line. With strict=on seabios will only boot from devices which have a boot priority explicitly assigned. With strict=off seabios will continue trying the remaining devices in default priority order in case all devices with an assigned priority failed. Extending the strict=on case to also skip the device initialization looks reasonable to me. Faster boot time, less memory consumption, and if you don't want that or need DOS access the second hard disk you can use strict=off (which is the default btw). cheers, Gerd _______________________________________________ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-leave@seabios.org
Explicitly handle the BBS_DO_NOT_BOOT_FROM and BBS_IGNORE_ENTRY values.
Also add one to the other priority values, as find_prio() does for
entries from bootorder. SeaBIOS uses zero for an item explicitly
selected in interactive_bootmenu().
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
---
v2: No code change, just correct the commit message.
v3: Add comment in code as requested by agraf, update commit message more.
src/fw/csm.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/src/fw/csm.c b/src/fw/csm.c
index 3fcc252..8359bcb 100644
--- a/src/fw/csm.c
+++ b/src/fw/csm.c
@@ -319,6 +319,23 @@ handle_csm(struct bregs *regs)
csm_return(regs);
}
+static int csm_prio_to_seabios(u16 csm_prio)
+{
+ switch (csm_prio) {
+ case BBS_DO_NOT_BOOT_FROM:
+ case BBS_IGNORE_ENTRY:
+ return -1;
+
+ case BBS_LOWEST_PRIORITY:
+ case BBS_UNPRIORITIZED_ENTRY:
+ default:
+ // SeaBIOS default priorities start at 1, with 0 being used for
+ // an item explicitly selected from interactive_bootmenu().
+ // As in find_prio(), add 1 to the value being returned.
+ return csm_prio + 1;
+ }
+}
+
int csm_bootprio_ata(struct pci_device *pci, int chanid, int slave)
{
if (!csm_boot_table)
@@ -327,7 +344,7 @@ int csm_bootprio_ata(struct pci_device *pci, int chanid, int slave)
int index = 1 + (chanid * 2) + slave;
dprintf(3, "CSM bootprio for ATA%d,%d (index %d) is %d\n", chanid, slave,
index, bbs[index].BootPriority);
- return bbs[index].BootPriority;
+ return csm_prio_to_seabios(bbs[index].BootPriority);
}
int csm_bootprio_fdc(struct pci_device *pci, int port, int fdid)
@@ -336,7 +353,7 @@ int csm_bootprio_fdc(struct pci_device *pci, int port, int fdid)
return -1;
BBS_TABLE *bbs = (void *)csm_boot_table->BbsTable;
dprintf(3, "CSM bootprio for FDC is %d\n", bbs[0].BootPriority);
- return bbs[0].BootPriority;
+ return csm_prio_to_seabios(bbs[0].BootPriority);
}
int csm_bootprio_pci(struct pci_device *pci)
@@ -350,7 +367,7 @@ int csm_bootprio_pci(struct pci_device *pci)
if (pci->bdf == pci_to_bdf(bbs[i].Bus, bbs[i].Device, bbs[i].Function)) {
dprintf(3, "CSM bootprio for PCI(%d,%d,%d) is %d\n", bbs[i].Bus,
bbs[i].Device, bbs[i].Function, bbs[i].BootPriority);
- return bbs[i].BootPriority;
+ return csm_prio_to_seabios(bbs[i].BootPriority);
}
}
return -1;
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
On Fri, Jun 28, 2019 at 02:57:47PM +0100, David Woodhouse wrote: > Explicitly handle the BBS_DO_NOT_BOOT_FROM and BBS_IGNORE_ENTRY values. > > Also add one to the other priority values, as find_prio() does for > entries from bootorder. SeaBIOS uses zero for an item explicitly > selected in interactive_bootmenu(). Thanks, I committed this change. -Kevin _______________________________________________ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-leave@seabios.org
© 2016 - 2023 Red Hat, Inc.