For whatever reason, when you compile SeaBIOS in Csm16 mode, and use it under EDK2's OvmfPkg, the ATA_CMD_IDENTIFY_PACKET_DEVICE command doesn't work properly, therefore, SeaBIOS detects the "SATA HARDDISK" as a "SATA CDROM" device, in QEMU. Despite the Tianocore developers seem to have removed support for Csm16 some time ago, if we decide to remove Csm16 mode in SeaBIOS in favor of that, at least we have the last commit of Csm16 working properly and not half-broken. In order to fix this bug, I decided to move the "detected as a disk" function to be a goto, so code isn't sloppy, by being copied and pasted 1:1 (though this can be changed back later if needed), and in the detected as CDROM function, when it fails at its point, assume its a harddisk, make sure it is with the ATA_CMD_IDENTIFY_DEVICE (which works fine in my testing) command, and continue from there.
Signed-off-by: Christopher Lentocha christopherericlentocha@gmail.com --- src/hw/ahci.c | 191 ++++++++++++++++++++++++++------------------------ 1 file changed, 101 insertions(+), 90 deletions(-)
diff --git a/src/hw/ahci.c b/src/hw/ahci.c index 4f0f640a..e53c26be 100644 --- a/src/hw/ahci.c +++ b/src/hw/ahci.c @@ -501,89 +501,7 @@ static int ahci_port_setup(struct ahci_port_s *port) port->drive.removable = (buffer[0] & 0x80) ? 1 : 0; if (!port->atapi) { - // found disk (ata) - port->drive.type = DTYPE_AHCI; - port->drive.blksize = DISK_SECTOR_SIZE; - port->drive.pchs.cylinder = buffer[1]; - port->drive.pchs.head = buffer[3]; - port->drive.pchs.sector = buffer[6]; - - u64 sectors; - if (buffer[83] & (1 << 10)) // word 83 - lba48 support - sectors = *(u64*)&buffer[100]; // word 100-103 - else - sectors = *(u32*)&buffer[60]; // word 60 and word 61 - port->drive.sectors = sectors; - u64 adjsize = sectors >> 11; - char adjprefix = 'M'; - if (adjsize >= (1 << 16)) { - adjsize >>= 10; - adjprefix = 'G'; - } - port->desc = znprintf(MAXDESCSIZE - , "AHCI/%d: %s ATA-%d Hard-Disk (%u %ciBytes)" - , port->pnr - , ata_extract_model(model, MAXMODEL, buffer) - , ata_extract_version(buffer) - , (u32)adjsize, adjprefix); - port->prio = bootprio_find_ata_device(ctrl->pci_tmp, pnr, 0); - - s8 multi_dma = -1; - s8 pio_mode = -1; - s8 udma_mode = -1; - // If bit 2 in word 53 is set, udma information is valid in word 88. - if (buffer[53] & 0x04) { - udma_mode = 6; - while ((udma_mode >= 0) && - !((buffer[88] & 0x7f) & ( 1 << udma_mode ))) { - udma_mode--; - } - } - // If bit 1 in word 53 is set, multiword-dma and advanced pio modes - // are available in words 63 and 64. - if (buffer[53] & 0x02) { - pio_mode = 4; - multi_dma = 3; - while ((multi_dma >= 0) && - !((buffer[63] & 0x7) & ( 1 << multi_dma ))) { - multi_dma--; - } - while ((pio_mode >= 3) && - !((buffer[64] & 0x3) & ( 1 << ( pio_mode - 3 ) ))) { - pio_mode--; - } - } - dprintf(2, "AHCI/%d: supported modes: udma %d, multi-dma %d, pio %d\n", - port->pnr, udma_mode, multi_dma, pio_mode); - - sata_prep_simple(&port->cmd->fis, ATA_CMD_SET_FEATURES); - port->cmd->fis.feature = ATA_SET_FEATRUE_TRANSFER_MODE; - // Select used mode. UDMA first, then Multi-DMA followed by - // advanced PIO modes 3 or 4. If non, set default PIO. - if (udma_mode >= 0) { - dprintf(1, "AHCI/%d: Set transfer mode to UDMA-%d\n", - port->pnr, udma_mode); - port->cmd->fis.sector_count = ATA_TRANSFER_MODE_ULTRA_DMA - | udma_mode; - } else if (multi_dma >= 0) { - dprintf(1, "AHCI/%d: Set transfer mode to Multi-DMA-%d\n", - port->pnr, multi_dma); - port->cmd->fis.sector_count = ATA_TRANSFER_MODE_MULTIWORD_DMA - | multi_dma; - } else if (pio_mode >= 3) { - dprintf(1, "AHCI/%d: Set transfer mode to PIO-%d\n", - port->pnr, pio_mode); - port->cmd->fis.sector_count = ATA_TRANSFER_MODE_PIO_FLOW_CTRL - | pio_mode; - } else { - dprintf(1, "AHCI/%d: Set transfer mode to default PIO\n", - port->pnr); - port->cmd->fis.sector_count = ATA_TRANSFER_MODE_DEFAULT_PIO; - } - rc = ahci_command(port, 1, 0, 0, 0); - if (rc < 0) { - dprintf(1, "AHCI/%d: Set transfer mode failed.\n", port->pnr); - } + goto is_ata_disk; } else { // found cdrom (atapi) port->drive.type = DTYPE_AHCI_ATAPI; @@ -592,14 +510,107 @@ static int ahci_port_setup(struct ahci_port_s *port) u8 iscd = ((buffer[0] >> 8) & 0x1f) == 0x05; if (!iscd) { dprintf(1, "AHCI/%d: atapi device isn't a cdrom\n", port->pnr); - return -1; + port->atapi = 0; + sata_prep_simple(&port->cmd->fis, ATA_CMD_IDENTIFY_DEVICE); + rc = ahci_command(port, 0, 0, buffer, sizeof(buffer)); + if (rc < 0) + return -1; + goto is_ata_disk; + } else { + port->desc = znprintf(MAXDESCSIZE + , "DVD/CD [AHCI/%d: %s ATAPI-%d DVD/CD]" + , port->pnr + , ata_extract_model(model, MAXMODEL, buffer) + , ata_extract_version(buffer)); + port->prio = bootprio_find_ata_device(ctrl->pci_tmp, pnr, 0); + boot_lchs_find_ata_device(ctrl->pci_tmp, pnr, 0, &(port->drive.lchs)); } - port->desc = znprintf(MAXDESCSIZE - , "DVD/CD [AHCI/%d: %s ATAPI-%d DVD/CD]" - , port->pnr - , ata_extract_model(model, MAXMODEL, buffer) - , ata_extract_version(buffer)); - port->prio = bootprio_find_ata_device(ctrl->pci_tmp, pnr, 0); + } + return 0; + +is_ata_disk: + // found disk (ata) + port->drive.type = DTYPE_AHCI; + port->drive.blksize = DISK_SECTOR_SIZE; + port->drive.pchs.cylinder = buffer[1]; + port->drive.pchs.head = buffer[3]; + port->drive.pchs.sector = buffer[6]; + + u64 sectors; + if (buffer[83] & (1 << 10)) // word 83 - lba48 support + sectors = *(u64*)&buffer[100]; // word 100-103 + else + sectors = *(u32*)&buffer[60]; // word 60 and word 61 + port->drive.sectors = sectors; + u64 adjsize = sectors >> 11; + char adjprefix = 'M'; + if (adjsize >= (1 << 16)) { + adjsize >>= 10; + adjprefix = 'G'; + } + port->desc = znprintf(MAXDESCSIZE + , "AHCI/%d: %s ATA-%d Hard-Disk (%u %ciBytes)" + , port->pnr + , ata_extract_model(model, MAXMODEL, buffer) + , ata_extract_version(buffer) + , (u32)adjsize, adjprefix); + port->prio = bootprio_find_ata_device(ctrl->pci_tmp, pnr, 0); + + s8 multi_dma = -1; + s8 pio_mode = -1; + s8 udma_mode = -1; + // If bit 2 in word 53 is set, udma information is valid in word 88. + if (buffer[53] & 0x04) { + udma_mode = 6; + while ((udma_mode >= 0) && + !((buffer[88] & 0x7f) & ( 1 << udma_mode ))) { + udma_mode--; + } + } + // If bit 1 in word 53 is set, multiword-dma and advanced pio modes + // are available in words 63 and 64. + if (buffer[53] & 0x02) { + pio_mode = 4; + multi_dma = 3; + while ((multi_dma >= 0) && + !((buffer[63] & 0x7) & ( 1 << multi_dma ))) { + multi_dma--; + } + while ((pio_mode >= 3) && + !((buffer[64] & 0x3) & ( 1 << ( pio_mode - 3 ) ))) { + pio_mode--; + } + } + dprintf(2, "AHCI/%d: supported modes: udma %d, multi-dma %d, pio %d\n", + port->pnr, udma_mode, multi_dma, pio_mode); + + sata_prep_simple(&port->cmd->fis, ATA_CMD_SET_FEATURES); + port->cmd->fis.feature = ATA_SET_FEATRUE_TRANSFER_MODE; + // Select used mode. UDMA first, then Multi-DMA followed by + // advanced PIO modes 3 or 4. If non, set default PIO. + if (udma_mode >= 0) { + dprintf(1, "AHCI/%d: Set transfer mode to UDMA-%d\n", + port->pnr, udma_mode); + port->cmd->fis.sector_count = ATA_TRANSFER_MODE_ULTRA_DMA + | udma_mode; + } else if (multi_dma >= 0) { + dprintf(1, "AHCI/%d: Set transfer mode to Multi-DMA-%d\n", + port->pnr, multi_dma); + port->cmd->fis.sector_count = ATA_TRANSFER_MODE_MULTIWORD_DMA + | multi_dma; + } else if (pio_mode >= 3) { + dprintf(1, "AHCI/%d: Set transfer mode to PIO-%d\n", + port->pnr, pio_mode); + port->cmd->fis.sector_count = ATA_TRANSFER_MODE_PIO_FLOW_CTRL + | pio_mode; + } else { + dprintf(1, "AHCI/%d: Set transfer mode to default PIO\n", + port->pnr); + port->cmd->fis.sector_count = ATA_TRANSFER_MODE_DEFAULT_PIO; + } + rc = ahci_command(port, 1, 0, 0, 0); + if (rc < 0) { + dprintf(1, "AHCI/%d: Set transfer mode failed.\n", port->pnr); } boot_lchs_find_ata_device(ctrl->pci_tmp, pnr, 0, &(port->drive.lchs)); return 0;
On Tue, Jan 14, 2025 at 11:10:26AM -0500, Christopher Lentocha wrote:
For whatever reason, when you compile SeaBIOS in Csm16 mode, and use it under EDK2's OvmfPkg, the ATA_CMD_IDENTIFY_PACKET_DEVICE command doesn't work properly,
Probably the device is in a different state due to the edk2 driver having talked to the device before.
Maybe add a controller reset to the initialization path? Currently the driver code only resets the ports.
In order to fix this bug, I decided to move the "detected as a disk" function to be a goto, so code isn't sloppy, by being copied and pasted 1:1 (though this can be changed back later if needed), and in the detected as CDROM function, when it fails at its point, assume its a harddisk, make sure it is with the ATA_CMD_IDENTIFY_DEVICE (which works fine in my testing) command, and continue from there.
That is more a workaround than a solution ...
take care, Gerd
-
Christopher Lentocha -
Gerd Hoffmann