Skip to content

Commit 7828d9c

Browse files
authoredJan 12, 2024
add QEMU pvpanic ISA device (#596)
This branch adds support for the [pvpanic] virtual device implemented by QEMU. This device allows guests to report kernel panics to the hypervisor. In `propolis-server`, guest-reported kernel panics are handled by incrementing an Oximeter metric. The pvpanic device can be exposed to the guest either as an ISA bus I/O port device or as a PCI bus device. This branch implements the ISA bus device. I'd like to also add a PCI pvpanic device, but will implement that in a subsequent pull request. In order for the guest to detect the ISA bus pvpanic device, it's necessary to add an entry for the panic device to the ACPI DSDT table. This is the AML that QEMU adds to its DSDT when the ISA bus pvpanic device is enabled: ``` // // QEMU panic device // Device (PEVT) { Name (_HID, "QEMU0001") // _HID: Hardware ID Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings { IO (Decode16, 0x0505, // Range Minimum 0x0505, // Range Maximum 0x01, // Alignment 0x01, // Length ) }) OperationRegion (PEOR, SystemIO, 0x0505, One) Field (PEOR, ByteAcc, NoLock, Preserve) { PEPT, 8 } Name (_STA, 0x0F) // _STA: Status Method (RDPT, 0, NotSerialized) { Local0 = PEPT /* \_SB_.PCI0.S08_.PEVT.PEPT */ Return (Local0) } Method (WRPT, 1, NotSerialized) { PEPT = Arg0 } } ``` This means that in order for guests to use this device, we need to boot with an ACPI table that contains this entry. For testing purposes, I modified EDK2 OVMF to add this entry to the DSDT. In the future, though, we'll likely want Propolis to generate ACPI tables dynamically on boot based on the instance spec. The EDK2 changes I used for testing this are available [here][edk2]. To test this change, I ran `propolis-standalone` with an Alpine Linux 3.19 guest,, and the following device added to the VM config file: ```toml [dev.pvpanic] driver = "qemu-pvpanic" enable_mmio = true ``` The guest correctly detects the panic device and loads the appropriate kernel module. If I then trigger a panic in the guest using SysRq, like this: ```console $ echo 1 > /proc/sys/kernel/sysrq $ echo c > /proc/sysrq-trigger ``` The guest crashes, and `propolis-standalone` logs: ``` dev: pvpanic Jan 11 18:14:13.494 DEBG guest kernel panic, guest_handled: false, host_handled: true ``` Closes #592 [pvpanic]: https://www.qemu.org/docs/master/specs/pvpanic.html [edk2]: oxidecomputer/edk2@6ca196f
1 parent 0cba3d1 commit 7828d9c

File tree

13 files changed

+377
-3
lines changed

13 files changed

+377
-3
lines changed
 

‎bin/propolis-server/src/lib/initializer.rs

+27-1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ use propolis::hw::chipset::Chipset;
2121
use propolis::hw::ibmpc;
2222
use propolis::hw::pci;
2323
use propolis::hw::ps2::ctrl::PS2Ctrl;
24+
use propolis::hw::qemu::pvpanic::QemuPvpanic;
2425
use propolis::hw::qemu::{debug::QemuDebugPort, fwcfg, ramfb};
2526
use propolis::hw::uart::LpcUart;
2627
use propolis::hw::{nvme, virtio};
@@ -34,7 +35,7 @@ use crate::serial::Serial;
3435
use crate::server::CrucibleBackendMap;
3536
pub use nexus_client::Client as NexusClient;
3637

37-
use anyhow::Result;
38+
use anyhow::{Context, Result};
3839

3940
// Arbitrary ROM limit for now
4041
const MAX_ROM_SIZE: usize = 0x20_0000;
@@ -276,6 +277,31 @@ impl<'a> MachineInitializer<'a> {
276277
Ok(())
277278
}
278279

280+
pub fn initialize_qemu_pvpanic(
281+
&self,
282+
uuid: uuid::Uuid,
283+
) -> Result<(), anyhow::Error> {
284+
if let Some(ref spec) = self.spec.devices.qemu_pvpanic {
285+
if spec.enable_isa {
286+
let pvpanic = QemuPvpanic::create(
287+
self.log.new(slog::o!("dev" => "qemu-pvpanic")),
288+
);
289+
pvpanic.attach_pio(&self.machine.bus_pio);
290+
self.inv.register(&pvpanic)?;
291+
292+
if let Some(ref registry) = self.producer_registry {
293+
let producer =
294+
crate::stats::PvpanicProducer::new(uuid, pvpanic);
295+
registry.register_producer(producer).context(
296+
"failed to register PVPANIC Oximeter producer",
297+
)?;
298+
}
299+
}
300+
}
301+
302+
Ok(())
303+
}
304+
279305
fn create_storage_backend_from_spec(
280306
&self,
281307
backend_spec: &instance_spec::v0::StorageBackendV0,

‎bin/propolis-server/src/lib/spec.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -222,9 +222,13 @@ impl ServerSpecBuilder {
222222
},
223223
)?;
224224

225-
let builder =
225+
let mut builder =
226226
SpecBuilder::new(properties.vcpus, properties.memory, enable_pcie);
227227

228+
builder.add_pvpanic_device(components::devices::QemuPvpanic {
229+
enable_isa: true,
230+
})?;
231+
228232
Ok(Self { builder })
229233
}
230234

