On Mon, Jul 06, 2015 at 08:59:58PM -0500, Jonathan A. Kollasch wrote:
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@i0cf8/*@4/drive@0/disk@0 but the first SATA port on /pci@i0cf8/*@5 is currently detected by SeaBIOS as something other than /pci@i0cf8/*@5/drive@0/disk@0 (the drive@ number is something like 2) and the second SATA port is something other than /pci@i0cf8/*@5/drive@1/disk@0 (drive@ number is usually more like 3) The /pci@i0cf8/*@5 controller only has two ports on it.
I agree the current code is incorrect. However, I don't think it requires changing the CSM or AHCI code. I think the ATA code should just track which controller number it is relative to the given PCI id and pass that to bootprio_find_ata_device() instead of chanid.
- 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.
I think the CSM code is just not capable of handling a system with more than one ATA controller or with drives of a non-ATA type. I vaguely recall David saying something like that when he committed the code. (I think he was going to try and chase it down with the CSM spec owners.) So, I would just pass in the updated "chanid" to CSM without worrying about the different behavior on secondary ATA 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...)
I'm not sure, but I don't think it's valid to call back into UEFI during the system runtime - only during the init phase. (And, SeaBIOS needs drivers through the lifetime of the system for old DOS apps.)
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.
Thanks. Are you willing to respin a simpler version of the patch?
-Kevin