Skip to content

Commit 3b06a84

Browse files
committed
fix(virtio): avoid panic on device activation failure
According to the VirtIO spec section 2.1.2, when we enter a failed state we need to to set `DEVICE_NEEDS_RESET` status field and if `DRIVER_OK` is set we need to must send a device configuration change interrupt to the driver. In MMIO code we were trying to follow this logic, but we were trying to get the interrupt object from the device, which fails (and panics) because interrupts are only set in the device upon successful activation. In the PCI transport, instead, we were not doing this at all. The transport layers hold a reference to the interrupts at all times. So, add the missing logic to the PCI transport and use the transport layer reference in both transports to avoid panics. Signed-off-by: Babis Chalios <[email protected]>
1 parent 39bdad6 commit 3b06a84

File tree

2 files changed

+15
-11
lines changed

2 files changed

+15
-11
lines changed

src/vmm/src/devices/virtio/transport/mmio.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -190,10 +190,7 @@ impl MmioTransport {
190190

191191
// Section 2.1.2 of the specification states that we need to send a device
192192
// configuration change interrupt
193-
let _ = self
194-
.locked_device()
195-
.interrupt_trigger()
196-
.trigger(VirtioInterruptType::Config);
193+
let _ = self.interrupt.trigger(VirtioInterruptType::Config);
197194

198195
error!("Failed to activate virtio device: {}", err)
199196
}

src/vmm/src/devices/virtio/transport/pci/device.rs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1029,15 +1029,22 @@ impl PciDevice for VirtioPciDevice {
10291029
// Try and activate the device if the driver status has changed
10301030
if self.needs_activation() {
10311031
debug!("Activating device");
1032-
self.virtio_device()
1032+
let interrupt = Arc::clone(self.virtio_interrupt.as_ref().unwrap());
1033+
match self
1034+
.virtio_device()
10331035
.lock()
10341036
.unwrap()
1035-
.activate(
1036-
self.memory.clone(),
1037-
Arc::clone(self.virtio_interrupt.as_ref().unwrap()),
1038-
)
1039-
.unwrap_or_else(|err| error!("Error activating device: {err:?}"));
1040-
self.device_activated.store(true, Ordering::SeqCst);
1037+
.activate(self.memory.clone(), interrupt.clone())
1038+
{
1039+
Ok(()) => self.device_activated.store(true, Ordering::SeqCst),
1040+
Err(err) => {
1041+
error!("Error activating device: {err:?}");
1042+
1043+
// Section 2.1.2 of the specification states that we need to send a device
1044+
// configuration change interrupt
1045+
let _ = interrupt.trigger(VirtioInterruptType::Config);
1046+
}
1047+
}
10411048
} else {
10421049
debug!("Device doesn't need activation");
10431050
}

0 commit comments

Comments
 (0)