‎bin/propolis-server/src/lib/stats.rs

+3
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ use uuid::Uuid;
2222

2323
use crate::server::MetricsEndpointConfig;
2424

25+
mod pvpanic;
26+
pub use self::pvpanic::PvpanicProducer;
27+
2528
const OXIMETER_STAT_INTERVAL: tokio::time::Duration =
2629
tokio::time::Duration::from_secs(30);
2730

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
// This Source Code Form is subject to the terms of the Mozilla Public
2+
// License, v. 2.0. If a copy of the MPL was not distributed with this
3+
// file, You can obtain one at https://mozilla.org/MPL/2.0/.
4+
5+
use super::InstanceUuid;
6+
use oximeter::{
7+
types::{Cumulative, Sample},
8+
Metric, MetricsError, Producer,
9+
};
10+
use propolis::hw::qemu::pvpanic;
11+
use std::sync::Arc;
12+
use uuid::Uuid;
13+
14+
#[derive(Clone, Debug)]
15+
pub struct PvpanicProducer {
16+
/// The name to use as the Oximeter target, i.e. the identifier of the
17+
/// source of these metrics.
18+
stat_name: InstanceUuid,
19+
20+
/// Kernel panic counts for the relevant instance.
21+
host_handled_panics: PvPanicHostHandled,
22+
guest_handled_panics: PvPanicGuestHandled,
23+
24+
pvpanic: Arc<pvpanic::QemuPvpanic>,
25+
}
26+
27+
/// An Oximeter `Metric` that specifies the number of times an instance's guest
28+
/// reported a guest-handled kernel panic using the QEMU `pvpanic` device.
29+
#[derive(Debug, Default, Copy, Clone, Metric)]
30+
struct PvPanicGuestHandled {
31+
/// The number of times this instance's guest handled a kernel panic.
32+
#[datum]
33+
pub count: Cumulative<i64>,
34+
}
35+
36+
/// An Oximeter `Metric` that specifies the number of times an instance's guest
37+
/// reported a host-handled kernel panic using the QEMU `pvpanic` device.
38+
#[derive(Debug, Default, Copy, Clone, Metric)]
39+
struct PvPanicHostHandled {
40+
/// The number of times this instance's reported a host-handled kernel panic.
41+
#[datum]
42+
pub count: Cumulative<i64>,
43+
}
44+
45+
impl PvpanicProducer {
46+
pub fn new(id: Uuid, pvpanic: Arc<pvpanic::QemuPvpanic>) -> Self {
47+
PvpanicProducer {
48+
stat_name: InstanceUuid { uuid: id },
49+
host_handled_panics: Default::default(),
50+
guest_handled_panics: Default::default(),
51+
pvpanic,
52+
}
53+
}
54+
}
55+
56+
impl Producer for PvpanicProducer {
57+
fn produce(
58+
&mut self,
59+
) -> Result<Box<dyn Iterator<Item = Sample> + 'static>, MetricsError> {
60+
let pvpanic::PanicCounts { guest_handled, host_handled } =
61+
self.pvpanic.panic_counts();
62+
63+
self.host_handled_panics.datum_mut().set(host_handled as i64);
64+
self.guest_handled_panics.datum_mut().set(guest_handled as i64);
65+
66+
let data = vec![
67+
Sample::new(&self.stat_name, &self.guest_handled_panics)?,
68+
Sample::new(&self.stat_name, &self.host_handled_panics)?,
69+
];
70+
71+
Ok(Box::new(data.into_iter()))
72+
}
73+
}

