Skip to content

Commit

Permalink
add QEMU pvpanic ISA device (#596)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
hawkw authored Jan 12, 2024
1 parent 0cba3d1 commit 7828d9c
Show file tree
Hide file tree
Showing 13 changed files with 377 additions and 3 deletions.
28 changes: 27 additions & 1 deletion bin/propolis-server/src/lib/initializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use propolis::hw::chipset::Chipset;
use propolis::hw::ibmpc;
use propolis::hw::pci;
use propolis::hw::ps2::ctrl::PS2Ctrl;
use propolis::hw::qemu::pvpanic::QemuPvpanic;
use propolis::hw::qemu::{debug::QemuDebugPort, fwcfg, ramfb};
use propolis::hw::uart::LpcUart;
use propolis::hw::{nvme, virtio};
Expand All @@ -34,7 +35,7 @@ use crate::serial::Serial;
use crate::server::CrucibleBackendMap;
pub use nexus_client::Client as NexusClient;

use anyhow::Result;
use anyhow::{Context, Result};

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

pub fn initialize_qemu_pvpanic(
&self,
uuid: uuid::Uuid,
) -> Result<(), anyhow::Error> {
if let Some(ref spec) = self.spec.devices.qemu_pvpanic {
if spec.enable_isa {
let pvpanic = QemuPvpanic::create(
self.log.new(slog::o!("dev" => "qemu-pvpanic")),
);
pvpanic.attach_pio(&self.machine.bus_pio);
self.inv.register(&pvpanic)?;

if let Some(ref registry) = self.producer_registry {
let producer =
crate::stats::PvpanicProducer::new(uuid, pvpanic);
registry.register_producer(producer).context(
"failed to register PVPANIC Oximeter producer",
)?;
}
}
}

Ok(())
}

fn create_storage_backend_from_spec(
&self,
backend_spec: &instance_spec::v0::StorageBackendV0,
Expand Down
6 changes: 5 additions & 1 deletion bin/propolis-server/src/lib/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,13 @@ impl ServerSpecBuilder {
},
)?;

let builder =
let mut builder =
SpecBuilder::new(properties.vcpus, properties.memory, enable_pcie);

builder.add_pvpanic_device(components::devices::QemuPvpanic {
enable_isa: true,
})?;

Ok(Self { builder })
}

Expand Down
3 changes: 3 additions & 0 deletions bin/propolis-server/src/lib/stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ use uuid::Uuid;

use crate::server::MetricsEndpointConfig;

mod pvpanic;
pub use self::pvpanic::PvpanicProducer;

const OXIMETER_STAT_INTERVAL: tokio::time::Duration =
tokio::time::Duration::from_secs(30);

Expand Down
73 changes: 73 additions & 0 deletions bin/propolis-server/src/lib/stats/pvpanic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

use super::InstanceUuid;
use oximeter::{
types::{Cumulative, Sample},
Metric, MetricsError, Producer,
};
use propolis::hw::qemu::pvpanic;
use std::sync::Arc;
use uuid::Uuid;

#[derive(Clone, Debug)]
pub struct PvpanicProducer {
/// The name to use as the Oximeter target, i.e. the identifier of the
/// source of these metrics.
stat_name: InstanceUuid,

/// Kernel panic counts for the relevant instance.
host_handled_panics: PvPanicHostHandled,
guest_handled_panics: PvPanicGuestHandled,

pvpanic: Arc<pvpanic::QemuPvpanic>,
}

/// An Oximeter `Metric` that specifies the number of times an instance's guest
/// reported a guest-handled kernel panic using the QEMU `pvpanic` device.
#[derive(Debug, Default, Copy, Clone, Metric)]
struct PvPanicGuestHandled {
/// The number of times this instance's guest handled a kernel panic.
#[datum]
pub count: Cumulative<i64>,
}

