PR feedback

oxidecomputer · Dec 20, 2023 · 122e564 · 122e564
1 parent 7981ac1
commit 122e564
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 18 deletions.
diff --git a/lib/propolis/src/hw/nvme/admin.rs b/lib/propolis/src/hw/nvme/admin.rs
@@ -372,8 +372,11 @@ impl NvmeCtrl {
             // Optional features
             cmds::FeatureIdent::VolatileWriteCache => {
                 // TODO: wire into actual write cache state
+                //
+                // Until that is done, indicate an enabled write cache to ensure
+                // IO flushes for the backends which require it for consistency.
                 cmds::Completion::success_val(
-                    cmds::FeatVolatileWriteCache { wce: false }.into(),
+                    cmds::FeatVolatileWriteCache { wce: true }.into(),
                 )
             }
 

diff --git a/lib/propolis/src/hw/nvme/bits.rs b/lib/propolis/src/hw/nvme/bits.rs
@@ -645,6 +645,11 @@ pub const FEAT_ID_ARBITRATION: u8 = 0x01;
 /// See NVMe 1.0e Section 5.12.1.2 Power Management (Feature Identifier 02h)
 pub const FEAT_ID_POWER_MGMT: u8 = 0x02;
 
+/// LBA Range Type
+///
+/// See NVMe 1.0e Section 5.12.1.3 LBA Range Type (Feature Identifier 03h)
+pub const FEAT_ID_LBA_RANGE_TYPE: u8 = 0x03;
+
 /// Temperature Threshold
 ///
 /// See NVMe 1.0e Section 5.12.1.4 Arbitration (Feature Identifier 04h)
@@ -655,6 +660,11 @@ pub const FEAT_ID_TEMP_THRESH: u8 = 0x04;
 /// See NVMe 1.0e Section 5.12.1.5 Error Recovery (Feature Identifier 05h)
 pub const FEAT_ID_ERROR_RECOVERY: u8 = 0x05;
 
+/// Volatile Write Cache
+///
+/// See NVMe 1.0e Section 5.12.1.6 Volatile Write Cache (Feature Identifier 06h)
+pub const FEAT_ID_VOLATILE_WRITE_CACHE: u8 = 0x06;
+
 /// Number of Queues
 ///
 /// See NVMe 1.0e Section 5.12.1.7 Number of Queues (Feature Identifier 07h)

diff --git a/lib/propolis/src/hw/nvme/cmds.rs b/lib/propolis/src/hw/nvme/cmds.rs
@@ -350,7 +350,7 @@ pub struct AbortCmd {
     pub sqid: u16,
 }
 
-/// Set Features Command Parameters
+/// Get Features Command Parameters
 #[derive(Debug)]
 pub struct GetFeaturesCmd {
     /// Feature Identifier (FID)
@@ -437,20 +437,21 @@ pub enum FeatureIdent {
 
 impl From<u8> for FeatureIdent {
     fn from(fid: u8) -> Self {
+        use super::bits::*;
         use FeatureIdent::*;
         match fid {
             0 => Reserved,
-            1 => Arbitration,
-            2 => PowerManagement,
-            3 => LbaRangeType,
-            4 => TemperatureThreshold,
-            5 => ErrorRecovery,
-            6 => VolatileWriteCache,
-            7 => NumberOfQueues,
-            8 => InterruptCoalescing,
-            9 => InterruptVectorConfiguration,
-            0xA => WriteAtomicity,
-            0xB => AsynchronousEventConfiguration,
+            FEAT_ID_ARBITRATION => Arbitration,
+            FEAT_ID_POWER_MGMT => PowerManagement,
+            FEAT_ID_LBA_RANGE_TYPE => LbaRangeType,
+            FEAT_ID_TEMP_THRESH => TemperatureThreshold,
+            FEAT_ID_ERROR_RECOVERY => ErrorRecovery,
+            FEAT_ID_VOLATILE_WRITE_CACHE => VolatileWriteCache,
+            FEAT_ID_NUM_QUEUES => NumberOfQueues,
+            FEAT_ID_INTR_COALESCE => InterruptCoalescing,
+            FEAT_ID_INTR_VEC_CFG => InterruptVectorConfiguration,
+            FEAT_ID_WRITE_ATOMIC => WriteAtomicity,
+            FEAT_ID_ASYNC_EVENT_CFG => AsynchronousEventConfiguration,
             0xC..=0x7F => Reserved,
             0x80 => SoftwareProgressMarker,
             0x81..=0xBF => Reserved,
@@ -568,11 +569,7 @@ impl From<u32> for FeatVolatileWriteCache {
 }
 impl From<FeatVolatileWriteCache> for u32 {
     fn from(value: FeatVolatileWriteCache) -> Self {
-        if value.wce {
-            0b1
-        } else {
-            0
-        }
+        value.wce as u32
     }
 }