‎bin/propolis-server/src/lib/vm/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,7 @@ impl VmController {
458458
let ps2ctrl_id = init.initialize_ps2(&chipset)?;
459459
let ps2ctrl: Option<Arc<PS2Ctrl>> = inv.get_concrete(ps2ctrl_id);
460460
init.initialize_qemu_debug_port()?;
461+
init.initialize_qemu_pvpanic(properties.id)?;
461462
init.initialize_network_devices(&chipset)?;
462463
#[cfg(feature = "falcon")]
463464
init.initialize_softnpu_ports(&chipset)?;

‎bin/propolis-standalone/src/main.rs

+16-1
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,16 @@ use std::time::{SystemTime, UNIX_EPOCH};
1515
use anyhow::Context;
1616
use clap::Parser;
1717
use futures::future::BoxFuture;
18+
use propolis::hw::qemu::pvpanic::QemuPvpanic;
1819
use slog::{o, Drain};
1920
use strum::IntoEnumIterator;
2021
use tokio::runtime;
2122

2223
use propolis::chardev::{BlockingSource, Sink, Source, UDSock};
2324
use propolis::hw::chipset::{i440fx, Chipset};
24-
use propolis::hw::ibmpc;
2525
use propolis::hw::ps2::ctrl::PS2Ctrl;
2626
use propolis::hw::uart::LpcUart;
27+
use propolis::hw::{ibmpc, qemu};
2728
use propolis::intr_pins::FuncPin;
2829
use propolis::usdt::register_probes;
2930
use propolis::vcpu::Vcpu;
@@ -929,6 +930,20 @@ fn setup_instance(
929930

930931
chipset.pci_attach(bdf, nvme);
931932
}
933+
qemu::pvpanic::DEVICE_NAME => {
934+
let enable_isa = dev
935+
.options
936+
.get("enable_isa")
937+
.and_then(|opt| opt.as_bool())
938+
.unwrap_or(false);
939+
if enable_isa {
940+
let pvpanic = QemuPvpanic::create(
941+
log.new(slog::o!("dev" => "pvpanic")),
942+
);
943+
pvpanic.attach_pio(pio);
944+
inv.register(&pvpanic)?;
945+
}
946+
}
932947
_ => {
933948
slog::error!(log, "unrecognized driver"; "name" => name);
934949
return Err(Error::new(

‎crates/propolis-api-types/src/instance_spec/components/devices.rs

+59
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,46 @@ pub enum MigrationCompatibilityError {
214214
ComponentConfiguration(String),
215215
}
216216

217+
#[derive(
218+
Clone,
219+
Copy,
220+
Deserialize,
221+
Serialize,
222+
Debug,
223+
PartialEq,
224+
Eq,
225+
JsonSchema,
226+
Default,
227+
)]
228+
#[serde(deny_unknown_fields)]
229+
pub struct QemuPvpanic {
230+
/// Enable the QEMU PVPANIC ISA bus device (I/O port 0x505).
231+
pub enable_isa: bool,
232+
// TODO(eliza): add support for the PCI PVPANIC device...
233+
}
234+
235+
impl MigrationElement for Option<QemuPvpanic> {
236+
fn kind(&self) -> &'static str {
237+
"QemuPvpanic"
238+
}
239+
240+
fn can_migrate_from_element(
241+
&self,
242+
other: &Self,
243+
) -> Result<(), crate::instance_spec::migration::ElementCompatibilityError>
244+
{
245+
if self != other {
246+
Err(MigrationCompatibilityError::ComponentConfiguration(format!(
247+
"pvpanic configuration mismatch (self: {0:?}, other: {1:?})",
248+
self, other
249+
))
250+
.into())
251+
} else {
252+
Ok(())
253+
}
254+
}
255+
}
256+
217257
//
218258
// Structs for Falcon devices. These devices don't support live migration.
219259
//
@@ -385,4 +425,23 @@ mod test {
385425
b2.pci_path = PciPath::new(4, 5, 6).unwrap();
386426
assert!(b1.can_migrate_from_element(&b2).is_err());
387427
}
428+
429+
#[test]
430+
fn incompatible_qemu_pvpanic() {
431+
let d1 = Some(QemuPvpanic { enable_isa: true });
432+
let d2 = Some(QemuPvpanic { enable_isa: false });
433+
assert!(d1.can_migrate_from_element(&d2).is_err());
434+
assert!(d1.can_migrate_from_element(&None).is_err());
435+
}
436+
437+
#[test]
438+
fn compatible_qemu_pvpanic() {
439+
let d1 = Some(QemuPvpanic { enable_isa: true });
440+
let d2 = Some(QemuPvpanic { enable_isa: true });
441+
assert!(d1.can_migrate_from_element(&d2).is_ok());
442+
443+
let d1 = Some(QemuPvpanic { enable_isa: false });
444+
let d2 = Some(QemuPvpanic { enable_isa: false });
445+
assert!(d1.can_migrate_from_element(&d2).is_ok());
446+
}
388447
}