/// An Oximeter `Metric` that specifies the number of times an instance's guest
/// reported a host-handled kernel panic using the QEMU `pvpanic` device.
#[derive(Debug, Default, Copy, Clone, Metric)]
struct PvPanicHostHandled {
/// The number of times this instance's reported a host-handled kernel panic.
#[datum]
pub count: Cumulative<i64>,
}

impl PvpanicProducer {
pub fn new(id: Uuid, pvpanic: Arc<pvpanic::QemuPvpanic>) -> Self {
PvpanicProducer {
stat_name: InstanceUuid { uuid: id },
host_handled_panics: Default::default(),
guest_handled_panics: Default::default(),
pvpanic,
}
}
}

impl Producer for PvpanicProducer {
fn produce(
&mut self,
) -> Result<Box<dyn Iterator<Item = Sample> + 'static>, MetricsError> {
let pvpanic::PanicCounts { guest_handled, host_handled } =
self.pvpanic.panic_counts();

self.host_handled_panics.datum_mut().set(host_handled as i64);
self.guest_handled_panics.datum_mut().set(guest_handled as i64);

let data = vec![
Sample::new(&self.stat_name, &self.guest_handled_panics)?,
Sample::new(&self.stat_name, &self.host_handled_panics)?,
];

Ok(Box::new(data.into_iter()))
}
}
1 change: 1 addition & 0 deletions bin/propolis-server/src/lib/vm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,7 @@ impl VmController {
let ps2ctrl_id = init.initialize_ps2(&chipset)?;
let ps2ctrl: Option<Arc<PS2Ctrl>> = inv.get_concrete(ps2ctrl_id);
init.initialize_qemu_debug_port()?;
init.initialize_qemu_pvpanic(properties.id)?;
init.initialize_network_devices(&chipset)?;
#[cfg(feature = "falcon")]
init.initialize_softnpu_ports(&chipset)?;
Expand Down
17 changes: 16 additions & 1 deletion bin/propolis-standalone/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,16 @@ use std::time::{SystemTime, UNIX_EPOCH};
use anyhow::Context;
use clap::Parser;
use futures::future::BoxFuture;
use propolis::hw::qemu::pvpanic::QemuPvpanic;
use slog::{o, Drain};
use strum::IntoEnumIterator;
use tokio::runtime;

use propolis::chardev::{BlockingSource, Sink, Source, UDSock};
use propolis::hw::chipset::{i440fx, Chipset};
use propolis::hw::ibmpc;
use propolis::hw::ps2::ctrl::PS2Ctrl;
use propolis::hw::uart::LpcUart;
use propolis::hw::{ibmpc, qemu};
use propolis::intr_pins::FuncPin;
use propolis::usdt::register_probes;
use propolis::vcpu::Vcpu;
Expand Down Expand Up @@ -929,6 +930,20 @@ fn setup_instance(

chipset.pci_attach(bdf, nvme);
}
qemu::pvpanic::DEVICE_NAME => {
let enable_isa = dev
.options
.get("enable_isa")
.and_then(|opt| opt.as_bool())
.unwrap_or(false);
if enable_isa {
let pvpanic = QemuPvpanic::create(
log.new(slog::o!("dev" => "pvpanic")),
);
pvpanic.attach_pio(pio);
inv.register(&pvpanic)?;
}
}
_ => {
slog::error!(log, "unrecognized driver"; "name" => name);
return Err(Error::new(
Expand Down
59 changes: 59 additions & 0 deletions crates/propolis-api-types/src/instance_spec/components/devices.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,46 @@ pub enum MigrationCompatibilityError {
ComponentConfiguration(String),
}

#[derive(
Clone,
Copy,
Deserialize,
Serialize,
Debug,
PartialEq,
Eq,
JsonSchema,
Default,
)]
#[serde(deny_unknown_fields)]
pub struct QemuPvpanic {
/// Enable the QEMU PVPANIC ISA bus device (I/O port 0x505).
pub enable_isa: bool,
// TODO(eliza): add support for the PCI PVPANIC device...
}

impl MigrationElement for Option<QemuPvpanic> {
fn kind(&self) -> &'static str {
"QemuPvpanic"
}

fn can_migrate_from_element(
&self,
other: &Self,
) -> Result<(), crate::instance_spec::migration::ElementCompatibilityError>
{
if self != other {
Err(MigrationCompatibilityError::ComponentConfiguration(format!(
"pvpanic configuration mismatch (self: {0:?}, other: {1:?})",
self, other
))
.into())
} else {
Ok(())
}
}
}

//
// Structs for Falcon devices. These devices don't support live migration.
//
Expand Down Expand Up @@ -385,4 +425,23 @@ mod test {
b2.pci_path = PciPath::new(4, 5, 6).unwrap();
assert!(b1.can_migrate_from_element(&b2).is_err());
}

#[test]
fn incompatible_qemu_pvpanic() {
let d1 = Some(QemuPvpanic { enable_isa: true });
let d2 = Some(QemuPvpanic { enable_isa: false });
assert!(d1.can_migrate_from_element(&d2).is_err());
assert!(d1.can_migrate_from_element(&None).is_err());
}

#[test]
fn compatible_qemu_pvpanic() {
let d1 = Some(QemuPvpanic { enable_isa: true });
let d2 = Some(QemuPvpanic { enable_isa: true });
assert!(d1.can_migrate_from_element(&d2).is_ok());

let d1 = Some(QemuPvpanic { enable_isa: false });
let d2 = Some(QemuPvpanic { enable_isa: false });
assert!(d1.can_migrate_from_element(&d2).is_ok());
}
}
16 changes: 16 additions & 0 deletions crates/propolis-api-types/src/instance_spec/v0/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,22 @@ impl SpecBuilder {
}
}

