Created a whole new struct for Pci related faults

gurasinghMS · gurasinghMS · commit e004a932a449 · 2025-08-27T17:18:52.000-07:00
diff --git a/vm/devices/storage/disk_nvme/nvme_driver/src/tests.rs b/vm/devices/storage/disk_nvme/nvme_driver/src/tests.rs
@@ -11,9 +11,9 @@ use inspect::InspectMut;
 use mesh::CellUpdater;
 use nvme::NvmeControllerCaps;
 use nvme_resources::fault::AdminQueueFaultConfig;
-use nvme_resources::fault::ControllerManagementFaultConfig;
-use nvme_resources::fault::FaultBehaviour;
 use nvme_resources::fault::FaultConfiguration;
+use nvme_resources::fault::PciFaultConfig;
+use nvme_resources::fault::QueueFaultBehavior;
 use nvme_spec::AdminOpcode;
 use nvme_spec::Cap;
 use nvme_spec::Command;
@@ -49,9 +49,9 @@ async fn test_nvme_command_fault(driver: DefaultDriver) {
             fault_active: CellUpdater::new(true).cell(),
             admin_fault: AdminQueueFaultConfig::new().with_submission_queue_fault(
                 AdminOpcode::CREATE_IO_COMPLETION_QUEUE.0,
-                FaultBehaviour::Update(output_cmd),
+                QueueFaultBehavior::Update(output_cmd),
             ),
-            controller_management_fault: ControllerManagementFaultConfig::new(),
+            pci_fault: PciFaultConfig::new(),
         },
     )
     .await;
diff --git a/vm/devices/storage/nvme_resources/src/fault.rs b/vm/devices/storage/nvme_resources/src/fault.rs
@@ -23,25 +23,27 @@ pub enum QueueFaultBehavior<T> {
     Panic(String),
 }
 