‎crates/propolis-api-types/src/instance_spec/v0/builder.rs

+16
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,22 @@ impl SpecBuilder {
176176
}
177177
}
178178

179+
/// Adds a QEMU pvpanic device.
180+
pub fn add_pvpanic_device(
181+
&mut self,
182+
pvpanic: components::devices::QemuPvpanic,
183+
) -> Result<&Self, SpecBuilderError> {
184+
if self.spec.devices.qemu_pvpanic.is_some() {
185+
return Err(SpecBuilderError::DeviceNameInUse(
186+
"pvpanic".to_string(),
187+
));
188+
}
189+
190+
self.spec.devices.qemu_pvpanic = Some(pvpanic);
191+
192+
Ok(self)
193+
}
194+
179195
#[cfg(feature = "falcon")]
180196
pub fn set_softnpu_pci_port(
181197
&mut self,

‎crates/propolis-api-types/src/instance_spec/v0/mod.rs

+23
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,20 @@ pub struct DeviceSpecV0 {
114114
pub serial_ports: HashMap<SpecKey, components::devices::SerialPort>,
115115
pub pci_pci_bridges: HashMap<SpecKey, components::devices::PciPciBridge>,
116116

117+
// This field has a default value (`None`) to allow for
118+
// backwards-compatibility when upgrading from a Propolis
119+
// version that does not support this device. If the pvpanic device was not
120+
// present in the spec being deserialized, a `None` will be produced,
121+
// rather than rejecting the spec.
122+
#[serde(default)]
123+
// Skip serializing this field if it is `None`. This is so that Propolis
124+
// versions with support for this device are backwards-compatible with
125+
// older versions that don't, as long as the spec doesn't define a pvpanic
126+
// device --- if there is no panic device, skipping the field from the spec
127+
// means that the older version will still accept the spec.
128+
#[serde(skip_serializing_if = "Option::is_none")]
129+
pub qemu_pvpanic: Option<components::devices::QemuPvpanic>,
130+
117131
#[cfg(feature = "falcon")]
118132
pub softnpu_pci_port: Option<components::devices::SoftNpuPciPort>,
119133
#[cfg(feature = "falcon")]
@@ -169,6 +183,15 @@ impl DeviceSpecV0 {
169183
)
170184
})?;
171185

