From 160b1330b5bb4ffe7bbf97952e22bd22a9b6e2fe Mon Sep 17 00:00:00 2001 From: John Starks Date: Thu, 23 Jul 2026 08:31:07 -0700 Subject: [PATCH 1/2] nvme: reset shadow doorbell state Restore and clear the controller-owned doorbell backing after queue workers drain so stale shadow doorbell and event-index mappings do not survive controller reset. Retain the original backing allocation for reuse and cover both the normal and fault-injection emulators. Thanks to @bitranox for identifying the stale mapping and suggesting the reset approach in #3915. --- vm/devices/storage/nvme/src/queue.rs | 14 +++++++- .../nvme/src/tests/shadow_doorbell_tests.rs | 36 +++++++++++++++++++ .../storage/nvme/src/workers/coordinator.rs | 1 + vm/devices/storage/nvme_test/src/queue.rs | 14 +++++++- .../src/tests/shadow_doorbell_tests.rs | 36 +++++++++++++++++++ .../nvme_test/src/workers/coordinator.rs | 1 + 6 files changed, 100 insertions(+), 2 deletions(-) diff --git a/vm/devices/storage/nvme/src/queue.rs b/vm/devices/storage/nvme/src/queue.rs index 7b46ae9266..2206f2f515 100644 --- a/vm/devices/storage/nvme/src/queue.rs +++ b/vm/devices/storage/nvme/src/queue.rs @@ -20,6 +20,7 @@ use vmcore::interrupt::Interrupt; pub struct DoorbellMemory { mem: GuestMemory, + private_mem: GuestMemory, offset: u64, event_idx_offset: Option, wakers: Vec>, @@ -29,14 +30,25 @@ pub struct InvalidDoorbell; impl DoorbellMemory { pub fn new(num_qids: u16) -> Self { + let private_mem = GuestMemory::allocate((num_qids as usize) << DOORBELL_STRIDE_BITS); Self { - mem: GuestMemory::allocate((num_qids as usize) << DOORBELL_STRIDE_BITS), + mem: private_mem.clone(), + private_mem, offset: 0, event_idx_offset: None, wakers: (0..num_qids).map(|_| None).collect(), } } + pub fn reset(&mut self) { + self.private_mem + .fill_at(0, 0, self.wakers.len() << DOORBELL_STRIDE_BITS) + .expect("private doorbell memory must be writable"); + self.mem = self.private_mem.clone(); + self.offset = 0; + self.event_idx_offset = None; + } + /// Update the memory used to store the doorbell values. This is used to /// support shadow doorbells, where the values are directly in guest memory. pub fn replace_mem( diff --git a/vm/devices/storage/nvme/src/tests/shadow_doorbell_tests.rs b/vm/devices/storage/nvme/src/tests/shadow_doorbell_tests.rs index 8581a191de..dd4e601599 100644 --- a/vm/devices/storage/nvme/src/tests/shadow_doorbell_tests.rs +++ b/vm/devices/storage/nvme/src/tests/shadow_doorbell_tests.rs @@ -3,6 +3,7 @@ use crate::PAGE_SIZE64; use crate::prp::PrpRange; +use crate::queue::DoorbellMemory; use crate::spec; use crate::tests::controller_tests::instantiate_and_build_admin_queue; use crate::tests::controller_tests::wait_for_msi; @@ -14,6 +15,7 @@ use pal_async::DefaultDriver; use pal_async::async_test; use pci_core::test_helpers::TestPciInterruptController; use user_driver::backoff::Backoff; +use vmcore::device_state::ChangeDeviceState; use zerocopy::FromZeros; use zerocopy::IntoBytes; @@ -157,6 +159,40 @@ async fn test_setup_shadow_doorbells(driver: DefaultDriver) { setup_shadow_doorbells(driver.clone(), &cq_buf, &sq_buf, &gm, &int_controller, None).await; } +#[async_test] +async fn test_reset_shadow_doorbells(driver: DefaultDriver) { + let cq_buf = PrpRange::new(vec![CQ_BASE], 0, PAGE_SIZE64).unwrap(); + let sq_buf = PrpRange::new(vec![SQ_BASE], 0, PAGE_SIZE64).unwrap(); + let gm = test_memory(); + let int_controller = TestPciInterruptController::new(); + + let mut nvmec = + setup_shadow_doorbells(driver, &cq_buf, &sq_buf, &gm, &int_controller, None).await; + + ChangeDeviceState::reset(&mut nvmec).await; + + let shadow_value = 0x1234; + gm.write_plain::(DOORBELL_BUFFER_BASE, &shadow_value) + .unwrap(); + nvmec.write_bar0(0x1000, 0x5678_u32.as_bytes()).unwrap(); + + assert_eq!( + gm.read_plain::(DOORBELL_BUFFER_BASE).unwrap(), + shadow_value + ); + + let mut doorbells = DoorbellMemory::new(2); + assert!(doorbells.try_write(0, shadow_value).is_ok()); + doorbells + .replace_mem(gm.clone(), DOORBELL_BUFFER_BASE, None) + .unwrap(); + doorbells.reset(); + doorbells + .replace_mem(gm.clone(), EVT_IDX_BUFFER_BASE, None) + .unwrap(); + assert_eq!(gm.read_plain::(EVT_IDX_BUFFER_BASE).unwrap(), 0); +} + #[async_test] async fn test_setup_sq_ring_with_shadow(driver: DefaultDriver) { let cq_buf = PrpRange::new(vec![CQ_BASE], 0, PAGE_SIZE64).unwrap(); diff --git a/vm/devices/storage/nvme/src/workers/coordinator.rs b/vm/devices/storage/nvme/src/workers/coordinator.rs index 365ec9cf7b..5f10416b87 100644 --- a/vm/devices/storage/nvme/src/workers/coordinator.rs +++ b/vm/devices/storage/nvme/src/workers/coordinator.rs @@ -174,6 +174,7 @@ impl NvmeWorkers { } } } + self.doorbells.write().reset(); } } diff --git a/vm/devices/storage/nvme_test/src/queue.rs b/vm/devices/storage/nvme_test/src/queue.rs index 5ac0d3b8b5..789b305ff8 100644 --- a/vm/devices/storage/nvme_test/src/queue.rs +++ b/vm/devices/storage/nvme_test/src/queue.rs @@ -20,6 +20,7 @@ use vmcore::interrupt::Interrupt; pub struct DoorbellMemory { mem: GuestMemory, + private_mem: GuestMemory, offset: u64, event_idx_offset: Option, wakers: Vec>, @@ -29,14 +30,25 @@ pub struct InvalidDoorbell; impl DoorbellMemory { pub fn new(num_qids: u16) -> Self { + let private_mem = GuestMemory::allocate((num_qids as usize) << DOORBELL_STRIDE_BITS); Self { - mem: GuestMemory::allocate((num_qids as usize) << DOORBELL_STRIDE_BITS), + mem: private_mem.clone(), + private_mem, offset: 0, event_idx_offset: None, wakers: (0..num_qids).map(|_| None).collect(), } } + pub fn reset(&mut self) { + self.private_mem + .fill_at(0, 0, self.wakers.len() << DOORBELL_STRIDE_BITS) + .expect("private doorbell memory must be writable"); + self.mem = self.private_mem.clone(); + self.offset = 0; + self.event_idx_offset = None; + } + /// Update the memory used to store the doorbell values. This is used to /// support shadow doorbells, where the values are directly in guest memory. pub fn replace_mem( 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 index a4bf449623..eda4611f4c 100644 --- a/vm/devices/storage/nvme_test/src/tests/shadow_doorbell_tests.rs +++ b/vm/devices/storage/nvme_test/src/tests/shadow_doorbell_tests.rs @@ -3,6 +3,7 @@ use crate::PAGE_SIZE64; use crate::prp::PrpRange; +use crate::queue::DoorbellMemory; use crate::spec; use crate::tests::controller_tests::instantiate_and_build_admin_queue; use crate::tests::controller_tests::wait_for_msi; @@ -16,6 +17,7 @@ use pal_async::DefaultDriver; use pal_async::async_test; use pci_core::test_helpers::TestPciInterruptController; use user_driver::backoff::Backoff; +use vmcore::device_state::ChangeDeviceState; use zerocopy::FromZeros; use zerocopy::IntoBytes; @@ -162,6 +164,40 @@ async fn test_setup_shadow_doorbells(driver: DefaultDriver) { setup_shadow_doorbells(driver.clone(), &cq_buf, &sq_buf, &gm, &int_controller, None).await; } +#[async_test] +async fn test_reset_shadow_doorbells(driver: DefaultDriver) { + let cq_buf = PrpRange::new(vec![CQ_BASE], 0, PAGE_SIZE64).unwrap(); + let sq_buf = PrpRange::new(vec![SQ_BASE], 0, PAGE_SIZE64).unwrap(); + let gm = test_memory(); + let int_controller = TestPciInterruptController::new(); + + let mut nvmec = + setup_shadow_doorbells(driver, &cq_buf, &sq_buf, &gm, &int_controller, None).await; + + ChangeDeviceState::reset(&mut nvmec).await; + + let shadow_value = 0x1234; + gm.write_plain::(DOORBELL_BUFFER_BASE, &shadow_value) + .unwrap(); + nvmec.write_bar0(0x1000, 0x5678_u32.as_bytes()).unwrap(); + + assert_eq!( + gm.read_plain::(DOORBELL_BUFFER_BASE).unwrap(), + shadow_value + ); + + let mut doorbells = DoorbellMemory::new(2); + assert!(doorbells.try_write(0, shadow_value).is_ok()); + doorbells + .replace_mem(gm.clone(), DOORBELL_BUFFER_BASE, None) + .unwrap(); + doorbells.reset(); + doorbells + .replace_mem(gm.clone(), EVT_IDX_BUFFER_BASE, None) + .unwrap(); + assert_eq!(gm.read_plain::(EVT_IDX_BUFFER_BASE).unwrap(), 0); +} + #[async_test] async fn test_setup_sq_ring_with_shadow(driver: DefaultDriver) { let cq_buf = PrpRange::new(vec![CQ_BASE], 0, PAGE_SIZE64).unwrap(); diff --git a/vm/devices/storage/nvme_test/src/workers/coordinator.rs b/vm/devices/storage/nvme_test/src/workers/coordinator.rs index a349c83743..f3ec555655 100644 --- a/vm/devices/storage/nvme_test/src/workers/coordinator.rs +++ b/vm/devices/storage/nvme_test/src/workers/coordinator.rs @@ -177,6 +177,7 @@ impl NvmeWorkers { } } } + self.doorbells.write().reset(); } } From 242fdf5a3092695ca03201aab950572c7ce1f8df Mon Sep 17 00:00:00 2001 From: John Starks Date: Fri, 24 Jul 2026 02:02:38 -0700 Subject: [PATCH 2/2] feedback --- vm/devices/storage/nvme/src/queue.rs | 18 +++++++++++++----- .../nvme/src/tests/shadow_doorbell_tests.rs | 12 ------------ vm/devices/storage/nvme_test/src/queue.rs | 18 +++++++++++++----- .../src/tests/shadow_doorbell_tests.rs | 12 ------------ 4 files changed, 26 insertions(+), 34 deletions(-) diff --git a/vm/devices/storage/nvme/src/queue.rs b/vm/devices/storage/nvme/src/queue.rs index 2206f2f515..6477d9fd9b 100644 --- a/vm/devices/storage/nvme/src/queue.rs +++ b/vm/devices/storage/nvme/src/queue.rs @@ -41,12 +41,20 @@ impl DoorbellMemory { } pub fn reset(&mut self) { - self.private_mem - .fill_at(0, 0, self.wakers.len() << DOORBELL_STRIDE_BITS) + let Self { + mem, + private_mem, + offset, + event_idx_offset, + wakers, + } = self; + private_mem + .fill_at(0, 0, wakers.len() << DOORBELL_STRIDE_BITS) .expect("private doorbell memory must be writable"); - self.mem = self.private_mem.clone(); - self.offset = 0; - self.event_idx_offset = None; + *mem = private_mem.clone(); + *offset = 0; + *event_idx_offset = None; + wakers.fill(None); } /// Update the memory used to store the doorbell values. This is used to diff --git a/vm/devices/storage/nvme/src/tests/shadow_doorbell_tests.rs b/vm/devices/storage/nvme/src/tests/shadow_doorbell_tests.rs index dd4e601599..c13cb69af9 100644 --- a/vm/devices/storage/nvme/src/tests/shadow_doorbell_tests.rs +++ b/vm/devices/storage/nvme/src/tests/shadow_doorbell_tests.rs @@ -3,7 +3,6 @@ use crate::PAGE_SIZE64; use crate::prp::PrpRange; -use crate::queue::DoorbellMemory; use crate::spec; use crate::tests::controller_tests::instantiate_and_build_admin_queue; use crate::tests::controller_tests::wait_for_msi; @@ -180,17 +179,6 @@ async fn test_reset_shadow_doorbells(driver: DefaultDriver) { gm.read_plain::(DOORBELL_BUFFER_BASE).unwrap(), shadow_value ); - - let mut doorbells = DoorbellMemory::new(2); - assert!(doorbells.try_write(0, shadow_value).is_ok()); - doorbells - .replace_mem(gm.clone(), DOORBELL_BUFFER_BASE, None) - .unwrap(); - doorbells.reset(); - doorbells - .replace_mem(gm.clone(), EVT_IDX_BUFFER_BASE, None) - .unwrap(); - assert_eq!(gm.read_plain::(EVT_IDX_BUFFER_BASE).unwrap(), 0); } #[async_test] diff --git a/vm/devices/storage/nvme_test/src/queue.rs b/vm/devices/storage/nvme_test/src/queue.rs index 789b305ff8..860560293e 100644 --- a/vm/devices/storage/nvme_test/src/queue.rs +++ b/vm/devices/storage/nvme_test/src/queue.rs @@ -41,12 +41,20 @@ impl DoorbellMemory { } pub fn reset(&mut self) { - self.private_mem - .fill_at(0, 0, self.wakers.len() << DOORBELL_STRIDE_BITS) + let Self { + mem, + private_mem, + offset, + event_idx_offset, + wakers, + } = self; + private_mem + .fill_at(0, 0, wakers.len() << DOORBELL_STRIDE_BITS) .expect("private doorbell memory must be writable"); - self.mem = self.private_mem.clone(); - self.offset = 0; - self.event_idx_offset = None; + *mem = private_mem.clone(); + *offset = 0; + *event_idx_offset = None; + wakers.fill(None); } /// Update the memory used to store the doorbell values. This is used to 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 index eda4611f4c..208f1ea4ea 100644 --- a/vm/devices/storage/nvme_test/src/tests/shadow_doorbell_tests.rs +++ b/vm/devices/storage/nvme_test/src/tests/shadow_doorbell_tests.rs @@ -3,7 +3,6 @@ use crate::PAGE_SIZE64; use crate::prp::PrpRange; -use crate::queue::DoorbellMemory; use crate::spec; use crate::tests::controller_tests::instantiate_and_build_admin_queue; use crate::tests::controller_tests::wait_for_msi; @@ -185,17 +184,6 @@ async fn test_reset_shadow_doorbells(driver: DefaultDriver) { gm.read_plain::(DOORBELL_BUFFER_BASE).unwrap(), shadow_value ); - - let mut doorbells = DoorbellMemory::new(2); - assert!(doorbells.try_write(0, shadow_value).is_ok()); - doorbells - .replace_mem(gm.clone(), DOORBELL_BUFFER_BASE, None) - .unwrap(); - doorbells.reset(); - doorbells - .replace_mem(gm.clone(), EVT_IDX_BUFFER_BASE, None) - .unwrap(); - assert_eq!(gm.read_plain::(EVT_IDX_BUFFER_BASE).unwrap(), 0); } #[async_test]