-#[derive(MeshPayload)]
-/// A buildable fault configuration for the controller management interface (cc.en(), csts.rdy(), ... )
-pub struct ControllerManagementFaultConfig {
-    /// Fault to apply to cc.en() bit during enablement
-    pub controller_management_fault_enable: FaultBehaviour<bool>,
+#[derive(Clone, MeshPayload)]
+/// Supported fault behaviour for PCI faults
+pub enum PciFaultBehavior {
+    /// Introduce a delay to the PCI operation
+    Delay(Duration),
+    /// Do nothing
+    Default,
 }
 
 #[derive(MeshPayload, Clone)]
 /// A buildable fault configuration for the controller management interface (cc.en(), csts.rdy(), ... )
-pub struct ControllerManagementFaultConfig {
+pub struct PciFaultConfig {
     /// Fault to apply to cc.en() bit during enablement
-    pub controller_management_fault_enable: FaultBehaviour<bool>,
+    pub controller_management_fault_enable: PciFaultBehavior,
 }
 
 #[derive(MeshPayload, Clone)]
 /// A buildable fault configuration
 pub struct AdminQueueFaultConfig {
     /// A map of NVME opcodes to the fault behavior for each. (This would ideally be a `HashMap`, but `mesh` doesn't support that type. Given that this is not performance sensitive, the lookup is okay)
-    pub admin_submission_queue_faults: Vec<(u8, FaultBehaviour<Command>)>,
+    pub admin_submission_queue_faults: Vec<(u8, QueueFaultBehavior<Command>)>,
 }
 
 #[derive(MeshPayload, Clone)]
@@ -52,22 +54,19 @@ pub struct FaultConfiguration {
     /// Fault to apply to the admin queues
     pub admin_fault: AdminQueueFaultConfig,
     /// Fault to apply to management layer of the controller
-    pub controller_management_fault: ControllerManagementFaultConfig,
+    pub pci_fault: PciFaultConfig,
 }
 
-impl ControllerManagementFaultConfig {
+impl PciFaultConfig {
     /// Create a new no-op fault configuration
     pub fn new() -> Self {
         Self {
-            controller_management_fault_enable: FaultBehaviour::Default,
+            controller_management_fault_enable: PciFaultBehavior::Default,
         }
     }
 
     /// Create a new fault configuration
-    pub fn with_controller_management_enable_fault(
-        mut self,
-        behaviour: FaultBehaviour<bool>,
-    ) -> Self {
+    pub fn with_cc_enable_fault(mut self, behaviour: PciFaultBehavior) -> Self {
         self.controller_management_fault_enable = behaviour;
         self
     }
@@ -85,7 +84,7 @@ impl AdminQueueFaultConfig {
     pub fn with_submission_queue_fault(
         mut self,
         opcode: u8,
-        behaviour: FaultBehaviour<Command>,
+        behaviour: QueueFaultBehavior<Command>,
     ) -> Self {
         if self
             .admin_submission_queue_faults
diff --git a/vm/devices/storage/nvme_test/src/pci.rs b/vm/devices/storage/nvme_test/src/pci.rs
@@ -30,8 +30,8 @@ use guestmem::GuestMemory;
 use guid::Guid;
 use inspect::Inspect;
 use inspect::InspectMut;
-use nvme_resources::fault::FaultBehaviour;
 use nvme_resources::fault::FaultConfiguration;
+use nvme_resources::fault::PciFaultBehavior;
 use parking_lot::Mutex;
 use pci_core::capabilities::msix::MsixEmulator;
 use pci_core::cfg_space_emu::BarMemoryKind;
@@ -348,19 +348,13 @@ impl NvmeFaultController {
                 // If any fault was configured for cc.en() process it here
                 match self
                     .fault_configuration
-                    .controller_management_fault
+                    .pci_fault
                     .controller_management_fault_enable
                 {
-                    FaultBehaviour::Delay(duration) => {
+                    PciFaultBehavior::Delay(duration) => {
                         std::thread::sleep(duration);
                     }
-                    FaultBehaviour::Drop => {
-                        tracelimit::warn_ratelimited!(
-                            "Dropping enable command due to fault injection"
-                        );
-                        return;
-                    }
-                    _ => {} // Update is not yet configured for this fault. Treat that as a default action for now
+                    PciFaultBehavior::Default => {}
                 }
 
                 // Some drivers will write zeros to IOSQES and IOCQES, assuming that the defaults will work.
diff --git a/vm/devices/storage/nvme_test/src/tests/controller_tests.rs b/vm/devices/storage/nvme_test/src/tests/controller_tests.rs
@@ -17,9 +17,9 @@ use guestmem::GuestMemory;
 use guid::Guid;
 use mesh::CellUpdater;
 use nvme_resources::fault::AdminQueueFaultConfig;
-use nvme_resources::fault::ControllerManagementFaultConfig;
-use nvme_resources::fault::FaultBehaviour;
 use nvme_resources::fault::FaultConfiguration;
+use nvme_resources::fault::PciFaultConfig;
+use nvme_resources::fault::QueueFaultBehavior;
 use nvme_spec::Command;
 use nvme_spec::Completion;
 use pal_async::DefaultDriver;
@@ -209,7 +209,7 @@ async fn test_basic_registers(driver: DefaultDriver) {
     let fault_configuration = FaultConfiguration {
         fault_active: CellUpdater::new(false).cell(),
         admin_fault: AdminQueueFaultConfig::new(),
-        controller_management_fault: ControllerManagementFaultConfig::new(),
+        pci_fault: PciFaultConfig::new(),
     };
     let mut nvmec = instantiate_controller(driver, &gm, None, fault_configuration);
     let mut dword = 0u32;
@@ -238,7 +238,7 @@ async fn test_invalid_configuration(driver: DefaultDriver) {
     let fault_configuration = FaultConfiguration {
         fault_active: CellUpdater::new(false).cell(),
         admin_fault: AdminQueueFaultConfig::new(),
-        controller_management_fault: ControllerManagementFaultConfig::new(),
+        pci_fault: PciFaultConfig::new(),
     };
     let mut nvmec = instantiate_controller(driver, &gm, None, fault_configuration);
     let mut dword = 0u32;
@@ -257,7 +257,7 @@ async fn test_enable_controller(driver: DefaultDriver) {
     let fault_configuration = FaultConfiguration {
         fault_active: CellUpdater::new(false).cell(),
         admin_fault: AdminQueueFaultConfig::new(),
-        controller_management_fault: ControllerManagementFaultConfig::new(),
+        pci_fault: PciFaultConfig::new(),
     };
     let mut nvmec = instantiate_controller(driver, &gm, None, fault_configuration);
 
@@ -289,7 +289,7 @@ async fn test_multi_page_admin_queues(driver: DefaultDriver) {
     let fault_configuration = FaultConfiguration {
         fault_active: CellUpdater::new(false).cell(),
         admin_fault: AdminQueueFaultConfig::new(),
-        controller_management_fault: ControllerManagementFaultConfig::new(),
+        pci_fault: PciFaultConfig::new(),
     };
     let mut nvmec = instantiate_controller(driver, &gm, None, fault_configuration);
 
@@ -364,7 +364,7 @@ async fn test_send_identify_no_fault(driver: DefaultDriver) {
     let fault_configuration = FaultConfiguration {
         fault_active: CellUpdater::new(false).cell(),
         admin_fault: AdminQueueFaultConfig::new(),
-        controller_management_fault: ControllerManagementFaultConfig::new(),
+        pci_fault: PciFaultConfig::new(),
     };
     let cqe = send_identify(driver, fault_configuration).await;
 
@@ -380,9 +380,9 @@ async fn test_send_identify_with_sq_fault(driver: DefaultDriver) {
         fault_active: CellUpdater::new(true).cell(),
         admin_fault: AdminQueueFaultConfig::new().with_submission_queue_fault(
             nvme_spec::AdminOpcode::IDENTIFY.0,
-            FaultBehaviour::Update(faulty_identify),
+            QueueFaultBehavior::Update(faulty_identify),
         ),
-        controller_management_fault: ControllerManagementFaultConfig::new(),
+        pci_fault: PciFaultConfig::new(),
     };
     let cqe = send_identify(driver, fault_configuration).await;
 
diff --git a/vm/devices/storage/nvme_test/src/tests/shadow_doorbell_tests.rs b/vm/devices/storage/nvme_test/src/tests/shadow_doorbell_tests.rs
@@ -14,8 +14,8 @@ use crate::tests::test_helpers::write_command_to_queue;
 use guestmem::GuestMemory;
 use mesh::CellUpdater;
 use nvme_resources::fault::AdminQueueFaultConfig;
-use nvme_resources::fault::ControllerManagementFaultConfig;
 use nvme_resources::fault::FaultConfiguration;
+use nvme_resources::fault::PciFaultConfig;
 use pal_async::DefaultDriver;
 use pal_async::async_test;
 use pci_core::test_helpers::TestPciInterruptController;
@@ -43,7 +43,7 @@ async fn setup_shadow_doorbells(
     let fault_configuration = FaultConfiguration {
         fault_active: CellUpdater::new(false).cell(),
         admin_fault: AdminQueueFaultConfig::new(),
-        controller_management_fault: ControllerManagementFaultConfig::new(),
+        pci_fault: PciFaultConfig::new(),
     }; // Build a controller with 64 entries in the admin queue (just so that the ASQ fits in one page).
     let mut nvmec = instantiate_and_build_admin_queue(
         cq_buf,
diff --git a/vm/devices/storage/nvme_test/src/workers/admin.rs b/vm/devices/storage/nvme_test/src/workers/admin.rs
@@ -31,8 +31,8 @@ use futures_concurrency::future::Race;
 use guestmem::GuestMemory;
 use guid::Guid;
 use inspect::Inspect;
-use nvme_resources::fault::FaultBehaviour;
 use nvme_resources::fault::FaultConfiguration;
+use nvme_resources::fault::QueueFaultBehavior;
 use pal_async::task::Spawn;
 use pal_async::task::Task;
 use pal_async::timer::PolledTimer;
@@ -481,22 +481,22 @@ impl AdminHandler {
                         .unwrap_or_else(|| QueueFaultBehavior::Default);
 
                     match fault {
-                        FaultBehaviour::Update(command_updated) => {
+                        QueueFaultBehavior::Update(command_updated) => {
                             tracing::warn!(
                                 "configured fault: admin command updated in sq. original: {:?},\n new: {:?}",
                                 &command,
                                 &command_updated
                             );
                             command = command_updated;
                         }
-                        FaultBehaviour::Drop => {
+                        QueueFaultBehavior::Drop => {
                             tracing::warn!(
                                 "configured fault: admin command dropped from sq {:?}",
                                 &command
                             );
                             return Ok(());
                         }
-                        FaultBehaviour::Delay(duration) => {
+                        QueueFaultBehavior::Delay(duration) => {
                             self.timer.sleep(duration).await;
                         }
                         QueueFaultBehavior::Panic(message) => {
diff --git a/vmm_tests/vmm_tests/tests/tests/multiarch/openhcl_servicing.rs b/vmm_tests/vmm_tests/tests/tests/multiarch/openhcl_servicing.rs
@@ -15,8 +15,9 @@ use mesh::CellUpdater;
 use nvme_resources::NamespaceDefinition;
 use nvme_resources::NvmeFaultControllerHandle;
 use nvme_resources::fault::AdminQueueFaultConfig;
-use nvme_resources::fault::FaultBehaviour;
 use nvme_resources::fault::FaultConfiguration;
+use nvme_resources::fault::PciFaultConfig;
+use nvme_resources::fault::QueueFaultBehavior;
 use petri::OpenHclServicingFlags;
 use petri::PetriVmBuilder;
 use petri::PetriVmmBackend;
@@ -256,8 +257,9 @@ async fn keepalive_with_nvme_fault(
         fault_active: fault_start_updater.cell(),
         admin_fault: AdminQueueFaultConfig::new().with_submission_queue_fault(
             nvme_spec::AdminOpcode::CREATE_IO_COMPLETION_QUEUE.0,
-            QueueFaultBehavior::Panic("Received a CREATE_IO_COMPLETION_QUEUE command during servicing with keepalive enabled. This should never happen.".to_string()),
+            QueueFaultBehavior::Panic("Received a CREATE_IO_COMPLETION_QUEUE command during servicing with keepalive enabled. THERE IS A BUG SOMEWHERE.".to_string()),
         ),
+        pci_fault: PciFaultConfig::new(),
     };
 
     let (mut vm, agent) = config