/// Adds a QEMU pvpanic device.
pub fn add_pvpanic_device(
&mut self,
pvpanic: components::devices::QemuPvpanic,
) -> Result<&Self, SpecBuilderError> {
if self.spec.devices.qemu_pvpanic.is_some() {
return Err(SpecBuilderError::DeviceNameInUse(
"pvpanic".to_string(),
));
}

self.spec.devices.qemu_pvpanic = Some(pvpanic);

Ok(self)
}

#[cfg(feature = "falcon")]
pub fn set_softnpu_pci_port(
&mut self,
Expand Down
23 changes: 23 additions & 0 deletions crates/propolis-api-types/src/instance_spec/v0/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,20 @@ pub struct DeviceSpecV0 {
pub serial_ports: HashMap<SpecKey, components::devices::SerialPort>,
pub pci_pci_bridges: HashMap<SpecKey, components::devices::PciPciBridge>,

// This field has a default value (`None`) to allow for
// backwards-compatibility when upgrading from a Propolis
// version that does not support this device. If the pvpanic device was not
// present in the spec being deserialized, a `None` will be produced,
// rather than rejecting the spec.
#[serde(default)]
// Skip serializing this field if it is `None`. This is so that Propolis
// versions with support for this device are backwards-compatible with
// older versions that don't, as long as the spec doesn't define a pvpanic
// device --- if there is no panic device, skipping the field from the spec
// means that the older version will still accept the spec.
#[serde(skip_serializing_if = "Option::is_none")]
pub qemu_pvpanic: Option<components::devices::QemuPvpanic>,

#[cfg(feature = "falcon")]
pub softnpu_pci_port: Option<components::devices::SoftNpuPciPort>,
#[cfg(feature = "falcon")]
Expand Down Expand Up @@ -169,6 +183,15 @@ impl DeviceSpecV0 {
)
})?;

self.qemu_pvpanic
.can_migrate_from_element(&other.qemu_pvpanic)
.map_err(|e| {
MigrationCompatibilityError::ElementMismatch(
"QEMU PVPANIC device".to_string(),
e,
)
})?;

Ok(())
}
}
Expand Down
1 change: 1 addition & 0 deletions lib/propolis/src/hw/qemu/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@

pub mod debug;
pub mod fwcfg;
pub mod pvpanic;
pub mod ramfb;
Loading

0 comments on commit 7828d9c

Please sign in to comment.