[SeaBIOS] [PATCH] ramdisk: search for all available floppy images instead of one

Mike Banon posted 1 patch 5 years, 5 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
[SeaBIOS] [PATCH] ramdisk: search for all available floppy images instead of one
Posted by Mike Banon 5 years, 5 months ago
All the floppy images available at CBFS will be found and listed in a
boot menu, instead of the first found. Could be highly valuable if you
are participating in a hobby OS development - would like to test
multiple versions of your floppy in the same coreboot image, to reduce
the amount of re-flashes and accelerate the development at bare metal
- or simply would like to access multiple floppies as a coreboot user;
for example: KolibriOS (nice assembly OS with GUI), FreeDOS, Visopsys
and memtest (coreboot's own memtest version is a bit buggy, e.g.
external USB keyboard isn't working there at some laptops)

Shortened patch description message:

All the available floppy images will be found and listed in a boot
menu, instead of the first found. Useful if you are participating in a
hobby OS development or would like to access multiple floppies as a
coreboot user.

Signed-off-by: Mike Banon <mikebdp2@gmail.com>
---
diff --git a/src/hw/ramdisk.c b/src/hw/ramdisk.c
index b9e9baa..0cd8470 100644
--- a/src/hw/ramdisk.c
+++ b/src/hw/ramdisk.c
@@ -23,40 +23,51 @@ ramdisk_setup(void)
     if (!CONFIG_FLASH_FLOPPY)
         return;

+    struct romfile_s *file = NULL;
+    char *filename, *desc;
+    u32 size;
+    int ftype, ret;
+    void *pos;
+    struct drive_s *drive;
+
+    for (;;) {
+
-    // Find image.
-    struct romfile_s *file = romfile_findprefix("floppyimg/", NULL);
-    if (!file)
-        return;
+        // Find the next image.
+        file = romfile_findprefix("floppyimg/", file);
+        if (!file)
+            return;
-    const char *filename = file->name;
-    u32 size = file->size;
-    dprintf(3, "Found floppy file %s of size %d\n", filename, size);
-    int ftype = find_floppy_type(size);
-    if (ftype < 0) {
-        dprintf(3, "No floppy type found for ramdisk size\n");
-        return;
-    }
+        filename = file->name;
+        size = file->size;
+        dprintf(3, "Found floppy file %s of size %d\n", filename, size);
+        ftype = find_floppy_type(size);
+        if (ftype < 0) {
+            dprintf(3, "No floppy type found for ramdisk size\n");
+            return;
+        }

-    // Allocate ram for image.
-    void *pos = memalign_tmphigh(PAGE_SIZE, size);
-    if (!pos) {
-        warn_noalloc();
-        return;
-    }
-    e820_add((u32)pos, size, E820_RESERVED);
+        // Allocate ram for image.
+        pos = memalign_tmphigh(PAGE_SIZE, size);
+        if (!pos) {
+            warn_noalloc();
+            return;
+        }
+        e820_add((u32)pos, size, E820_RESERVED);

-    // Copy image into ram.
-    int ret = file->copy(file, pos, size);
-    if (ret < 0)
-        return;
+        // Copy image into ram.
+        ret = file->copy(file, pos, size);
+        if (ret < 0)
+            return;

-    // Setup driver.
-    struct drive_s *drive = init_floppy((u32)pos, ftype);
-    if (!drive)
-        return;
+        // Setup driver.
+        drive = init_floppy((u32)pos, ftype);
+        if (!drive)
+            return;
-    drive->type = DTYPE_RAMDISK;
-    dprintf(1, "Mapping floppy %s to addr %p\n", filename, pos);
-    char *desc = znprintf(MAXDESCSIZE, "Ramdisk [%s]", &filename[10]);
-    boot_add_floppy(drive, desc, bootprio_find_named_rom(filename, 0));
+        drive->type = DTYPE_RAMDISK;
+        dprintf(1, "Mapping floppy %s to addr %p\n", filename, pos);
+        desc = znprintf(MAXDESCSIZE, "Ramdisk [%s]", &filename[10]);
+        boot_add_floppy(drive, desc, bootprio_find_named_rom(filename, 0));
+
+    }
 }

 static int
