nvme: don't fail on abort cmd (#581)

`Abort` is a mandatory NVMe command and we were previously just returning `STS_INTERNAL_ERR` (`06h - Internal Device Error` ). Even without fully supporting aborting commands we can do better than that since it's a best effort command.

This change just adds some validation and immediately completes an abort request indicating the command in question _wasn't_ aborted.

lib/propolis/src/hw/nvme/admin.rs

@@ -10,9 +10,35 @@ use crate::vmm::MemCtx;
 
 use super::bits::*;
 use super::queue::{QueueId, ADMIN_QUEUE_ID};
-use super::{cmds, NvmeCtrl, NvmeError, MAX_NUM_IO_QUEUES};
+use super::{cmds, NvmeCtrl, NvmeError, MAX_NUM_IO_QUEUES, MAX_NUM_QUEUES};
+
+#[usdt::provider(provider = "propolis")]
+mod probes {
+    fn nvme_abort(cid: u16, sqid: u16) {}
+}
 
 impl NvmeCtrl {
+    /// Abort command.
+    ///
+    /// See NVMe 1.0e Section 5.1 Abort command
+    pub(super) fn acmd_abort(&self, cmd: &cmds::AbortCmd) -> cmds::Completion {
+        probes::nvme_abort!(|| (cmd.cid, cmd.sqid));
+
+        // Verify the SQ in question currently exists
+        let sqid = cmd.sqid as usize;
+        if sqid >= MAX_NUM_QUEUES || self.sqs[sqid].is_none() {
+            return cmds::Completion::generic_err_dnr(STS_INVAL_FIELD);
+        }
+
+        // TODO: Support aborting in-flight commands.
+
+        // The NVMe spec does not make any guarantees about being able to
+        // successfully abort commands and allows indicating a failure to
+        // do so back to the host software. We do so here by returning a
+        // "success" value with bit 0 set to '1'.
+        cmds::Completion::success_val(1)
+    }
+
     /// Service Create I/O Completion Queue command.
     ///
     /// See NVMe 1.0e Section 5.3 Create I/O Completion Queue command

lib/propolis/src/hw/nvme/cmds.rs

@@ -45,7 +45,7 @@ pub enum AdminCmd {
     /// Identify Command
     Identify(IdentifyCmd),
     /// Abort Command
-    Abort,
+    Abort(AbortCmd),
     /// Set Features Command
     SetFeatures(SetFeaturesCmd),
     /// Get Features Command
@@ -110,7 +110,10 @@ impl AdminCmd {
                 prp1: raw.prp1,
                 prp2: raw.prp2,
             }),
-            bits::ADMIN_OPC_ABORT => AdminCmd::Abort,
+            bits::ADMIN_OPC_ABORT => AdminCmd::Abort(AbortCmd {
+                cid: (raw.cdw10 >> 16) as u16,
+                sqid: raw.cdw10 as u16,
+            }),
             bits::ADMIN_OPC_SET_FEATURES => {
                 AdminCmd::SetFeatures(SetFeaturesCmd {
                     fid: FeatureIdent::from((raw.cdw10 as u8, raw.cdw11)),
@@ -329,6 +332,17 @@ impl IdentifyCmd {
     }
 }
 
+/// Abort Command Parameters
+#[derive(Debug)]
+pub struct AbortCmd {
+    /// The command identifier of the command to be aborted.
+    pub cid: u16,
+
+    /// The ID of the Submission Queue asssociated with the command to be
+    /// aborted.
+    pub sqid: u16,
+}
+
 /// Set Features Command Parameters
 #[derive(Debug)]
 pub struct SetFeaturesCmd {

lib/propolis/src/hw/nvme/mod.rs

@@ -618,6 +618,8 @@ impl PciNvme {
 
         let cur = state.ctrl.cc;
         if new.enabled() && !cur.enabled() {
+            slog::info!(self.log, "Enabling controller");
+
             let mem = self.mem_access();
             let mem = mem.ok_or(NvmeError::MemoryInaccessible)?;
 
@@ -633,6 +635,8 @@ impl PciNvme {
                 state.ctrl.csts.set_ready(true);
             }
         } else if !new.enabled() && cur.enabled() {
+            slog::info!(self.log, "Disabling controller");
+
             // Reset controller state which will set CC.EN=0 and CSTS.RDY=0
             state.reset();
         }
@@ -885,6 +889,7 @@ impl PciNvme {
                 AdminCmd::Unknown(sub)
             });
             let comp = match cmd {
+                AdminCmd::Abort(cmd) => state.acmd_abort(&cmd),
                 AdminCmd::CreateIOCompQ(cmd) => {
                     state.acmd_create_io_cq(&cmd, &mem)
                 }
@@ -910,9 +915,7 @@ impl PciNvme {
                     // this can detect it and stop posting async events.
                     cmds::Completion::generic_err_dnr(bits::STS_INVAL_OPC)
                 }
-                AdminCmd::Abort
-                | AdminCmd::GetFeatures
-                | AdminCmd::Unknown(_) => {
+                AdminCmd::GetFeatures | AdminCmd::Unknown(_) => {
                     cmds::Completion::generic_err(bits::STS_INTERNAL_ERR)
                 }
             };