From 4ee6faea9e5c3aea9c4c219471473d7bed5b7b1a Mon Sep 17 00:00:00 2001 From: Patrick Mooney Date: Tue, 5 Oct 2021 22:27:05 -0500 Subject: [PATCH 1/2] Rework PCI emulation to better expose inner data The previous structure for PCI device emulation made it inconvenient to access the inner data of devices for initialization during setup. This attempts to flatten some of the data model at the cost of more monoporphization. --- propolis/src/block/file.rs | 7 +- propolis/src/hw/chipset/i440fx.rs | 422 +++++++-------- propolis/src/hw/chipset/mod.rs | 2 - propolis/src/hw/nvme/mod.rs | 36 +- propolis/src/hw/pci/bar.rs | 260 ++++++++++ propolis/src/hw/pci/bits.rs | 9 + propolis/src/hw/pci/bus.rs | 180 +++++++ propolis/src/hw/pci/device.rs | 816 ++++++++---------------------- propolis/src/hw/pci/mod.rs | 228 ++++----- propolis/src/hw/ps2ctrl.rs | 50 +- propolis/src/hw/qemu/debug.rs | 20 +- propolis/src/hw/qemu/fwcfg.rs | 178 ++++--- propolis/src/hw/uart/lpc.rs | 86 ++-- propolis/src/hw/virtio/block.rs | 5 +- propolis/src/hw/virtio/mod.rs | 2 - propolis/src/hw/virtio/pci.rs | 213 ++++---- propolis/src/hw/virtio/viona.rs | 2 +- propolis/src/instance.rs | 6 +- propolis/src/inventory.rs | 5 + propolis/src/mmio.rs | 37 +- propolis/src/pio.rs | 38 +- propolis/src/vmm/machine.rs | 14 +- server/src/lib/initializer.rs | 8 +- server/src/lib/server.rs | 11 +- standalone/src/config.rs | 2 +- standalone/src/main.rs | 15 +- 26 files changed, 1352 insertions(+), 1300 deletions(-) create mode 100644 propolis/src/hw/pci/bar.rs create mode 100644 propolis/src/hw/pci/bus.rs diff --git a/propolis/src/block/file.rs b/propolis/src/block/file.rs index d5aaf1049..8ee01da45 100644 --- a/propolis/src/block/file.rs +++ b/propolis/src/block/file.rs @@ -105,6 +105,7 @@ impl Driver { }) } fn blocking_loop(&self, sctx: &mut SyncCtx) { + let mut idled = false; loop { if sctx.check_yield() { break; @@ -113,6 +114,7 @@ impl Driver { let mut guard = self.queue.lock().unwrap(); if let Some(req) = guard.pop_front() { drop(guard); + idled = false; let ctx = sctx.dispctx(); match process_request(&self.fp, &req, &ctx) { Ok(_) => req.complete(block::Result::Success, &ctx), @@ -120,7 +122,10 @@ impl Driver { } } else { // wait until more requests are available - self.idle_threads.add_permits(1); + if !idled { + self.idle_threads.add_permits(1); + idled = true; + } let _guard = self .cv .wait_while(guard, |g| { diff --git a/propolis/src/hw/chipset/i440fx.rs b/propolis/src/hw/chipset/i440fx.rs index 8a53834b1..60f4aa317 100644 --- a/propolis/src/hw/chipset/i440fx.rs +++ b/propolis/src/hw/chipset/i440fx.rs @@ -1,17 +1,17 @@ use std::sync::atomic::{AtomicU8, Ordering}; -use std::sync::{Arc, Mutex, Weak}; +use std::sync::{Arc, Mutex}; use super::Chipset; use crate::common::*; use crate::dispatch::DispCtx; use crate::hw::ibmpc; -use crate::hw::pci::{self, Bdf, INTxPinID, PioCfgDecoder}; +use crate::hw::pci::{self, Bdf, BusNum, INTxPinID, PioCfgDecoder}; use crate::instance; use crate::intr_pins::{IntrPin, LegacyPIC, LegacyPin}; -use crate::pio::{PioBus, PioDev}; +use crate::inventory; +use crate::pio::{PioBus, PioFn}; use crate::util::regmap::RegMap; -use crate::util::self_arc::*; -use crate::vmm::{MachineCtx, VmmHdl}; +use crate::vmm::{Machine, VmmHdl}; use lazy_static::lazy_static; @@ -23,119 +23,96 @@ const PM_DEV: u8 = 1; const PM_FUNC: u8 = 3; pub struct I440Fx { - pic: Arc, - pci_bus: Mutex, + pci_bus: pci::Bus, pci_cfg: PioCfgDecoder, + irq_config: Arc, - lnk_pins: [Arc; 4], - - #[allow(unused)] - // XXX: wire up SCI notifications - sci_pin: Arc, - - sa_cell: SelfArcCell, + dev_hb: Arc, + dev_lpc: Arc, + dev_pm: Arc, } impl I440Fx { - pub fn create(hdl: Arc) -> Arc { - let pic = LegacyPIC::new(hdl); - - let sci_pin = Arc::new(LNKPin::new()); - sci_pin.reassign(pic.pin_handle(SCI_IRQ)); + pub fn create(machine: &Machine) -> Arc { + let hdl = machine.hdl.clone(); + let irq_config = IrqConfig::create(hdl); - let mut this = Arc::new(Self { - pic, - pci_bus: Mutex::new(pci::Bus::new()), + let this = Arc::new(Self { + pci_bus: pci::Bus::new( + BusNum::new(0).unwrap(), + &machine.bus_pio, + &machine.bus_mmio, + ), pci_cfg: PioCfgDecoder::new(), + irq_config: irq_config.clone(), - lnk_pins: [ - Arc::new(LNKPin::new()), - Arc::new(LNKPin::new()), - Arc::new(LNKPin::new()), - Arc::new(LNKPin::new()), - ], - sci_pin, - - sa_cell: SelfArcCell::new(), + dev_hb: Piix4HostBridge::create(), + dev_lpc: Piix3Lpc::create(irq_config), + dev_pm: Piix3PM::create(), }); - SelfArc::self_arc_init(&mut this); - - let hbdev = Piix4HostBridge::create(); - let lpcdev = Piix3Lpc::create(Arc::downgrade(&this)); - let pmdev = Piix3PM::create(); - this.pci_attach(Bdf::new(0, HB_DEV, HB_FUNC), hbdev); - this.pci_attach(Bdf::new(0, LPC_DEV, LPC_FUNC), lpcdev); - this.pci_attach(Bdf::new(0, PM_DEV, PM_FUNC), pmdev); + this.pci_attach( + Bdf::new(0, HB_DEV, HB_FUNC).unwrap(), + this.dev_hb.clone(), + ); + this.pci_attach( + Bdf::new(0, LPC_DEV, LPC_FUNC).unwrap(), + this.dev_lpc.clone(), + ); + this.pci_attach( + Bdf::new(0, PM_DEV, PM_FUNC).unwrap(), + this.dev_pm.clone(), + ); + + // Attach chipset devices + let pio = &machine.bus_pio; + let hdl = &machine.hdl; + this.dev_lpc.attach(pio); + this.dev_pm.attach(pio, hdl); + + let pio_dev = Arc::clone(&this); + let piofn = Arc::new(move |port: u16, rwo: RWOp, ctx: &DispCtx| { + pio_dev.pio_rw(port, rwo, ctx) + }) as Arc; + pio.register( + pci::bits::PORT_PCI_CONFIG_ADDR, + pci::bits::LEN_PCI_CONFIG_ADDR, + Arc::clone(&piofn), + ) + .unwrap(); + pio.register( + pci::bits::PORT_PCI_CONFIG_DATA, + pci::bits::LEN_PCI_CONFIG_DATA, + piofn, + ) + .unwrap(); this } - pub fn attach(&self, mctx: &MachineCtx) { - let bus = self.pci_bus.lock().unwrap(); - - // XXX: hardcoded attachments for now - let lpc = bus.device_at(1, 0).unwrap(); - lpc.as_devinst().unwrap().inner_dev::().attach(mctx.pio()); - - let pm = bus.device_at(1, 3).unwrap(); - pm.as_devinst().unwrap().inner_dev::().attach(mctx); - } - - fn set_lnk_route(&self, idx: usize, irq: Option) { - assert!(idx <= 3); - self.lnk_pins[idx].reassign(irq.and_then(|i| self.pic.pin_handle(i))); - } - fn route_lintr(&self, bdf: &Bdf) -> (INTxPinID, Arc) { - let intx_pin = match (bdf.func() + 1) % 4 { - 1 => INTxPinID::IntA, - 2 => INTxPinID::IntB, - 3 => INTxPinID::IntC, - 4 => INTxPinID::IntD, + let intx_pin = match (bdf.func.get() + 1) % 4 { + 0 => INTxPinID::IntA, + 1 => INTxPinID::IntB, + 2 => INTxPinID::IntC, + 3 => INTxPinID::IntD, _ => panic!(), }; // D->A->B->C starting at 0:0.0 - let pin_route = (bdf.dev() + intx_pin as u8 + 2) % 4; - ( - intx_pin, - Arc::clone(&self.lnk_pins[pin_route as usize]) as Arc, - ) + let pin_route = (bdf.dev.get() + intx_pin as u8 + 2) % 4; + (intx_pin, self.irq_config.intr_pin(pin_route as usize)) } -} -impl Chipset for I440Fx { - fn pci_attach(&self, bdf: Bdf, dev: Arc) { - assert!(bdf.bus() == 0); - dev.attach(&|| self.route_lintr(&bdf)); - let mut bus = self.pci_bus.lock().unwrap(); - bus.attach(bdf.dev(), bdf.func(), dev); - } - fn pci_finalize(&self, mctx: &MachineCtx) { - let cfg_pio = self.self_weak() as Weak; - let pio = mctx.pio(); - let cfg_pio2 = Weak::clone(&cfg_pio); - pio.register(pci::PORT_PCI_CONFIG_ADDR, 4, cfg_pio, 0).unwrap(); - pio.register(pci::PORT_PCI_CONFIG_DATA, 4, cfg_pio2, 0).unwrap(); - } - fn irq_pin(&self, irq: u8) -> Option { - self.pic.pin_handle(irq) - } -} -impl PioDev for I440Fx { - fn pio_rw(&self, port: u16, _ident: usize, rwo: RWOp, ctx: &DispCtx) { + fn pio_rw(&self, port: u16, rwo: RWOp, ctx: &DispCtx) { match port { - pci::PORT_PCI_CONFIG_ADDR => { + pci::bits::PORT_PCI_CONFIG_ADDR => { self.pci_cfg.service_addr(rwo); } - pci::PORT_PCI_CONFIG_DATA => { + pci::bits::PORT_PCI_CONFIG_DATA => { self.pci_cfg.service_data(rwo, |bdf, rwo| { - if bdf.bus() != 0 { + if bdf.bus.get() != 0 { return None; } - let bus = self.pci_bus.lock().unwrap(); - if let Some(dev) = bus.device_at(bdf.dev(), bdf.func()) { - let dev = Arc::clone(dev); - drop(bus); + if let Some(dev) = self.pci_bus.device_at(*bdf) { dev.cfg_rw(rwo, ctx); // let opname = match rwo { // RWOp::Read(_) => "cfgread", @@ -162,22 +139,26 @@ impl PioDev for I440Fx { } } } -impl SelfArc for I440Fx { - fn self_arc_cell(&self) -> &SelfArcCell { - &self.sa_cell +impl Chipset for I440Fx { + fn pci_attach(&self, bdf: Bdf, dev: Arc) { + assert!(bdf.bus.get() == 0); + + self.pci_bus.attach(bdf, dev, Some(self.route_lintr(&bdf))); + } + fn irq_pin(&self, irq: u8) -> Option { + self.irq_config.pic.pin_handle(irq) } } impl Entity for I440Fx { - fn state_transition( - &self, - next: instance::State, - target: Option, - ctx: &DispCtx, - ) { - // TODO: make it easier to do this automatically - let bus = self.pci_bus.lock().unwrap(); - let pm = bus.device_at(PM_DEV, PM_FUNC).unwrap().as_devinst().unwrap(); - pm.state_transition(next, target, ctx); + fn child_register(&self) -> Option> { + Some(vec![ + inventory::ChildRegister::new( + &self.dev_hb, + "hostbridge".to_string(), + ), + inventory::ChildRegister::new(&self.dev_lpc, "lpc".to_string()), + inventory::ChildRegister::new(&self.dev_pm, "pm".to_string()), + ]) } } @@ -235,6 +216,41 @@ impl IntrPin for LNKPin { } } +struct IrqConfig { + pic: Arc, + + lnk_pins: [Arc; 4], + + #[allow(unused)] + // XXX: wire up SCI notifications + sci_pin: Arc, +} +impl IrqConfig { + fn create(hdl: Arc) -> Arc { + let pic = LegacyPIC::new(hdl); + let sci_pin = Arc::new(LNKPin::new()); + sci_pin.reassign(pic.pin_handle(SCI_IRQ)); + Arc::new(Self { + pic, + lnk_pins: [ + Arc::new(LNKPin::new()), + Arc::new(LNKPin::new()), + Arc::new(LNKPin::new()), + Arc::new(LNKPin::new()), + ], + sci_pin, + }) + } + fn set_lnk_route(&self, idx: usize, irq: Option) { + assert!(idx <= 3); + self.lnk_pins[idx].reassign(irq.and_then(|i| self.pic.pin_handle(i))); + } + fn intr_pin(&self, idx: usize) -> Arc { + assert!(idx <= 3); + Arc::clone(&self.lnk_pins[idx]) as Arc + } +} + const PIR_OFFSET: usize = 0x60; const PIR_LEN: usize = 4; const PIR_END: usize = PIR_OFFSET + PIR_LEN; @@ -249,35 +265,41 @@ fn valid_pir_irq(irq: u8) -> bool { matches!(irq, 3..=7 | 9..=12 | 14 | 15) } -struct Piix4HostBridge {} +struct Piix4HostBridge { + pci_state: pci::DeviceState, +} impl Piix4HostBridge { - pub fn create() -> Arc { - pci::Builder::new(pci::Ident { + pub fn create() -> Arc { + let pci_state = pci::Builder::new(pci::Ident { vendor_id: 0x8086, device_id: 0x1237, class: 0x06, ..Default::default() }) - .finish(Arc::new(Self {})) + .finish(); + Arc::new(Self { pci_state }) + } +} +impl pci::Device for Piix4HostBridge { + fn device_state(&self) -> &pci::DeviceState { + &self.pci_state + } +} +impl Entity for Piix4HostBridge { + fn reset(&self, _ctx: &DispCtx) { + self.pci_state.reset(self); } } -impl pci::Device for Piix4HostBridge {} -impl Entity for Piix4HostBridge {} pub struct Piix3Lpc { + pci_state: pci::DeviceState, reg_pir: Mutex<[u8; PIR_LEN]>, post_code: AtomicU8, - chipset: Weak, + irq_config: Arc, } impl Piix3Lpc { - pub fn create(chipset: Weak) -> Arc { - let this = Arc::new(Self { - reg_pir: Mutex::new([0u8; PIR_LEN]), - post_code: AtomicU8::new(0), - chipset, - }); - - pci::Builder::new(pci::Ident { + fn create(irq_config: Arc) -> Arc { + let pci_state = pci::Builder::new(pci::Ident { vendor_id: 0x8086, device_id: 0x7000, class: 0x06, @@ -285,24 +307,55 @@ impl Piix3Lpc { ..Default::default() }) .add_custom_cfg(PIR_OFFSET as u8, PIR_LEN as u8) - .finish(this) + .finish(); + + Arc::new(Self { + pci_state, + reg_pir: Mutex::new([0u8; PIR_LEN]), + post_code: AtomicU8::new(0), + irq_config, + }) } fn attach(self: &Arc, pio: &PioBus) { + let this = Arc::clone(self); + let piofn = Arc::new(move |port: u16, rwo: RWOp, ctx: &DispCtx| { + this.pio_rw(port, rwo, ctx) + }) as Arc; pio.register( ibmpc::PORT_FAST_A20, ibmpc::LEN_FAST_A20, - Arc::downgrade(self) as Weak, - 0, - ) - .unwrap(); - pio.register( - ibmpc::PORT_POST_CODE, - ibmpc::LEN_POST_CODE, - Arc::downgrade(self) as Weak, - 0, + Arc::clone(&piofn), ) .unwrap(); + pio.register(ibmpc::PORT_POST_CODE, ibmpc::LEN_POST_CODE, piofn) + .unwrap(); + } + + fn pio_rw(&self, port: u16, rwo: RWOp, _ctx: &DispCtx) { + match port { + ibmpc::PORT_FAST_A20 => { + match rwo { + RWOp::Read(ro) => { + // A20 is always enabled + ro.write_u8(0x02); + } + RWOp::Write(wo) => { + let _ = wo.read_u8(); + // TODO: handle FAST_INIT request + } + } + } + ibmpc::PORT_POST_CODE => match rwo { + RWOp::Read(ro) => { + ro.write_u8(self.post_code.load(Ordering::SeqCst)); + } + RWOp::Write(wo) => { + self.post_code.store(wo.read_u8(), Ordering::SeqCst); + } + }, + _ => {} + } } fn write_pir(&self, idx: usize, val: u8) { @@ -314,17 +367,20 @@ impl Piix3Lpc { let irq = val & PIR_MASK_IRQ; // XXX better integrate with PCI interrupt routing - let chipset = Weak::upgrade(&self.chipset).unwrap(); if !disabled && valid_pir_irq(irq) { - chipset.set_lnk_route(idx, Some(irq)); + self.irq_config.set_lnk_route(idx, Some(irq)); } else { - chipset.set_lnk_route(idx, None); + self.irq_config.set_lnk_route(idx, None); } regs[idx] = val; } } } impl pci::Device for Piix3Lpc { + fn device_state(&self) -> &pci::DeviceState { + &self.pci_state + } + fn cfg_rw(&self, region: u8, rwo: RWOp) { assert_eq!(region as usize, PIR_OFFSET); assert!(rwo.offset() + rwo.len() <= PIR_END - PIR_OFFSET); @@ -344,32 +400,9 @@ impl pci::Device for Piix3Lpc { } } } -impl Entity for Piix3Lpc {} -impl PioDev for Piix3Lpc { - fn pio_rw(&self, port: u16, _ident: usize, rwo: RWOp, _ctx: &DispCtx) { - match port { - ibmpc::PORT_FAST_A20 => { - match rwo { - RWOp::Read(ro) => { - // A20 is always enabled - ro.write_u8(0x02); - } - RWOp::Write(wo) => { - let _ = wo.read_u8(); - // TODO: handle FAST_INIT request - } - } - } - ibmpc::PORT_POST_CODE => match rwo { - RWOp::Read(ro) => { - ro.write_u8(self.post_code.load(Ordering::SeqCst)); - } - RWOp::Write(wo) => { - self.post_code.store(wo.read_u8(), Ordering::SeqCst); - } - }, - _ => {} - } +impl Entity for Piix3Lpc { + fn reset(&self, _ctx: &DispCtx) { + self.pci_state.reset(self); } } @@ -543,19 +576,12 @@ impl PMRegs { } pub struct Piix3PM { + pci_state: pci::DeviceState, regs: Mutex, - sa_cell: SelfArcCell, } impl Piix3PM { - pub fn create() -> Arc { - let regs = PMRegs::default(); - let mut this = Arc::new(Self { - regs: Mutex::new(regs), - sa_cell: SelfArcCell::new(), - }); - SelfArc::self_arc_init(&mut this); - - pci::Builder::new(pci::Ident { + pub fn create() -> Arc { + let pci_state = pci::Builder::new(pci::Ident { vendor_id: 0x8086, device_id: 0x7113, class: 0x06, @@ -563,20 +589,26 @@ impl Piix3PM { ..Default::default() }) .add_custom_cfg(PMCFG_OFFSET as u8, PMCFG_LEN as u8) - .finish(this) + .finish(); + + Arc::new(Self { pci_state, regs: Mutex::new(PMRegs::default()) }) } - fn attach(self: &Arc, mctx: &MachineCtx) { + fn attach(self: &Arc, pio: &PioBus, hdl: &VmmHdl) { // XXX: static registration for now - mctx.pio() - .register( - PMBASE_DEFAULT, - PMBASE_LEN, - Arc::downgrade(self) as Weak, - 0, - ) - .unwrap(); - mctx.hdl().pmtmr_locate(PMBASE_DEFAULT + PM_TMR_OFFSET).unwrap(); + let this = Arc::clone(&self); + let piofn = Arc::new(move |port: u16, rwo: RWOp, ctx: &DispCtx| { + this.pio_rw(port, rwo, ctx) + }) as Arc; + pio.register(PMBASE_DEFAULT, PMBASE_LEN, piofn).unwrap(); + hdl.pmtmr_locate(PMBASE_DEFAULT + PM_TMR_OFFSET).unwrap(); + } + + fn pio_rw(&self, _port: u16, mut rwo: RWOp, ctx: &DispCtx) { + PM_REGS.process(&mut rwo, |id, rwo| match rwo { + RWOp::Read(ro) => self.pmreg_read(id, ro), + RWOp::Write(wo) => self.pmreg_write(id, wo, ctx), + }); } fn pmcfg_read(&self, id: &PmCfg, ro: &mut ReadOp) { @@ -690,6 +722,9 @@ impl Piix3PM { } } impl pci::Device for Piix3PM { + fn device_state(&self) -> &pci::DeviceState { + &self.pci_state + } fn cfg_rw(&self, region: u8, mut rwo: RWOp) { assert_eq!(region as usize, PMCFG_OFFSET); @@ -700,27 +735,8 @@ impl pci::Device for Piix3PM { } } impl Entity for Piix3PM { - fn state_transition( - &self, - next: instance::State, - _target: Option, - ctx: &DispCtx, - ) { - if matches!(next, instance::State::Reset) { - self.reset(ctx); - } - } -} -impl PioDev for Piix3PM { - fn pio_rw(&self, _port: u16, _ident: usize, mut rwo: RWOp, ctx: &DispCtx) { - PM_REGS.process(&mut rwo, |id, rwo| match rwo { - RWOp::Read(ro) => self.pmreg_read(id, ro), - RWOp::Write(wo) => self.pmreg_write(id, wo, ctx), - }); - } -} -impl SelfArc for Piix3PM { - fn self_arc_cell(&self) -> &SelfArcCell { - &self.sa_cell + fn reset(&self, ctx: &DispCtx) { + self.pci_state.reset(self); + self.reset(ctx); } } diff --git a/propolis/src/hw/chipset/mod.rs b/propolis/src/hw/chipset/mod.rs index 059586b4b..0bb7f4a80 100644 --- a/propolis/src/hw/chipset/mod.rs +++ b/propolis/src/hw/chipset/mod.rs @@ -2,12 +2,10 @@ use std::sync::Arc; use crate::hw::pci::{Bdf, Endpoint}; use crate::intr_pins::LegacyPin; -use crate::vmm::MachineCtx; pub mod i440fx; pub trait Chipset { fn pci_attach(&self, bdf: Bdf, dev: Arc); - fn pci_finalize(&self, mctx: &MachineCtx); fn irq_pin(&self, irq: u8) -> Option; } diff --git a/propolis/src/hw/nvme/mod.rs b/propolis/src/hw/nvme/mod.rs index 2e0ddaecf..b35b0805a 100644 --- a/propolis/src/hw/nvme/mod.rs +++ b/propolis/src/hw/nvme/mod.rs @@ -280,6 +280,9 @@ pub struct PciNvme { /// Underlying Block Device notifier notifier: block::Notifier, + + /// PCI device state + pci_state: pci::DeviceState, } impl PciNvme { @@ -288,7 +291,7 @@ impl PciNvme { vendor: u16, device: u16, binfo: block::DeviceInfo, - ) -> Arc { + ) -> Arc { let builder = pci::Builder::new(pci::Ident { vendor_id: vendor, device_id: device, @@ -393,17 +396,20 @@ impl PciNvme { binfo, }; - let notifier = block::Notifier::new(); - let nvme = PciNvme { state: Mutex::new(state), notifier }; - - builder + let pci_state = builder // XXX: add room for doorbells .add_bar_mmio64(pci::BarN::BAR0, CONTROLLER_REG_SZ as u64) // BAR0/1 are used for the main config and doorbell registers // BAR2 is for the optional index/data registers // Place MSIX in BAR4 for now .add_cap_msix(pci::BarN::BAR4, NVME_MSIX_COUNT) - .finish(Arc::new(nvme)) + .finish(); + + Arc::new(PciNvme { + state: Mutex::new(state), + notifier: block::Notifier::new(), + pci_state, + }) } /// Service a write to the NVMe Controller Configuration from the VM @@ -684,14 +690,16 @@ impl pci::Device for PciNvme { } } - fn attach( - &self, - lintr_pin: Option, - msix_hdl: Option, - ) { - assert!(lintr_pin.is_none()); - assert!(msix_hdl.is_some()); - self.state.lock().unwrap().msix_hdl = msix_hdl; + fn attach(&self) { + // TODO: Update the controller logic to reach out to `pci_state` to get + // access to the MSIX handle, rather than caching it internally + let mut state = self.state.lock().unwrap(); + state.msix_hdl = self.pci_state.msix_hdl(); + assert!(state.msix_hdl.is_some()); + } + + fn device_state(&self) -> &pci::DeviceState { + &self.pci_state } } impl Entity for PciNvme {} diff --git a/propolis/src/hw/pci/bar.rs b/propolis/src/hw/pci/bar.rs new file mode 100644 index 000000000..da58fb8fc --- /dev/null +++ b/propolis/src/hw/pci/bar.rs @@ -0,0 +1,260 @@ +use std::convert::TryFrom; + +use super::bits; +use super::BarN; + +pub const BAR_COUNT: usize = 6; + +#[derive(Eq, PartialEq, Clone, Copy, Debug)] +pub enum BarDefine { + Pio(u16), + Mmio(u32), + Mmio64(u64), +} +impl BarDefine { + /// Definition represent PIO-backed BAR + pub fn is_pio(&self) -> bool { + matches!(self, BarDefine::Pio(_)) + } + /// Definition represent MMIO-backed (32-bit or 64-bit) BAR + pub fn is_mmio(&self) -> bool { + matches!(self, BarDefine::Mmio(_) | BarDefine::Mmio64(_)) + } + /// Get the size of the BAR definition, regardless of type + pub fn size(&self) -> u64 { + match self { + BarDefine::Pio(sz) => *sz as u64, + BarDefine::Mmio(sz) => *sz as u64, + BarDefine::Mmio64(sz) => *sz as u64, + } + } +} +impl TryFrom for BarDefine { + type Error = (); + + fn try_from(value: EntryKind) -> Result { + match value { + EntryKind::Empty | EntryKind::Mmio64High => Err(()), + EntryKind::Pio(sz) => Ok(BarDefine::Pio(sz)), + EntryKind::Mmio(sz) => Ok(BarDefine::Mmio(sz)), + EntryKind::Mmio64(sz) => Ok(BarDefine::Mmio64(sz)), + } + } +} + +#[derive(Copy, Clone)] +enum EntryKind { + Empty, + Pio(u16), + Mmio(u32), + Mmio64(u64), + Mmio64High, +} + +#[derive(Copy, Clone)] +struct Entry { + kind: EntryKind, + value: u64, +} +impl Default for Entry { + fn default() -> Self { + Self { kind: EntryKind::Empty, value: 0 } + } +} + +pub(super) struct Bars { + entries: [Entry; BAR_COUNT], +} + +impl Bars { + pub(super) fn new(defs: &[Option; BAR_COUNT]) -> Self { + let mut this = Self { entries: Default::default() }; + for (idx, def) in defs.iter().enumerate() { + match def { + None => continue, + Some(def) => { + assert!(matches!(this.entries[idx].kind, EntryKind::Empty)); + this.entries[idx].kind = match def { + BarDefine::Pio(sz) => EntryKind::Pio(*sz), + BarDefine::Mmio(sz) => EntryKind::Mmio(*sz), + BarDefine::Mmio64(sz) => { + // Make sure 64-bit BAR definitions are playing by + // the rules + assert!(idx < (BarN::BAR5 as usize)); + this.entries[idx + 1].kind = EntryKind::Mmio64High; + EntryKind::Mmio64(*sz) + } + } + } + } + } + + this + } + pub(super) fn reg_read(&self, bar: BarN) -> u32 { + let idx = bar as usize; + let ent = self.entries[idx]; + match ent.kind { + EntryKind::Empty => 0, + EntryKind::Pio(_) => (ent.value as u16) as u32 | bits::BAR_TYPE_IO, + EntryKind::Mmio(_) => ent.value as u32 | bits::BAR_TYPE_MEM, + EntryKind::Mmio64(_) => ent.value as u32 | bits::BAR_TYPE_MEM64, + EntryKind::Mmio64High => { + assert_ne!(idx, 0); + let ent = self.entries[idx - 1]; + assert!(matches!(ent.kind, EntryKind::Mmio64(_))); + + (ent.value >> 32) as u32 + } + } + } + pub(super) fn reg_write( + &mut self, + bar: BarN, + val: u32, + ) -> Option<(BarDefine, u64, u64)> { + let idx = bar as usize; + let ent = &mut self.entries[idx]; + let (def, old, new) = match ent.kind { + EntryKind::Empty => return None, + EntryKind::Pio(size) => { + let mask = !(size - 1) as u32; + let old = ent.value; + ent.value = (val & mask) as u64; + (BarDefine::Pio(size), old, ent.value) + } + EntryKind::Mmio(size) => { + let mask = !(size - 1); + let old = ent.value; + ent.value = (val & mask) as u64; + (BarDefine::Mmio(size), old, ent.value) + } + EntryKind::Mmio64(size) => { + let old = ent.value; + let mask = !(size - 1) as u32; + let low = val as u32 & mask; + ent.value = (old & (0xffffffff << 32)) | low as u64; + (BarDefine::Mmio64(size), old, ent.value) + } + EntryKind::Mmio64High => { + assert!(idx > 0); + let mut ent = &mut self.entries[idx - 1]; + let size = match ent.kind { + EntryKind::Mmio64(sz) => sz, + _ => panic!(), + }; + let mask = !(size - 1); + let old = ent.value; + let high = (((val as u64) << 32) & mask) & 0xffffffff00000000; + ent.value = high | (old & 0xffffffff); + (BarDefine::Mmio64(size), old, ent.value) + } + }; + if old != new { + return Some((def, old, new)); + } + None + } + + /// Get BAR definition and current value + pub fn get(&self, n: BarN) -> Option<(BarDefine, u64)> { + let ent = &self.entries[n as usize]; + let def = BarDefine::try_from(ent.kind).ok()?; + Some((def, ent.value)) + } + + /// Set BAR value directly + /// + /// May only be called on BARs which are defined (not on empty BARs or the + /// high portions of 64-bit MMIO BARs). Furthermore, the value must be + /// valid for the BAR type (ie. not > u32::MAX for 32-bit MMIO BAR). + pub fn set(&mut self, n: BarN, value: u64) { + let ent = &mut self.entries[n as usize]; + match ent.kind { + EntryKind::Empty => panic!("{:?} not defined", n), + EntryKind::Mmio64High => { + panic!("high BAR bits not to be set directly") + } + EntryKind::Pio(_) => { + assert!(value <= u16::MAX as u64); + ent.value = value; + } + EntryKind::Mmio(_) => { + assert!(value <= u32::MAX as u64); + } + EntryKind::Mmio64(_) => {} + } + ent.value = value; + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::convert::TryFrom; + + fn setup() -> Bars { + let bar_defs = [ + Some(BarDefine::Pio(0x100)), + Some(BarDefine::Mmio(0x20000)), + Some(BarDefine::Mmio64(0x40000)), + None, // high bits + Some(BarDefine::Mmio64(0x200000000)), + None, // high bits + ]; + let bars = Bars::new(&bar_defs); + + bars + } + #[test] + fn init() { + let _ = setup(); + } + + #[test] + fn read_type() { + let mut bars = setup(); + bars.set(BarN::BAR0, 0x1000); + bars.set(BarN::BAR1, 0xc000000); + bars.set(BarN::BAR2, 0xd000000); + bars.set(BarN::BAR4, 0x800000000); + + assert_eq!(bars.reg_read(BarN::BAR0), 0x1001); + assert_eq!(bars.reg_read(BarN::BAR1), 0xc000000); + assert_eq!(bars.reg_read(BarN::BAR2), 0xd000004); + assert_eq!(bars.reg_read(BarN::BAR3), 0); + assert_eq!(bars.reg_read(BarN::BAR4), 0x4); + assert_eq!(bars.reg_read(BarN::BAR5), 0x8); + } + + #[test] + fn write_place() { + let mut bars = setup(); + bars.reg_write(BarN::BAR0, 0x1000); + bars.reg_write(BarN::BAR1, 0xc000000); + bars.reg_write(BarN::BAR2, 0xd000000); + bars.reg_write(BarN::BAR5, 0x8); + bars.reg_write(BarN::BAR4, 0x0); + + assert_eq!(bars.reg_read(BarN::BAR0), 0x1001); + assert_eq!(bars.reg_read(BarN::BAR1), 0xc000000); + assert_eq!(bars.reg_read(BarN::BAR2), 0xd000004); + assert_eq!(bars.reg_read(BarN::BAR3), 0); + assert_eq!(bars.reg_read(BarN::BAR4), 0x4); + assert_eq!(bars.reg_read(BarN::BAR5), 0x8); + } + + #[test] + fn limits() { + let mut bars = setup(); + for i in 0..=5u8 { + bars.reg_write(BarN::try_from(i).unwrap(), 0xffffffff); + } + assert_eq!(bars.reg_read(BarN::BAR0), 0x0000ff01); + assert_eq!(bars.reg_read(BarN::BAR1), 0xfffe0000); + assert_eq!(bars.reg_read(BarN::BAR2), 0xfffc0004); + assert_eq!(bars.reg_read(BarN::BAR3), 0xffffffff); + assert_eq!(bars.reg_read(BarN::BAR4), 0x00000004); + assert_eq!(bars.reg_read(BarN::BAR5), 0xfffffffe); + } +} diff --git a/propolis/src/hw/pci/bits.rs b/propolis/src/hw/pci/bits.rs index b833063ee..a6d3eebeb 100644 --- a/propolis/src/hw/pci/bits.rs +++ b/propolis/src/hw/pci/bits.rs @@ -51,3 +51,12 @@ pub const CLASS_BRIDGE: u8 = 6; pub const SUBCLASS_NVM: u8 = 8; pub const PROGIF_ENTERPRISE_NVME: u8 = 2; + +pub(super) const MASK_FUNC: u8 = 0x07; +pub(super) const MASK_DEV: u8 = 0x1f; +pub(super) const MASK_BUS: u8 = 0xff; + +pub const PORT_PCI_CONFIG_ADDR: u16 = 0xcf8; +pub const LEN_PCI_CONFIG_ADDR: u16 = 4; +pub const PORT_PCI_CONFIG_DATA: u16 = 0xcfc; +pub const LEN_PCI_CONFIG_DATA: u16 = 4; diff --git a/propolis/src/hw/pci/bus.rs b/propolis/src/hw/pci/bus.rs new file mode 100644 index 000000000..0d089fe07 --- /dev/null +++ b/propolis/src/hw/pci/bus.rs @@ -0,0 +1,180 @@ +use std::collections::BTreeMap; +use std::sync::{Arc, Mutex, Weak}; + +use super::bar::BarDefine; +use super::{BarN, Bdf, BusNum, Endpoint, LintrCfg}; +use crate::common::RWOp; +use crate::dispatch::DispCtx; +use crate::mmio::{MmioBus, MmioFn}; +use crate::pio::{PioBus, PioFn}; + +pub struct Bus { + n: BusNum, + inner: Arc>, +} + +impl Bus { + pub fn new(n: BusNum, pio: &Arc, mmio: &Arc) -> Self { + Self { n, inner: Arc::new(Mutex::new(Inner::new(pio, mmio))) } + } + + pub fn attach( + &self, + bdf: Bdf, + dev: Arc, + lintr_cfg: Option, + ) { + assert_eq!(bdf.bus, self.n); + + let mut inner = self.inner.lock().unwrap(); + inner.attach(bdf, dev.clone()); + + let attached = + Attachment { inner: Arc::downgrade(&self.inner), bdf, lintr_cfg }; + dev.attach(attached); + } + + pub fn device_at(&self, bdf: Bdf) -> Option> { + assert_eq!(bdf.bus, self.n); + + let inner = self.inner.lock().unwrap(); + inner.device_at(bdf) + } +} + +pub struct Attachment { + inner: Weak>, + bdf: Bdf, + lintr_cfg: Option, +} +impl Attachment { + pub fn bar_register(&self, n: BarN, def: BarDefine, addr: u64) { + if let Some(inner) = self.inner.upgrade() { + let mut guard = inner.lock().unwrap(); + guard.bar_register(self.bdf, n, def, addr); + } + } + pub fn bar_unregister(&self, n: BarN) { + if let Some(inner) = self.inner.upgrade() { + let mut guard = inner.lock().unwrap(); + guard.bar_unregister(self.bdf, n); + } + } + pub fn lintr_cfg(&self) -> Option<&LintrCfg> { + self.lintr_cfg.as_ref() + } + pub fn bdf(&self) -> Bdf { + self.bdf + } +} + +const SLOTS_PER_BUS: usize = 32; +const FUNCS_PER_SLOT: usize = 8; + +#[derive(Default)] +struct Slot { + funcs: [Option>; FUNCS_PER_SLOT], +} + +struct BarState { + def: BarDefine, + value: u64, + live: bool, +} + +struct Inner { + slots: [Slot; SLOTS_PER_BUS], + bar_state: BTreeMap<(Bdf, BarN), BarState>, + bus_pio: Weak, + bus_mmio: Weak, +} +impl Inner { + fn new(pio: &Arc, mmio: &Arc) -> Self { + Self { + slots: Default::default(), + bar_state: BTreeMap::new(), + bus_pio: Arc::downgrade(pio), + bus_mmio: Arc::downgrade(mmio), + } + } + fn device_at(&self, bdf: Bdf) -> Option> { + let res = self.slots[bdf.dev.get() as usize].funcs + [bdf.func.get() as usize] + .as_ref() + .map(|d| Arc::clone(d)); + res + } + fn attach(&mut self, bdf: Bdf, dev: Arc) { + let _old = self.slots[bdf.dev.get() as usize].funcs + [bdf.func.get() as usize] + .replace(dev); + // XXX be strict for now + assert!(matches!(_old, None)); + } + fn bar_register(&mut self, bdf: Bdf, n: BarN, def: BarDefine, value: u64) { + let dev = self.device_at(bdf).unwrap(); + + let live = match def { + BarDefine::Pio(sz) => { + if let Some(pio) = self.bus_pio.upgrade() { + let func = + Arc::new(move |_port: u16, rwo: RWOp, ctx: &DispCtx| { + dev.bar_rw(n, rwo, ctx) + }) as Arc; + pio.register(value as u16, sz, func).is_ok() + } else { + false + } + } + BarDefine::Mmio(sz) => { + if let Some(mmio) = self.bus_mmio.upgrade() { + let func = Arc::new( + move |_addr: usize, rwo: RWOp, ctx: &DispCtx| { + dev.bar_rw(n, rwo, ctx) + }, + ) as Arc; + mmio.register(value as usize, sz as usize, func).is_ok() + } else { + false + } + } + BarDefine::Mmio64(sz) => { + if let Some(mmio) = self.bus_mmio.upgrade() { + let func = Arc::new( + move |_addr: usize, rwo: RWOp, ctx: &DispCtx| { + dev.bar_rw(n, rwo, ctx) + }, + ) as Arc; + mmio.register(value as usize, sz as usize, func).is_ok() + } else { + false + } + } + }; + let _old = + self.bar_state.insert((bdf, n), BarState { def, value, live }); + // XXX be strict for now + assert!(_old.is_none()); + } + fn bar_unregister(&mut self, bdf: Bdf, n: BarN) { + if let Some(state) = self.bar_state.remove(&(bdf, n)) { + if !state.live { + // when BAR was registered, it conflicted with something else on + // the bus, so no further action is necessary + return; + } + match state.def { + BarDefine::Pio(_) => { + if let Some(pio) = self.bus_pio.upgrade() { + pio.unregister(state.value as u16).unwrap(); + } + } + BarDefine::Mmio(_) | BarDefine::Mmio64(_) => { + if let Some(mmio) = self.bus_mmio.upgrade() { + mmio.unregister(state.value as usize).unwrap(); + } + } + } + } + } +} diff --git a/propolis/src/hw/pci/device.rs b/propolis/src/hw/pci/device.rs index 4607d2c6f..efe813142 100644 --- a/propolis/src/hw/pci/device.rs +++ b/propolis/src/hw/pci/device.rs @@ -1,22 +1,85 @@ -use std::any::Any; -use std::convert::TryFrom; -use std::marker::PhantomData; -use std::sync::{Arc, Condvar, Mutex, MutexGuard, Weak}; +use std::sync::{Arc, Condvar, Mutex, MutexGuard}; +use super::bar::{BarDefine, Bars}; use super::bits::*; -use super::{Endpoint, INTxPinID}; +use super::{bus, BarN, Endpoint}; use crate::common::*; use crate::dispatch::DispCtx; -use crate::instance; use crate::intr_pins::IntrPin; -use crate::inventory::Entity; -use crate::mmio::MmioDev; -use crate::pio::PioDev; use crate::util::regmap::{Flags, RegMap}; -use crate::util::self_arc::*; use lazy_static::lazy_static; -use num_enum::TryFromPrimitive; + +pub trait Device: Send + Sync + 'static { + fn device_state(&self) -> &DeviceState; + + #[allow(unused_variables)] + fn bar_rw(&self, bar: BarN, rwo: RWOp, ctx: &DispCtx) { + match rwo { + RWOp::Read(ro) => { + unimplemented!("BAR read ({:?} @ {:x})", bar, ro.offset()) + } + RWOp::Write(wo) => { + unimplemented!("BAR write ({:?} @ {:x})", bar, wo.offset()) + } + } + } + + fn cfg_rw(&self, region: u8, rwo: RWOp) { + match rwo { + RWOp::Read(ro) => { + unimplemented!("CFG read ({:x} @ {:x})", region, ro.offset()) + } + RWOp::Write(wo) => { + unimplemented!("CFG write ({:x} @ {:x})", region, wo.offset()) + } + } + } + fn attach(&self) {} + #[allow(unused_variables)] + fn interrupt_mode_change(&self, mode: IntrMode) {} + #[allow(unused_variables)] + fn msi_update(&self, info: MsiUpdate, ctx: &DispCtx) {} + // TODO + // fn cap_read(&self); + // fn cap_write(&self); +} + +impl Endpoint for D { + fn attach(&self, attachment: bus::Attachment) { + let ds = self.device_state(); + ds.attach(attachment); + } + fn cfg_rw(&self, mut rwo: RWOp, ctx: &DispCtx) { + let ds = self.device_state(); + ds.cfg_space.process(&mut rwo, |id, mut rwo| match id { + CfgReg::Std => { + STD_CFG_MAP.process(&mut rwo, |id, rwo| match rwo { + RWOp::Read(ro) => ds.cfg_std_read(id, ro, ctx), + RWOp::Write(wo) => ds.cfg_std_write(self, id, wo, ctx), + }); + } + CfgReg::Custom(region) => Device::cfg_rw(self, *region, rwo), + CfgReg::CapId(_) | CfgReg::CapNext(_) | CfgReg::CapBody(_) => { + ds.cfg_cap_rw(self, id, rwo, ctx) + } + }); + } + fn bar_rw(&self, bar: BarN, rwo: RWOp, ctx: &DispCtx) { + let ds = self.device_state(); + if let Some(msix) = ds.msix_cfg.as_ref() { + if msix.bar_match(bar) { + msix.bar_rw( + rwo, + |info| ds.notify_msi_update(self, info, ctx), + ctx, + ); + return; + } + } + Device::bar_rw(self, bar, rwo, ctx); + } +} enum CfgReg { Std, @@ -53,25 +116,6 @@ enum StdCfgReg { MaxLatency, } -#[derive(Copy, Clone, Eq, PartialEq, Debug, TryFromPrimitive)] -#[repr(u8)] -pub enum BarN { - BAR0 = 0, - BAR1, - BAR2, - BAR3, - BAR4, - BAR5, -} - -#[derive(Eq, PartialEq, Clone, Copy, Debug)] -pub enum BarDefine { - Pio(u16), - Mmio(u32), - Mmio64(u64), - Mmio64High, -} - lazy_static! { static ref STD_CFG_MAP: RegMap = { let layout = [ @@ -120,215 +164,29 @@ pub struct Ident { pub sub_device_id: u16, } -#[derive(Default)] struct State { reg_command: RegCmd, reg_intr_line: u8, reg_intr_pin: u8, - lintr_pin: Option>, + attach: Option, + bars: Bars, update_in_progress: bool, } - -#[derive(Default)] -struct BarState { - addr: u64, - registered: bool, -} -struct BarEntry { - define: Option, - state: Mutex, -} -impl BarEntry { - fn new() -> Self { - Self { define: None, state: Mutex::new(Default::default()) } - } -} - -struct Bars { - entries: [BarEntry; 6], -} - -impl Bars { - fn new(defs: &[Option; 6]) -> Self { - let mut this = Self { - entries: [ - BarEntry::new(), - BarEntry::new(), - BarEntry::new(), - BarEntry::new(), - BarEntry::new(), - BarEntry::new(), - ], - }; - for (idx, def) in - defs.iter().enumerate().filter_map(|(n, def)| def.map(|d| (n, d))) - { - // Make sure 64-bit BAR definitions are playing by the rules - if matches!(def, BarDefine::Mmio64(_)) { - assert!(idx < 5); - assert!(matches!(defs[idx + 1], Some(BarDefine::Mmio64High))); - } - if matches!(def, BarDefine::Mmio64High) { - assert_ne!(idx, 0); - assert!(matches!(defs[idx - 1], Some(BarDefine::Mmio64(_)))); - } - this.entries[idx].define = Some(def); - } - - this - } - fn reg_read(&self, bar: BarN) -> u32 { - let idx = bar as usize; - let ent = &self.entries[idx]; - if ent.define.is_none() { - return 0; - } - let state = ent.state.lock().unwrap(); - match ent.define.as_ref().unwrap() { - BarDefine::Pio(_) => state.addr as u32 | BAR_TYPE_IO, - BarDefine::Mmio(_) => state.addr as u32 | BAR_TYPE_MEM, - BarDefine::Mmio64(_) => state.addr as u32 | BAR_TYPE_MEM64, - BarDefine::Mmio64High => { - assert_ne!(idx, 0); - drop(state); - let prev = self.entries[idx - 1].state.lock().unwrap(); - (prev.addr >> 32) as u32 - } - } - } - fn reg_write(&self, bar: BarN, val: u32, register: F) - where - F: Fn(&BarDefine, u64, u64) -> bool, - { - let idx = bar as usize; - if self.entries[idx].define.is_none() { - return; - } - let mut ent = &self.entries[idx]; - let mut state = self.entries[idx].state.lock().unwrap(); - let (old, mut state) = match ent.define.as_ref().unwrap() { - BarDefine::Pio(size) => { - let mask = !(size - 1) as u32; - let old = state.addr; - state.addr = (val & mask) as u64; - (old, state) - } - BarDefine::Mmio(size) => { - let mask = !(size - 1); - let old = state.addr; - state.addr = (val & mask) as u64; - (old, state) - } - BarDefine::Mmio64(size) => { - let old = state.addr; - let mask = !(size - 1) as u32; - let low = val as u32 & mask; - state.addr = (old & (0xffffffff << 32)) | low as u64; - (old, state) - } - BarDefine::Mmio64High => { - assert!(idx > 0); - drop(state); - ent = &self.entries[idx - 1]; - let mut state = ent.state.lock().unwrap(); - let size = match ent.define.as_ref().unwrap() { - BarDefine::Mmio64(sz) => sz, - _ => panic!(), - }; - let mask = !(size - 1); - let old = state.addr; - let high = (((val as u64) << 32) & mask) & 0xffffffff00000000; - state.addr = high | (old & 0xffffffff); - (old, state) - } - }; - if state.registered && old != state.addr { - // attempt to register BAR at new location - state.registered = - register(ent.define.as_ref().unwrap(), old, state.addr); - } - } - fn change_registrations(&self, changef: F) - where - F: Fn(BarN, &BarDefine, u64, bool) -> Option, - { - self.for_each(|barn, def| { - if def == &BarDefine::Mmio64High { - // The high portion of 64-bit BARs does not require direct - // handling, as the low portion bears the necessary information. - return; - } - let mut state = self.entries[barn as usize].state.lock().unwrap(); - if let Some(new_reg_state) = - changef(barn, def, state.addr, state.registered) - { - state.registered = new_reg_state; - } - }); - } - fn for_each(&self, mut f: F) - where - F: FnMut(BarN, &BarDefine), - { - for (n, bar) in - self.entries.iter().enumerate().filter(|(_n, b)| b.define.is_some()) - { - let barn = BarN::try_from(n as u8).unwrap(); - f(barn, bar.define.as_ref().unwrap()); - } - } - fn place(&self, bar: BarN, addr: u64) { - let idx = bar as usize; - assert!(self.entries[idx].define.is_some()); - - let ent = &self.entries[idx].define.as_ref().unwrap(); - let mut state = self.entries[idx].state.lock().unwrap(); - match ent { - BarDefine::Pio(_) => { - assert!(addr <= u16::MAX as u64); - } - BarDefine::Mmio(_) => { - assert!(addr <= u32::MAX as u64); - } - BarDefine::Mmio64(_) => {} - BarDefine::Mmio64High => panic!(), +impl State { + fn new(bars: Bars) -> Self { + Self { + reg_command: RegCmd::empty(), + reg_intr_line: 0xff, + reg_intr_pin: 0, + attach: None, + bars, + update_in_progress: false, } - // initial BAR placement is a necessary step prior to registration - assert!(!state.registered); - state.addr = addr; } - fn reset(&self, changef: F, ctx: &DispCtx) - where - F: Fn(BarN, &BarDefine, u64), - { - self.for_each(|barn, def| { - if def == &BarDefine::Mmio64High { - // The high portion of 64-bit BARs does not require direct - // handling, as the low portion bears the necessary information. - return; - } - let mut state = self.entries[barn as usize].state.lock().unwrap(); - if state.registered { - match def { - BarDefine::Pio(_) => { - ctx.mctx.pio().unregister(state.addr as u16).unwrap(); - } - BarDefine::Mmio(_) | BarDefine::Mmio64(_) => { - ctx.mctx - .mmio() - .unregister(state.addr as usize) - .unwrap(); - } - // Already filtered out - BarDefine::Mmio64High => panic!(), - } - state.registered = false; - changef(barn, def, state.addr); - } - state.addr = 0; - }); + fn attached(&self) -> &bus::Attachment { + self.attach.as_ref().unwrap() } } @@ -337,7 +195,7 @@ struct Cap { offset: u8, } -pub struct DeviceInst { +pub struct DeviceState { ident: Ident, lintr_req: bool, cfg_space: RegMap, @@ -345,48 +203,27 @@ pub struct DeviceInst { caps: Vec, state: Mutex, - bars: Bars, cond: Condvar, - - sa_cell: SelfArcCell, - - inner: Arc, - // Keep a 'dyn Any' copy around for downcasting - inner_any: Arc, } -impl DeviceInst { - fn new( +impl DeviceState { + fn new( ident: Ident, + lintr_req: bool, cfg_space: RegMap, msix_cfg: Option>, caps: Vec, bars: Bars, - inner: Arc, - ) -> Self - where - D: Device + Send + Sync + 'static, - { - let inner_any = - Arc::clone(&inner) as Arc; + ) -> Self { Self { ident, - lintr_req: false, + lintr_req, cfg_space, msix_cfg, caps, - state: Mutex::new(State { - reg_intr_line: 0xff, - ..Default::default() - }), - bars, + state: Mutex::new(State::new(bars)), cond: Condvar::new(), - - sa_cell: SelfArcCell::new(), - - inner: inner as Arc, - inner_any, } } @@ -397,22 +234,24 @@ impl DeviceInst { /// protection provided against other such updates which might race. fn affects_intr_mode( &self, + dev: &dyn Device, mut state: MutexGuard, f: impl FnOnce(&mut State), - ) { + ) -> MutexGuard { state = self.cond.wait_while(state, |s| s.update_in_progress).unwrap(); f(&mut state); let next_mode = self.next_intr_mode(&state); state.update_in_progress = true; drop(state); - // inner is notified of mode change w/o state locked - self.inner.interrupt_mode_change(next_mode); + // device is notified of mode change w/o state locked + dev.interrupt_mode_change(next_mode); let mut state = self.state.lock().unwrap(); assert!(state.update_in_progress); state.update_in_progress = false; self.cond.notify_all(); + state } fn cfg_std_read(&self, id: &StdCfgReg, ro: &mut ReadOp, _ctx: &DispCtx) { @@ -436,7 +275,7 @@ impl DeviceInst { let mut val = RegStatus::empty(); if self.lintr_req { let state = self.state.lock().unwrap(); - if let Some(pin) = state.lintr_pin.as_ref() { + if let Some((_id, pin)) = state.attached().lintr_cfg() { if pin.is_asserted() { val.insert(RegStatus::INTR_STATUS); } @@ -453,7 +292,10 @@ impl DeviceInst { StdCfgReg::IntrPin => { ro.write_u8(self.state.lock().unwrap().reg_intr_pin) } - StdCfgReg::Bar(bar) => ro.write_u32(self.bars.reg_read(*bar)), + StdCfgReg::Bar(bar) => { + let state = self.state.lock().unwrap(); + ro.write_u32(state.bars.reg_read(*bar)) + } StdCfgReg::ExpansionRomAddr => { // no rom for now ro.write_u32(0); @@ -483,57 +325,37 @@ impl DeviceInst { } } } - fn cfg_std_write(&self, id: &StdCfgReg, wo: &mut WriteOp, ctx: &DispCtx) { + fn cfg_std_write( + &self, + dev: &dyn Device, + id: &StdCfgReg, + wo: &mut WriteOp, + _ctx: &DispCtx, + ) { assert!(wo.offset() == 0 || *id == StdCfgReg::Reserved); match id { StdCfgReg::Command => { let new = RegCmd::from_bits_truncate(wo.read_u16()); - self.reg_cmd_write(new, ctx); + self.reg_cmd_write(dev, new); } StdCfgReg::IntrLine => { self.state.lock().unwrap().reg_intr_line = wo.read_u8(); } StdCfgReg::Bar(bar) => { let val = wo.read_u32(); - let state = self.state.lock().unwrap(); - self.bars.reg_write(*bar, val, |def, old, new| { - // fail move for now - match def { - BarDefine::Pio(sz) => { - if !state.reg_command.contains(RegCmd::IO_EN) { - // pio mappings are disabled via cmd reg - return false; - } - let bus = ctx.mctx.pio(); - // We know this was previously registered - let (dev, old_bar) = - bus.unregister(old as u16).unwrap(); - assert_eq!(old_bar, *bar as usize); - bus.register(new as u16, *sz, dev, *bar as usize) - .is_err() - } - BarDefine::Mmio(_) | BarDefine::Mmio64(_) => { - if !state.reg_command.contains(RegCmd::MMIO_EN) { - // mmio mappings are disabled via cmd reg - return false; - } - let sz = match def { - BarDefine::Mmio(s) => *s as usize, - BarDefine::Mmio64(s) => *s as usize, - _ => panic!(), - }; - let bus = ctx.mctx.mmio(); - // We know this was previously registered - let (dev, old_bar) = - bus.unregister(old as usize).unwrap(); - assert_eq!(old_bar, *bar as usize); - bus.register(new as usize, sz, dev, *bar as usize) - .is_err() - } - BarDefine::Mmio64High => panic!(), + let mut state = self.state.lock().unwrap(); + if let Some((def, _old, new)) = state.bars.reg_write(*bar, val) + { + let pio_en = state.reg_command.contains(RegCmd::IO_EN); + let mmio_en = state.reg_command.contains(RegCmd::MMIO_EN); + + let attach = state.attached(); + if (pio_en && def.is_pio()) || (mmio_en && def.is_mmio()) { + attach.bar_unregister(*bar); + attach.bar_register(*bar, def, new); } - }); + } } StdCfgReg::VendorId | StdCfgReg::DeviceId @@ -566,13 +388,40 @@ impl DeviceInst { } } } - fn reg_cmd_write(&self, val: RegCmd, ctx: &DispCtx) { + fn reg_cmd_write(&self, dev: &dyn Device, val: RegCmd) { let mut state = self.state.lock().unwrap(); + let attach = state.attached(); let diff = val ^ state.reg_command; - self.update_bar_registration(diff, val, ctx); + + // Update BAR registrations + if diff.intersects(RegCmd::IO_EN | RegCmd::MMIO_EN) { + for n in BarN::iter() { + let bar = state.bars.get(n); + if bar.is_none() { + continue; + } + let (def, v) = bar.unwrap(); + + if diff.contains(RegCmd::IO_EN) && def.is_pio() { + if val.contains(RegCmd::IO_EN) { + attach.bar_register(n, def, v); + } else { + attach.bar_unregister(n); + } + } + if diff.contains(RegCmd::MMIO_EN) && def.is_mmio() { + if val.contains(RegCmd::MMIO_EN) { + attach.bar_register(n, def, v); + } else { + attach.bar_unregister(n); + } + } + } + } + if diff.intersects(RegCmd::INTX_DIS) { // special handling required for INTx enable/disable - self.affects_intr_mode(state, |state| { + let _state = self.affects_intr_mode(dev, state, |state| { state.reg_command = val; }); } else { @@ -586,98 +435,24 @@ impl DeviceInst { { return IntrMode::Msix; } - if state.lintr_pin.is_some() - && !state.reg_command.contains(RegCmd::INTX_DIS) - { - return IntrMode::INTxPin; + if let Some(attach) = state.attach.as_ref() { + if attach.lintr_cfg().is_some() + && !state.reg_command.contains(RegCmd::INTX_DIS) + { + return IntrMode::INTxPin; + } } IntrMode::Disabled } - fn update_bar_registration( + fn cfg_cap_rw( &self, - diff: RegCmd, - new: RegCmd, + dev: &dyn Device, + id: &CfgReg, + rwo: RWOp, ctx: &DispCtx, ) { - if !diff.intersects(RegCmd::IO_EN | RegCmd::MMIO_EN) { - return; - } - - self.bars.change_registrations( - |bar, def, addr, registered| match def { - BarDefine::Pio(sz) => { - if !diff.intersects(RegCmd::IO_EN) { - return None; - } - - if registered && !new.contains(RegCmd::IO_EN) { - ctx.mctx.pio().unregister(addr as u16).unwrap(); - return Some(false); - } else if !registered && new.contains(RegCmd::IO_EN) { - let reg_attempt = ctx - .mctx - .pio() - .register( - addr as u16, - *sz as u16, - self.self_weak(), - bar as usize, - ) - .is_ok(); - return Some(reg_attempt); - } - None - } - BarDefine::Mmio(_) | BarDefine::Mmio64(_) => { - if !diff.intersects(RegCmd::MMIO_EN) { - return None; - } - - let sz = match def { - BarDefine::Mmio(s) => *s as u64, - BarDefine::Mmio64(s) => *s, - _ => panic!(), - }; - - if registered && !new.contains(RegCmd::IO_EN) { - ctx.mctx.mmio().unregister(addr as usize).unwrap(); - return Some(false); - } else if !registered && new.contains(RegCmd::IO_EN) { - let reg_attempt = ctx - .mctx - .mmio() - .register( - addr as usize, - sz as usize, - self.self_weak(), - bar as usize, - ) - .is_ok(); - return Some(reg_attempt); - } - - None - } - // Registration for the high portion of a 64-bit BAR is not - // handled separately. - BarDefine::Mmio64High => panic!(), - }, - ); - } - fn bar_rw(&self, ident: usize, rwo: RWOp, ctx: &DispCtx) { - let bar = BarN::try_from(ident as u8).unwrap(); - if let Some(msix) = self.msix_cfg.as_ref() { - if msix.bar_match(bar) { - msix.bar_rw(rwo, |info| self.notify_msi_update(info, ctx), ctx); - return; - } - } - self.inner.bar_rw(bar, rwo, ctx); - } - - fn cfg_cap_rw(&self, id: &CfgReg, rwo: RWOp, ctx: &DispCtx) { match id { CfgReg::CapId(i) => { if let RWOp::Read(ro) = rwo { @@ -694,13 +469,13 @@ impl DeviceInst { } } } - CfgReg::CapBody(i) => self.do_cap_rw(*i, rwo, ctx), + CfgReg::CapBody(i) => self.do_cap_rw(dev, *i, rwo, ctx), // Should be filtered down to only cap regs by now _ => panic!(), } } - fn do_cap_rw(&self, idx: u8, rwo: RWOp, ctx: &DispCtx) { + fn do_cap_rw(&self, dev: &dyn Device, idx: u8, rwo: RWOp, ctx: &DispCtx) { assert!(idx < self.caps.len() as u8); // XXX: no fancy capability support for now let cap = &self.caps[idx as usize]; @@ -711,17 +486,17 @@ impl DeviceInst { // MSI-X cap writes may result in a change to the interrupt // mode of the device which requires extra locking concerns. let state = self.state.lock().unwrap(); - self.affects_intr_mode(state, |_state| { + let _state = self.affects_intr_mode(dev, state, |_state| { msix_cfg.cfg_rw( rwo, - |info| self.notify_msi_update(info, ctx), + |info| self.notify_msi_update(dev, info, ctx), ctx, ); }); } else { msix_cfg.cfg_rw( rwo, - |info| self.notify_msi_update(info, ctx), + |info| self.notify_msi_update(dev, info, ctx), ctx, ); } @@ -735,134 +510,53 @@ impl DeviceInst { } } } - fn notify_msi_update(&self, info: MsiUpdate, ctx: &DispCtx) { - self.inner.msi_update(info, ctx); - } - /// Get access to the inner device emulation. - /// - /// This will panic if the provided type does not match. - pub fn inner_dev(&self) -> Arc { - let inner = Arc::clone(&self.inner_any); - Arc::downcast(inner).unwrap() + fn notify_msi_update( + &self, + dev: &dyn Device, + info: MsiUpdate, + ctx: &DispCtx, + ) { + dev.msi_update(info, ctx); } - fn do_reset(&self, ctx: &DispCtx) { + pub fn reset(&self, dev: &dyn Device) { let state = self.state.lock().unwrap(); - self.affects_intr_mode(state, |state| { + + let mut state = self.affects_intr_mode(dev, state, |state| { state.reg_command.reset(); if let Some(msix) = &self.msix_cfg { msix.reset(); } }); - self.bars.reset( - |_bar, _def, _addr| { - // TODO: notify device of unregistered BARs - }, - ctx, - ); - } -} -impl Endpoint for DeviceInst { - fn cfg_rw(&self, mut rwo: RWOp, ctx: &DispCtx) { - self.cfg_space.process(&mut rwo, |id, mut rwo| match id { - CfgReg::Std => { - STD_CFG_MAP.process(&mut rwo, |id, rwo| match rwo { - RWOp::Read(ro) => self.cfg_std_read(id, ro, ctx), - RWOp::Write(wo) => self.cfg_std_write(id, wo, ctx), - }); + // Both IO and MMIO BARs should be disabled at this point + debug_assert!(!state + .reg_command + .intersects(RegCmd::IO_EN | RegCmd::MMIO_EN)); + for n in BarN::iter() { + if let Some(_) = state.bars.get(n) { + state.bars.set(n, 0); + let attach = state.attached(); + attach.bar_unregister(n); + // TODO: notify device of zeroed BARs } - CfgReg::Custom(region) => self.inner.cfg_rw(*region, rwo), - CfgReg::CapId(_) | CfgReg::CapNext(_) | CfgReg::CapBody(_) => { - self.cfg_cap_rw(id, rwo, ctx) - } - }); - } - fn attach(&self, get_lintr: &dyn Fn() -> (INTxPinID, Arc)) { - let mut state = self.state.lock().unwrap(); - if self.lintr_req { - let (intx, isa_pin) = get_lintr(); - state.reg_intr_pin = intx as u8; - state.lintr_pin = Some(isa_pin); } - drop(state); - - let lintr_pin = match self.lintr_req { - true => Some(INTxPin::new(self.self_weak())), - false => None, - }; - let msix_hdl = self.msix_cfg.as_ref().map(|msix| MsixHdl::new(msix)); - self.inner.attach(lintr_pin, msix_hdl); } - - fn bar_for_each(&self, cb: &mut dyn FnMut(BarN, &BarDefine)) { - self.bars.for_each(cb) + fn attach(&self, attachment: bus::Attachment) { + let mut state = self.state.lock().unwrap(); + let _old = state.attach.replace(attachment); + assert!(_old.is_none()); } - fn bar_place(&self, bar: BarN, addr: u64) { - // Expect that IO/MMIO is disabled while we are placing BARs + pub fn lintr_pin(&self) -> Option> { let state = self.state.lock().unwrap(); - assert!(state.reg_command == RegCmd::INTX_DIS); - - self.bars.place(bar, addr); + let attach = state.attach.as_ref()?; + let (_id, pin) = attach.lintr_cfg()?; + Some(Arc::clone(pin)) } - fn as_devinst(&self) -> Option<&DeviceInst> { - Some(self) - } -} -impl PioDev for DeviceInst { - fn pio_rw(&self, _port: u16, ident: usize, rwo: RWOp, ctx: &DispCtx) { - self.bar_rw(ident, rwo, ctx); - } -} -impl MmioDev for DeviceInst { - fn mmio_rw(&self, _addr: usize, ident: usize, rwo: RWOp, ctx: &DispCtx) { - self.bar_rw(ident, rwo, ctx); - } -} - -impl Entity for DeviceInst { - fn state_transition( - &self, - next: instance::State, - target: Option, - ctx: &DispCtx, - ) { - if matches!(next, instance::State::Reset) { - self.do_reset(ctx); - } - self.inner.state_transition(next, target, ctx); - } -} - -impl SelfArc for DeviceInst { - fn self_arc_cell(&self) -> &SelfArcCell { - &self.sa_cell - } -} - -#[derive(Clone)] -pub struct INTxPin { - outer: Weak, -} -impl INTxPin { - fn new(outer: Weak) -> Self { - Self { outer } - } - pub fn assert(&self) { - self.with_pin(|pin| pin.assert()); - } - pub fn deassert(&self) { - self.with_pin(|pin| pin.deassert()); - } - pub fn pulse(&self) { - self.with_pin(|pin| pin.pulse()); - } - fn with_pin(&self, f: impl FnOnce(&dyn IntrPin)) { - if let Some(dev) = Weak::upgrade(&self.outer) { - let state = dev.state.lock().unwrap(); - f(state.lintr_pin.as_ref().unwrap().as_ref()); - } + pub fn msix_hdl(&self) -> Option { + let cfg = self.msix_cfg.as_ref()?; + Some(MsixHdl::new(cfg)) } } @@ -879,44 +573,6 @@ pub enum MsiUpdate { Modify(u16), } -pub trait Device: Send + Sync + 'static + Entity { - #[allow(unused_variables)] - fn bar_rw(&self, bar: BarN, rwo: RWOp, ctx: &DispCtx) { - match rwo { - RWOp::Read(ro) => { - unimplemented!("BAR read ({:?} @ {:x})", bar, ro.offset()) - } - RWOp::Write(wo) => { - unimplemented!("BAR write ({:?} @ {:x})", bar, wo.offset()) - } - } - } - - fn cfg_rw(&self, region: u8, rwo: RWOp) { - match rwo { - RWOp::Read(ro) => { - unimplemented!("CFG read ({:x} @ {:x})", region, ro.offset()) - } - RWOp::Write(wo) => { - unimplemented!("CFG write ({:x} @ {:x})", region, wo.offset()) - } - } - } - fn attach(&self, lintr_pin: Option, msix_hdl: Option) { - // A device model has no reason to request interrupt resources but not - // make use of them. - assert!(lintr_pin.is_none()); - assert!(msix_hdl.is_none()); - } - #[allow(unused_variables)] - fn interrupt_mode_change(&self, mode: IntrMode) {} - #[allow(unused_variables)] - fn msi_update(&self, info: MsiUpdate, ctx: &DispCtx) {} - // TODO - // fn cap_read(&self); - // fn cap_write(&self); -} - #[derive(Debug)] enum MsixBarReg { Addr(u16), @@ -1286,7 +942,7 @@ impl Clone for MsixHdl { } } -pub struct Builder { +pub struct Builder { ident: Ident, lintr_req: bool, msix_cfg: Option>, @@ -1295,11 +951,9 @@ pub struct Builder { cap_next_alloc: usize, caps: Vec, - - _phantom: PhantomData, } -impl Builder { +impl Builder { pub fn new(ident: Ident) -> Self { let mut cfgmap = RegMap::new(LEN_CFG); cfgmap.define_with_flags(0, LEN_CFG_STD, CfgReg::Std, Flags::PASSTHRU); @@ -1313,8 +967,6 @@ impl Builder { caps: Vec::new(), // capabilities can start immediately after std cfg area cap_next_alloc: LEN_CFG_STD, - - _phantom: PhantomData, } } @@ -1368,7 +1020,7 @@ impl Builder { assert!(self.bars[idx + 1].is_none()); self.bars[idx] = Some(BarDefine::Mmio64(size)); - self.bars[idx + 1] = Some(BarDefine::Mmio64High); + // TODO: prevent later BAR definition from occupying high word self } @@ -1429,22 +1081,15 @@ impl Builder { self } - pub fn finish(self, inner: Arc) -> Arc { - let bars = Bars::new(&self.bars); - - let mut inst = DeviceInst::new( + pub fn finish(self) -> DeviceState { + DeviceState::new( self.ident, + self.lintr_req, self.cfgmap, self.msix_cfg, self.caps, - bars, - inner, - ); - inst.lintr_req = self.lintr_req; - - let mut done = Arc::new(inst); - SelfArc::self_arc_init(&mut done); - done + Bars::new(&self.bars), + ) } } @@ -1452,53 +1097,6 @@ impl Builder { mod test { use super::*; - fn bar_setup() -> Bars { - let bar_defs = [ - Some(BarDefine::Pio(0x100)), - Some(BarDefine::Mmio(0x20000)), - Some(BarDefine::Mmio64(0x40000)), - Some(BarDefine::Mmio64High), - Some(BarDefine::Mmio64(0x200000000)), - Some(BarDefine::Mmio64High), - ]; - let bars = Bars::new(&bar_defs); - bars.place(BarN::BAR0, 0x1000); - bars.place(BarN::BAR1, 0xc000000); - bars.place(BarN::BAR2, 0xd000000); - bars.place(BarN::BAR4, 0x800000000); - - bars - } - #[test] - fn bar_init() { - let _ = bar_setup(); - } - - #[test] - fn bar_limits() { - let bars = bar_setup(); - - assert_eq!(bars.reg_read(BarN::BAR0), 0x1001); - assert_eq!(bars.reg_read(BarN::BAR1), 0xc000000); - assert_eq!(bars.reg_read(BarN::BAR2), 0xd000004); - assert_eq!(bars.reg_read(BarN::BAR3), 0); - assert_eq!(bars.reg_read(BarN::BAR4), 0x4); - assert_eq!(bars.reg_read(BarN::BAR5), 0x8); - for i in 0..=5u8 { - bars.reg_write( - BarN::try_from(i).unwrap(), - 0xffffffff, - |_, _, _| false, - ); - } - assert_eq!(bars.reg_read(BarN::BAR0), 0x0000ff01); - assert_eq!(bars.reg_read(BarN::BAR1), 0xfffe0000); - assert_eq!(bars.reg_read(BarN::BAR2), 0xfffc0004); - assert_eq!(bars.reg_read(BarN::BAR3), 0xffffffff); - assert_eq!(bars.reg_read(BarN::BAR4), 0x00000004); - assert_eq!(bars.reg_read(BarN::BAR5), 0xfffffffe); - } - #[test] #[should_panic] fn msix_cfg_zero() { diff --git a/propolis/src/hw/pci/mod.rs b/propolis/src/hw/pci/mod.rs index 069f5b223..c0b120b81 100644 --- a/propolis/src/hw/pci/mod.rs +++ b/propolis/src/hw/pci/mod.rs @@ -1,3 +1,4 @@ +use std::convert::TryFrom; use std::fmt::Result as FmtResult; use std::fmt::{Display, Formatter}; use std::io::{Error, ErrorKind}; @@ -8,26 +9,63 @@ use crate::common::*; use crate::dispatch::DispCtx; use crate::intr_pins::IntrPin; +use num_enum::TryFromPrimitive; + +pub mod bar; pub mod bits; +pub mod bus; mod device; +pub use bus::Bus; pub use device::*; -pub const PORT_PCI_CONFIG_ADDR: u16 = 0xcf8; -pub const PORT_PCI_CONFIG_DATA: u16 = 0xcfc; - -const MASK_FUNC: u8 = 0x07; -const MASK_DEV: u8 = 0x1f; -const MASK_BUS: u8 = 0xff; +#[derive(Copy, Clone, Eq, PartialEq, Debug, Ord, PartialOrd)] +pub struct BusNum(u8); +impl BusNum { + pub const fn new(n: u8) -> Option { + Some(Self(n)) + } + pub const fn get(&self) -> u8 { + self.0 + } +} +#[derive(Copy, Clone, Eq, PartialEq, Debug, Ord, PartialOrd)] +pub struct DevNum(u8); +impl DevNum { + pub const fn new(n: u8) -> Option { + if n <= bits::MASK_DEV { + Some(Self(n)) + } else { + None + } + } + pub const fn get(&self) -> u8 { + self.0 + } +} +#[derive(Copy, Clone, Eq, PartialEq, Debug, Ord, PartialOrd)] +pub struct FuncNum(u8); +impl FuncNum { + pub const fn new(n: u8) -> Option { + if n <= bits::MASK_FUNC { + Some(Self(n)) + } else { + None + } + } + pub const fn get(&self) -> u8 { + self.0 + } +} /// Bus, Device, Function. /// /// Acts as an address for PCI and PCIe device functionality. -#[derive(Copy, Clone, Eq, PartialEq, Debug)] +#[derive(Copy, Clone, Eq, PartialEq, Debug, Ord, PartialOrd)] pub struct Bdf { - inner_bus: u8, - inner_dev: u8, - inner_func: u8, + pub bus: BusNum, + pub dev: DevNum, + pub func: FuncNum, } impl FromStr for Bdf { @@ -55,7 +93,7 @@ impl FromStr for Bdf { )); } - Bdf::try_new(fields[0], fields[1], fields[2]).ok_or_else(|| { + Bdf::new(fields[0], fields[1], fields[2]).ok_or_else(|| { Error::new( ErrorKind::InvalidInput, "Failed to parse as BDF".to_string(), @@ -65,46 +103,54 @@ impl FromStr for Bdf { } impl Bdf { - /// Creates a new Bdf. - /// - /// The bus/device/function values must be within the acceptable range for - /// PCI addressing. If they could be invalid, `Bdf::try_new` should be used - /// instead. - /// - /// # Panics - /// - /// - Panics if `dev` is larger than 0x1F. - /// - Panics if `func` is larger than 0x07. - pub fn new(bus: u8, dev: u8, func: u8) -> Self { - assert!(dev <= MASK_DEV); - assert!(func <= MASK_FUNC); - - Self { inner_bus: bus, inner_dev: dev, inner_func: func } - } - /// Attempts to make a new BDF. /// /// Returns [`Option::None`] if the values would not fit within a BDF. - pub fn try_new(bus: u8, dev: u8, func: u8) -> Option { - if dev <= MASK_DEV && func <= MASK_FUNC { - Some(Self::new(bus, dev, func)) - } else { - None + pub const fn new(bus: u8, dev: u8, func: u8) -> Option { + let bnum = BusNum::new(bus); + let dnum = DevNum::new(dev); + let fnum = FuncNum::new(func); + match (bnum, dnum, fnum) { + (Some(b), Some(d), Some(f)) => { + Some(Self { bus: b, dev: d, func: f }) + } + _ => None, } } - pub fn bus(&self) -> u8 { - self.inner_bus - } - pub fn dev(&self) -> u8 { - self.inner_dev - } - pub fn func(&self) -> u8 { - self.inner_func - } } impl Display for Bdf { fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult { - write!(f, "{}.{}.{}", self.inner_bus, self.inner_dev, self.inner_func) + write!(f, "{}.{}.{}", self.bus.0, self.dev.0, self.func.0) + } +} + +#[derive( + Copy, Clone, Eq, PartialEq, Debug, Ord, PartialOrd, TryFromPrimitive, +)] +#[repr(u8)] +pub enum BarN { + BAR0 = 0, + BAR1, + BAR2, + BAR3, + BAR4, + BAR5, +} +impl BarN { + fn iter() -> BarIter { + BarIter { n: 0 } + } +} +struct BarIter { + n: u8, +} +impl Iterator for BarIter { + type Item = BarN; + + fn next(&mut self) -> Option { + let res = BarN::try_from(self.n).ok()?; + self.n += 1; + Some(res) } } @@ -117,85 +163,12 @@ pub enum INTxPinID { IntD = 4, } +pub type LintrCfg = (INTxPinID, Arc); + pub trait Endpoint: Send + Sync { + fn attach(&self, attachment: bus::Attachment); fn cfg_rw(&self, op: RWOp<'_, '_>, ctx: &DispCtx); - fn attach(&self, get_lintr: &dyn Fn() -> (INTxPinID, Arc)); - fn bar_for_each(&self, cb: &mut dyn FnMut(BarN, &BarDefine)); - fn bar_place(&self, bar: BarN, addr: u64); - fn as_devinst(&self) -> Option<&DeviceInst>; -} - -const SLOTS_PER_BUS: usize = 32; -const FUNCS_PER_SLOT: usize = 8; - -#[derive(Default)] -pub struct Slot { - funcs: [Option>; FUNCS_PER_SLOT], -} - -#[derive(Default)] -pub struct Bus { - slots: [Slot; SLOTS_PER_BUS], -} - -impl Bus { - pub fn new() -> Self { - Self::default() - } - - pub fn attach(&mut self, slot: u8, func: u8, dev: Arc) { - assert!((slot as usize) < SLOTS_PER_BUS); - assert!((func as usize) < FUNCS_PER_SLOT); - - // XXX be strict for now - assert!(self.slots[slot as usize].funcs[func as usize].is_none()); - self.slots[slot as usize].funcs[func as usize] = Some(dev); - } - - pub fn iter(&self) -> Iter { - Iter::new(self) - } - - pub fn device_at(&self, slot: u8, func: u8) -> Option<&Arc> { - assert!((slot as usize) < SLOTS_PER_BUS); - assert!((func as usize) < FUNCS_PER_SLOT); - - self.slots[slot as usize].funcs[func as usize].as_ref() - } -} - -pub struct Iter<'a> { - bus: &'a Bus, - pos: usize, -} -impl<'a> Iter<'a> { - fn new(bus: &'a Bus) -> Self { - Self { bus, pos: 0 } - } - fn slot_func(&self) -> Option<(usize, usize)> { - if self.pos < (SLOTS_PER_BUS * FUNCS_PER_SLOT) as usize { - Some((self.pos / FUNCS_PER_SLOT, self.pos & MASK_FUNC as usize)) - } else { - None - } - } -} -impl<'a> Iterator for Iter<'a> { - type Item = (u8, u8, &'a Arc); - - fn next(&mut self) -> Option { - while let Some((slot, func)) = self.slot_func() { - self.pos += 1; - if self.bus.slots[slot].funcs[func].is_some() { - return Some(( - slot as u8, - func as u8, - self.bus.slots[slot].funcs[func].as_ref().unwrap(), - )); - } - } - None - } + fn bar_rw(&self, bar: BarN, rwo: RWOp, ctx: &DispCtx); } fn cfg_addr_parse(addr: u32) -> Option<(Bdf, u8)> { @@ -203,12 +176,15 @@ fn cfg_addr_parse(addr: u32) -> Option<(Bdf, u8)> { // Enable bit not set None } else { - let offset = addr & 0xff; - let func = (addr >> 8) as u8 & MASK_FUNC; - let device = (addr >> 11) as u8 & MASK_DEV; - let bus = (addr >> 16) as u8 & MASK_BUS; - - Some((Bdf::new(bus, device, func), offset as u8)) + Some(( + Bdf::new( + (addr >> 16) as u8 & bits::MASK_BUS, + (addr >> 11) as u8 & bits::MASK_DEV, + (addr >> 8) as u8 & bits::MASK_FUNC, + ) + .unwrap(), + (addr & 0xff) as u8, + )) } } diff --git a/propolis/src/hw/ps2ctrl.rs b/propolis/src/hw/ps2ctrl.rs index c8ef95b51..1f6618dc7 100644 --- a/propolis/src/hw/ps2ctrl.rs +++ b/propolis/src/hw/ps2ctrl.rs @@ -1,6 +1,6 @@ use std::collections::VecDeque; use std::mem::replace; -use std::sync::{Arc, Mutex, Weak}; +use std::sync::{Arc, Mutex}; use crate::common::*; use crate::dispatch::DispCtx; @@ -8,7 +8,7 @@ use crate::hw::chipset::Chipset; use crate::hw::ibmpc; use crate::instance; use crate::intr_pins::LegacyPin; -use crate::pio::{PioBus, PioDev}; +use crate::pio::{PioBus, PioFn}; bitflags! { #[derive(Default)] @@ -99,16 +99,36 @@ impl PS2Ctrl { Arc::new(Self { state: Mutex::new(PS2State::default()) }) } pub fn attach(self: &Arc, bus: &PioBus, chipset: &dyn Chipset) { - let data_ref = Arc::downgrade(self) as Weak; - let cmd_ref = Weak::clone(&data_ref); - bus.register(ibmpc::PORT_PS2_DATA, 1, data_ref, 0).unwrap(); - bus.register(ibmpc::PORT_PS2_CMD_STATUS, 1, cmd_ref, 0).unwrap(); + let this = Arc::clone(self); + let piofn = Arc::new(move |port: u16, rwo: RWOp, ctx: &DispCtx| { + this.pio_rw(port, rwo, ctx) + }) as Arc; + + bus.register(ibmpc::PORT_PS2_DATA, 1, Arc::clone(&piofn)).unwrap(); + bus.register(ibmpc::PORT_PS2_CMD_STATUS, 1, piofn).unwrap(); let mut state = self.state.lock().unwrap(); state.pri_pin = Some(chipset.irq_pin(ibmpc::IRQ_PS2_PRI).unwrap()); state.aux_pin = Some(chipset.irq_pin(ibmpc::IRQ_PS2_AUX).unwrap()); } + fn pio_rw(&self, port: u16, rwo: RWOp, ctx: &DispCtx) { + assert_eq!(rwo.len(), 1); + match port { + ibmpc::PORT_PS2_DATA => match rwo { + RWOp::Read(ro) => ro.write_u8(self.data_read()), + RWOp::Write(wo) => self.data_write(wo.read_u8()), + }, + ibmpc::PORT_PS2_CMD_STATUS => match rwo { + RWOp::Read(ro) => ro.write_u8(self.status_read()), + RWOp::Write(wo) => self.cmd_write(wo.read_u8(), ctx), + }, + _ => { + panic!("unexpected pio in {:x}", port); + } + } + } + fn data_write(&self, v: u8) { let mut state = self.state.lock().unwrap(); let cmd_prefix = replace(&mut state.cmd_prefix, None); @@ -259,24 +279,6 @@ impl PS2Ctrl { ); } } -impl PioDev for PS2Ctrl { - fn pio_rw(&self, port: u16, _ident: usize, rwo: RWOp, ctx: &DispCtx) { - assert_eq!(rwo.len(), 1); - match port { - ibmpc::PORT_PS2_DATA => match rwo { - RWOp::Read(ro) => ro.write_u8(self.data_read()), - RWOp::Write(wo) => self.data_write(wo.read_u8()), - }, - ibmpc::PORT_PS2_CMD_STATUS => match rwo { - RWOp::Read(ro) => ro.write_u8(self.status_read()), - RWOp::Write(wo) => self.cmd_write(wo.read_u8(), ctx), - }, - _ => { - panic!("unexpected pio in {:x}", port); - } - } - } -} impl Entity for PS2Ctrl {} const PS2K_CMD_SET_LEDS: u8 = 0xed; diff --git a/propolis/src/hw/qemu/debug.rs b/propolis/src/hw/qemu/debug.rs index 64bbf2a20..a7805717d 100644 --- a/propolis/src/hw/qemu/debug.rs +++ b/propolis/src/hw/qemu/debug.rs @@ -1,9 +1,9 @@ -use std::sync::{Arc, Weak}; +use std::sync::Arc; use crate::chardev::{BlockingSource, BlockingSourceConsumer, ConsumerCell}; use crate::common::*; use crate::dispatch::DispCtx; -use crate::pio::{PioBus, PioDev}; +use crate::pio::{PioBus, PioFn}; const QEMU_DEBUG_IOPORT: u16 = 0x0402; const QEMU_DEBUG_IDENT: u8 = 0xe9; @@ -15,19 +15,15 @@ impl QemuDebugPort { pub fn create(pio: &PioBus) -> Arc { let this = Arc::new(Self { consumer: ConsumerCell::new() }); - pio.register( - QEMU_DEBUG_IOPORT, - 1, - Arc::downgrade(&this) as Weak, - 0, - ) - .unwrap(); + let piodev = this.clone(); + let piofn = Arc::new(move |_port: u16, rwo: RWOp, ctx: &DispCtx| { + piodev.pio_rw(rwo, ctx) + }) as Arc; + pio.register(QEMU_DEBUG_IOPORT, 1, piofn).unwrap(); this } -} -impl PioDev for QemuDebugPort { - fn pio_rw(&self, _port: u16, _ident: usize, rwo: RWOp, ctx: &DispCtx) { + fn pio_rw(&self, rwo: RWOp, ctx: &DispCtx) { match rwo { RWOp::Read(ro) => { ro.write_u8(QEMU_DEBUG_IDENT); diff --git a/propolis/src/hw/qemu/fwcfg.rs b/propolis/src/hw/qemu/fwcfg.rs index e46653b47..5d1c39d72 100644 --- a/propolis/src/hw/qemu/fwcfg.rs +++ b/propolis/src/hw/qemu/fwcfg.rs @@ -1,9 +1,9 @@ use std::collections::BTreeMap; -use std::sync::{Arc, Mutex, MutexGuard, Weak}; +use std::sync::{Arc, Mutex, MutexGuard}; use crate::common::*; use crate::dispatch::DispCtx; -use crate::pio::{PioBus, PioDev}; +use crate::pio::{PioBus, PioFn}; use bits::*; use byteorder::{ByteOrder, BE, LE}; @@ -378,14 +378,92 @@ impl FwCfg { (FW_CFG_IOP_DMA_HI, 4), (FW_CFG_IOP_DMA_LO, 4), ]; + let this = self.clone(); + let piofn = Arc::new(move |port: u16, rwo: RWOp, ctx: &DispCtx| { + this.pio_rw(port, rwo, ctx) + }) as Arc; for (port, len) in ports.iter() { - pio.register( - *port, - *len, - Arc::downgrade(self) as Weak, - 0, - ) - .unwrap() + pio.register(*port, *len, piofn.clone()).unwrap() + } + } + + fn pio_rw(&self, port: u16, rwo: RWOp, ctx: &DispCtx) { + let mut state = self.state.lock().unwrap(); + match port { + FW_CFG_IOP_SELECTOR => match rwo { + RWOp::Read(ro) => match ro.len() { + 2 => ro.write_u16(state.selector), + 1 => ro.write_u8(state.selector as u8), + _ => {} + }, + RWOp::Write(wo) => { + match wo.len() { + 2 => state.selector = wo.read_u16(), + 1 => state.selector = wo.read_u8() as u16, + _ => {} + } + state.offset = 0; + } + }, + FW_CFG_IOP_DATA => { + match rwo { + RWOp::Read(mut ro) => { + if ro.len() != 1 { + ro.fill(0); + return; + } + + let res = self.xfer( + state.selector, + RWOp::Read(&mut ReadOp::new_child( + state.offset as usize, + &mut ro, + 0..1, + )), + ctx, + ); + if res.is_err() { + ro.write_u8(0); + } + state.offset = state.offset.saturating_add(1); + } + RWOp::Write(_wo) => { + // XXX: ignore writes to data area + } + } + } + FW_CFG_IOP_DMA_HI => match rwo { + RWOp::Read(ro) => { + if ro.len() != 4 { + ro.fill(0); + } else { + ro.write_u32(state.addr_high.to_be()); + } + } + RWOp::Write(wo) => { + if wo.len() == 4 { + state.addr_high = u32::from_be(wo.read_u32()); + } + } + }, + FW_CFG_IOP_DMA_LO => match rwo { + RWOp::Read(ro) => { + if ro.len() != 4 { + ro.fill(0); + } else { + ro.write_u32(state.addr_low.to_be()); + } + } + RWOp::Write(wo) => { + if wo.len() == 4 { + state.addr_low = u32::from_be(wo.read_u32()); + let _ = self.dma_initiate(state, ctx); + } + } + }, + _ => { + panic!("unexpected port {:x}", port); + } } } @@ -566,88 +644,6 @@ impl FwCfg { } } -impl PioDev for FwCfg { - fn pio_rw(&self, port: u16, _ident: usize, rwo: RWOp, ctx: &DispCtx) { - let mut state = self.state.lock().unwrap(); - match port { - FW_CFG_IOP_SELECTOR => match rwo { - RWOp::Read(ro) => match ro.len() { - 2 => ro.write_u16(state.selector), - 1 => ro.write_u8(state.selector as u8), - _ => {} - }, - RWOp::Write(wo) => { - match wo.len() { - 2 => state.selector = wo.read_u16(), - 1 => state.selector = wo.read_u8() as u16, - _ => {} - } - state.offset = 0; - } - }, - FW_CFG_IOP_DATA => { - match rwo { - RWOp::Read(mut ro) => { - if ro.len() != 1 { - ro.fill(0); - return; - } - - let res = self.xfer( - state.selector, - RWOp::Read(&mut ReadOp::new_child( - state.offset as usize, - &mut ro, - 0..1, - )), - ctx, - ); - if res.is_err() { - ro.write_u8(0); - } - state.offset = state.offset.saturating_add(1); - } - RWOp::Write(_wo) => { - // XXX: ignore writes to data area - } - } - } - FW_CFG_IOP_DMA_HI => match rwo { - RWOp::Read(ro) => { - if ro.len() != 4 { - ro.fill(0); - } else { - ro.write_u32(state.addr_high.to_be()); - } - } - RWOp::Write(wo) => { - if wo.len() == 4 { - state.addr_high = u32::from_be(wo.read_u32()); - } - } - }, - FW_CFG_IOP_DMA_LO => match rwo { - RWOp::Read(ro) => { - if ro.len() != 4 { - ro.fill(0); - } else { - ro.write_u32(state.addr_low.to_be()); - } - } - RWOp::Write(wo) => { - if wo.len() == 4 { - state.addr_low = u32::from_be(wo.read_u32()); - let _ = self.dma_initiate(state, ctx); - } - } - }, - _ => { - panic!("unexpected port {:x}", port); - } - } - } -} - impl Entity for FwCfg {} mod bits { diff --git a/propolis/src/hw/uart/lpc.rs b/propolis/src/hw/uart/lpc.rs index 8f46697c8..ad4eaebe8 100644 --- a/propolis/src/hw/uart/lpc.rs +++ b/propolis/src/hw/uart/lpc.rs @@ -1,4 +1,4 @@ -use std::sync::{Arc, Mutex, Weak}; +use std::sync::{Arc, Mutex}; use super::base::Uart; use crate::chardev::*; @@ -6,7 +6,7 @@ use crate::common::*; use crate::dispatch::DispCtx; use crate::instance; use crate::intr_pins::{IntrPin, LegacyPin}; -use crate::pio::{PioBus, PioDev}; +use crate::pio::{PioBus, PioFn}; pub const REGISTER_LEN: usize = 8; @@ -45,13 +45,45 @@ impl LpcUart { }) } pub fn attach(self: &Arc, bus: &PioBus, port: u16) { - bus.register( - port, - REGISTER_LEN as u16, - Arc::downgrade(self) as Weak, - 0, - ) - .unwrap(); + let this = self.clone(); + let piofn = Arc::new(move |_port: u16, rwo: RWOp, ctx: &DispCtx| { + this.pio_rw(rwo, ctx) + }) as Arc; + bus.register(port, REGISTER_LEN as u16, piofn).unwrap(); + } + fn pio_rw(&self, rwo: RWOp, ctx: &DispCtx) { + assert!(rwo.offset() < REGISTER_LEN); + assert!(rwo.len() != 0); + let mut state = self.state.lock().unwrap(); + let readable_before = state.uart.is_readable(); + let writable_before = state.uart.is_writable(); + + match rwo { + RWOp::Read(ro) => { + ro.write_u8(state.uart.reg_read(ro.offset() as u8)); + } + RWOp::Write(wo) => { + state.uart.reg_write(wo.offset() as u8, wo.read_u8()); + } + } + if state.auto_discard { + while let Some(_val) = state.uart.data_read() {} + } + + state.sync_intr_pin(); + + let read_notify = !readable_before && state.uart.is_readable(); + let write_notify = !writable_before && state.uart.is_writable(); + + // The uart state lock cannot be held while dispatching notifications since those callbacks + // could immediately attempt to read/write the pending data. + drop(state); + if read_notify { + self.notify_readable.notify(self as &dyn Source, ctx); + } + if write_notify { + self.notify_writable.notify(self as &dyn Sink, ctx); + } } fn reset(&self) { let mut state = self.state.lock().unwrap(); @@ -100,42 +132,6 @@ impl Source for LpcUart { } } -impl PioDev for LpcUart { - fn pio_rw(&self, _port: u16, _ident: usize, rwo: RWOp, ctx: &DispCtx) { - assert!(rwo.offset() < REGISTER_LEN); - assert!(rwo.len() != 0); - let mut state = self.state.lock().unwrap(); - let readable_before = state.uart.is_readable(); - let writable_before = state.uart.is_writable(); - - match rwo { - RWOp::Read(ro) => { - ro.write_u8(state.uart.reg_read(ro.offset() as u8)); - } - RWOp::Write(wo) => { - state.uart.reg_write(wo.offset() as u8, wo.read_u8()); - } - } - if state.auto_discard { - while let Some(_val) = state.uart.data_read() {} - } - - state.sync_intr_pin(); - - let read_notify = !readable_before && state.uart.is_readable(); - let write_notify = !writable_before && state.uart.is_writable(); - - // The uart state lock cannot be held while dispatching notifications since those callbacks - // could immediately attempt to read/write the pending data. - drop(state); - if read_notify { - self.notify_readable.notify(self as &dyn Source, ctx); - } - if write_notify { - self.notify_writable.notify(self as &dyn Sink, ctx); - } - } -} impl Entity for LpcUart { fn state_transition( &self, diff --git a/propolis/src/hw/virtio/block.rs b/propolis/src/hw/virtio/block.rs index 650e17686..95e87c2a7 100644 --- a/propolis/src/hw/virtio/block.rs +++ b/propolis/src/hw/virtio/block.rs @@ -23,10 +23,7 @@ pub struct VirtioBlock { notifier: block::Notifier, } impl VirtioBlock { - pub fn create( - queue_size: u16, - info: block::DeviceInfo, - ) -> Arc { + pub fn create(queue_size: u16, info: block::DeviceInfo) -> Arc { // virtio-block only needs two MSI-X entries for its interrupt needs: // - device config changes // - queue 0 notification diff --git a/propolis/src/hw/virtio/mod.rs b/propolis/src/hw/virtio/mod.rs index 3d5f37293..f040cff3f 100644 --- a/propolis/src/hw/virtio/mod.rs +++ b/propolis/src/hw/virtio/mod.rs @@ -21,8 +21,6 @@ trait VirtioDevice: Send + Sync + 'static + Entity { fn queue_notify(&self, vq: &Arc, ctx: &DispCtx); fn queues(&self) -> &VirtQueues; - #[allow(unused_variables)] - fn device_reset(&self, ctx: &DispCtx) {} #[allow(unused_variables)] fn queue_change( &self, diff --git a/propolis/src/hw/virtio/pci.rs b/propolis/src/hw/virtio/pci.rs index d0119eed8..b2c94bb49 100644 --- a/propolis/src/hw/virtio/pci.rs +++ b/propolis/src/hw/virtio/pci.rs @@ -9,8 +9,8 @@ use crate::common::*; use crate::dispatch::DispCtx; use crate::hw::pci; use crate::instance; +use crate::intr_pins::IntrPin; use crate::util::regmap::RegMap; -use crate::util::self_arc::*; use lazy_static::lazy_static; @@ -42,11 +42,8 @@ struct VirtioState { status: Status, queue_sel: u16, nego_feat: u32, - isr_status: u8, intr_mode: IntrMode, intr_mode_updating: bool, - lintr_pin: Option, - msix_hdl: Option, msix_cfg_vec: u16, msix_queue_vec: Vec, } @@ -59,11 +56,8 @@ impl VirtioState { status: Status::RESET, queue_sel: 0, nego_feat: 0, - isr_status: 0, intr_mode: IntrMode::IsrOnly, intr_mode_updating: false, - lintr_pin: None, - msix_hdl: None, msix_cfg_vec: VIRTIO_MSI_NO_VECTOR, msix_queue_vec, } @@ -72,15 +66,13 @@ impl VirtioState { self.status = Status::RESET; self.queue_sel = 0; self.nego_feat = 0; - self.isr_status = 0; - if let Some(pin) = self.lintr_pin.as_ref() { - pin.deassert(); - } self.msix_cfg_vec = VIRTIO_MSI_NO_VECTOR; } } pub struct PciVirtio { + pci_state: pci::DeviceState, + map: RegMap, map_nomsix: RegMap, /// Quick access to register map for MSIX (true) or non-MSIX (false) @@ -88,8 +80,7 @@ pub struct PciVirtio { state: Mutex, state_cv: Condvar, - - sa_cell: SelfArcCell, + isr_state: Arc, dev: Arc, dev_any: Arc, @@ -101,10 +92,28 @@ impl PciVirtio { dev_class: u8, cfg_sz: usize, inner: Arc, - ) -> Arc + ) -> Arc where D: VirtioDevice + Send + Sync + 'static, { + let mut builder = pci::Builder::new(pci::Ident { + vendor_id: VIRTIO_VENDOR, + device_id: dev_id, + sub_vendor_id: VIRTIO_VENDOR, + sub_device_id: dev_id - 0xfff, + class: dev_class, + ..Default::default() + }) + .add_lintr(); + + if let Some(count) = msix_count { + builder = builder.add_cap_msix(pci::BarN::BAR1, count); + } + + // XXX: properly size the legacy cfg BAR + builder = builder.add_bar_io(pci::BarN::BAR0, 0x200); + let pci_state = builder.finish(); + let layout = [ (VirtioTop::LegacyConfig, LEGACY_REG_SZ), (VirtioTop::DeviceConfig, cfg_sz), @@ -116,7 +125,9 @@ impl PciVirtio { let dev_any = Arc::clone(&inner) as Arc; - let mut this = Arc::new(Self { + let this = Arc::new(Self { + pci_state, + map: RegMap::create_packed_passthru( cfg_sz + LEGACY_REG_SZ, &layout, @@ -129,34 +140,17 @@ impl PciVirtio { state: Mutex::new(VirtioState::new(inner.queues().count().get())), state_cv: Condvar::new(), + isr_state: IsrState::new(), dev: inner, dev_any, - - sa_cell: SelfArcCell::new(), }); - SelfArc::self_arc_init(&mut this); for queue in this.dev.queues()[..].iter() { - queue.set_interrupt(IsrIntr::new(this.self_weak())); - } - - let mut builder = pci::Builder::new(pci::Ident { - vendor_id: VIRTIO_VENDOR, - device_id: dev_id, - sub_vendor_id: VIRTIO_VENDOR, - sub_device_id: dev_id - 0xfff, - class: dev_class, - ..Default::default() - }) - .add_lintr(); - - if let Some(count) = msix_count { - builder = builder.add_cap_msix(pci::BarN::BAR1, count); + queue.set_interrupt(IsrIntr::new(&this.isr_state)); } - // XXX: properly size the legacy cfg BAR - builder.add_bar_io(pci::BarN::BAR0, 0x200).finish(this) + this } fn legacy_read(&self, id: &LegacyReg, ro: &mut ReadOp, _ctx: &DispCtx) { @@ -191,15 +185,8 @@ impl PciVirtio { ro.write_u8(state.status.bits()); } LegacyReg::IsrStatus => { - let mut state = self.state.lock().unwrap(); - let isr = state.isr_status; - if isr != 0 { - // reading ISR Status clears it as well - state.isr_status = 0; - if let Some(pin) = state.lintr_pin.as_ref() { - pin.deassert(); - } - } + // reading ISR Status clears it as well + let isr = self.isr_state.read_clear(); ro.write_u8(isr); } LegacyReg::MsixVectorConfig => { @@ -252,6 +239,7 @@ impl PciVirtio { state.msix_cfg_vec = wo.read_u16(); } LegacyReg::MsixVectorQueue => { + let hdl = self.pci_state.msix_hdl().unwrap(); let mut state = self.state.lock().unwrap(); let sel = state.queue_sel as usize; if let Some(queue) = self.vq(state.queue_sel) { @@ -267,7 +255,6 @@ impl PciVirtio { .unwrap(); state.intr_mode_updating = true; state.msix_queue_vec[sel] = val; - let hdl = state.msix_hdl.as_ref().unwrap().clone(); // State lock cannot be held while updating queue // interrupt handlers due to deadlock possibility. @@ -296,7 +283,7 @@ impl PciVirtio { let mut state = self.state.lock().unwrap(); let val = Status::from_bits_truncate(status); if val == Status::RESET && state.status != Status::RESET { - self.device_reset(state, ctx) + self.virtio_reset(state, ctx) } else { // XXX: better device status FSM state.status = val; @@ -316,21 +303,18 @@ impl PciVirtio { ) { self.dev.queue_change(vq, change, ctx); } - fn device_reset(&self, mut state: MutexGuard, ctx: &DispCtx) { + + /// Reset the virtio portion of the device + /// + /// This leaves PCI state (such as configured BARs) unchanged + fn virtio_reset(&self, mut state: MutexGuard, ctx: &DispCtx) { for queue in self.dev.queues()[..].iter() { queue.reset(); self.queue_change(queue, VqChange::Reset, ctx); } state.reset(); - self.dev.device_reset(ctx); - } - - fn raise_isr(&self) { - let mut state = self.state.lock().unwrap(); - state.isr_status |= 1; - if let Some(pin) = state.lintr_pin.as_ref() { - pin.assert() - } + let _ = self.isr_state.read_clear(); + self.dev.reset(ctx); } fn set_intr_mode(&self, new_mode: IntrMode) { @@ -347,9 +331,7 @@ impl PciVirtio { match old_mode { IntrMode::IsrLintr => { // When leaving lintr-pin mode, deassert anything on said pin - if let Some(pin) = state.lintr_pin.as_ref() { - pin.deassert(); - } + self.isr_state.disable(); } IntrMode::Msi => { // When leaving MSI mode, re-wire the Isr interrupt handling @@ -358,7 +340,7 @@ impl PciVirtio { // updating the interrupts handlers on queues. drop(state); for queue in self.dev.queues()[..].iter() { - queue.set_interrupt(IsrIntr::new(self.self_weak())); + queue.set_interrupt(IsrIntr::new(&self.isr_state)); } state = self.state.lock().unwrap(); } @@ -368,21 +350,17 @@ impl PciVirtio { state.intr_mode = new_mode; match new_mode { IntrMode::IsrLintr => { - if let Some(pin) = state.lintr_pin.as_ref() { - if state.isr_status != 0 { - pin.assert() - } - } + self.isr_state.enable(); } IntrMode::Msi => { + let hdl = self.pci_state.msix_hdl().unwrap(); for (idx, queue) in self.dev.queues()[..].iter().enumerate() { let vec = *state.msix_queue_vec.get(idx).unwrap(); - let hdl = state.msix_hdl.as_ref().unwrap().clone(); // State lock cannot be held while updating queue interrupt // handlers due to deadlock possibility. drop(state); - queue.set_interrupt(MsiIntr::new(hdl, vec)); + queue.set_interrupt(MsiIntr::new(hdl.clone(), vec)); state = self.state.lock().unwrap(); } } @@ -405,13 +383,10 @@ impl PciVirtio { } } -impl SelfArc for PciVirtio { - fn self_arc_cell(&self) -> &SelfArcCell { - &self.sa_cell - } -} - impl pci::Device for PciVirtio { + fn device_state(&self) -> &pci::DeviceState { + &self.pci_state + } fn bar_rw(&self, bar: pci::BarN, mut rwo: RWOp, ctx: &DispCtx) { assert_eq!(bar, pci::BarN::BAR0); let map = match self.map_which.load(Ordering::SeqCst) { @@ -428,14 +403,10 @@ impl pci::Device for PciVirtio { VirtioTop::DeviceConfig => self.dev.device_cfg_rw(rwo), }); } - fn attach( - &self, - lintr_pin: Option, - msix_hdl: Option, - ) { - let mut state = self.state.lock().unwrap(); - state.lintr_pin = lintr_pin; - state.msix_hdl = msix_hdl; + fn attach(&self) { + if let Some(pin) = self.pci_state.lintr_pin() { + self.isr_state.set_pin(pin); + } } fn interrupt_mode_change(&self, mode: pci::IntrMode) { self.set_intr_mode(match mode { @@ -487,26 +458,88 @@ impl Entity for PciVirtio { target: Option, ctx: &DispCtx, ) { - if next == instance::State::Reset { - let state = self.state.lock().unwrap(); - self.device_reset(state, ctx); - } self.dev.state_transition(next, target, ctx) } + fn reset(&self, ctx: &DispCtx) { + let state = self.state.lock().unwrap(); + self.virtio_reset(state, ctx); + self.pci_state.reset(self); + } +} + +#[derive(Default)] +struct IsrInner { + disabled: bool, + value: u8, + pin: Option>, +} +struct IsrState { + inner: Mutex, +} +impl IsrState { + fn new() -> Arc { + Arc::new(Self { inner: Mutex::new(IsrInner::default()) }) + } + fn raise(&self) { + let mut inner = self.inner.lock().unwrap(); + inner.value |= 1; + if !inner.disabled { + if let Some(pin) = inner.pin.as_ref() { + pin.assert() + } + } + } + fn read_clear(&self) -> u8 { + let mut inner = self.inner.lock().unwrap(); + let val = inner.value; + if val != 0 { + inner.value = 0; + if let Some(pin) = inner.pin.as_ref() { + pin.deassert(); + } + } + val + } + fn disable(&self) { + let mut inner = self.inner.lock().unwrap(); + if !inner.disabled { + if let Some(pin) = inner.pin.as_ref() { + pin.deassert(); + } + inner.disabled = true; + } + } + fn enable(&self) { + let mut inner = self.inner.lock().unwrap(); + if inner.disabled { + if inner.value != 0 { + if let Some(pin) = inner.pin.as_ref() { + pin.deassert(); + } + } + inner.disabled = false; + } + } + fn set_pin(&self, pin: Arc) { + let mut inner = self.inner.lock().unwrap(); + let old = inner.pin.replace(pin); + // XXX: strict for now + assert!(old.is_none()); + } } struct IsrIntr { - outer: Weak, + state: Weak, } impl IsrIntr { - fn new(outer: Weak) -> Box { - Box::new(Self { outer }) + fn new(state: &Arc) -> Box { + Box::new(Self { state: Arc::downgrade(state) }) } } impl VirtioIntr for IsrIntr { fn notify(&self, _ctx: &DispCtx) { - if let Some(dev) = Weak::upgrade(&self.outer) { - dev.raise_isr(); + if let Some(state) = Weak::upgrade(&self.state) { + state.raise() } } fn read(&self) -> VqIntr { diff --git a/propolis/src/hw/virtio/viona.rs b/propolis/src/hw/virtio/viona.rs index 6383dd465..fd19577c4 100644 --- a/propolis/src/hw/virtio/viona.rs +++ b/propolis/src/hw/virtio/viona.rs @@ -52,7 +52,7 @@ impl VirtioViona { vnic_name: &str, queue_size: u16, vm: &VmmHdl, - ) -> Result> { + ) -> Result> { let dlhdl = dladm::Handle::new()?; let info = dlhdl.query_vnic(vnic_name)?; let hdl = VionaHdl::new(info.link_id, vm.fd())?; diff --git a/propolis/src/instance.rs b/propolis/src/instance.rs index c12ff39da..cb6687574 100644 --- a/propolis/src/instance.rs +++ b/propolis/src/instance.rs @@ -315,7 +315,11 @@ impl Instance { self.disp.with_ctx(|ctx| { // Allow any entity to act on the new state inner.inv.for_each_node(inventory::Order::Pre, |_id, rec| { - rec.entity().state_transition(state, target, ctx); + let ent = rec.entity(); + if matches!(state, State::Reset) { + ent.reset(ctx); + } + ent.state_transition(state, target, ctx); }); for f in inner.transition_funcs.iter() { diff --git a/propolis/src/inventory.rs b/propolis/src/inventory.rs index 36b4546fd..39cc8e5a7 100644 --- a/propolis/src/inventory.rs +++ b/propolis/src/inventory.rs @@ -430,6 +430,11 @@ pub trait Entity: Send + Sync + 'static { ctx: &DispCtx, ) { } + /// Function dedicated to `State::Reset` event delivery so implementers do + /// not need to create a more verbose `state_transition` implementation for + /// emulating device reset. + #[allow(unused_variables)] + fn reset(&self, ctx: &DispCtx) {} #[allow(unused_variables)] fn child_register(&self) -> Option> { None diff --git a/propolis/src/mmio.rs b/propolis/src/mmio.rs index cd23c05d4..3f8d8718c 100644 --- a/propolis/src/mmio.rs +++ b/propolis/src/mmio.rs @@ -1,4 +1,4 @@ -use std::sync::{Arc, Mutex, Weak}; +use std::sync::{Arc, Mutex}; use crate::common::*; use crate::dispatch::DispCtx; @@ -7,12 +7,10 @@ pub use crate::util::aspace::{Error, Result}; use byteorder::{ByteOrder, LE}; -pub trait MmioDev: Send + Sync { - fn mmio_rw(&self, addr: usize, ident: usize, rwop: RWOp, ctx: &DispCtx); -} +pub type MmioFn = dyn Fn(usize, RWOp, &DispCtx) + Send + Sync + 'static; pub struct MmioBus { - map: Mutex, usize)>>, + map: Mutex>>, } impl MmioBus { pub fn new(max: usize) -> Self { @@ -24,16 +22,12 @@ impl MmioBus { &self, start: usize, len: usize, - dev: Weak, - ident: usize, + func: Arc, ) -> Result<()> { - self.map.lock().unwrap().register(start, len, (dev, ident)) + self.map.lock().unwrap().register(start, len, func) } - pub fn unregister( - &self, - addr: usize, - ) -> Result<(Weak, usize)> { - self.map.lock().unwrap().unregister(addr) + pub fn unregister(&self, addr: usize) -> Result<()> { + self.map.lock().unwrap().unregister(addr).map(|_| ()) } pub fn handle_write( @@ -51,9 +45,9 @@ impl MmioBus { 8 => &buf[0..], _ => panic!(), }; - let handled = self.do_mmio(addr, |a, o, dev, ident| { + let handled = self.do_mmio(addr, |a, o, func| { let mut wo = WriteOp::from_buf(o as usize, data); - dev.mmio_rw(a, ident, RWOp::Write(&mut wo), ctx) + func(a, RWOp::Write(&mut wo), ctx) }); if !handled { println!("unhandled MMIO write - addr:{:x} len:{}", addr, bytes); @@ -69,9 +63,9 @@ impl MmioBus { 8 => &mut buf[0..], _ => panic!(), }; - let handled = self.do_mmio(addr, |a, o, dev, ident| { + let handled = self.do_mmio(addr, |a, o, func| { let mut ro = ReadOp::from_buf(o as usize, &mut data); - dev.mmio_rw(a, ident, RWOp::Read(&mut ro), ctx) + func(a, RWOp::Read(&mut ro), ctx) }); if !handled { println!("unhandled MMIO read - addr:{:x} len:{}", addr, bytes); @@ -84,15 +78,14 @@ impl MmioBus { fn do_mmio(&self, addr: usize, f: F) -> bool where - F: FnOnce(usize, usize, &Arc, usize), + F: FnOnce(usize, usize, &Arc), { let map = self.map.lock().unwrap(); - if let Ok((start, _len, (weak, ident))) = map.region_at(addr) { - let dev = Weak::upgrade(weak).unwrap(); - let identv = *ident; + if let Ok((start, _len, func)) = map.region_at(addr) { + let func = Arc::clone(func); // unlock map before entering handler drop(map); - f(start, addr - start, &dev, identv); + f(start, addr - start, &func); true } else { false diff --git a/propolis/src/pio.rs b/propolis/src/pio.rs index 38387c11b..fedacb059 100644 --- a/propolis/src/pio.rs +++ b/propolis/src/pio.rs @@ -1,4 +1,4 @@ -use std::sync::{Arc, Mutex, Weak}; +use std::sync::{Arc, Mutex}; use crate::common::*; use crate::dispatch::DispCtx; @@ -7,13 +7,11 @@ pub use crate::util::aspace::{Error, Result}; use byteorder::{ByteOrder, LE}; -pub trait PioDev: Send + Sync { - fn pio_rw(&self, port: u16, ident: usize, rwop: RWOp, ctx: &DispCtx); -} +pub type PioFn = dyn Fn(u16, RWOp<'_, '_>, &DispCtx) + Send + Sync + 'static; /// Port IO bus. pub struct PioBus { - map: Mutex, usize)>>, + map: Mutex>>, } impl PioBus { @@ -25,17 +23,12 @@ impl PioBus { &self, start: u16, len: u16, - dev: Weak, - ident: usize, + func: Arc, ) -> Result<()> { - self.map.lock().unwrap().register( - start as usize, - len as usize, - (dev, ident), - ) + self.map.lock().unwrap().register(start as usize, len as usize, func) } - pub fn unregister(&self, start: u16) -> Result<(Weak, usize)> { - self.map.lock().unwrap().unregister(start as usize) + pub fn unregister(&self, start: u16) -> Result<()> { + self.map.lock().unwrap().unregister(start as usize).map(|_| ()) } pub fn handle_out(&self, port: u16, bytes: u8, val: u32, ctx: &DispCtx) { @@ -46,9 +39,9 @@ impl PioBus { 4 => &buf[0..], _ => panic!(), }; - let handled = self.do_pio(port, |p, o, dev, ident| { + let handled = self.do_pio(port, |a, o, func| { let mut wo = WriteOp::from_buf(o as usize, data); - dev.pio_rw(p, ident, RWOp::Write(&mut wo), ctx) + func(a, RWOp::Write(&mut wo), ctx) }); if !handled { println!("unhandled IO out - port:{:x} len:{}", port, bytes); @@ -64,9 +57,9 @@ impl PioBus { 4 => &mut buf[0..], _ => panic!(), }; - let handled = self.do_pio(port, |p, o, dev, ident| { + let handled = self.do_pio(port, |a, o, func| { let mut ro = ReadOp::from_buf(o as usize, &mut data); - dev.pio_rw(p, ident, RWOp::Read(&mut ro), ctx) + func(a, RWOp::Read(&mut ro), ctx) }); if !handled { println!("unhandled IO in - port:{:x} len:{}", port, bytes); @@ -80,15 +73,14 @@ impl PioBus { fn do_pio(&self, port: u16, f: F) -> bool where - F: FnOnce(u16, u16, &Arc, usize), + F: FnOnce(u16, u16, &Arc), { let map = self.map.lock().unwrap(); - if let Ok((start, _len, (weak, ident))) = map.region_at(port as usize) { - let dev = Weak::upgrade(weak).unwrap(); - let identv = *ident; + if let Ok((start, _len, func)) = map.region_at(port as usize) { + let func = Arc::clone(func); // unlock map before entering handler drop(map); - f(start as u16, port - start as u16, &dev, identv); + f(start as u16, port - start as u16, &func); true } else { false diff --git a/propolis/src/vmm/machine.rs b/propolis/src/vmm/machine.rs index 6f4d0b867..49da72a54 100644 --- a/propolis/src/vmm/machine.rs +++ b/propolis/src/vmm/machine.rs @@ -39,13 +39,13 @@ struct MapEnt { /// - The device's physical memory representation /// - Buses. pub struct Machine { - hdl: Arc, + pub hdl: Arc, max_cpu: u8, _guard_space: GuardSpace, map_physmem: ASpace, - bus_mmio: MmioBus, - bus_pio: PioBus, + pub bus_mmio: Arc, + pub bus_pio: Arc, } impl Machine { @@ -169,8 +169,8 @@ impl Machine { _guard_space: guard_space, map_physmem: map, - bus_mmio: MmioBus::new(MAX_PHYSMEM), - bus_pio: PioBus::new(), + bus_mmio: Arc::new(MmioBus::new(MAX_PHYSMEM)), + bus_pio: Arc::new(PioBus::new()), }) } } @@ -576,8 +576,8 @@ impl Builder { _guard_space: guard_space, map_physmem: map, - bus_mmio: MmioBus::new(MAX_PHYSMEM), - bus_pio: PioBus::new(), + bus_mmio: Arc::new(MmioBus::new(MAX_PHYSMEM)), + bus_pio: Arc::new(PioBus::new()), }; Ok(machine) } diff --git a/server/src/lib/initializer.rs b/server/src/lib/initializer.rs index 1f4cc109e..a827f0853 100644 --- a/server/src/lib/initializer.rs +++ b/server/src/lib/initializer.rs @@ -133,9 +133,7 @@ impl<'a> MachineInitializer<'a> { } pub fn initialize_chipset(&self) -> Result { - let hdl = self.machine.get_hdl(); - let chipset = I440Fx::create(Arc::clone(&hdl)); - chipset.attach(self.mctx); + let chipset = I440Fx::create(self.machine); let id = self .inv .register(&chipset, "chipset".to_string(), None) @@ -212,9 +210,7 @@ impl<'a> MachineInitializer<'a> { .map_err(|e| -> std::io::Error { e.into() })?; let _ = self.inv.register_child(be_register, id).unwrap(); - let blk = vioblk - .inner_dev::() - .inner_dev::(); + let blk = vioblk.inner_dev::(); backend.attach(blk, self.disp); chipset.device().pci_attach(bdf, vioblk); diff --git a/server/src/lib/server.rs b/server/src/lib/server.rs index 92519928b..b743b435f 100644 --- a/server/src/lib/server.rs +++ b/server/src/lib/server.rs @@ -26,7 +26,6 @@ use uuid::Uuid; use propolis::bhyve_api; use propolis::dispatch::{AsyncCtx, AsyncTaskId}; -use propolis::hw::chipset::Chipset; use propolis::hw::pci; use propolis::hw::uart::LpcUart; use propolis::instance::Instance; @@ -143,9 +142,13 @@ enum SlotType { fn slot_to_bdf(slot: api::Slot, ty: SlotType) -> Result { match ty { // Slots for NICS: 0x08 -> 0x0F - SlotType::NIC if slot.0 <= 7 => Ok(pci::Bdf::new(0, slot.0 + 0x8, 0)), + SlotType::NIC if slot.0 <= 7 => { + Ok(pci::Bdf::new(0, slot.0 + 0x8, 0).unwrap()) + } // Slots for Disks: 0x10 -> 0x17 - SlotType::Disk if slot.0 <= 7 => Ok(pci::Bdf::new(0, slot.0 + 0x10, 0)), + SlotType::Disk if slot.0 <= 7 => { + Ok(pci::Bdf::new(0, slot.0 + 0x10, 0).unwrap()) + } _ => Err(anyhow::anyhow!( "PCI Slot {} has no translation to BDF for type {:?}", slot.0, @@ -334,8 +337,6 @@ async fn instance_ensure( } } - // Finalize device. - chipset.device().pci_finalize(mctx); init.initialize_fwcfg(&chipset, properties.vcpus)?; init.initialize_cpus()?; Ok(()) diff --git a/standalone/src/config.rs b/standalone/src/config.rs index c4311bed5..efab43232 100644 --- a/standalone/src/config.rs +++ b/standalone/src/config.rs @@ -134,7 +134,7 @@ pub fn parse_bdf(v: &str) -> Option { } if fields.len() == 3 { - pci::Bdf::try_new(fields[0], fields[1], fields[2]) + pci::Bdf::new(fields[0], fields[1], fields[2]) } else { None } diff --git a/standalone/src/main.rs b/standalone/src/main.rs index c1506ce7f..f3931de7d 100644 --- a/standalone/src/main.rs +++ b/standalone/src/main.rs @@ -132,8 +132,7 @@ fn main() { machine.initialize_rtc(lowmem, highmem).unwrap(); let hdl = machine.get_hdl(); - let chipset = hw::chipset::i440fx::I440Fx::create(Arc::clone(&hdl)); - chipset.attach(mctx); + let chipset = hw::chipset::i440fx::I440Fx::create(machine); let chipset_id = inv .register(&chipset, "chipset".to_string(), None) .map_err(|e| -> std::io::Error { e.into() })?; @@ -209,9 +208,8 @@ fn main() { .register_child(creg, id) .map_err(|e| -> std::io::Error { e.into() })?; - let blk = vioblk - .inner_dev::() - .inner_dev::(); + let blk = + vioblk.inner_dev::(); backend.attach(blk as Arc, disp); chipset.pci_attach(bdf.unwrap(), vioblk); @@ -241,8 +239,7 @@ fn main() { inv.register(&nvme, format!("nvme-{}", name), None)?; let _be_id = inv.register_child(creg, id)?; - let blk = nvme.inner_dev::(); - backend.attach(blk, disp); + backend.attach(nvme.clone(), disp); chipset.pci_attach(bdf.unwrap(), nvme); } @@ -253,10 +250,6 @@ fn main() { } } - // with all pci devices attached, place their BARs and wire up access to PCI - // configuration space - chipset.pci_finalize(mctx); - let mut fwcfg = hw::qemu::fwcfg::FwCfgBuilder::new(); fwcfg .add_legacy( From 308228939ca4823cd27c34711d922496a5856351 Mon Sep 17 00:00:00 2001 From: Patrick Mooney Date: Mon, 18 Oct 2021 00:24:40 -0500 Subject: [PATCH 2/2] CR feedback, round 1 --- propolis/src/hw/pci/bus.rs | 2 +- propolis/src/hw/pci/mod.rs | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/propolis/src/hw/pci/bus.rs b/propolis/src/hw/pci/bus.rs index 0d089fe07..6dcdba5ef 100644 --- a/propolis/src/hw/pci/bus.rs +++ b/propolis/src/hw/pci/bus.rs @@ -101,7 +101,7 @@ impl Inner { let res = self.slots[bdf.dev.get() as usize].funcs [bdf.func.get() as usize] .as_ref() - .map(|d| Arc::clone(d)); + .map(Arc::clone); res } fn attach(&mut self, bdf: Bdf, dev: Arc) { diff --git a/propolis/src/hw/pci/mod.rs b/propolis/src/hw/pci/mod.rs index c0b120b81..4d65c897e 100644 --- a/propolis/src/hw/pci/mod.rs +++ b/propolis/src/hw/pci/mod.rs @@ -107,6 +107,8 @@ impl Bdf { /// /// Returns [`Option::None`] if the values would not fit within a BDF. pub const fn new(bus: u8, dev: u8, func: u8) -> Option { + // Until the `?` operator is supported in `const fn`s, this more verbose + // implementation is required. let bnum = BusNum::new(bus); let dnum = DevNum::new(dev); let fnum = FuncNum::new(func);