_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] ramdisk: search for all available floppy images instead of one
Posted by Kevin O'Connor 5 years, 5 months ago
On Fri, Nov 02, 2018 at 01:07:20AM +0300, Mike Banon wrote:
> All the floppy images available at CBFS will be found and listed in a
> boot menu, instead of the first found. Could be highly valuable if you
> are participating in a hobby OS development - would like to test
> multiple versions of your floppy in the same coreboot image, to reduce
> the amount of re-flashes and accelerate the development at bare metal
> - or simply would like to access multiple floppies as a coreboot user;
> for example: KolibriOS (nice assembly OS with GUI), FreeDOS, Visopsys
> and memtest (coreboot's own memtest version is a bit buggy, e.g.
> external USB keyboard isn't working there at some laptops)

Thanks.  However I'm concerned that this would reserve memory for all
of the floppy images in the e820 map (and thus the final OS would not
be able to use that memory).  I think if multiple floppy support was
desired then we'd want to delay memory reservation until after a
particular image was selected in the boot menu.

-Kevin

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] ramdisk: search for all available floppy images instead of one
Posted by Mike Banon 5 years, 5 months ago
> However I'm concerned that this would reserve memory for all
> of the floppy images in the e820 map (and thus the final OS
> would not be able to use that memory).

sorry Kevin I'm not fully understanding the consequences. Would
reserving e820 memory for all the floppies - result in a slightly
lower amount of available RAM to OS (e.g. just 14 MB lower if I add 10
floppies) or there could be more serious problems? When I've been
testing this patch - together with "35 boot menu entries" patch by
Ivan - https://mail.coreboot.org/pipermail/seabios/2017-June/011416.html
- I added about 30 floppies and everything seemed to work perfectly.
If there are no other possible problems, I am okay with slightly lower
amount of RAM - and only the "multiple floppies" users will be
affected. Also, what's good about this patch is that its' only a small
change for SeaBIOS source code - mostly placing the "floppy searching"
code inside the "for" cycle. But I could try to do something with the
allocation part if that wouldn't turn out too difficult




On Sat, Nov 10, 2018 at 9:16 PM Kevin O'Connor <kevin@koconnor.net> wrote:
>
> On Fri, Nov 02, 2018 at 01:07:20AM +0300, Mike Banon wrote:
> > All the floppy images available at CBFS will be found and listed in a
> > boot menu, instead of the first found. Could be highly valuable if you
> > are participating in a hobby OS development - would like to test
> > multiple versions of your floppy in the same coreboot image, to reduce
> > the amount of re-flashes and accelerate the development at bare metal
> > - or simply would like to access multiple floppies as a coreboot user;
> > for example: KolibriOS (nice assembly OS with GUI), FreeDOS, Visopsys
> > and memtest (coreboot's own memtest version is a bit buggy, e.g.
> > external USB keyboard isn't working there at some laptops)
>
> Thanks.  However I'm concerned that this would reserve memory for all
> of the floppy images in the e820 map (and thus the final OS would not
> be able to use that memory).  I think if multiple floppy support was
> desired then we'd want to delay memory reservation until after a
> particular image was selected in the boot menu.
>
> -Kevin

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] ramdisk: search for all available floppy images instead of one
Posted by Kevin O'Connor 5 years, 5 months ago
On Sun, Nov 11, 2018 at 02:12:09AM +0300, Mike Banon wrote:
> > However I'm concerned that this would reserve memory for all
> > of the floppy images in the e820 map (and thus the final OS
> > would not be able to use that memory).
> 
> sorry Kevin I'm not fully understanding the consequences. Would
> reserving e820 memory for all the floppies - result in a slightly
> lower amount of available RAM to OS (e.g. just 14 MB lower if I add 10
> floppies) or there could be more serious problems?

