From 15d2078a13dddb5f09d9313635ac39a432b788ad Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 20 Jul 2024 15:11:34 +0200 Subject: [PATCH] Handle daemonization of pcid in a better way Currently pcid exits the main thread once it is done spawning drivers and expects all background threads handling driver communication to stay around. This only works as exitting the main thread on redox os currently doesn't cause the whole process to die. This will have to be fixed at some point for compatibility with programs that expect that exitting the main thread kills all other threads in the process, at which point pcid would break without the changes in this commit. --- Cargo.lock | 1 + fmt.sh | 1 + pcid/Cargo.toml | 1 + pcid/src/driver_handler.rs | 7 +++---- pcid/src/driver_interface/cap.rs | 5 +---- pcid/src/driver_interface/irq_helpers.rs | 24 +++++++++++++++--------- pcid/src/main.rs | 8 ++++++++ 7 files changed, 30 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c3bfee3bd9..d971355db9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -937,6 +937,7 @@ dependencies = [ "paw", "pci_types", "plain", + "redox-daemon", "redox-log", "redox_syscall 0.5.3", "serde", diff --git a/fmt.sh b/fmt.sh index e867ac215f..f62b954766 100755 --- a/fmt.sh +++ b/fmt.sh @@ -22,4 +22,5 @@ fmt graphics/bgad \ graphics/virtio-gpud \ inputd \ net/virtio-netd \ + pcid \ virtio-core diff --git a/pcid/Cargo.toml b/pcid/Cargo.toml index 8e7db297ac..a0b6fcc152 100644 --- a/pcid/Cargo.toml +++ b/pcid/Cargo.toml @@ -21,6 +21,7 @@ log = "0.4" paw = "1.0" pci_types = "0.10" plain = "0.2" +redox-daemon = "0.1" redox-log = "0.1" redox_syscall = "0.5" serde = { version = "1", features = ["derive"] } diff --git a/pcid/src/driver_handler.rs b/pcid/src/driver_handler.rs index d34a067d3c..cbaf221e54 100644 --- a/pcid/src/driver_handler.rs +++ b/pcid/src/driver_handler.rs @@ -74,18 +74,17 @@ impl DriverHandler { Ok(mut child) => { let driver_handler = DriverHandler { addr: func.addr, - state, + state: state.clone(), capabilities, }; - let _handle = thread::spawn(move || { + let handle = thread::spawn(move || { driver_handler.handle_spawn( pcid_to_client_write, pcid_from_client_read, subdriver_args, ); }); - // FIXME this currently deadlocks as pcid doesn't daemonize - //state.threads.lock().unwrap().push(handle); + state.threads.lock().unwrap().push(handle); match child.wait() { Ok(_status) => (), Err(err) => error!("pcid: failed to wait for {:?}: {}", command, err), diff --git a/pcid/src/driver_interface/cap.rs b/pcid/src/driver_interface/cap.rs index aa3f5540de..4e1b746a6d 100644 --- a/pcid/src/driver_interface/cap.rs +++ b/pcid/src/driver_interface/cap.rs @@ -8,10 +8,7 @@ pub struct VendorSpecificCapability { } impl VendorSpecificCapability { - pub unsafe fn parse( - addr: PciCapabilityAddress, - access: &dyn ConfigRegionAccess, - ) -> Self { + pub unsafe fn parse(addr: PciCapabilityAddress, access: &dyn ConfigRegionAccess) -> Self { let dword = access.read(addr.address, addr.offset); let next = (dword >> 8) & 0xFF; let length = ((dword >> 16) & 0xFF) as u16; diff --git a/pcid/src/driver_interface/irq_helpers.rs b/pcid/src/driver_interface/irq_helpers.rs index b900068456..6c7b010485 100644 --- a/pcid/src/driver_interface/irq_helpers.rs +++ b/pcid/src/driver_interface/irq_helpers.rs @@ -137,14 +137,15 @@ pub fn allocate_aligned_interrupt_vectors( } // if found, reserve the irq - let irq_handle = match File::create(format!("/scheme/irq/cpu-{:02x}/{}", cpu_id, irq_number)) { - Ok(handle) => handle, + let irq_handle = + match File::create(format!("/scheme/irq/cpu-{:02x}/{}", cpu_id, irq_number)) { + Ok(handle) => handle, - // return early if the entire range couldn't be allocated - Err(err) if err.kind() == io::ErrorKind::NotFound => break, + // return early if the entire range couldn't be allocated + Err(err) if err.kind() == io::ErrorKind::NotFound => break, - Err(err) => return Err(err), - }; + Err(err) => return Err(err), + }; handles.push(irq_handle); index += 1; } @@ -191,8 +192,13 @@ pub fn allocate_single_interrupt_vector_for_msi(cpu_id: usize) -> (MsiAddrAndDat let (vector, interrupt_handle) = allocate_single_interrupt_vector(cpu_id) .expect("failed to allocate interrupt vector") .expect("no interrupt vectors left"); - let msg_data = - x86_msix::message_data_edge_triggered(x86_msix::DeliveryMode::Fixed, vector); + let msg_data = x86_msix::message_data_edge_triggered(x86_msix::DeliveryMode::Fixed, vector); - (MsiAddrAndData { addr, data: msg_data }, interrupt_handle) + ( + MsiAddrAndData { + addr, + data: msg_data, + }, + interrupt_handle, + ) } diff --git a/pcid/src/main.rs b/pcid/src/main.rs index e392ba37b0..ca435e1635 100644 --- a/pcid/src/main.rs +++ b/pcid/src/main.rs @@ -248,6 +248,10 @@ fn main(args: Args) { let _logger_ref = setup_logging(args.verbose); + redox_daemon::Daemon::new(move |daemon| main_inner(config, daemon)).unwrap(); +} + +fn main_inner(config: Config, daemon: redox_daemon::Daemon) -> ! { let state = Arc::new(State { pcie: Pcie::new(), threads: Mutex::new(Vec::new()), @@ -312,7 +316,11 @@ fn main(args: Args) { } } + daemon.ready().unwrap(); + for thread in state.threads.lock().unwrap().drain(..) { thread.join().unwrap(); } + + std::process::exit(0); }