[PATCH 5/5] esp.c: only allow ESP commands permitted in the current asc_mode

Mark Cave-Ayland posted 5 patches 3 weeks ago
[PATCH 5/5] esp.c: only allow ESP commands permitted in the current asc_mode
Posted by Mark Cave-Ayland 3 weeks ago
If an ESP command is issued in an incorrect mode then an illegal command
interrupt should be generated. Add a new esp_cmd_is_valid() function to
indicate whether the ESP command is valid for the current mode, and if not
then raise the illegal command interrupt.

This fixes WinNT MIPS which issues ICCS after a Chip Reset which is not
permitted, but will fail with an INACCESSIBLE_BOOT_DEVICE error unless an
interrupt is generated.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Fixes: 83428f7a97 ("esp.c: move write_response() non-DMA logic to esp_do_nodma()")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2464
---
 hw/scsi/esp.c         | 37 +++++++++++++++++++++++++++++++++++++
 hw/scsi/trace-events  |  1 +
 include/hw/scsi/esp.h |  7 +++++++
 3 files changed, 45 insertions(+)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 3370546287..7b6b8a4a57 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -1122,6 +1122,38 @@ static void parent_esp_reset(ESPState *s, int irq, int level)
     }
 }
 
+static bool esp_cmd_is_valid(ESPState *s, uint8_t cmd)
+{
+    uint8_t cmd_group = (cmd & CMD_GRP_MASK) >> 4;
+
+    /* Always allow misc commands */
+    if (cmd_group == CMD_GRP_MISC) {
+        return true;
+    }
+
+    switch (s->asc_mode) {
+    case ESP_ASC_MODE_DIS:
+        /* Disconnected mode: only allow disconnected commands */
+        if (cmd_group == CMD_GRP_DISC) {
+            return true;
+        }
+        break;
+
+    case ESP_ASC_MODE_INI:
+        /* Initiator mode: allow initiator commands */
+        if (cmd_group == CMD_GRP_INIT) {
+            return true;
+        }
+        break;
+
+    default:
+        g_assert_not_reached();
+    }
+
+    trace_esp_invalid_cmd(cmd, s->asc_mode);
+    return false;
+}
+
 static void esp_run_cmd(ESPState *s)
 {
     uint8_t cmd = s->rregs[ESP_CMD];
@@ -1278,6 +1310,11 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t val)
         break;
     case ESP_CMD:
         s->rregs[saddr] = val;
+        if (!esp_cmd_is_valid(s, s->rregs[saddr])) {
+            s->rregs[ESP_RSTAT] |= INTR_IL;
+            esp_raise_irq(s);
+            break;
+        }
         esp_run_cmd(s);
         break;
     case ESP_WBUSID ... ESP_WSYNO:
diff --git a/hw/scsi/trace-events b/hw/scsi/trace-events
index f0f2a98c2e..6c2788e202 100644
--- a/hw/scsi/trace-events
+++ b/hw/scsi/trace-events
@@ -198,6 +198,7 @@ esp_mem_writeb_cmd_ensel(uint32_t val) "Enable selection (0x%2.2x)"
 esp_mem_writeb_cmd_dissel(uint32_t val) "Disable selection (0x%2.2x)"
 esp_mem_writeb_cmd_ti(uint32_t val) "Transfer Information (0x%2.2x)"
 esp_set_phase(const char *phase) "setting bus phase to %s"
+esp_invalid_cmd(uint8_t cmd, uint8_t asc_mode) "command 0x%x asc_mode 0x%x"
 
 # esp-pci.c
 esp_pci_error_invalid_dma_direction(void) "invalid DMA transfer direction"
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index 64cbd11765..3526bad746 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -111,6 +111,13 @@ struct SysBusESPState {
 #define CMD_DMA 0x80
 #define CMD_CMD 0x7f
 
+#define CMD_GRP_MASK 0x70
+
+#define CMD_GRP_MISC 0x00
+#define CMD_GRP_INIT 0x01
+#define CMD_GRP_TRGT 0x02
+#define CMD_GRP_DISC 0x04
+
 #define CMD_NOP      0x00
 #define CMD_FLUSH    0x01
 #define CMD_RESET    0x02
-- 
2.39.5