[SeaBIOS] [PATCH] Support multiple hard disks in boot list.

Gaurav Poothia posted 1 patch 6 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/seabios tags/patchew/20170701004827.GA19037@centos7.nutanix.com
src/block.c |  3 ++-
src/block.h |  2 +-
src/boot.c  | 30 +++++++++++++++++++-----------
3 files changed, 22 insertions(+), 13 deletions(-)
[SeaBIOS] [PATCH] Support multiple hard disks in boot list.
Posted by Gaurav Poothia 6 years, 9 months ago
If multiple disks in boot order and first is unbootable
then fallback to other disks in list.

Signed-off-by: Gaurav Poothia <gaurav.poothia@gmail.com>
---
 src/block.c |  3 ++-
 src/block.h |  2 +-
 src/boot.c  | 30 +++++++++++++++++++-----------
 3 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/src/block.c b/src/block.c
index d104f6d..5c78533 100644
--- a/src/block.c
+++ b/src/block.c
@@ -238,7 +238,7 @@ add_drive(struct drive_s **idmap, u8 *count, struct drive_s *drive)
 }
 
 // Map a hard drive
-void
+int
 map_hd_drive(struct drive_s *drive)
 {
     ASSERT32FLAT();
@@ -252,6 +252,7 @@ map_hd_drive(struct drive_s *drive)
 
     // Fill "fdpt" structure.
     fill_fdpt(drive, hdid);
+    return hdid;
 }
 
 // Map a cd
diff --git a/src/block.h b/src/block.h
index f03ec38..68f2f32 100644
--- a/src/block.h
+++ b/src/block.h
@@ -109,7 +109,7 @@ extern u8 *bounce_buf_fl;
 struct drive_s *getDrive(u8 exttype, u8 extdriveoffset);
 int getDriveId(u8 exttype, struct drive_s *drive);
 void map_floppy_drive(struct drive_s *drive);
-void map_hd_drive(struct drive_s *drive);
+int map_hd_drive(struct drive_s *drive);
 void map_cd_drive(struct drive_s *drive);
 struct int13dpt_s;
 int fill_edd(struct segoff_s edd, struct drive_s *drive_gf);
diff --git a/src/boot.c b/src/boot.c
index 59623fb..9b9cf91 100644
--- a/src/boot.c
+++ b/src/boot.c
@@ -527,16 +527,16 @@ interactive_bootmenu(void)
 struct bev_s {
     int type;
     u32 vector;
+    // Applicable to hard disks only
+    int hdid;
 };
 static struct bev_s BEV[20];
 static int BEVCount;
 static int HaveHDBoot, HaveFDBoot;
 
 static void
-add_bev(int type, u32 vector)
+add_bev(int type, u32 vector, int hdid)
 {
-    if (type == IPL_TYPE_HARDDISK && HaveHDBoot++)
-        return;
     if (type == IPL_TYPE_FLOPPY && HaveFDBoot++)
         return;
     if (BEVCount >= ARRAY_SIZE(BEV))
@@ -544,12 +544,15 @@ add_bev(int type, u32 vector)
     struct bev_s *bev = &BEV[BEVCount++];
     bev->type = type;
     bev->vector = vector;
+    bev->hdid = hdid;
 }
 
+
 // Prepare for boot - show menu and run bcvs.
 void
 bcv_prepboot(void)
 {
+    int hdid = -1;
     if (! CONFIG_BOOT)
         return;
 
@@ -563,28 +566,30 @@ bcv_prepboot(void)
         switch (pos->type) {
         case IPL_TYPE_BCV:
             call_bcv(pos->vector.seg, pos->vector.offset);
-            add_bev(IPL_TYPE_HARDDISK, 0);
+            add_bev(IPL_TYPE_HARDDISK, 0, -1);
             break;
         case IPL_TYPE_FLOPPY:
             map_floppy_drive(pos->drive);
-            add_bev(IPL_TYPE_FLOPPY, 0);
+            add_bev(IPL_TYPE_FLOPPY, 0, -1);
             break;
         case IPL_TYPE_HARDDISK:
-            map_hd_drive(pos->drive);
-            add_bev(IPL_TYPE_HARDDISK, 0);
+            hdid = map_hd_drive(pos->drive);
+            add_bev(IPL_TYPE_HARDDISK, 0, hdid);
+            HaveHDBoot++;
             break;
         case IPL_TYPE_CDROM:
             map_cd_drive(pos->drive);
             // NO BREAK
         default:
-            add_bev(pos->type, pos->data);
+            add_bev(pos->type, pos->data, -1);
             break;
         }
     }
 
     // If nothing added a floppy/hd boot - add it manually.