Just the loss of 14MB of ram.  It seems odd to reserve that ram when I
expect one would rarely use it.  Arguably, seabios shouldn't reserve
even 1.4MB of ram if one isn't actually booting from the given image,
but extending this to every image found when most wouldn't even be
accessible seems even worse.

>When I've been
> testing this patch - together with "35 boot menu entries" patch by
> Ivan - https://mail.coreboot.org/pipermail/seabios/2017-June/011416.html
> - I added about 30 floppies and everything seemed to work perfectly.
> If there are no other possible problems, I am okay with slightly lower
> amount of RAM - and only the "multiple floppies" users will be
> affected. Also, what's good about this patch is that its' only a small
> change for SeaBIOS source code - mostly placing the "floppy searching"
> code inside the "for" cycle. But I could try to do something with the
> allocation part if that wouldn't turn out too difficult

Okay, thanks.  We're planning to make a SeaBIOS release in a few days
and this change would be post release anyway.

-Kevin

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] ramdisk: search for all available floppy images instead of one
Posted by Mike Banon 5 years, 5 months ago
Thank you very much for clarifying, Kevin. I finally moved this ram
allocation to do_boot function (line 800) at boot.c - after the
printf("Booting from Floppy...\n") - but sadly it seems that
memalign_tmphigh and similar malloc'ing functions (like
malloc_tmphigh) are not allowed at this stage:

ERROR: .data.varinit.. is VARVERIFY32INIT but used from
['.text.runtime..', '.text.do_boot', '.text.malloc_tmphigh']
Makefile:162: recipe for target 'out/romlayout16.lds' failed
make[2]: *** [out/romlayout16.lds] Error 1

I can't understand this cryptic error, and especially why it's
happening with "malloc functions" at do_boot while they're working
fine at loadBootOrder function from the same boot.c file (line 50) :
Bootorder = malloc_tmphigh(BootorderCount*sizeof(char*)); Out of
curiosity I tried restoring do_boot to its' original state then
inserted this line of code there ( Bootorder and BootorderCount are
globally declared ) and got a similar error.

Is it true that malloc'ing during the do_boot is impossible (why?) -
and, if yes, what are the possible workarounds? Maybe I could rewrite
it to leave a single "malloc_tmphigh" inside ramdisk_setup / ramdisk.c
for the size of largest floppy (will look through the floppies at CBFS
and find the largest one) and use this allocated RAM space for any
floppy that would be selected later. <--- This way I would allocate
just 1.4MB if there are 10 * 1.4M floppies and should be less code
changes
On Tue, Nov 13, 2018 at 9:06 PM Kevin O'Connor <kevin@koconnor.net> wrote:
>
> On Sun, Nov 11, 2018 at 02:12:09AM +0300, Mike Banon wrote:
> > > However I'm concerned that this would reserve memory for all
> > > of the floppy images in the e820 map (and thus the final OS
> > > would not be able to use that memory).
> >
> > sorry Kevin I'm not fully understanding the consequences. Would
> > reserving e820 memory for all the floppies - result in a slightly
> > lower amount of available RAM to OS (e.g. just 14 MB lower if I add 10
> > floppies) or there could be more serious problems?
>
> Just the loss of 14MB of ram.  It seems odd to reserve that ram when I
> expect one would rarely use it.  Arguably, seabios shouldn't reserve
> even 1.4MB of ram if one isn't actually booting from the given image,
> but extending this to every image found when most wouldn't even be
> accessible seems even worse.
>
> >When I've been
> > testing this patch - together with "35 boot menu entries" patch by
> > Ivan - https://mail.coreboot.org/pipermail/seabios/2017-June/011416.html
> > - I added about 30 floppies and everything seemed to work perfectly.
> > If there are no other possible problems, I am okay with slightly lower
> > amount of RAM - and only the "multiple floppies" users will be
> > affected. Also, what's good about this patch is that its' only a small
> > change for SeaBIOS source code - mostly placing the "floppy searching"
> > code inside the "for" cycle. But I could try to do something with the
> > allocation part if that wouldn't turn out too difficult
>
> Okay, thanks.  We're planning to make a SeaBIOS release in a few days
> and this change would be post release anyway.
>
> -Kevin

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] ramdisk: search for all available floppy images instead of one
Posted by Mike Banon 5 years, 4 months ago
Dear Kevin,

