[PATCH 1/4] pci: Allow scanning pci bus number up to 255 in CSM mode
On real hardware especially server platforms, there might be multiple root buses, thus pci bus number could run up to 255. This patch fixed pci_probe_devices() by allowing to scan all 256 pci bus numbers(0-255). Signed-off-by: Bin Gao <gaobin@amazon.com> --- src/hw/pcidevice.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/hw/pcidevice.c b/src/hw/pcidevice.c index 8853cf7..8d9c401 100644 --- a/src/hw/pcidevice.c +++ b/src/hw/pcidevice.c @@ -26,6 +26,14 @@ pci_probe_devices(void) struct hlist_node **pprev = &PCIDevices.first; int extraroots = romfile_loadint("etc/extra-pci-roots", 0); int bus = -1, lastbus = 0, rootbuses = 0, count=0; + + // There might be multiple PCI root buses on physical hardware especially + // server platforms, thus bus number could run up to the top value, + // i.e. 0xff. Setting extraroots to 0xff to ensure we can enumerate all + // PCI bus numbers. + if (CONFIG_CSM) + extraroots = 0xff; + while (bus < 0xff && (bus < MaxPCIBus || rootbuses < extraroots)) { bus++; int bdf; -- 2.17.1
In csm mode, the bev pointer of the pci option rom was not added to the bootentry list, resulting in failure to boot from pci option rom. This patch fixed it. Also we enabled hw timer interrupt and clock update before we call the option rom's init() funtion, so the clock tick update at BDA address 40:6C will work during the rom's init() execution. Withou this change, iPXE would hang on reading BDA 40:6C due to the value at this address is not changed. Signed-off-by: Bin Gao <gaobin@amazon.com> --- src/fw/csm.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/fw/csm.c b/src/fw/csm.c index 8359bcb..f64bc73 100644 --- a/src/fw/csm.c +++ b/src/fw/csm.c @@ -19,6 +19,7 @@ #include "std/acpi.h" // RSDP_SIGNATURE #include "std/bda.h" // struct bios_data_area_s #include "std/optionrom.h" // struct rom_header +#include "std/pnpbios.h" // PNP_SIGNATURE #include "util.h" // copy_smbios #define UINT8 u8 @@ -219,6 +220,7 @@ handle_csm_0005(struct bregs *regs) { EFI_DISPATCH_OPROM_TABLE *table = MAKE_FLATPTR(regs->es, regs->bx); struct rom_header *rom; + struct pnp_data *pnp; u16 bdf; if (!CONFIG_OPTIONROMS) { @@ -240,11 +242,36 @@ handle_csm_0005(struct bregs *regs) rom_reserve(rom->size * 512); + // callrom() typically calls the rom's init() functions. + // If the rom's init() function does a delay by detecting the value + // at address 40:6C, it will fail because the value at 40:6C is + // updated only in clock_update() which is called in hw timer ISR. + // Thus we have to enable hw timer interrupt and set up clock before + // calling callrom(). + pic_setup(); + timer_setup(); + clock_setup(); + // XX PnP seg/ofs should never be other than default callrom(rom, bdf); rom_confirm(rom->size * 512); + // PnP rom - check for BEV and BCV boot capabilities. + pnp = (void*)((u8*)rom + rom->pnpoffset); + if (pnp->signature == PNP_SIGNATURE) { + struct pci_device pci_dev = { + .bdf = bdf, + }; + int prio = csm_bootprio_pci(&pci_dev); + if (pnp->bev) + boot_add_bev(FLATPTR_TO_SEG(rom), pnp->bev, pnp->productname, prio); + else if (pnp->bcv) + boot_add_bcv(FLATPTR_TO_SEG(rom), pnp->bcv, pnp->productname, prio); + } else { + dprintf(3, "rom %p: not a PnP rom, no BEV or BCV capability\n", table); + } + regs->bx = 0; // FIXME regs->ax = 0; } -- 2.17.1
On physical hardware, some PCI devices are not detectable when UEFI invokes our InitializeYourself() function. But they are guaranteed to be available before UEFI invokes our PreparToBoot() function. Signed-off-by: Bin Gao <gaobin@amazon.com> --- src/fw/csm.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/fw/csm.c b/src/fw/csm.c index f64bc73..1bd821e 100644 --- a/src/fw/csm.c +++ b/src/fw/csm.c @@ -64,7 +64,9 @@ static void csm_maininit(struct bregs *regs) { interface_init(); - pci_probe_devices(); + + if (CONFIG_CSM && CONFIG_QEMU_HARDWARE) + pci_probe_devices(); csm_compat_table.PnPInstallationCheckSegment = SEG_BIOS; csm_compat_table.PnPInstallationCheckOffset = get_pnp_offset(); @@ -144,6 +146,13 @@ handle_csm_0002(struct bregs *regs) return; } + // On physical hardware, some PCI devices are not detectable when + // UEFI invokes our InitializeYourself() function. But they + // are guaranteed to be available before UEFI invokes our + // PreparToBoot() function. + if (CONFIG_CSM && !CONFIG_QEMU_HARDWARE) + pci_probe_devices(); + dprintf(3, "PrepareToBoot table %04x:%04x\n", regs->es, regs->bx); struct e820entry *p = (void *)csm_compat_table.E820Pointer; -- 2.17.1
By default SeaBIOS can map up to 2 hard disks, and more hard disks beyond 2 will be rejected. This restriction is caused by limited BDA slots. This patch added support for mapping more than 2 hard disks by dynamically mapping the hard disk right before we're about to boot the hard disk. Signed-off-by: Bin Gao <gaobin@amazon.com> --- src/Kconfig | 8 ++++++++ src/block.c | 17 ++++++++++++++++- src/boot.c | 14 +++++++++++--- 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/Kconfig b/src/Kconfig index 3a8ffa1..d6031e2 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -380,6 +380,14 @@ menu "BIOS interfaces" help Support int13 disk/floppy drive functions. + config DYNAMIC_MAP_HD + depends on DRIVES + bool "Dynamically map hard drives" + default n + help + Support dynamically map hard drives so that the boot logic can + support more than 2 hard drives. + config CDROM_BOOT depends on DRIVES bool "DVD/CDROM booting" diff --git a/src/block.c b/src/block.c index 1f600b8..904e5f4 100644 --- a/src/block.c +++ b/src/block.c @@ -253,6 +253,10 @@ add_drive(struct drive_s **idmap, u8 *count, struct drive_s *drive) } // Map a hard drive +// +// By default SeaBIOS can map up to 2 hard disks, and more hard disks beyond +// 2 will be rejected. This restriction is caused by limited BDA slots. +// To support more than 2 hard drives, we need to enable CONFIG_DYNAMIC_MAP_HD. void map_hd_drive(struct drive_s *drive) { @@ -260,7 +264,18 @@ map_hd_drive(struct drive_s *drive) struct bios_data_area_s *bda = MAKE_FLATPTR(SEG_BDA, 0); int hdid = bda->hdcount; dprintf(3, "Mapping hd drive %p to %d\n", drive, hdid); - add_drive(IDMap[EXTTYPE_HD], &bda->hdcount, drive); + // To implement dynamic hard drive maping, we constantly keep + // bda->hdcount = 1, and use hdid = 0 all the times. + // map_hd_drive() must be called each time before boot to a + // specific hard drive. + if (CONFIG_DYNAMIC_MAP_HD) { + hdid = 0; // 0 - always the first hard drive + u8 tmp = 0; + add_drive(IDMap[EXTTYPE_HD], &tmp, drive); + bda->hdcount = 1; + } else { + add_drive(IDMap[EXTTYPE_HD], &bda->hdcount, drive); + } // Setup disk geometry translation. setup_translation(drive); diff --git a/src/boot.c b/src/boot.c index 1effd80..2e6e356 100644 --- a/src/boot.c +++ b/src/boot.c @@ -802,7 +802,7 @@ static int HaveHDBoot, HaveFDBoot; static void add_bev(int type, u32 vector) { - if (type == IPL_TYPE_HARDDISK && HaveHDBoot++) + if (type == IPL_TYPE_HARDDISK && HaveHDBoot++ && !CONFIG_DYNAMIC_MAP_HD) return; if (type == IPL_TYPE_FLOPPY && HaveFDBoot++) return; @@ -837,8 +837,14 @@ bcv_prepboot(void) add_bev(IPL_TYPE_FLOPPY, 0); break; case IPL_TYPE_HARDDISK: - map_hd_drive(pos->drive); - add_bev(IPL_TYPE_HARDDISK, 0); + if (CONFIG_DYNAMIC_MAP_HD) { + // Pass the drive_s pointer to bev_s struct so that + // we can do dynamic hard disk map in do_boot(). + add_bev(IPL_TYPE_HARDDISK, (u32)pos->drive); + } else { + map_hd_drive(pos->drive); + add_bev(IPL_TYPE_HARDDISK, 0); + } break; case IPL_TYPE_CDROM: map_cd_drive(pos->drive); @@ -998,6 +1004,8 @@ do_boot(int seq_nr) break; case IPL_TYPE_HARDDISK: printf("Booting from Hard Disk...\n"); + if (CONFIG_DYNAMIC_MAP_HD) + map_hd_drive((struct drive_s *)ie->vector); boot_disk(0x80, 1); break; case IPL_TYPE_CDROM: -- 2.17.1
Dear Bin, Thank you for your patches. Am 01.01.21 um 04:08 schrieb Bin Gao:
On physical hardware, some PCI devices are not detectable when UEFI invokes our InitializeYourself() function.
It would be great, if you gave a concrete example of hardware (board, firmware version and PCI devices).
But they are guaranteed to be available before UEFI invokes our PreparToBoot() function.
Nit: Small typo (also in summary and comment below): Prepar*e*ToBoot.
Signed-off-by: Bin Gao <gaobin@amazon.com> --- src/fw/csm.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/fw/csm.c b/src/fw/csm.c index f64bc73..1bd821e 100644 --- a/src/fw/csm.c +++ b/src/fw/csm.c @@ -64,7 +64,9 @@ static void csm_maininit(struct bregs *regs) { interface_init(); - pci_probe_devices(); + + if (CONFIG_CSM && CONFIG_QEMU_HARDWARE) + pci_probe_devices();
csm_compat_table.PnPInstallationCheckSegment = SEG_BIOS; csm_compat_table.PnPInstallationCheckOffset = get_pnp_offset(); @@ -144,6 +146,13 @@ handle_csm_0002(struct bregs *regs) return; }
+ // On physical hardware, some PCI devices are not detectable when + // UEFI invokes our InitializeYourself() function. But they + // are guaranteed to be available before UEFI invokes our + // PreparToBoot() function. + if (CONFIG_CSM && !CONFIG_QEMU_HARDWARE) + pci_probe_devices(); + dprintf(3, "PrepareToBoot table %04x:%04x\n", regs->es, regs->bx);
struct e820entry *p = (void *)csm_compat_table.E820Pointer;
Why don’t we want to do the same for QEMU? Kind regards, Paul
Dear Bin, Thank you very much for your patches. Am 01.01.21 um 04:08 schrieb Bin Gao:
On real hardware especially server platforms, there might be multiple
Please give a concrete example of the hardware.
root buses, thus pci bus number could run up to 255. This patch fixed
Nit: I’d use present tense in commit messages: This patch fixes ….
pci_probe_devices() by allowing to scan all 256 pci bus numbers(0-255).
Nit: Space before (.
Signed-off-by: Bin Gao <gaobin@amazon.com> --- src/hw/pcidevice.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/src/hw/pcidevice.c b/src/hw/pcidevice.c index 8853cf7..8d9c401 100644 --- a/src/hw/pcidevice.c +++ b/src/hw/pcidevice.c @@ -26,6 +26,14 @@ pci_probe_devices(void) struct hlist_node **pprev = &PCIDevices.first; int extraroots = romfile_loadint("etc/extra-pci-roots", 0); int bus = -1, lastbus = 0, rootbuses = 0, count=0; + + // There might be multiple PCI root buses on physical hardware especially + // server platforms, thus bus number could run up to the top value, + // i.e. 0xff. Setting extraroots to 0xff to ensure we can enumerate all + // PCI bus numbers. + if (CONFIG_CSM) + extraroots = 0xff; +
Can there be a romfile with CSM? If yes, the configured value would be overwritten. Maybe add a log message in that case?
while (bus < 0xff && (bus < MaxPCIBus || rootbuses < extraroots)) { bus++; int bdf;
Kind regards, Paul
Hi,
+ // On physical hardware, some PCI devices are not detectable when + // UEFI invokes our InitializeYourself() function. But they + // are guaranteed to be available before UEFI invokes our + // PreparToBoot() function. + if (CONFIG_CSM && !CONFIG_QEMU_HARDWARE) + pci_probe_devices(); + dprintf(3, "PrepareToBoot table %04x:%04x\n", regs->es, regs->bx); struct e820entry *p = (void *)csm_compat_table.E820Pointer;
Why don’t we want to do the same for QEMU?
I think so, there is no reason why qemu should be handled in a different way. IIRC I've even mentioned that before. Do you guys read the review comments before submitting a new version of the patch set? take care, Gerd
Dear Bin,
Thank you for your patches.
Am 01.01.21 um 04:08 schrieb Bin Gao:
On physical hardware, some PCI devices are not detectable when UEFI invokes our InitializeYourself() function.
It would be great, if you gave a concrete example of hardware (board, firmware version and PCI devices).
Will add this formation in my next revision.
But they are guaranteed to be available before UEFI invokes our PreparToBoot() function.
Nit: Small typo (also in summary and comment below): Prepar*e*ToBoot.
Will fix in next revision.
Signed-off-by: Bin Gao <gaobin@amazon.com> --- src/fw/csm.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/fw/csm.c b/src/fw/csm.c index f64bc73..1bd821e 100644 --- a/src/fw/csm.c +++ b/src/fw/csm.c @@ -64,7 +64,9 @@ static void csm_maininit(struct bregs *regs) { interface_init(); - pci_probe_devices(); + + if (CONFIG_CSM && CONFIG_QEMU_HARDWARE) + pci_probe_devices();
csm_compat_table.PnPInstallationCheckSegment = SEG_BIOS; csm_compat_table.PnPInstallationCheckOffset = get_pnp_offset(); @@ -144,6 +146,13 @@ handle_csm_0002(struct bregs *regs) return; }
+ // On physical hardware, some PCI devices are not detectable when + // UEFI invokes our InitializeYourself() function. But they + // are guaranteed to be available before UEFI invokes our + // PreparToBoot() function. + if (CONFIG_CSM && !CONFIG_QEMU_HARDWARE) + pci_probe_devices(); + dprintf(3, "PrepareToBoot table %04x:%04x\n", regs->es, regs->bx);
struct e820entry *p = (void *)csm_compat_table.E820Pointer;
Why don’t we want to do the same for QEMU?
This problem was only seen on physical hardware, so I wanted this fix to be applied only to physical hardware.
Kind regards,
Paul _______________________________________________ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-leave@seabios.org
Dear Bin,
Thank you very much for your patches.
Am 01.01.21 um 04:08 schrieb Bin Gao:
On real hardware especially server platforms, there might be multiple
Please give a concrete example of the hardware.
All Intel server processors have multiple PCI roots, called IIO.
root buses, thus pci bus number could run up to 255. This patch fixed
Nit: I’d use present tense in commit messages: This patch fixes ….
pci_probe_devices() by allowing to scan all 256 pci bus numbers(0-255).
Nit: Space before (.
Will fix this (and tense) in next revision.
Signed-off-by: Bin Gao <gaobin@amazon.com> --- src/hw/pcidevice.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/src/hw/pcidevice.c b/src/hw/pcidevice.c index 8853cf7..8d9c401 100644 --- a/src/hw/pcidevice.c +++ b/src/hw/pcidevice.c @@ -26,6 +26,14 @@ pci_probe_devices(void) struct hlist_node **pprev = &PCIDevices.first; int extraroots = romfile_loadint("etc/extra-pci-roots", 0); int bus = -1, lastbus = 0, rootbuses = 0, count=0; + + // There might be multiple PCI root buses on physical hardware especially + // server platforms, thus bus number could run up to the top value, + // i.e. 0xff. Setting extraroots to 0xff to ensure we can enumerate all + // PCI bus numbers. + if (CONFIG_CSM) + extraroots = 0xff; +
Can there be a romfile with CSM? If yes, the configured value would be overwritten. Maybe add a log message in that case?
There is no romfile with CSM. I do think the correct fix is to scan all the PCI buses, instead of to get a default max bus number. The fact is, on physical hardware, we can't assume the max bus number. On typical server design, it easily consumes up all the bus numbers, e.g. some PCIe bridges could pad up with many buses. On Intel server processors, for instance, we may want to split the 256 bus number to all IIOs so it's almost sure that we can reach up the maximal possible bus number (in a single PC segment). Yes, I can add a log message in that case.
while (bus < 0xff && (bus < MaxPCIBus || rootbuses < extraroots)) { bus++; int bdf;
Kind regards,
Paul
Dear Bin, Am 13.01.21 um 04:43 schrieb Gao, Bin:
Am 01.01.21 um 04:08 schrieb Bin Gao:
[…]
+ // On physical hardware, some PCI devices are not detectable when + // UEFI invokes our InitializeYourself() function. But they + // are guaranteed to be available before UEFI invokes our + // PreparToBoot() function. + if (CONFIG_CSM && !CONFIG_QEMU_HARDWARE) + pci_probe_devices(); + dprintf(3, "PrepareToBoot table %04x:%04x\n", regs->es, regs->bx);
struct e820entry *p = (void *)csm_compat_table.E820Pointer;
Why don’t we want to do the same for QEMU?
This problem was only seen on physical hardware, so I wanted this fix to be applied only to physical hardware.
Yes, I understand, but less code “complexity” and having the same behavior on QEMU and real hardware is preferred. The maintainer Gerd replied yesterday. Did you get his reply (Message-ID: <20210111143917.i6tsrnaymcly5plq@sirius.home.kraxel.org>)? Kind regards, Paul
On Thu, Dec 31, 2020 at 07:08:23PM -0800, Bin Gao wrote:
On real hardware especially server platforms, there might be multiple root buses, thus pci bus number could run up to 255. This patch fixed pci_probe_devices() by allowing to scan all 256 pci bus numbers(0-255).
Signed-off-by: Bin Gao <gaobin@amazon.com> --- src/hw/pcidevice.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/src/hw/pcidevice.c b/src/hw/pcidevice.c index 8853cf7..8d9c401 100644 --- a/src/hw/pcidevice.c +++ b/src/hw/pcidevice.c @@ -26,6 +26,14 @@ pci_probe_devices(void) struct hlist_node **pprev = &PCIDevices.first; int extraroots = romfile_loadint("etc/extra-pci-roots", 0); int bus = -1, lastbus = 0, rootbuses = 0, count=0; + + // There might be multiple PCI root buses on physical hardware especially + // server platforms, thus bus number could run up to the top value, + // i.e. 0xff. Setting extraroots to 0xff to ensure we can enumerate all + // PCI bus numbers. + if (CONFIG_CSM) + extraroots = 0xff;
This doesn't look correct to me. It would take a noticeable amount of time to scan every possible PCI root bus. It would introduce a regression for any current users of CONFIG_CSM. The "etc/extra-pci-roots" config is intended to allow users to make these choices at runtime. If more configurability is desired on CONFIG_CSM, then it will be needed to figure out a way to pass options to SeaBIOS when in that mode. -Kevin
On Thu, Dec 31, 2020 at 07:08:24PM -0800, Bin Gao wrote:
In csm mode, the bev pointer of the pci option rom was not added to the bootentry list, resulting in failure to boot from pci option rom. This patch fixed it. Also we enabled hw timer interrupt and clock update before we call the option rom's init() funtion, so the clock tick update at BDA address 40:6C will work during the rom's init() execution. Withou this change, iPXE would hang on reading BDA 40:6C due to the value at this address is not changed.
Signed-off-by: Bin Gao <gaobin@amazon.com> --- src/fw/csm.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
diff --git a/src/fw/csm.c b/src/fw/csm.c index 8359bcb..f64bc73 100644 --- a/src/fw/csm.c +++ b/src/fw/csm.c @@ -19,6 +19,7 @@ #include "std/acpi.h" // RSDP_SIGNATURE #include "std/bda.h" // struct bios_data_area_s #include "std/optionrom.h" // struct rom_header +#include "std/pnpbios.h" // PNP_SIGNATURE #include "util.h" // copy_smbios
#define UINT8 u8 @@ -219,6 +220,7 @@ handle_csm_0005(struct bregs *regs) { EFI_DISPATCH_OPROM_TABLE *table = MAKE_FLATPTR(regs->es, regs->bx); struct rom_header *rom; + struct pnp_data *pnp; u16 bdf;
if (!CONFIG_OPTIONROMS) { @@ -240,11 +242,36 @@ handle_csm_0005(struct bregs *regs)
rom_reserve(rom->size * 512);
+ // callrom() typically calls the rom's init() functions. + // If the rom's init() function does a delay by detecting the value + // at address 40:6C, it will fail because the value at 40:6C is + // updated only in clock_update() which is called in hw timer ISR. + // Thus we have to enable hw timer interrupt and set up clock before + // calling callrom(). + pic_setup(); + timer_setup(); + clock_setup();
This doesn't look correct to me. The CSM code was originally designed not to directly setup the PIC hardware - IIRC we don't know if the OS is a "legacy OS" or not at this point. Maybe David has a more clear recollection. In any case, it would not make sense to call clock_setup() and timer_setup() on every option rom and then again in PrepareToBoot(). -Kevin
On Thu, Dec 31, 2020 at 07:08:25PM -0800, Bin Gao wrote:
On physical hardware, some PCI devices are not detectable when UEFI invokes our InitializeYourself() function. But they are guaranteed to be available before UEFI invokes our PreparToBoot() function.
Signed-off-by: Bin Gao <gaobin@amazon.com> --- src/fw/csm.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/fw/csm.c b/src/fw/csm.c index f64bc73..1bd821e 100644 --- a/src/fw/csm.c +++ b/src/fw/csm.c @@ -64,7 +64,9 @@ static void csm_maininit(struct bregs *regs) { interface_init(); - pci_probe_devices(); + + if (CONFIG_CSM && CONFIG_QEMU_HARDWARE) + pci_probe_devices();
csm_compat_table.PnPInstallationCheckSegment = SEG_BIOS; csm_compat_table.PnPInstallationCheckOffset = get_pnp_offset(); @@ -144,6 +146,13 @@ handle_csm_0002(struct bregs *regs) return; }
+ // On physical hardware, some PCI devices are not detectable when + // UEFI invokes our InitializeYourself() function. But they + // are guaranteed to be available before UEFI invokes our + // PreparToBoot() function. + if (CONFIG_CSM && !CONFIG_QEMU_HARDWARE) + pci_probe_devices();
This looks like a hack. I'm not sure what's being attempted, but a more scalable solution will be needed. -Kevin
On Thu, Dec 31, 2020 at 07:08:26PM -0800, Bin Gao wrote:
By default SeaBIOS can map up to 2 hard disks, and more hard disks beyond 2 will be rejected. This restriction is caused by limited BDA slots. This patch added support for mapping more than 2 hard disks by dynamically mapping the hard disk right before we're about to boot the hard disk.
Signed-off-by: Bin Gao <gaobin@amazon.com> --- src/Kconfig | 8 ++++++++ src/block.c | 17 ++++++++++++++++- src/boot.c | 14 +++++++++++--- 3 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/src/Kconfig b/src/Kconfig index 3a8ffa1..d6031e2 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -380,6 +380,14 @@ menu "BIOS interfaces" help Support int13 disk/floppy drive functions.
+ config DYNAMIC_MAP_HD + depends on DRIVES + bool "Dynamically map hard drives" + default n + help + Support dynamically map hard drives so that the boot logic can + support more than 2 hard drives. + config CDROM_BOOT depends on DRIVES bool "DVD/CDROM booting" diff --git a/src/block.c b/src/block.c index 1f600b8..904e5f4 100644 --- a/src/block.c +++ b/src/block.c @@ -253,6 +253,10 @@ add_drive(struct drive_s **idmap, u8 *count, struct drive_s *drive) }
// Map a hard drive +// +// By default SeaBIOS can map up to 2 hard disks, and more hard disks beyond +// 2 will be rejected. This restriction is caused by limited BDA slots. +// To support more than 2 hard drives, we need to enable CONFIG_DYNAMIC_MAP_HD. void map_hd_drive(struct drive_s *drive) { @@ -260,7 +264,18 @@ map_hd_drive(struct drive_s *drive) struct bios_data_area_s *bda = MAKE_FLATPTR(SEG_BDA, 0); int hdid = bda->hdcount; dprintf(3, "Mapping hd drive %p to %d\n", drive, hdid); - add_drive(IDMap[EXTTYPE_HD], &bda->hdcount, drive); + // To implement dynamic hard drive maping, we constantly keep + // bda->hdcount = 1, and use hdid = 0 all the times. + // map_hd_drive() must be called each time before boot to a + // specific hard drive. + if (CONFIG_DYNAMIC_MAP_HD) { + hdid = 0; // 0 - always the first hard drive + u8 tmp = 0; + add_drive(IDMap[EXTTYPE_HD], &tmp, drive); + bda->hdcount = 1; + } else { + add_drive(IDMap[EXTTYPE_HD], &bda->hdcount, drive); + }
// Setup disk geometry translation. setup_translation(drive); diff --git a/src/boot.c b/src/boot.c index 1effd80..2e6e356 100644 --- a/src/boot.c +++ b/src/boot.c @@ -802,7 +802,7 @@ static int HaveHDBoot, HaveFDBoot; static void add_bev(int type, u32 vector) { - if (type == IPL_TYPE_HARDDISK && HaveHDBoot++) + if (type == IPL_TYPE_HARDDISK && HaveHDBoot++ && !CONFIG_DYNAMIC_MAP_HD) return; if (type == IPL_TYPE_FLOPPY && HaveFDBoot++) return; @@ -837,8 +837,14 @@ bcv_prepboot(void) add_bev(IPL_TYPE_FLOPPY, 0); break; case IPL_TYPE_HARDDISK: - map_hd_drive(pos->drive); - add_bev(IPL_TYPE_HARDDISK, 0); + if (CONFIG_DYNAMIC_MAP_HD) { + // Pass the drive_s pointer to bev_s struct so that + // we can do dynamic hard disk map in do_boot(). + add_bev(IPL_TYPE_HARDDISK, (u32)pos->drive); + } else { + map_hd_drive(pos->drive); + add_bev(IPL_TYPE_HARDDISK, 0); + } break; case IPL_TYPE_CDROM: map_cd_drive(pos->drive); @@ -998,6 +1004,8 @@ do_boot(int seq_nr) break; case IPL_TYPE_HARDDISK: printf("Booting from Hard Disk...\n"); + if (CONFIG_DYNAMIC_MAP_HD) + map_hd_drive((struct drive_s *)ie->vector); boot_disk(0x80, 1);
This is not valid. Once we start the boot phase (via INT19), it's not valid to change the hard drive mappings. Once we invoke INT19, it's possible for external code to alter memory - including any temporary storage that SeaBIOS allocated during the post phase. Indeed, on normal QEMU the memory holding the active drives is marked as read-only after the POST phase. Also, external code can invoke the SeaBIOS int19 handler multiple times. In accordance with the "BIOS Boot Specification", SeaBIOS does all its setup in the POST phase, and then just starts the boot process in the "BOOT phase". -Kevin
On Thu, Dec 31, 2020 at 07:08:23PM -0800, Bin Gao wrote:
On real hardware especially server platforms, there might be multiple root buses, thus pci bus number could run up to 255. This patch fixed pci_probe_devices() by allowing to scan all 256 pci bus numbers(0-255).
Signed-off-by: Bin Gao <gaobin@amazon.com> --- src/hw/pcidevice.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/src/hw/pcidevice.c b/src/hw/pcidevice.c index 8853cf7..8d9c401 100644 --- a/src/hw/pcidevice.c +++ b/src/hw/pcidevice.c @@ -26,6 +26,14 @@ pci_probe_devices(void) struct hlist_node **pprev = &PCIDevices.first; int extraroots = romfile_loadint("etc/extra-pci-roots", 0); int bus = -1, lastbus = 0, rootbuses = 0, count=0; + + // There might be multiple PCI root buses on physical hardware especially + // server platforms, thus bus number could run up to the top value, + // i.e. 0xff. Setting extraroots to 0xff to ensure we can enumerate all + // PCI bus numbers. + if (CONFIG_CSM) + extraroots = 0xff;
This doesn't look correct to me. It would take a noticeable amount of time to scan every possible PCI root bus. It would introduce a regression for any current users of CONFIG_CSM.
On real hardware scanning every possible PCI root bus wouldn't be slow so it's not a problem. Would it be reasonable to scan all pci buses only on physical hardware? if (CONFIG_CSM && ! CONFIG_QEMU_HARDWARE) extraroots = 0xff;
The "etc/extra-pci-roots" config is intended to allow users to make these choices at runtime. If more configurability is desired on CONFIG_CSM, then it will be needed to figure out a way to pass options to SeaBIOS when in that mode.
So far I only need this extraroots config on CONFIG_CSM. I didn't see an easy way to pass options from UEFI BIOS to SeaBIOS, unless we extend the CSM protocol defined by Intel.
-Kevin
In csm mode, the bev pointer of the pci option rom was not added to the bootentry list, resulting in failure to boot from pci option rom. This patch fixed it. Also we enabled hw timer interrupt and clock update before we call the option rom's init() funtion, so the clock tick update at BDA address 40:6C will work during the rom's init() execution. Withou this change, iPXE would hang on reading BDA 40:6C due to the value at this address is not changed.
Signed-off-by: Bin Gao <gaobin@amazon.com> --- src/fw/csm.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
diff --git a/src/fw/csm.c b/src/fw/csm.c index 8359bcb..f64bc73 100644 --- a/src/fw/csm.c +++ b/src/fw/csm.c @@ -19,6 +19,7 @@ #include "std/acpi.h" // RSDP_SIGNATURE #include "std/bda.h" // struct bios_data_area_s #include "std/optionrom.h" // struct rom_header +#include "std/pnpbios.h" // PNP_SIGNATURE #include "util.h" // copy_smbios
#define UINT8 u8 @@ -219,6 +220,7 @@ handle_csm_0005(struct bregs *regs) { EFI_DISPATCH_OPROM_TABLE *table = MAKE_FLATPTR(regs->es, regs->bx); struct rom_header *rom; + struct pnp_data *pnp; u16 bdf;
if (!CONFIG_OPTIONROMS) { @@ -240,11 +242,36 @@ handle_csm_0005(struct bregs *regs)
rom_reserve(rom->size * 512);
+ // callrom() typically calls the rom's init() functions. + // If the rom's init() function does a delay by detecting the value + // at address 40:6C, it will fail because the value at 40:6C is + // updated only in clock_update() which is called in hw timer ISR. + // Thus we have to enable hw timer interrupt and set up clock before + // calling callrom(). + pic_setup(); + timer_setup(); + clock_setup();
This doesn't look correct to me. The CSM code was originally designed not to directly setup the PIC hardware - IIRC we don't know if the OS is a "legacy OS" or not at this point. Maybe David has a more clear recollection.
In any case, it would not make sense to call clock_setup() and timer_setup() on every option rom and then again in PrepareToBoot().
After each rom dispatch, the execution returns back to BIOS, so we lose all the hardware context from SeaBIOS. That's why I did a per-dispatch time init. The ROM init() depends on the ticks updating if the ROM init() uses standard BDA 40:6C based time polling to implement a delay. Probably no one really tried PCI oprom on physical hardware so this problem was not disclosed...
-Kevin
On Thu, Dec 31, 2020 at 07:08:25PM -0800, Bin Gao wrote:
On physical hardware, some PCI devices are not detectable when UEFI invokes our InitializeYourself() function. But they are guaranteed to be available before UEFI invokes our PreparToBoot() function.
Signed-off-by: Bin Gao <gaobin@amazon.com> --- src/fw/csm.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/fw/csm.c b/src/fw/csm.c index f64bc73..1bd821e 100644 --- a/src/fw/csm.c +++ b/src/fw/csm.c @@ -64,7 +64,9 @@ static void csm_maininit(struct bregs *regs) { interface_init(); - pci_probe_devices(); + + if (CONFIG_CSM && CONFIG_QEMU_HARDWARE) + pci_probe_devices();
csm_compat_table.PnPInstallationCheckSegment = SEG_BIOS; csm_compat_table.PnPInstallationCheckOffset = get_pnp_offset(); @@ -144,6 +146,13 @@ handle_csm_0002(struct bregs *regs) return; }
+ // On physical hardware, some PCI devices are not detectable when + // UEFI invokes our InitializeYourself() function. But they + // are guaranteed to be available before UEFI invokes our + // PreparToBoot() function. + if (CONFIG_CSM && !CONFIG_QEMU_HARDWARE) + pci_probe_devices();
This looks like a hack. I'm not sure what's being attempted, but a more scalable solution will be needed.
As the commit message described, the goal is to make sure all the pci devices can be found in pci_probe_devices(). Due to the BIOS internal logic, some PCI devices may not be visible yet when InitializeYourself() is called. Thus we wanted to run pci_probe_devices() later, i.e. in PrepareToBoot() instead of in InitializeYourself(). As Paul and Gerd suggested, I will remove CONFIG_QEMU_HARDWARE and let the code be common for both qemu and bare metal.
-Kevin
diff --git a/src/boot.c b/src/boot.c index 1effd80..2e6e356 100644 --- a/src/boot.c +++ b/src/boot.c @@ -802,7 +802,7 @@ static int HaveHDBoot, HaveFDBoot; static void add_bev(int type, u32 vector) { - if (type == IPL_TYPE_HARDDISK && HaveHDBoot++) + if (type == IPL_TYPE_HARDDISK && HaveHDBoot++ && !CONFIG_DYNAMIC_MAP_HD) return; if (type == IPL_TYPE_FLOPPY && HaveFDBoot++) return; @@ -837,8 +837,14 @@ bcv_prepboot(void) add_bev(IPL_TYPE_FLOPPY, 0); break; case IPL_TYPE_HARDDISK: - map_hd_drive(pos->drive); - add_bev(IPL_TYPE_HARDDISK, 0); + if (CONFIG_DYNAMIC_MAP_HD) { + // Pass the drive_s pointer to bev_s struct so that + // we can do dynamic hard disk map in do_boot(). + add_bev(IPL_TYPE_HARDDISK, (u32)pos->drive); + } else { + map_hd_drive(pos->drive); + add_bev(IPL_TYPE_HARDDISK, 0); + } break; case IPL_TYPE_CDROM: map_cd_drive(pos->drive); @@ -998,6 +1004,8 @@ do_boot(int seq_nr) break; case IPL_TYPE_HARDDISK: printf("Booting from Hard Disk...\n"); + if (CONFIG_DYNAMIC_MAP_HD) + map_hd_drive((struct drive_s *)ie->vector); boot_disk(0x80, 1);
This is not valid. Once we start the boot phase (via INT19), it's not valid to change the hard drive mappings. Once we invoke INT19, it's possible for external code to alter memory - including any temporary storage that SeaBIOS allocated during the post phase. Indeed, on normal QEMU the memory holding the active drives is marked as read-only after the POST phase. Also, external code can invoke the SeaBIOS int19 handler multiple times.
In accordance with the "BIOS Boot Specification", SeaBIOS does all its setup in the POST phase, and then just starts the boot process in the "BOOT phase".
Essentially the map_hd_drive() function only touches struct drive_s{} and its parent container, IDMap[], BEV[] and BDA. The first one is typically malloc_high()ed, the last three are static/BSS. So if external code alters memory it will not affect map_hd_drive(). With that said, the dynamic hd map and the original static hd map don't have any difference, except that dynamic map can support more hard drives. Also the new code is gated by a configuration option CONFIG_DYNAMIC_MAP_HD and it's disabled by default, so it won't break the BBS spec. Would you consider this as an experiment feature?
-Kevin
+ // On physical hardware, some PCI devices are not detectable when + // UEFI invokes our InitializeYourself() function. But they + // are guaranteed to be available before UEFI invokes our + // PreparToBoot() function. + if (CONFIG_CSM && !CONFIG_QEMU_HARDWARE) + pci_probe_devices(); + dprintf(3, "PrepareToBoot table %04x:%04x\n", regs->es, regs->bx);
struct e820entry *p = (void *)csm_compat_table.E820Pointer;
Why don’t we want to do the same for QEMU?
This problem was only seen on physical hardware, so I wanted this fix to be applied only to physical hardware.
Yes, I understand, but less code “complexity” and having the same behavior on QEMU and real hardware is preferred. The maintainer Gerd replied yesterday. Did you get his reply (Message-ID: <20210111143917.i6tsrnaymcly5plq@sirius.home.kraxel.org>)?
Thanks Paul and sorry for late catch-up. Yes I saw Gerd's reply and we agreed to keep this change common to both QEMU and bare metal.
Kind regards,
Paul _______________________________________________ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-leave@seabios.org
participants (5)
-
Bin Gao -
Gao, Bin
-
Gerd Hoffmann -
Kevin O'Connor -
Paul Menzel