-    add_bev(IPL_TYPE_FLOPPY, 0);
-    add_bev(IPL_TYPE_HARDDISK, 0);
+    add_bev(IPL_TYPE_FLOPPY, 0, -1);
+    if (HaveHDBoot == 0)
+      add_bev(IPL_TYPE_HARDDISK, 0, -1);
 }
 
 
@@ -731,7 +736,10 @@ do_boot(int seq_nr)
         break;
     case IPL_TYPE_HARDDISK:
         printf("Booting from Hard Disk...\n");
-        boot_disk(0x80, 1);
+        if (ie->hdid == -1)
+          boot_disk(0x80 , 1);
+        else
+          boot_disk(0x80 + ie->hdid, 1);
         break;
     case IPL_TYPE_CDROM:
         boot_cdrom((void*)ie->vector);
-- 
1.8.3.1


_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] Support multiple hard disks in boot list.
Posted by Kevin O'Connor 6 years, 9 months ago
On Fri, Jun 30, 2017 at 08:48:27PM -0400, Gaurav Poothia wrote:
> If multiple disks in boot order and first is unbootable
> then fallback to other disks in list.
[...]
> @@ -731,7 +736,10 @@ do_boot(int seq_nr)
>          break;
>      case IPL_TYPE_HARDDISK:
>          printf("Booting from Hard Disk...\n");
> -        boot_disk(0x80, 1);
> +        if (ie->hdid == -1)
> +          boot_disk(0x80 , 1);
> +        else
> +          boot_disk(0x80 + ie->hdid, 1);
>          break;
>      case IPL_TYPE_CDROM:
>          boot_cdrom((void*)ie->vector);

Ultimately, what this patch does is set the DL register to something
other than 0x80.  I tested this approach a few years ago, and at the
time I could not find a single bootloader that used DL.  What
bootloaders did you test with this, and which ones worked and did not
work?

-Kevin

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] Support multiple hard disks in boot list.
Posted by Gaurav Poothia 6 years, 9 months ago
Hi Kevin,
Thanks for your response!

I have tested this successfully against grub (specifically GRUB 2)
Since you asked I went digging for the relevant code in grub - here is
the line of code that will validate range of acceptable DL values and
if acceptable jump ahead to boot from that disk and skip hardcoding DL
to 0x80:
https://github.com/coreos/grub/blob/master/grub-core/boot/i386/pc/boot.S#L222

So for example if bios were to set DL to 0x81 (by fallback logic in
the patch) that will be acceptable to grub

On Fri, Jul 7, 2017 at 7:55 AM, Kevin O'Connor <kevin@koconnor.net> wrote:
> On Fri, Jun 30, 2017 at 08:48:27PM -0400, Gaurav Poothia wrote:
>> If multiple disks in boot order and first is unbootable
>> then fallback to other disks in list.
> [...]
>> @@ -731,7 +736,10 @@ do_boot(int seq_nr)
>>          break;
>>      case IPL_TYPE_HARDDISK:
>>          printf("Booting from Hard Disk...\n");
>> -        boot_disk(0x80, 1);
>> +        if (ie->hdid == -1)
>> +          boot_disk(0x80 , 1);
>> +        else
>> +          boot_disk(0x80 + ie->hdid, 1);
>>          break;
>>      case IPL_TYPE_CDROM:
>>          boot_cdrom((void*)ie->vector);
>
> Ultimately, what this patch does is set the DL register to something
> other than 0x80.  I tested this approach a few years ago, and at the
> time I could not find a single bootloader that used DL.  What
> bootloaders did you test with this, and which ones worked and did not
> work?
>
> -Kevin