Please could you tell me, why the memory allocation functions like
malloc_tmphigh() are failing at do_boot() stage? (boot.c) . Because of
this "VARVERIFY32INIT" problem described in my letter above -
https://mail.coreboot.org/pipermail/seabios/2018-November/012575.html
- I rewrote all my code to allocate the RAM for one floppy only (space
equal to max floppy size, which could be used by any selected floppy)
. But when I'm trying to boot that selected floppy - it is failing,
because these floppies are LZMA compressed inside CBFS and to
decompress them another malloc_tmphigh() is needed (from
cbfs_copyfile() / coreboot.c) / Although this code compiles fine (no
VARVERIFY32INIT problem) - this malloc_tmphigh() function is always
failing and SeaBIOS is freezing.

Spent all my weekend debugging this problem and really stuck, any help
will be highly appreciated

Best regards,
Mike Banon
On Sat, Nov 17, 2018 at 2:30 AM Mike Banon <mikebdp2@gmail.com> wrote:
>
> Thank you very much for clarifying, Kevin. I finally moved this ram
> allocation to do_boot function (line 800) at boot.c - after the
> printf("Booting from Floppy...\n") - but sadly it seems that
> memalign_tmphigh and similar malloc'ing functions (like
> malloc_tmphigh) are not allowed at this stage:
>
> ERROR: .data.varinit.. is VARVERIFY32INIT but used from
> ['.text.runtime..', '.text.do_boot', '.text.malloc_tmphigh']
> Makefile:162: recipe for target 'out/romlayout16.lds' failed
> make[2]: *** [out/romlayout16.lds] Error 1
>
> I can't understand this cryptic error, and especially why it's
> happening with "malloc functions" at do_boot while they're working
> fine at loadBootOrder function from the same boot.c file (line 50) :
> Bootorder = malloc_tmphigh(BootorderCount*sizeof(char*)); Out of
> curiosity I tried restoring do_boot to its' original state then
> inserted this line of code there ( Bootorder and BootorderCount are
> globally declared ) and got a similar error.
>
> Is it true that malloc'ing during the do_boot is impossible (why?) -
> and, if yes, what are the possible workarounds? Maybe I could rewrite
> it to leave a single "malloc_tmphigh" inside ramdisk_setup / ramdisk.c
> for the size of largest floppy (will look through the floppies at CBFS
> and find the largest one) and use this allocated RAM space for any
> floppy that would be selected later. <--- This way I would allocate
> just 1.4MB if there are 10 * 1.4M floppies and should be less code
> changes
> On Tue, Nov 13, 2018 at 9:06 PM Kevin O'Connor <kevin@koconnor.net> wrote:
> >
> > On Sun, Nov 11, 2018 at 02:12:09AM +0300, Mike Banon wrote:
> > > > However I'm concerned that this would reserve memory for all
> > > > of the floppy images in the e820 map (and thus the final OS
> > > > would not be able to use that memory).
> > >
> > > sorry Kevin I'm not fully understanding the consequences. Would
> > > reserving e820 memory for all the floppies - result in a slightly
> > > lower amount of available RAM to OS (e.g. just 14 MB lower if I add 10
> > > floppies) or there could be more serious problems?
> >
> > Just the loss of 14MB of ram.  It seems odd to reserve that ram when I
> > expect one would rarely use it.  Arguably, seabios shouldn't reserve
> > even 1.4MB of ram if one isn't actually booting from the given image,
> > but extending this to every image found when most wouldn't even be
> > accessible seems even worse.
> >
> > >When I've been
> > > testing this patch - together with "35 boot menu entries" patch by
> > > Ivan - https://mail.coreboot.org/pipermail/seabios/2017-June/011416.html
> > > - I added about 30 floppies and everything seemed to work perfectly.
> > > If there are no other possible problems, I am okay with slightly lower
> > > amount of RAM - and only the "multiple floppies" users will be
> > > affected. Also, what's good about this patch is that its' only a small
> > > change for SeaBIOS source code - mostly placing the "floppy searching"
> > > code inside the "for" cycle. But I could try to do something with the
> > > allocation part if that wouldn't turn out too difficult
> >
> > Okay, thanks.  We're planning to make a SeaBIOS release in a few days
> > and this change would be post release anyway.
> >
> > -Kevin

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] ramdisk: search for all available floppy images instead of one
Posted by Kevin O'Connor 5 years, 4 months ago
On Sat, Nov 24, 2018 at 11:46:53PM +0300, Mike Banon wrote:
> Dear Kevin,
> 
> Please could you tell me, why the memory allocation functions like
> malloc_tmphigh() are failing at do_boot() stage? (boot.c) . Because of
> this "VARVERIFY32INIT" problem described in my letter above -
> https://mail.coreboot.org/pipermail/seabios/2018-November/012575.html
> - I rewrote all my code to allocate the RAM for one floppy only (space
> equal to max floppy size, which could be used by any selected floppy)
> . But when I'm trying to boot that selected floppy - it is failing,
> because these floppies are LZMA compressed inside CBFS and to
> decompress them another malloc_tmphigh() is needed (from
> cbfs_copyfile() / coreboot.c) / Although this code compiles fine (no
> VARVERIFY32INIT problem) - this malloc_tmphigh() function is always
> failing and SeaBIOS is freezing.
> 
> Spent all my weekend debugging this problem and really stuck, any help
> will be highly appreciated

Hi,

Only the "post" stage is able to allocate ram.  Once the "boot" phase
starts executing, areas of memory are locked down, and it's possible
for 3rd party software to become resident in various areas of ram.
SeaBIOS thus can't touch those areas of ram or modify the e820 map.
The VARVERIFY32INIT flag is there to catch this type of common error.

There's some info on this in the docs at:
https://www.seabios.org/Memory_Model#Memory_available_during_initialization
and:
https://www.seabios.org/Execution_and_code_flow

So, any type of allocation would have to occur before the boot stage.
It should be possible (though not necessarily easy) to perform the
allocation in the map_floppy_drive() phase of the code.

-Kevin

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
[SeaBIOS] [PATCH v2] ramdisk: search for all available floppy images instead of one
Posted by Mike Banon 5 years, 4 months ago
Hi Kevin, thank you very much for the suggestions! This is a second
version of my "more_than_one_floppy" patch : now it will search for
the max size of floppy stored inside CBFS, then allocate this size of
RAM and use it later for any floppy selected by user. sha256 =
2f8bb18c1606d437de49e585769f100c085234de29d8520e2dc74d25b7f7645a
---
All the floppy images available at CBFS will be found and listed in a
boot menu, instead of the first found. Could be highly valuable if you
are participating in a hobby OS development - would like to test
multiple versions of your floppy in the same coreboot image, to reduce
the amount of re-flashes and accelerate the development at bare metal
- or simply would like to access multiple floppies as a coreboot user;
for example: KolibriOS (nice assembly OS with GUI), FreeDOS, Visopsys
and memtest (coreboot's own memtest version is a bit buggy, e.g.
external USB keyboard isn't working there at some laptops)

Shortened patch description message:

All the available floppy images will be found and listed in a boot
menu, instead of the first found. Useful if you are participating in a
hobby OS development or would like to access multiple floppies as a
coreboot user.

Signed-off-by: Mike Banon <mikebdp2 at gmail.com>
---
diff --git a/src/block.h b/src/block.h
index f64e880..aaa236f 100644
--- a/src/block.h
+++ b/src/block.h
@@ -2,7 +2,7 @@
 #define __BLOCK_H

 #include "types.h" // u32
-
+#include "romfile.h" // struct romfile_s

 /****************************************************************
  * Disk command request
@@ -48,6 +48,7 @@ struct drive_s {
 struct drive_s {
     u8 type;            // Driver type (DTYPE_*)
     u8 floppy_type;     // Type of floppy (only for floppy drives).
+    struct romfile_s *floppy_file; // Floppy file (only for virtual floppies).
     struct chs_s lchs;  // Logical CHS
     u64 sectors;        // Total sectors count
     u32 cntl_id;        // Unique id for a given driver type.
diff --git a/src/boot.c b/src/boot.c
index 9f82f3c..79f1e7d 100644
--- a/src/boot.c
+++ b/src/boot.c
@@ -584,7 +584,7 @@ bcv_prepboot(void)
             break;
         case IPL_TYPE_FLOPPY:
             map_floppy_drive(pos->drive);
-            add_bev(IPL_TYPE_FLOPPY, 0);
+            add_bev(IPL_TYPE_FLOPPY, (u32)pos->drive);
             break;
         case IPL_TYPE_HARDDISK:
             map_hd_drive(pos->drive);
@@ -733,6 +733,12 @@ do_boot(int seq_nr)
 static void
 do_boot(int seq_nr)
 {
+
+    int ret;
+    void *pos;
+    struct romfile_s *file;
+    struct drive_s *drive;
+
     if (! CONFIG_BOOT)
         panic("Boot support not compiled in.\n");

@@ -744,6 +750,16 @@ do_boot(int seq_nr)
     switch (ie->type) {
     case IPL_TYPE_FLOPPY:
         printf("Booting from Floppy...\n");
+        drive = (struct drive_s *)ie->vector;
+        file = drive->floppy_file;
+        // File is NULL if a floppy is physical.
+        if (file) {
+            // Copy virtual floppy image into ram.
+            pos = (void *)drive->cntl_id;
+            ret = file->copy(file, pos, file->size);
+            if (ret < 0)
+                break;
+        }
         boot_disk(0x00, CheckFloppySig);
         break;
     case IPL_TYPE_HARDDISK:
diff --git a/src/hw/floppy.c b/src/hw/floppy.c
index 9e6647d..5b37c6c 100644
--- a/src/hw/floppy.c
+++ b/src/hw/floppy.c
@@ -107,7 +107,7 @@ struct floppyinfo_s FloppyInfo[] VARFSEG = {
 };

 struct drive_s *
-init_floppy(int floppyid, int ftype)
+init_floppy(int floppyid, int ftype, struct romfile_s *ffile)
 {
     if (ftype <= 0 || ftype >= ARRAY_SIZE(FloppyInfo)) {
         dprintf(1, "Bad floppy type %d\n", ftype);
@@ -124,6 +124,7 @@ init_floppy(int floppyid, int ftype)
     drive->type = DTYPE_FLOPPY;
     drive->blksize = DISK_SECTOR_SIZE;
     drive->floppy_type = ftype;
+    drive->floppy_file = ffile;
     drive->sectors = (u64)-1;

     memcpy(&drive->lchs, &FloppyInfo[ftype].chs
@@ -134,7 +135,7 @@ addFloppy(int floppyid, int ftype)
 static void
 addFloppy(int floppyid, int ftype)
 {
-    struct drive_s *drive = init_floppy(floppyid, ftype);
+    struct drive_s *drive = init_floppy(floppyid, ftype, 0);
     if (!drive)
         return;
     char *desc = znprintf(MAXDESCSIZE, "Floppy [drive %c]", 'A' + floppyid);
diff --git a/src/hw/ramdisk.c b/src/hw/ramdisk.c
index b9e9baa..a679385 100644
--- a/src/hw/ramdisk.c
+++ b/src/hw/ramdisk.c
@@ -23,40 +23,69 @@ ramdisk_setup(void)
     if (!CONFIG_FLASH_FLOPPY)
         return;

-    // Find image.
-    struct romfile_s *file = romfile_findprefix("floppyimg/", NULL);
-    if (!file)
-        return;
-    const char *filename = file->name;
-    u32 size = file->size;
-    dprintf(3, "Found floppy file %s of size %d\n", filename, size);
-    int ftype = find_floppy_type(size);
-    if (ftype < 0) {
-        dprintf(3, "No floppy type found for ramdisk size\n");
+    struct romfile_s *file = NULL;
+    char *filename, *desc;
+    u32 size, max_size = 0;
+    int ftype;
+    void *pos;
+    struct drive_s *drive;
+
+    // Find the max floppy size
+    for (;;) {
+        // Find the next image.
+        file = romfile_findprefix("floppyimg/", file);
+        if (!file)
+            break;
+        filename = file->name;
+        size = file->size;
+        dprintf(3, "Found floppy file %s of size %d\n", filename, size);
+        // Check if this size is valid.
+        ftype = find_floppy_type(size);
+        if (ftype < 0) {
+            dprintf(3, "No floppy type found for ramdisk size\n");
+        } else {
+            if (size > max_size)
+                max_size = size;
+        }
+    }
+    if (max_size == 0) {
+        dprintf(3, "No floppies found\n");
         return;
     }

     // Allocate ram for image.
-    void *pos = memalign_tmphigh(PAGE_SIZE, size);
+    pos = memalign_tmphigh(PAGE_SIZE, max_size);
     if (!pos) {
         warn_noalloc();
         return;
     }
-    e820_add((u32)pos, size, E820_RESERVED);
+    e820_add((u32)pos, max_size, E820_RESERVED);
+    dprintf(3, "Allocate %u bytes for a floppy\n", max_size);

-    // Copy image into ram.
-    int ret = file->copy(file, pos, size);
-    if (ret < 0)
-        return;
-
-    // Setup driver.
-    struct drive_s *drive = init_floppy((u32)pos, ftype);
-    if (!drive)
-        return;
-    drive->type = DTYPE_RAMDISK;
-    dprintf(1, "Mapping floppy %s to addr %p\n", filename, pos);
-    char *desc = znprintf(MAXDESCSIZE, "Ramdisk [%s]", &filename[10]);
-    boot_add_floppy(drive, desc, bootprio_find_named_rom(filename, 0));
+    // Setup the floppy drivers.
+    file = NULL;
+    for (;;) {
+        // Find the next image.
+        file = romfile_findprefix("floppyimg/", file);
+        if (!file)
+            return;
+        filename = file->name;
+        size = file->size;
+        dprintf(3, "Found floppy file %s of size %d\n", filename, size);
+        ftype = find_floppy_type(size);
+        if (ftype < 0) {
+            dprintf(3, "No floppy type found for ramdisk size\n");
+        } else {
+            // Setup driver.
+            drive = init_floppy((u32)pos, ftype, file);
+            if (!drive)
+                return;
+            drive->type = DTYPE_RAMDISK;
+            dprintf(1, "Mapping floppy %s to addr %p\n", filename, pos);
+            desc = znprintf(MAXDESCSIZE, "Ramdisk [%s]", &filename[10]);
+            boot_add_floppy(drive, desc, bootprio_find_named_rom(filename, 0));
+        }
+    }
 }

 static int
diff --git a/src/util.h b/src/util.h
index 6dd080f..04f8018 100644
--- a/src/util.h
+++ b/src/util.h
@@ -147,7 +147,8 @@ void dma_setup(void);
 // hw/floppy.c
 extern struct floppy_ext_dbt_s diskette_param_table2;
 void floppy_setup(void);
-struct drive_s *init_floppy(int floppyid, int ftype);
+extern struct romfile_s *ffile;
+struct drive_s *init_floppy(int floppyid, int ftype, struct romfile_s *ffile);
 int find_floppy_type(u32 size);
 int floppy_process_op(struct disk_op_s *op);
 void floppy_tick(void);
_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v2] ramdisk: search for all available floppy images instead of one
Posted by Kevin O'Connor 5 years, 4 months ago
On Thu, Dec 06, 2018 at 07:12:19PM +0300, Mike Banon wrote:
> Hi Kevin, thank you very much for the suggestions! This is a second
> version of my "more_than_one_floppy" patch : now it will search for
> the max size of floppy stored inside CBFS, then allocate this size of
> RAM and use it later for any floppy selected by user. sha256 =
> 2f8bb18c1606d437de49e585769f100c085234de29d8520e2dc74d25b7f7645a

Thanks, see my comments below.

[...]
> ---
> diff --git a/src/block.h b/src/block.h
> index f64e880..aaa236f 100644
> --- a/src/block.h
> +++ b/src/block.h
> @@ -2,7 +2,7 @@
>  #define __BLOCK_H
> 
>  #include "types.h" // u32
> -
> +#include "romfile.h" // struct romfile_s
> 
>  /****************************************************************
>   * Disk command request
> @@ -48,6 +48,7 @@ struct drive_s {
>  struct drive_s {
>      u8 type;            // Driver type (DTYPE_*)
>      u8 floppy_type;     // Type of floppy (only for floppy drives).
> +    struct romfile_s *floppy_file; // Floppy file (only for virtual floppies).
>      struct chs_s lchs;  // Logical CHS
>      u64 sectors;        // Total sectors count
>      u32 cntl_id;        // Unique id for a given driver type.

Driver specific fields should not be added to drive_s - the code
should use the container_of() trick to allocate driver specific
fields.  (Create and allocate a custom driver struct that contains a
'struct drive_s' and when called with a drive_s use container_of() to
get a pointer back to that main struct - for an example, take a look
at what src/hw/usb-msc.c does.)

> diff --git a/src/boot.c b/src/boot.c
> index 9f82f3c..79f1e7d 100644
> --- a/src/boot.c
> +++ b/src/boot.c
> @@ -584,7 +584,7 @@ bcv_prepboot(void)
>              break;
>          case IPL_TYPE_FLOPPY:
>              map_floppy_drive(pos->drive);
> -            add_bev(IPL_TYPE_FLOPPY, 0);
> +            add_bev(IPL_TYPE_FLOPPY, (u32)pos->drive);
>              break;
>          case IPL_TYPE_HARDDISK:
>              map_hd_drive(pos->drive);
> @@ -733,6 +733,12 @@ do_boot(int seq_nr)
>  static void
>  do_boot(int seq_nr)
>  {
> +
> +    int ret;
> +    void *pos;
> +    struct romfile_s *file;
> +    struct drive_s *drive;
> +
>      if (! CONFIG_BOOT)
>          panic("Boot support not compiled in.\n");
> 
> @@ -744,6 +750,16 @@ do_boot(int seq_nr)
>      switch (ie->type) {
>      case IPL_TYPE_FLOPPY:
>          printf("Booting from Floppy...\n");
> +        drive = (struct drive_s *)ie->vector;
> +        file = drive->floppy_file;
> +        // File is NULL if a floppy is physical.
> +        if (file) {
> +            // Copy virtual floppy image into ram.
> +            pos = (void *)drive->cntl_id;
> +            ret = file->copy(file, pos, file->size);
> +            if (ret < 0)
> +                break;
> +        }

Similarly, we shouldn't add driver specific code into the boot phase.
A better place to do this would be during the BCV mapping phase (eg,
bcv_prepboot() ).  The idea is to do the work after the boot menu, but
before the boot phase.  Ideally the code would detect the driver wants
a callback and then call driver specific code that resides in
src/hw/ramdisk.c.

-Kevin

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