[SeaBIOS] bootorder, ATA-SFF, AHCI and CSM; oh my!

Jonathan A. Kollasch jakllsch at kollasch.net
Tue Jul 7 03:59:58 CEST 2015


Hi,

I've discovered a mess:

 - bootorder paths for ATA-SFF devices do not use the
   channel/port-on-controller number in the path, instead they use a
   volatile and unique-to-each-channel number that has no relation
   to the actual arrangement of the hardware.

   For instance, on a MCP55-based machine (no AHCI, but plenty of
   ATA-SFF channels/ports:
   
   The master drive on the PATA channel is (and should be, but is only
   by coincidence) /pci at i0cf8/*@4/drive at 0/disk at 0 but the first SATA port
   on /pci at i0cf8/*@5 is currently detected by SeaBIOS as something other
   than /pci at i0cf8/*@5/drive at 0/disk at 0 (the drive@ number is something
   like 2) and the second SATA port is something other than
   /pci at i0cf8/*@5/drive at 1/disk at 0 (drive@ number is usually more like 3)
   The /pci at i0cf8/*@5 controller only has two ports on it.

 - bootprio_find_ata_device() is shared between the ahci.c and ata.c
   drivers.  This becomes interesting in a system using SeaBIOS as CSM
   and/or having certain combinations of ATA-SFF and/or AHCI controllers.

   (And why does the SeaBIOS CSM even have drivers in it?  I would have
   thought it would be using the UEFI drivers...but maybe we had to
   ExitBootServices() to work properly...)

 - Thus the chanid argument to csm_bootprio_ata() could be any of two
   unrelated things:

   - the global-based index of a ata.c channel
   - the controller-based port index of any of the ahci.c controllers in
     the system

I've attached a patch that attempts to address the issue with the ata.c
bootorder paths, but it doesn't really fix the actual problem.

	Jonathan Kollasch
-------------- next part --------------
diff --git a/src/boot.c b/src/boot.c
index e0f73a3..b8659a5 100644
--- a/src/boot.c
+++ b/src/boot.c
@@ -150,7 +150,7 @@ int bootprio_find_scsi_device(struct pci_device *pci, int target, int lun)
     return find_prio(desc);
 }
 
-int bootprio_find_ata_device(struct pci_device *pci, int chanid, int slave)
+int bootprio_find_ata_device(struct pci_device *pci, int chanid, int pnr, int slave)
 {
     if (CONFIG_CSM)
         return csm_bootprio_ata(pci, chanid, slave);
@@ -162,7 +162,7 @@ int bootprio_find_ata_device(struct pci_device *pci, int chanid, int slave)
     // Find ata drive - for example: /pci at i0cf8/ide at 1,1/drive at 1/disk at 0
     char desc[256], *p;
     p = build_pci_path(desc, sizeof(desc), "*", pci);
-    snprintf(p, desc+sizeof(desc)-p, "/drive@%x/disk@%x", chanid, slave);
+    snprintf(p, desc+sizeof(desc)-p, "/drive@%x/disk@%x", pnr, slave);
     return find_prio(desc);
 }
 
diff --git a/src/hw/ahci.c b/src/hw/ahci.c
index 0d71cc4..ca9851a 100644
--- a/src/hw/ahci.c
+++ b/src/hw/ahci.c
@@ -349,6 +349,7 @@ ahci_port_reset(struct ahci_ctrl_s *ctrl, u32 pnr)
 static struct ahci_port_s*
 ahci_port_alloc(struct ahci_ctrl_s *ctrl, u32 pnr)
 {
+    static int chanid = 0;
     struct ahci_port_s *port = malloc_tmp(sizeof(*port));
 
     if (!port) {
@@ -356,6 +357,7 @@ ahci_port_alloc(struct ahci_ctrl_s *ctrl, u32 pnr)
         return NULL;
     }
     port->pnr = pnr;
+    port->chanid = chanid++;
     port->ctrl = ctrl;
     port->list = memalign_tmp(1024, 1024);
     port->fis = memalign_tmp(256, 256);
@@ -431,6 +433,7 @@ static int ahci_port_setup(struct ahci_port_s *port)
 {
     struct ahci_ctrl_s *ctrl = port->ctrl;
     u32 pnr = port->pnr;
+    u32 chanid = port->chanid;
     char model[MAXMODEL+1];
     u16 buffer[256];
     u32 cmd, stat, err, tf;
@@ -523,7 +526,7 @@ static int ahci_port_setup(struct ahci_port_s *port)
                               , ata_extract_model(model, MAXMODEL, buffer)
                               , ata_extract_version(buffer)
                               , (u32)adjsize, adjprefix);
-        port->prio = bootprio_find_ata_device(ctrl->pci_tmp, pnr, 0);
+        port->prio = bootprio_find_ata_device(ctrl->pci_tmp, chanid, pnr, 0);
     } else {
         // found cdrom (atapi)
         port->drive.type = DTYPE_AHCI_ATAPI;
@@ -539,7 +542,7 @@ static int ahci_port_setup(struct ahci_port_s *port)
                               , port->pnr
                               , ata_extract_model(model, MAXMODEL, buffer)
                               , ata_extract_version(buffer));
-        port->prio = bootprio_find_ata_device(ctrl->pci_tmp, pnr, 0);
+        port->prio = bootprio_find_ata_device(ctrl->pci_tmp, chanid, pnr, 0);
     }
     return 0;
 }
diff --git a/src/hw/ahci.h b/src/hw/ahci.h
index c8c755a..270ae58 100644
--- a/src/hw/ahci.h
+++ b/src/hw/ahci.h
@@ -76,6 +76,7 @@ struct ahci_port_s {
     struct ahci_list_s *list;
     struct ahci_fis_s  *fis;
     struct ahci_cmd_s  *cmd;
+    u32                chanid;
     u32                pnr;
     u32                atapi;
     char               *desc;
diff --git a/src/hw/ata.c b/src/hw/ata.c
index d805706..f43280f 100644
--- a/src/hw/ata.c
+++ b/src/hw/ata.c
@@ -750,6 +750,7 @@ init_drive_atapi(struct atadrive_s *dummy, u16 *buffer)
     if (iscd) {
         int prio = bootprio_find_ata_device(adrive->chan_gf->pci_tmp,
                                             adrive->chan_gf->chanid,
+                                            adrive->chan_gf->pnr,
                                             adrive->slave);
         boot_add_cd(&adrive->drive, desc, prio);
     }
@@ -800,6 +801,7 @@ init_drive_ata(struct atadrive_s *dummy, u16 *buffer)
 
     int prio = bootprio_find_ata_device(adrive->chan_gf->pci_tmp,
                                         adrive->chan_gf->chanid,
+                                        adrive->chan_gf->pnr,
                                         adrive->slave);
     // Register with bcv system.
     boot_add_hd(&adrive->drive, desc, prio);
@@ -913,7 +915,7 @@ ata_detect(void *data)
 
 // Initialize an ata controller and detect its drives.
 static void
-init_controller(struct pci_device *pci, int irq
+init_controller(struct pci_device *pci, int irq, u32 pnr
                 , u32 port1, u32 port2, u32 master)
 {
     static int chanid = 0;
@@ -929,8 +931,9 @@ init_controller(struct pci_device *pci, int irq
     chan_gf->iobase1 = port1;
     chan_gf->iobase2 = port2;
     chan_gf->iomaster = master;
-    dprintf(1, "ATA controller %d at %x/%x/%x (irq %d dev %x)\n"
-            , chanid, port1, port2, master, irq, chan_gf->pci_bdf);
+    chan_gf->pnr = pnr;
+    dprintf(1, "ATA controller %d at %x/%x/%x (irq %d dev %x chan %u)\n"
+            , chanid, port1, port2, master, irq, chan_gf->pci_bdf, pnr);
     run_thread(ata_detect, chan_gf);
 }
 
@@ -966,7 +969,7 @@ init_pciata(struct pci_device *pci, u8 prog_if)
         port2 = PORT_ATA1_CTRL_BASE;
         irq = IRQ_ATA1;
     }
-    init_controller(pci, irq, port1, port2, master);
+    init_controller(pci, irq, 0, port1, port2, master);
 
     if (prog_if & 4) {
         port1 = (pci_config_readl(bdf, PCI_BASE_ADDRESS_2)
@@ -979,7 +982,7 @@ init_pciata(struct pci_device *pci, u8 prog_if)
         port2 = PORT_ATA2_CTRL_BASE;
         irq = IRQ_ATA2;
     }
-    init_controller(pci, irq, port1, port2, master ? master + 8 : 0);
+    init_controller(pci, irq, 1, port1, port2, master ? master + 8 : 0);
 }
 
 static void
@@ -1011,9 +1014,9 @@ ata_scan(void)
     if (CONFIG_QEMU && hlist_empty(&PCIDevices)) {
         // No PCI devices found - probably a QEMU "-M isapc" machine.
         // Try using ISA ports for ATA controllers.
-        init_controller(NULL, IRQ_ATA1
+        init_controller(NULL, IRQ_ATA1, 0
                         , PORT_ATA1_CMD_BASE, PORT_ATA1_CTRL_BASE, 0);
-        init_controller(NULL, IRQ_ATA2
+        init_controller(NULL, IRQ_ATA2, 1
                         , PORT_ATA2_CMD_BASE, PORT_ATA2_CTRL_BASE, 0);
         return;
     }
diff --git a/src/hw/ata.h b/src/hw/ata.h
index c73892b..04164fa 100644
--- a/src/hw/ata.h
+++ b/src/hw/ata.h
@@ -13,6 +13,7 @@ struct ata_channel_s {
     u8  chanid;
     int pci_bdf;
     struct pci_device *pci_tmp;
+    u8  pnr; // channel on controller
 };
 
 struct atadrive_s {
diff --git a/src/util.h b/src/util.h
index 6250c12..8061986 100644
--- a/src/util.h
+++ b/src/util.h
@@ -30,7 +30,7 @@ void bcv_prepboot(void);
 struct pci_device;
 int bootprio_find_pci_device(struct pci_device *pci);
 int bootprio_find_scsi_device(struct pci_device *pci, int target, int lun);
-int bootprio_find_ata_device(struct pci_device *pci, int chanid, int slave);
+int bootprio_find_ata_device(struct pci_device *pci, int chanid, int channel, int slave);
 int bootprio_find_fdc_device(struct pci_device *pci, int port, int fdid);
 int bootprio_find_pci_rom(struct pci_device *pci, int instance);
 int bootprio_find_named_rom(const char *name, int instance);


More information about the SeaBIOS mailing list