-- 
Kiva.org - Loans That Change Lives

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] Support multiple hard disks in boot list.
Posted by Gaurav Poothia 6 years, 9 months ago
Hi Kevin,
I also tried this patch to fallback from non bootable SCSI disk to
bootable SCSI disk with Windows installed. That worked too.
So while we cannot see the Windows bootloader source it sure seems to
be honoring the DL values other than 0x80

Thanks
Gaurav

On Mon, Jul 10, 2017 at 1:49 PM, Gaurav Poothia
<gaurav.poothia@gmail.com> wrote:
> Hi Kevin,
> Thanks for your response!
>
> I have tested this successfully against grub (specifically GRUB 2)
> Since you asked I went digging for the relevant code in grub - here is
> the line of code that will validate range of acceptable DL values and
> if acceptable jump ahead to boot from that disk and skip hardcoding DL
> to 0x80:
> https://github.com/coreos/grub/blob/master/grub-core/boot/i386/pc/boot.S#L222
>
> So for example if bios were to set DL to 0x81 (by fallback logic in
> the patch) that will be acceptable to grub
>
> On Fri, Jul 7, 2017 at 7:55 AM, Kevin O'Connor <kevin@koconnor.net> wrote:
>> On Fri, Jun 30, 2017 at 08:48:27PM -0400, Gaurav Poothia wrote:
>>> If multiple disks in boot order and first is unbootable
>>> then fallback to other disks in list.
>> [...]
>>> @@ -731,7 +736,10 @@ do_boot(int seq_nr)
>>>          break;
>>>      case IPL_TYPE_HARDDISK:
>>>          printf("Booting from Hard Disk...\n");
>>> -        boot_disk(0x80, 1);
>>> +        if (ie->hdid == -1)
>>> +          boot_disk(0x80 , 1);
>>> +        else
>>> +          boot_disk(0x80 + ie->hdid, 1);
>>>          break;
>>>      case IPL_TYPE_CDROM:
>>>          boot_cdrom((void*)ie->vector);
>>
>> Ultimately, what this patch does is set the DL register to something
>> other than 0x80.  I tested this approach a few years ago, and at the
>> time I could not find a single bootloader that used DL.  What
>> bootloaders did you test with this, and which ones worked and did not
>> work?
>>
>> -Kevin
>
>
>
> --
> Kiva.org - Loans That Change Lives



-- 
Kiva.org - Loans That Change Lives

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] Support multiple hard disks in boot list.
Posted by Kevin O'Connor 6 years, 9 months ago
On Thu, Jul 13, 2017 at 05:30:43PM -0700, Gaurav Poothia wrote:
> Hi Kevin,
> I also tried this patch to fallback from non bootable SCSI disk to
> bootable SCSI disk with Windows installed. That worked too.
> So while we cannot see the Windows bootloader source it sure seems to
> be honoring the DL values other than 0x80

Thanks.  I tried a handful of images I had handy and found a bunch of
failures (see testing patch I used below).

lilo - failed
freedos - failed subtly
winxp - failed
winvista - success
fedora 13 disk - failed

Unfortunately, I don't think this is a change that could be made
without near universal support.  As, when it does fail, it fails in a
very cryptic way.  I think it would be more pain for users than gain.

-Kevin


--- a/src/boot.c
+++ b/src/boot.c
@@ -731,7 +731,7 @@ do_boot(int seq_nr)
         break;
     case IPL_TYPE_HARDDISK:
         printf("Booting from Hard Disk...\n");
-        boot_disk(0x80, 1);
+        boot_disk(0x81, 1);
         break;
     case IPL_TYPE_CDROM:
         boot_cdrom((void*)ie->vector);

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios