[Qemu-devel] [PATCH 5/5] ahci: factor out FIS decomposition from handle_cmd

[John Snow <jsnow@redhat.com>]

In order to make handle_cmd more readable at the macro level,
the details of how to decompose particular types of FIS packets
are left to helper functions.

In our case, the only type of FIS packet we currently expect to
see is a Register H2D FIS packet, but the gory details of its
decomposition are of no particular interest in handle_cmd.

This patch keeps the receipt of FIS packets and the decomposition
thereof separated to two different functions.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/ide/ahci.c | 169 ++++++++++++++++++++++++++++++----------------------------
 1 file changed, 86 insertions(+), 83 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 536b455..4f4bda0 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -946,10 +946,94 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis,
     }
 }
 
+static void handle_reg_h2d_fis(AHCIState *s, int port,
+                               int slot, uint8_t *cmd_fis)
+{
+    IDEState *ide_state = &s->dev[port].port.ifs[0];
+    AHCICmdHdr *cmd = s->dev[port].cur_cmd;
+    uint32_t opts = le32_to_cpu(cmd->opts);
+
+    if (cmd_fis[1] & 0x0F) {
+        DPRINTF(port, "Port Multiplier not supported."
+                " cmd_fis[0]=%02x cmd_fis[1]=%02x cmd_fis[2]=%02x\n",
+                cmd_fis[0], cmd_fis[1], cmd_fis[2]);
+        return;
+    }
+
+    if (cmd_fis[1] & 0x70) {
+        DPRINTF(port, "Reserved flags set in H2D Register FIS."
+                " cmd_fis[0]=%02x cmd_fis[1]=%02x cmd_fis[2]=%02x\n",
+                cmd_fis[0], cmd_fis[1], cmd_fis[2]);
+        return;
+    }
+
+    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;
+            }
+            break;
+        case STATE_RESET:
+            if (!(cmd_fis[15] & ATA_SRST)) {
+                ahci_reset_port(s, port);
+            }
+            break;
+        }
+        return;
+    }
+
+    /* Check for NCQ command */
+    if (is_ncq(cmd_fis[2])) {
+        process_ncq_command(s, port, cmd_fis, slot);
+        return;
+    }
+
+    /* 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];
+    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);
+        debug_print_fis(ide_state->io_buffer, 0x10);
+        s->dev[port].done_atapi_packet = false;
+        /* XXX send PIO setup FIS */
+    }
+
+    ide_state->error = 0;
+
+    /* Reset transferred byte counter */
+    cmd->status = 0;
+
+    /* We're ready to process the command in FIS byte 2. */
+    ide_exec_cmd(&s->dev[port].port, cmd_fis[2]);
+}
+
 static int handle_cmd(AHCIState *s, int port, int slot)
 {
     IDEState *ide_state;
-    uint32_t opts;
     uint64_t tbl_addr;
     AHCICmdHdr *cmd;
     uint8_t *cmd_fis;
@@ -976,7 +1060,6 @@ static int handle_cmd(AHCIState *s, int port, int slot)
         return -1;
     }
 
-    opts = le32_to_cpu(cmd->opts);
     tbl_addr = le64_to_cpu(cmd->tbl_addr);
     cmd_len = 0x80;
     cmd_fis = dma_memory_map(s->as, tbl_addr, &cmd_len,
@@ -995,93 +1078,13 @@ static int handle_cmd(AHCIState *s, int port, int slot)
 
     switch (cmd_fis[0]) {
         case SATA_FIS_TYPE_REGISTER_H2D:
+            handle_reg_h2d_fis(s, port, slot, cmd_fis);
             break;
         default:
             DPRINTF(port, "unknown command cmd_fis[0]=%02x cmd_fis[1]=%02x "
                           "cmd_fis[2]=%02x\n", cmd_fis[0], cmd_fis[1],
                           cmd_fis[2]);
-            goto out;
-            break;
-    }
-
-    if (cmd_fis[1] & 0x0F) {
-        DPRINTF(port, "Port Multiplier not supported."
-                " cmd_fis[0]=%02x cmd_fis[1]=%02x cmd_fis[2]=%02x\n",
-                cmd_fis[0], cmd_fis[1], cmd_fis[2]);
-        goto out;
-    }
-
-    if (cmd_fis[1] & 0x70) {
-        DPRINTF(port, "Reserved flags set in H2D Register FIS."
-                " cmd_fis[0]=%02x cmd_fis[1]=%02x cmd_fis[2]=%02x\n",
-                cmd_fis[0], cmd_fis[1], cmd_fis[2]);
-        goto out;
-    }
-
-    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;
-            }
             break;
-        case STATE_RESET:
-            if (!(cmd_fis[15] & ATA_SRST)) {
-                ahci_reset_port(s, port);
-            }
-            break;
-        }
-    }
-
-    else if (cmd_fis[1] & SATA_FIS_REG_H2D_UPDATE_COMMAND_REGISTER) {
-
-        /* Check for NCQ command */
-        if (is_ncq(cmd_fis[2])) {
-            process_ncq_command(s, port, cmd_fis, slot);
-            goto out;
-        }
-
-        /* 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];
-        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);
-            debug_print_fis(ide_state->io_buffer, 0x10);
-            s->dev[port].done_atapi_packet = false;
-            /* XXX send PIO setup FIS */
-        }
-
-        ide_state->error = 0;
-
-        /* Reset transferred byte counter */
-        cmd->status = 0;
-
-        /* We're ready to process the command in FIS byte 2. */
-        ide_exec_cmd(&s->dev[port].port, cmd_fis[2]);
     }
 
 out:
-- 
1.9.3