[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [RFC 06/10] AHCI: Fix FIS decomposition
From: |
John Snow |
Subject: |
[Qemu-devel] [RFC 06/10] AHCI: Fix FIS decomposition |
Date: |
Sat, 13 Sep 2014 00:34:11 -0400 |
This patch introduces a few changes to how FIS packets are
deciphered in the AHCI virtual device.
(1) Packets may now either update the Control register or
the Command register, but not both. This is according
to the SATA 3.2 specification which states:
"...the device either initiates processing of the command
indicated in the Command register or initiates processing
of the control request indicated [...] depending on the
state of the C bit in the FIS."
(2) Instead of trying to extract the sector number out of the
FIS from bytes 4-10 and setting it with ide_set_sector,
we set the appropriate IDEState registers and trust that
ide_get_sector can retrieve the correct sector later.
(3) We save cmd_fis[7] as ide_state->select, which informs
decisions about if we are using LBA or CHS.
This corrects a bug in AHCI wherein we attempt to set and/or
retrieve the sector number by using ide_set_sector and
ide_get_sector, which depend on the select register to
determine if we are using LBA or CHS.
(4) Save cmd_fis[11] as ide_state->hob_feature, as defined in AHCI.
(5) For several ATA commands, the sector count register set to 0
is a magic number that means 256 sectors. For LBA48 commands,
this means 65,536 sectors. We drop the magic sector correction
here, and trust the ide core layer to handle the conversion
appropriately, in ide_cmd_lba48_transform
(6) We expand FIS decomposition to include both ATAPI and IDE devices.
We leave the logic of determining if the fields are valid or not
to the respective layers.
(7) Forcefully setting the feature, hcyl and lcyl registers for ATAPI
commands is removed.
- The hcyl and lcyl magic present here is valid at boot only,
and should not be overridden for every PACKET command.
- The feature register is defined as valid for the PACKET command,
so we should not suppress it. The ATAPI layer does not even
currently depend on or require 0x01 as mandatory.
Signed-off-by: John Snow <address@hidden>
---
hw/ide/ahci.c | 64 ++++++++++++++++++++++++-----------------------------------
1 file changed, 26 insertions(+), 38 deletions(-)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 4a1af3b..c2fa733 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1008,7 +1008,8 @@ static int handle_cmd(AHCIState *s, int port, int slot)
break;
}
- switch (s->dev[port].port_state) {
+ if (!(cmd_fis[1] & SATA_FIS_REG_H2D_UPDATE_COMMAND_REGISTER)) {
+ switch (s->dev[port].port_state) {
case STATE_RUN:
if (cmd_fis[15] & ATA_SRST) {
s->dev[port].port_state = STATE_RESET;
@@ -1019,9 +1020,10 @@ static int handle_cmd(AHCIState *s, int port, int slot)
ahci_reset_port(s, port);
}
break;
+ }
}
- if (cmd_fis[1] == SATA_FIS_REG_H2D_UPDATE_COMMAND_REGISTER) {
+ else if (cmd_fis[1] & SATA_FIS_REG_H2D_UPDATE_COMMAND_REGISTER) {
/* Check for NCQ command */
if (is_ncq(cmd_fis[2])) {
@@ -1029,50 +1031,36 @@ static int handle_cmd(AHCIState *s, int port, int slot)
goto out;
}
- /* Decompose the FIS */
- ide_state->nsector = (int64_t)((cmd_fis[13] << 8) | cmd_fis[12]);
+ /* Decompose the FIS:
+ * AHCI does not interpret FIS packets, it only forwards them.
+ * SATA 1.0 describes how to decode LBA28 and CHS FIS packets.
+ * Later specifications, e.g, SATA 3.2, describe LBA48 FIS packets.
+ *
+ * ATA4 describes sector number for LBA28/CHS commands.
+ * ATA6 describes sector number for LBA48 commands.
+ * ATA8 deprecates CHS fully, describing only LBA28/48.
+ *
+ * We dutifully convert the FIS into IDE registers, and allow the
+ * core layer to interpret them as needed. */
ide_state->feature = cmd_fis[3];
- if (!ide_state->nsector) {
- ide_state->nsector = 256;
- }
-
- if (ide_state->drive_kind != IDE_CD) {
- /*
- * We set the sector depending on the sector defined in the FIS.
- * Unfortunately, the spec isn't exactly obvious on this one.
- *
- * Apparently LBA48 commands set fis bytes 10,9,8,6,5,4 to the
- * 48 bit sector number. ATA_CMD_READ_DMA_EXT is an example for
- * such a command.
- *
- * Non-LBA48 commands however use 7[lower 4 bits],6,5,4 to define a
- * 28-bit sector number. ATA_CMD_READ_DMA is an example for such
- * a command.
- *
- * Since the spec doesn't explicitly state what each field should
- * do, I simply assume non-used fields as reserved and OR
everything
- * together, independent of the command.
- */
- ide_set_sector(ide_state, ((uint64_t)cmd_fis[10] << 40)
- | ((uint64_t)cmd_fis[9] << 32)
- /* This is used for LBA48 commands */
- | ((uint64_t)cmd_fis[8] << 24)
- /* This is used for non-LBA48 commands */
- | ((uint64_t)(cmd_fis[7] & 0xf) << 24)
- | ((uint64_t)cmd_fis[6] << 16)
- | ((uint64_t)cmd_fis[5] << 8)
- | cmd_fis[4]);
- }
+ ide_state->sector = cmd_fis[4]; /* LBA 7:0 */
+ ide_state->lcyl = cmd_fis[5]; /* LBA 15:8 */
+ ide_state->hcyl = cmd_fis[6]; /* LBA 23:16 */
+ ide_state->select = cmd_fis[7]; /* LBA 27:24 (LBA28) */
+ ide_state->hob_sector = cmd_fis[8]; /* LBA 31:24 */
+ ide_state->hob_lcyl = cmd_fis[9]; /* LBA 39:32 */
+ ide_state->hob_hcyl = cmd_fis[10]; /* LBA 47:40 */
+ ide_state->hob_feature = cmd_fis[11];
+ ide_state->nsector = (int64_t)((cmd_fis[13] << 8) | cmd_fis[12]);
+ /* 14, 16, 17, 18, 19: Reserved (SATA 1.0) */
+ /* 15: Only valid when UPDATE_COMMAND not set. */
/* Copy the ACMD field (ATAPI packet, if any) from the AHCI command
* table to ide_state->io_buffer
*/
if (opts & AHCI_CMD_ATAPI) {
memcpy(ide_state->io_buffer, &cmd_fis[AHCI_COMMAND_TABLE_ACMD],
0x10);
- ide_state->lcyl = 0x14;
- ide_state->hcyl = 0xeb;
debug_print_fis(ide_state->io_buffer, 0x10);
- ide_state->feature = IDE_FEATURE_DMA;
s->dev[port].done_atapi_packet = false;
/* XXX send PIO setup FIS */
}
--
1.9.3
- [Qemu-devel] [RFC 02/10] AHCI: Update byte count after DMA completion, (continued)
- [Qemu-devel] [RFC 02/10] AHCI: Update byte count after DMA completion, John Snow, 2014/09/13
- [Qemu-devel] [RFC 03/10] AHCI: Add PRD interrupt, John Snow, 2014/09/13
- [Qemu-devel] [RFC 04/10] ide: Correct handling of malformed/short PRDTs, John Snow, 2014/09/13
- [Qemu-devel] [RFC 05/10] AHCI: Rename NCQFIS structure fields, John Snow, 2014/09/13
- [Qemu-devel] [RFC 01/10] ide: add is_write() macro for semantic consistency, John Snow, 2014/09/13
- [Qemu-devel] [RFC 06/10] AHCI: Fix FIS decomposition,
John Snow <=
- [Qemu-devel] [RFC 07/10] ide/ahci: Reorder error cases in handle_cmd, John Snow, 2014/09/13
- [Qemu-devel] [RFC 10/10] AHCI: Fix SDB FIS Construction, John Snow, 2014/09/13
- [Qemu-devel] [RFC 09/10] ahci: factor out FIS decomposition, John Snow, 2014/09/13
- [Qemu-devel] [RFC 08/10] ahci: Check cmd_fis[1] more explicitly, John Snow, 2014/09/13