186+
self.qemu_pvpanic
187+
.can_migrate_from_element(&other.qemu_pvpanic)
188+
.map_err(|e| {
189+
MigrationCompatibilityError::ElementMismatch(
190+
"QEMU PVPANIC device".to_string(),
191+
e,
192+
)
193+
})?;
194+
172195
Ok(())
173196
}
174197
}

‎lib/propolis/src/hw/qemu/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,5 @@
44

55
pub mod debug;
66
pub mod fwcfg;
7+
pub mod pvpanic;
78
pub mod ramfb;

‎lib/propolis/src/hw/qemu/pvpanic.rs

+111
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
// This Source Code Form is subject to the terms of the Mozilla Public
2+
// License, v. 2.0. If a copy of the MPL was not distributed with this
3+
// file, You can obtain one at https://mozilla.org/MPL/2.0/.
4+
5+
use std::sync::{Arc, Mutex};
6+
7+
use crate::common::*;
8+
use crate::pio::{PioBus, PioFn};
9+
10+
/// Implements the QEMU [pvpanic device], which
11+
/// may be used by guests to notify the host when a kernel panic has occurred.
12+
///
13+
/// QEMU exposes the pvpanic virtual device as a device on the ISA bus (I/O port
14+
/// 0x505), a PCI device, and through ACPI. Currently, Propolis only implements
15+
/// the ISA bus pvpanic device, but the PCI device may be implemented in the
16+
/// future.
17+
///
18+
/// [pvpanic device]: https://www.qemu.org/docs/master/specs/pvpanic.html
19+
#[derive(Debug)]
20+
pub struct QemuPvpanic {
21+
counts: Mutex<PanicCounts>,
22+
log: slog::Logger,
23+
}
24+
25+
/// Counts the number of guest kernel panics reported using the [`QemuPvpanic`]
26+
/// virtual device.
27+
#[derive(Copy, Clone, Debug)]
28+
pub struct PanicCounts {
29+
/// Counts the number of guest kernel panics handled by the host.
30+
pub host_handled: usize,
31+
/// Counts the number of guest kernel panics handled by the guest.
32+
pub guest_handled: usize,
33+
}
34+
35+
pub const DEVICE_NAME: &str = "qemu-pvpanic";
36+
37+
/// Indicates that a guest panic has happened and should be processed by the
38+
/// host
39+
const HOST_HANDLED: u8 = 0b01;
40+
/// Indicates a guest panic has happened and will be handled by the guest; the
41+
/// host should record it or report it, but should not affect the execution of
42+
/// the guest.
43+
const GUEST_HANDLED: u8 = 0b10;
44+
45+
#[usdt::provider(provider = "propolis")]
46+
mod probes {
47+
fn pvpanic_pio_write(value: u8) {}
48+
}
49+
50+
impl QemuPvpanic {
51+
const IOPORT: u16 = 0x505;
52+
53+
pub fn create(log: slog::Logger) -> Arc<Self> {
54+
Arc::new(Self {
55+
counts: Mutex::new(PanicCounts {
56+
host_handled: 0,
57+
guest_handled: 0,
58+
}),
59+
log,
60+
})
61+
}
62+
63+
/// Attaches this pvpanic device to the provided [`PioBus`].
64+
pub fn attach_pio(self: &Arc<Self>, pio: &PioBus) {
65+
let piodev = self.clone();
66+
let piofn = Arc::new(move |_port: u16, rwo: RWOp| piodev.pio_rw(rwo))
67+
as Arc<PioFn>;
68+
pio.register(Self::IOPORT, 1, piofn).unwrap();
69+
}
70+
71+
/// Returns the current panic counts reported by the guest.
72+
pub fn panic_counts(&self) -> PanicCounts {
73+
*self.counts.lock().unwrap()
74+
}
75+
76+
fn pio_rw(&self, rwo: RWOp) {
77+
match rwo {
78+
RWOp::Read(ro) => {
79+
ro.write_u8(HOST_HANDLED | GUEST_HANDLED);
80+
}
81+
RWOp::Write(wo) => {
82+
let value = wo.read_u8();
83+
probes::pvpanic_pio_write!(|| value);
84+
let host_handled = value & HOST_HANDLED != 0;
85+
let guest_handled = value & GUEST_HANDLED != 0;
86+
slog::debug!(
87+
self.log,
88+
"guest kernel panic";
89+
"host_handled" => host_handled,
90+
"guest_handled" => guest_handled,
91+
);
92+
93+
let mut counts = self.counts.lock().unwrap();
94+
95+
if host_handled {
96+
counts.host_handled += 1;
97+
}
98+
99+
if guest_handled {
100+
counts.guest_handled += 1;
101+
}
102+
}
103+
}
104+
}
105+
}
106+
107+
impl Entity for QemuPvpanic {
108+
fn type_name(&self) -> &'static str {
109+
DEVICE_NAME
110+
}
111+
}

