Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions bin/propolis-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use clap::{Args, Parser, Subcommand};
use futures::{future, SinkExt};
use newtype_uuid::{GenericUuid, TypedUuid, TypedUuidKind, TypedUuidTag};
use propolis_client::instance_spec::{
BlobStorageBackend, Board, Chipset, ComponentV0, CrucibleStorageBackend,
BlobStorageBackend, Board, Chipset, Component, CrucibleStorageBackend,
GuestHypervisorInterface, HyperVFeatureFlag, I440Fx, InstanceMetadata,
InstanceProperties, InstanceSpec, InstanceSpecGetResponse, NvmeDisk,
PciPath, QemuPvpanic, ReplacementComponent, SerialPort, SerialPortNumber,
Expand Down Expand Up @@ -207,7 +207,7 @@ struct VmConfig {
fn add_component_to_spec(
spec: &mut InstanceSpec,
id: SpecKey,
component: ComponentV0,
component: Component,
) -> anyhow::Result<()> {
use std::collections::btree_map::Entry;
match spec.components.entry(id) {
Expand Down Expand Up @@ -236,7 +236,7 @@ struct DiskRequest {
#[derive(Clone, Debug)]
struct ParsedDiskRequest {
device_id: SpecKey,
device_spec: ComponentV0,
device_spec: Component,
backend_id: SpecKey,
backend_spec: CrucibleStorageBackend,
}
Expand All @@ -255,11 +255,11 @@ impl DiskRequest {
format!("processing disk request {:?}", self.name)
})?;
let device_spec = match self.device.as_ref() {
"virtio" => ComponentV0::VirtioDisk(VirtioDisk {
"virtio" => Component::VirtioDisk(VirtioDisk {
backend_id: backend_id.clone(),
pci_path,
}),
"nvme" => ComponentV0::NvmeDisk(NvmeDisk {
"nvme" => Component::NvmeDisk(NvmeDisk {
backend_id: backend_id.clone(),
pci_path,
serial_number: nvme_serial_from_str(&self.name, b' '),
Expand Down Expand Up @@ -373,7 +373,7 @@ impl VmConfig {
add_component_to_spec(
&mut spec,
backend_id,
ComponentV0::CrucibleStorageBackend(backend_spec),
Component::CrucibleStorageBackend(backend_spec),
)?;
}

Expand All @@ -389,7 +389,7 @@ impl VmConfig {
add_component_to_spec(
&mut spec,
SpecKey::Name(CLOUD_INIT_NAME.to_owned()),
ComponentV0::VirtioDisk(VirtioDisk {
Component::VirtioDisk(VirtioDisk {
backend_id: SpecKey::Name(
CLOUD_INIT_BACKEND_NAME.to_owned(),
),
Expand All @@ -400,7 +400,7 @@ impl VmConfig {
add_component_to_spec(
&mut spec,
SpecKey::Name(CLOUD_INIT_BACKEND_NAME.to_owned()),
ComponentV0::BlobStorageBackend(BlobStorageBackend {
Component::BlobStorageBackend(BlobStorageBackend {
base64: bytes,
readonly: true,
}),
Expand All @@ -415,20 +415,20 @@ impl VmConfig {
add_component_to_spec(
&mut spec,
SpecKey::Name(name.to_owned()),
ComponentV0::SerialPort(SerialPort { num: port }),
Component::SerialPort(SerialPort { num: port }),
)?;
}

// If there are no SoftNPU devices, also enable COM4.
if !spec
.components
.iter()
.any(|(_, c)| matches!(c, ComponentV0::SoftNpuPort(_)))
.any(|(_, c)| matches!(c, Component::SoftNpuPort(_)))
{
add_component_to_spec(
&mut spec,
SpecKey::Name("com4".to_owned()),
ComponentV0::SerialPort(SerialPort {
Component::SerialPort(SerialPort {
num: SerialPortNumber::Com4,
}),
)?;
Expand All @@ -437,7 +437,7 @@ impl VmConfig {
add_component_to_spec(
&mut spec,
SpecKey::Name("pvpanic".to_owned()),
ComponentV0::QemuPvpanic(QemuPvpanic { enable_isa: true }),
Component::QemuPvpanic(QemuPvpanic { enable_isa: true }),
)?;

Ok(spec)
Expand Down
45 changes: 45 additions & 0 deletions bin/propolis-server/src/lib/initializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use std::convert::TryInto;
use std::fs::File;
use std::net::{IpAddr, Ipv4Addr, SocketAddr};
use std::num::{NonZeroU8, NonZeroUsize};
use std::os::unix::fs::FileTypeExt;
use std::sync::Arc;
Expand Down Expand Up @@ -46,6 +47,7 @@ use propolis::hw::uart::LpcUart;
use propolis::hw::{nvme, virtio};
use propolis::intr_pins;
use propolis::vmm::{self, Builder, Machine};
use propolis::vsock::GuestCid;
use propolis_api_types::instance::InstanceProperties;
use propolis_api_types::instance_spec::components::devices::SerialPortNumber;
use propolis_api_types::instance_spec::{self, SpecKey};
Expand Down Expand Up @@ -476,6 +478,49 @@ impl MachineInitializer<'_> {
Ok(())
}

pub fn initialize_vsock(
&mut self,
chipset: &RegisteredChipset,
) -> Result<(), MachineInitError> {
use propolis::vsock::proxy::VsockPortMapping;

// OANA Port 605 - VM Attestation RFD 605
const ATTESTATION_PORT: u16 = 605;
const ATTESTATION_ADDR: SocketAddr = SocketAddr::new(
IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)),
ATTESTATION_PORT,
);

if let Some(vsock) = &self.spec.vsock {
let bdf: pci::Bdf = vsock.spec.pci_path.into();

let mappings = vec![VsockPortMapping::new(
ATTESTATION_PORT.into(),
ATTESTATION_ADDR,
)];

let guest_cid = GuestCid::try_from(vsock.spec.guest_cid)
.context("guest cid")?;
// While the spec does not recommend how large the virtio descriptor
// table should be we sized this appropriately in testing so
// that the guest is able to move vsock packets at a reasonable
// throughput without the need to be much larger.
let num_queues = 256;

let device = virtio::PciVirtioSock::new(
num_queues,
guest_cid,
self.log.new(slog::o!("dev" => "virtio-socket")),
mappings,
);

self.devices.insert(vsock.id.clone(), device.clone());
chipset.pci_attach(bdf, device);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wanted to double-check my understanding of how the multi-function device bit would get set and currently it's only if there are multiple functions attached on the same bus/device (managed over in Slot::attach). that makes sense, but it also means this is a non-multi-function device from the guest's POV until we attach another function later on.

my read of the spec (and seems to agree with at least linux and illumos) is that it'd be legal to report this device as multi-function that happens to only implement a single function right now. but that it's also pretty low-risk for this device to suddenly become multi-function when there's another function here. so i think being not multi-function today while knowing it'll become multi-function in the future is ok?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it from chat, this is okay to leave as is for merging?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, whatever we do with the multifunction-ness of this device we can (should!) do as a separate change.

}

Ok(())
}

async fn create_storage_backend_from_spec(
&mut self,
backend_spec: &StorageBackend,
Expand Down
Loading
Loading