‎openapi/propolis-server-falcon.json

+21
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,14 @@
631631
"$ref": "#/components/schemas/PciPciBridge"
632632
}
633633
},
634+
"qemu_pvpanic": {
635+
"nullable": true,
636+
"allOf": [
637+
{
638+
"$ref": "#/components/schemas/QemuPvpanic"
639+
}
640+
]
641+
},
634642
"serial_ports": {
635643
"type": "object",
636644
"additionalProperties": {
@@ -1405,6 +1413,19 @@
14051413
],
14061414
"additionalProperties": false
14071415
},
1416+
"QemuPvpanic": {
1417+
"type": "object",
1418+
"properties": {
1419+
"enable_isa": {
1420+
"description": "Enable the QEMU PVPANIC ISA bus device (I/O port 0x505).",
1421+
"type": "boolean"
1422+
}
1423+
},
1424+
"required": [
1425+
"enable_isa"
1426+
],
1427+
"additionalProperties": false
1428+
},
14081429
"SerialPort": {
14091430
"description": "A serial port device.",
14101431
"type": "object",

‎openapi/propolis-server.json

+21
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,14 @@
623623
"$ref": "#/components/schemas/PciPciBridge"
624624
}
625625
},
626+
"qemu_pvpanic": {
627+
"nullable": true,
628+
"allOf": [
629+
{
630+
"$ref": "#/components/schemas/QemuPvpanic"
631+
}
632+
]
633+
},
626634
"serial_ports": {
627635
"type": "object",
628636
"additionalProperties": {
@@ -1340,6 +1348,19 @@
13401348
],
13411349
"additionalProperties": false
13421350
},
1351+
"QemuPvpanic": {
1352+
"type": "object",
1353+
"properties": {
1354+
"enable_isa": {
1355+
"description": "Enable the QEMU PVPANIC ISA bus device (I/O port 0x505).",
1356+
"type": "boolean"
1357+
}
1358+
},
1359+
"required": [
1360+
"enable_isa"
1361+
],
1362+
"additionalProperties": false
1363+
},
13431364
"SerialPort": {
13441365
"description": "A serial port device.",
13451366
"type": "object",

0 commit comments

Comments
 (0)
Please sign in to comment.