From 84a3b95f17b6ea851d6eb4d5f15baa388c04daaf Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Thu, 25 Jun 2026 22:35:42 +0000 Subject: [PATCH] =?UTF-8?q?refactor(windows-host):=20delete=20the=20SudoVD?= =?UTF-8?q?A=20backend=20=E2=80=94=20pf-vdisplay=20is=20the=20sole=20vdisp?= =?UTF-8?q?lay=20(Goal=202)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Goal 2 ("drop every trace of SudoVDA") is done. The SudoVDA driver is no longer shipped (only pf-vdisplay; the old vdisplay-driver tree was deleted in a2bd0cd), and F1 (d638a93/e60cda3) already moved the display-utility helpers out of the backend into neutral modules (win_adapter/win_display), breaking the reach-in. So the backend is now cleanly removable: - Deleted crates/punktfunk-host/src/vdisplay/windows/sudovda.rs (350 lines: the SudoVdaDisplay VirtualDisplay impl + its VdisplayDriver/probe). - vdisplay::open()/probe() are now unconditional pf-vdisplay; deleted the windows_use_pf_vdisplay() backend selector. open() now ensure!s pf_vdisplay::is_available() with a clear "driver not installed" error instead of the old silent SudoVDA fallback (no fallback driver exists anymore). - Scrubbed the dangling references to the deleted symbols (manager/sendinput/dxgi comments, the config + host.env PUNKTFUNK_VDISPLAY docs); the var stays as an informational forward-seam. Updated the F1 module docs (Goal 2 now done). All changes are #[cfg(windows)] except the config doc; Linux clippy -p punktfunk-host -D warnings clean; zero `sudovda::`/`SudoVdaDisplay` code refs remain (comments only). Windows build is CI-gated. Scorecard Goal 2 -> DONE; recorded the E1 "do NOT do it" stability decision in windows-host-rewrite.md §4 (the process-global driver design is sound given ProcessSharingDisabled; a device-owned variant adds a use-after-free window for no gain). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/capture/windows/dxgi.rs | 6 +- crates/punktfunk-host/src/config.rs | 4 +- .../src/inject/windows/sendinput.rs | 2 +- crates/punktfunk-host/src/vdisplay.rs | 37 +- .../src/vdisplay/windows/manager.rs | 2 +- .../src/vdisplay/windows/sudovda.rs | 350 ------------------ .../punktfunk-host/src/windows/win_adapter.rs | 4 +- .../punktfunk-host/src/windows/win_display.rs | 4 +- docs/windows-host-rewrite.md | 42 ++- scripts/windows/host.env.example | 4 +- 10 files changed, 50 insertions(+), 405 deletions(-) delete mode 100644 crates/punktfunk-host/src/vdisplay/windows/sudovda.rs diff --git a/crates/punktfunk-host/src/capture/windows/dxgi.rs b/crates/punktfunk-host/src/capture/windows/dxgi.rs index 156f659..927a109 100644 --- a/crates/punktfunk-host/src/capture/windows/dxgi.rs +++ b/crates/punktfunk-host/src/capture/windows/dxgi.rs @@ -2186,9 +2186,9 @@ impl DuplCapturer { let context = context.context("null D3D11 context")?; // 3) duplicate the output. Attach to the current input desktop first (as SYSTEM this can // be the Winlogon secure desktop) so a session that starts at the lock/login screen works. - // The SudoVDA is kept the sole desktop via the CCD isolation in sudovda::create_monitor - // (registry-persisted), so the secure desktop has nowhere to render but the output we - // capture — no per-open re-isolation needed. + // The virtual display is kept the sole desktop via the CCD isolation the pf-vdisplay backend + // applies at monitor creation (registry-persisted), so the secure desktop has nowhere to render + // but the output we capture — no per-open re-isolation needed. attach_input_desktop(); let dupl = duplicate_output(&output, &device, want_hdr) .context("DuplicateOutput (already duplicated by another app?)")?; diff --git a/crates/punktfunk-host/src/config.rs b/crates/punktfunk-host/src/config.rs index a22eabf..054c0df 100644 --- a/crates/punktfunk-host/src/config.rs +++ b/crates/punktfunk-host/src/config.rs @@ -72,7 +72,9 @@ pub struct HostConfig { pub compositor: Option, /// `PUNKTFUNK_GAMEPAD` — client/operator virtual-pad backend preference (fed to `pick_gamepad`). pub gamepad: Option, - /// `PUNKTFUNK_VDISPLAY` — Windows virtual-display backend select (`pf`/`pfvd` vs `sudovda`; else auto-detect). + /// `PUNKTFUNK_VDISPLAY` — Windows virtual-display backend. The pf-vdisplay IddCx driver is now the only + /// backend (the legacy SudoVDA backend was removed), so this is currently informational — kept for the + /// shipped `host.env` and as a forward seam if a second backend is ever added. pub vdisplay: Option, } diff --git a/crates/punktfunk-host/src/inject/windows/sendinput.rs b/crates/punktfunk-host/src/inject/windows/sendinput.rs index 208702b..d3973d0 100644 --- a/crates/punktfunk-host/src/inject/windows/sendinput.rs +++ b/crates/punktfunk-host/src/inject/windows/sendinput.rs @@ -35,7 +35,7 @@ pub struct SendInputInjector { desktop: Option, } -// Only ever used from the host's single injector thread (like SudoVdaDisplay). +// Only ever used from the host's single injector thread. unsafe impl Send for SendInputInjector {} impl SendInputInjector { diff --git a/crates/punktfunk-host/src/vdisplay.rs b/crates/punktfunk-host/src/vdisplay.rs index e1606f8..d57e804 100644 --- a/crates/punktfunk-host/src/vdisplay.rs +++ b/crates/punktfunk-host/src/vdisplay.rs @@ -529,15 +529,15 @@ pub fn open(compositor: Compositor) -> Result> { } #[cfg(target_os = "windows")] { - // Two virtual-display backends: the new pf-vdisplay IddCx driver (pf_vdisplay_proto) and the - // shipping SudoVDA fallback. The compositor arg is moot on Windows. PUNKTFUNK_VDISPLAY overrides; - // default auto-detects (prefer pf-vdisplay if its driver interface is present). + // The pf-vdisplay all-Rust IddCx driver is the sole virtual-display backend (the legacy SudoVDA + // fallback was removed — its driver is no longer shipped). The compositor arg is moot on Windows. let _ = compositor; - if windows_use_pf_vdisplay() { - Ok(Box::new(pf_vdisplay::PfVdisplayDisplay::new()?)) - } else { - Ok(Box::new(sudovda::SudoVdaDisplay::new()?)) - } + anyhow::ensure!( + pf_vdisplay::is_available(), + "pf-vdisplay driver interface not found — the pf-vdisplay IddCx driver is not installed or \ + not loaded (the host installer bundles it; reinstall or check the driver state)" + ); + Ok(Box::new(pf_vdisplay::PfVdisplayDisplay::new()?)) } #[cfg(not(any(target_os = "linux", target_os = "windows")))] { @@ -546,18 +546,6 @@ pub fn open(compositor: Compositor) -> Result> { } } -/// Pick the Windows virtual-display backend. `PUNKTFUNK_VDISPLAY=pf|pf-vdisplay|pfvd` forces the new -/// pf-vdisplay IddCx driver; `=sudovda|sudo` forces the shipping SudoVDA driver; anything else (the -/// default) auto-detects, preferring pf-vdisplay if its device interface is enumerable. -#[cfg(target_os = "windows")] -fn windows_use_pf_vdisplay() -> bool { - match crate::config::config().vdisplay.as_deref().map(str::trim) { - Some("pf") | Some("pf-vdisplay") | Some("pfvd") => true, - Some("sudovda") | Some("sudo") => false, - _ => pf_vdisplay::is_available(), - } -} - /// Readiness probe for `compositor`: is it up and able to create a virtual output *right /// now*? A session-bringup script polls this (via `punktfunk-host probe-compositor`) to gate /// on actual readiness instead of racing the compositor with a blind sleep. @@ -578,11 +566,7 @@ pub fn probe(compositor: Compositor) -> Result<()> { #[cfg(target_os = "windows")] { let _ = compositor; - if windows_use_pf_vdisplay() { - pf_vdisplay::probe() - } else { - sudovda::probe() - } + pf_vdisplay::probe() } #[cfg(not(any(target_os = "linux", target_os = "windows")))] { @@ -640,9 +624,6 @@ pub(crate) mod manager; #[cfg(target_os = "windows")] #[path = "vdisplay/windows/pf_vdisplay.rs"] pub(crate) mod pf_vdisplay; -#[cfg(target_os = "windows")] -#[path = "vdisplay/windows/sudovda.rs"] -pub(crate) mod sudovda; #[cfg(target_os = "linux")] #[path = "vdisplay/linux/wlroots.rs"] mod wlroots; diff --git a/crates/punktfunk-host/src/vdisplay/windows/manager.rs b/crates/punktfunk-host/src/vdisplay/windows/manager.rs index 077e4e9..54d8a5a 100644 --- a/crates/punktfunk-host/src/vdisplay/windows/manager.rs +++ b/crates/punktfunk-host/src/vdisplay/windows/manager.rs @@ -178,7 +178,7 @@ impl VirtualDisplayManager { } /// Open + initialise the backend (validates the driver is present). Mirrors the old - /// `SudoVdaDisplay::new`/`PfVdisplayDisplay::new`. + /// `PfVdisplayDisplay::new`. pub(crate) fn open_backend(&self) -> Result<()> { // Hold the state lock across the open so two racing backends can't double-open the device. let _guard = self.state.lock().unwrap(); diff --git a/crates/punktfunk-host/src/vdisplay/windows/sudovda.rs b/crates/punktfunk-host/src/vdisplay/windows/sudovda.rs deleted file mode 100644 index 372488e..0000000 --- a/crates/punktfunk-host/src/vdisplay/windows/sudovda.rs +++ /dev/null @@ -1,350 +0,0 @@ -//! Windows virtual-display backend driving **SudoVDA** (the SudoMaker Virtual Display Adapter — -//! the Indirect Display Driver the Apollo Sunshine-fork ships). The Windows analogue of the -//! Linux per-compositor backends: [`create`](VirtualDisplay::create) adds a virtual monitor at the -//! client's exact `WxH@Hz` (the mode is baked into the ADD IOCTL — no EDID seeding), starts the -//! mandatory watchdog ping, and the returned [`VirtualOutput`]'s keepalive `Drop` removes it (RAII). -//! -//! Control surface (verified live against SudoVDA 0.2.1): a device-interface-GUID + `CreateFileW` -//! + `DeviceIoControl` IOCTL protocol. No DLL, no named pipe. See `docs/windows-host.md`. - -use std::ffi::c_void; -use std::mem::size_of; -use std::os::windows::io::{FromRawHandle, OwnedHandle}; -use std::sync::atomic::Ordering; - -use anyhow::{Context, Result}; -use windows::core::{GUID, PCWSTR}; -use windows::Win32::Devices::DeviceAndDriverInstallation::{ - SetupDiDestroyDeviceInfoList, SetupDiEnumDeviceInterfaces, SetupDiGetClassDevsW, - SetupDiGetDeviceInterfaceDetailW, DIGCF_DEVICEINTERFACE, DIGCF_PRESENT, - SP_DEVICE_INTERFACE_DATA, SP_DEVICE_INTERFACE_DETAIL_DATA_W, -}; -// (CCD `Devices::Display` + `Graphics::Gdi` imports moved with the display helpers to `win_display`.) -use windows::Win32::Foundation::{CloseHandle, HANDLE, LUID}; -use windows::Win32::Storage::FileSystem::{ - CreateFileW, FILE_FLAGS_AND_ATTRIBUTES, FILE_SHARE_READ, FILE_SHARE_WRITE, OPEN_EXISTING, -}; -use windows::Win32::System::IO::DeviceIoControl; - -use super::manager::{AddedMonitor, MonitorKey, VdisplayDriver}; -use super::{Mode, VirtualDisplay, VirtualOutput}; - -// SudoVDA device-interface GUID (Common/Include/sudovda-ioctl.h). -const SUVDA_INTERFACE: GUID = GUID::from_u128(0xE5BC_C234_1E0C_418A_A0D4_EF8B_7501_414D); - -// CTL_CODE(FILE_DEVICE_UNKNOWN=0x22, func, METHOD_BUFFERED=0, FILE_ANY_ACCESS=0). -const fn ctl(func: u32) -> u32 { - (0x22u32 << 16) | (func << 2) -} -const IOCTL_ADD: u32 = ctl(0x800); -const IOCTL_REMOVE: u32 = ctl(0x801); -const IOCTL_SET_RENDER_ADAPTER: u32 = ctl(0x802); // == 0x0022_2008 -const IOCTL_GET_WATCHDOG: u32 = ctl(0x803); -/// pf-vdisplay extension (NOT in SudoVDA): tear down every virtual monitor. Sent once on host startup -/// to reap monitors orphaned by a crashed/killed previous host. SudoVDA returns invalid (ignored). -const IOCTL_CLEAR_ALL: u32 = ctl(0x804); -const IOCTL_DRIVER_PING: u32 = ctl(0x888); -const IOCTL_GET_VERSION: u32 = ctl(0x8FF); - -/// A UNIQUE-per-session SudoVDA monitor GUID. The monitor is keyed by GUID for IOCTL_ADD/REMOVE, so a -/// FIXED GUID makes overlapping sessions (a client reconnecting after a freeze before the old session -/// has torn down, or genuine concurrent sessions) all map to the SAME monitor — then one session's -/// IOCTL_REMOVE on teardown tears the monitor down OUT FROM UNDER a still-live session ("display -/// disconnected" sound + freeze, even with no context change — observed live). Make it unique per -/// (process, session): base GUID with the low 48-bit node = (pid << 16 | session#). -fn next_monitor_guid() -> GUID { - use std::sync::atomic::AtomicU32; - static N: AtomicU32 = AtomicU32::new(0); - let n = N.fetch_add(1, Ordering::Relaxed) as u128; - let pid = std::process::id() as u128; - GUID::from_u128(0x70756E6B_7466_756E_6B30_000000000000u128 | (pid << 16) | (n & 0xFFFF)) -} - -#[repr(C)] -#[derive(Clone, Copy)] -struct AddParams { - width: u32, - height: u32, - refresh: u32, - guid: GUID, - device_name: [u8; 14], - serial: [u8; 14], -} - -#[repr(C)] -#[derive(Clone, Copy)] -struct AddOut { - luid: LUID, - target_id: u32, -} - -// SET_RENDER_ADAPTER input — byte-identical to SudoVDA's `{ LUID AdapterLuid; }` (8 bytes). The -// windows `LUID` is `{ LowPart: u32, HighPart: i32 }` == the C `LUID`, so `#[repr(C)]` is exact. -#[repr(C)] -#[derive(Clone, Copy)] -struct SetRenderAdapterParams { - luid: LUID, -} - -/// Pin the SudoVDA IDD's RENDER GPU to `luid` (Apollo's `SetRenderAdapter`). No output buffer. MUST be -/// issued on the driver handle BEFORE `IOCTL_ADD` to steer which GPU the new target renders on — on a -/// multi-adapter box (SudoVDA IDD + a discrete GPU) this stops DXGI from reparenting the virtual -/// output onto a different adapter than the one we duplicate/encode on (the ACCESS_LOST storm). -unsafe fn set_render_adapter(h: HANDLE, luid: LUID) -> Result<()> { - let p = SetRenderAdapterParams { luid }; - let bytes = std::slice::from_raw_parts( - &p as *const _ as *const u8, - size_of::(), - ); - let mut none: [u8; 0] = []; - ioctl(h, IOCTL_SET_RENDER_ADAPTER, bytes, &mut none) - .map(|_| ()) - .context("SudoVDA SET_RENDER_ADAPTER") -} - -#[repr(C)] -struct RemoveParams { - guid: GUID, -} - -/// One `DeviceIoControl` round trip (METHOD_BUFFERED). `input`/`output` may be empty. -unsafe fn ioctl(h: HANDLE, code: u32, input: &[u8], output: &mut [u8]) -> Result { - let mut returned = 0u32; - let inp = (!input.is_empty()).then_some(input.as_ptr() as *const c_void); - let outp = (!output.is_empty()).then_some(output.as_mut_ptr() as *mut c_void); - DeviceIoControl( - h, - code, - inp, - input.len() as u32, - outp, - output.len() as u32, - Some(&mut returned), - None, - ) - .with_context(|| format!("DeviceIoControl(code={code:#x})"))?; - Ok(returned) -} - -unsafe fn open_device() -> Result { - let hdev = SetupDiGetClassDevsW( - Some(&SUVDA_INTERFACE), - PCWSTR::null(), - None, - DIGCF_DEVICEINTERFACE | DIGCF_PRESENT, - ) - .context("SetupDiGetClassDevsW(SudoVDA) — is the SudoVDA driver installed?")?; - - let mut idata = SP_DEVICE_INTERFACE_DATA { - cbSize: size_of::() as u32, - ..Default::default() - }; - SetupDiEnumDeviceInterfaces(hdev, None, &SUVDA_INTERFACE, 0, &mut idata) - .context("SetupDiEnumDeviceInterfaces(SudoVDA)")?; - - let mut required = 0u32; - let _ = SetupDiGetDeviceInterfaceDetailW(hdev, &idata, None, 0, Some(&mut required), None); - let mut buf = vec![0u8; required as usize]; - let detail = buf.as_mut_ptr() as *mut SP_DEVICE_INTERFACE_DETAIL_DATA_W; - (*detail).cbSize = size_of::() as u32; - SetupDiGetDeviceInterfaceDetailW(hdev, &idata, Some(detail), required, None, None) - .context("SetupDiGetDeviceInterfaceDetailW(SudoVDA)")?; - - let handle = CreateFileW( - PCWSTR((*detail).DevicePath.as_ptr()), - 0xC000_0000, // GENERIC_READ | GENERIC_WRITE - FILE_SHARE_READ | FILE_SHARE_WRITE, - None, - OPEN_EXISTING, - FILE_FLAGS_AND_ATTRIBUTES(0), - None, - ) - .context("CreateFileW(SudoVDA device)")?; - let _ = SetupDiDestroyDeviceInfoList(hdev); - Ok(handle) -} - -/// The SudoVDA IOCTL surface behind the shared [`VirtualDisplayManager`](super::manager::VirtualDisplayManager) -/// (Goal-1 §2.5) — the only SudoVDA-specific code left; the monitor lifecycle is the shared state machine. -pub(crate) struct SudoVdaDriver; - -impl VdisplayDriver for SudoVdaDriver { - fn name(&self) -> &'static str { - "sudovda" - } - - unsafe fn open(&self) -> Result<(OwnedHandle, u32)> { - let device = unsafe { open_device()? }; - let mut ver = [0u8; 4]; - if unsafe { ioctl(device, IOCTL_GET_VERSION, &[], &mut ver) }.is_ok() { - tracing::info!( - "SudoVDA protocol {}.{}.{} (test={})", - ver[0], - ver[1], - ver[2], - ver[3] - ); - } - let mut wd = [0u8; 8]; - let watchdog_s = if unsafe { ioctl(device, IOCTL_GET_WATCHDOG, &[], &mut wd) }.is_ok() { - u32::from_le_bytes([wd[0], wd[1], wd[2], wd[3]]).max(1) - } else { - 3 - }; - tracing::info!("SudoVDA watchdog timeout {}s", watchdog_s); - // Reap monitors orphaned by a crashed previous host (SudoVDA returns invalid for CLEAR_ALL — - // ignored; pf-vdisplay honors it). - let mut none: [u8; 0] = []; - if unsafe { ioctl(device, IOCTL_CLEAR_ALL, &[], &mut none) }.is_ok() { - tracing::info!("cleared orphaned virtual monitors on host startup"); - } - // Take ownership — the OwnedHandle CloseHandle's the control device on drop (it was leaked before). - Ok((unsafe { OwnedHandle::from_raw_handle(device.0 as _) }, watchdog_s)) - } - - unsafe fn add_monitor( - &self, - dev: HANDLE, - mode: Mode, - render_luid: Option, - ) -> Result { - // SET_RENDER_ADAPTER (opt-in). On this box SudoVDA IGNORES the pin and the IDD lands on a different - // adapter than its DXGI output is enumerated under — the cross-GPU ACCESS_LOST source — so the - // manager only pins under PUNKTFUNK_RENDER_ADAPTER / IDD-push. - if let Some(luid) = render_luid { - match unsafe { set_render_adapter(dev, luid) } { - Ok(()) => tracing::info!( - luid = format!("{:08x}:{:08x}", luid.HighPart, luid.LowPart), - "SudoVDA SET_RENDER_ADAPTER: pinned IDD render GPU" - ), - Err(e) => tracing::warn!("SudoVDA SET_RENDER_ADAPTER failed (continuing): {e:#}"), - } - } - let mut device_name = [0u8; 14]; - let nm = b"punktfunk"; - device_name[..nm.len()].copy_from_slice(nm); - let session_guid = next_monitor_guid(); - let add = AddParams { - width: mode.width, - height: mode.height, - refresh: mode.refresh_hz, - guid: session_guid, - device_name, - serial: [0u8; 14], - }; - let add_bytes = unsafe { - std::slice::from_raw_parts(&add as *const _ as *const u8, size_of::()) - }; - let mut out = [0u8; size_of::()]; - unsafe { ioctl(dev, IOCTL_ADD, add_bytes, &mut out) }.with_context(|| { - format!( - "SudoVDA ADD {}x{}@{}", - mode.width, mode.height, mode.refresh_hz - ) - })?; - let ao = unsafe { *(out.as_ptr() as *const AddOut) }; - tracing::info!( - "SudoVDA created {}x{}@{} (target_id={}, adapter_luid={:#x})", - mode.width, - mode.height, - mode.refresh_hz, - ao.target_id, - ao.luid.LowPart - ); - if let Some(luid) = render_luid { - if ao.luid.LowPart == luid.LowPart && ao.luid.HighPart == luid.HighPart { - tracing::info!("SudoVDA ADD render adapter matches the pinned GPU (pin took)"); - } else { - tracing::warn!( - add = format!("{:08x}:{:08x}", ao.luid.HighPart, ao.luid.LowPart), - pinned = format!("{:08x}:{:08x}", luid.HighPart, luid.LowPart), - "SudoVDA ADD render adapter DIFFERS from pinned — driver ignored SET_RENDER_ADAPTER?" - ); - } - } - Ok(AddedMonitor { - key: MonitorKey::Guid(session_guid), - target_id: ao.target_id, - luid: ao.luid, - }) - } - - unsafe fn remove_monitor(&self, dev: HANDLE, key: &MonitorKey) -> Result<()> { - let MonitorKey::Guid(guid) = key else { - anyhow::bail!("sudovda: unexpected monitor key kind"); - }; - let rp = RemoveParams { guid: *guid }; - let rp_bytes = unsafe { - std::slice::from_raw_parts(&rp as *const _ as *const u8, size_of::()) - }; - let mut none: [u8; 0] = []; - unsafe { ioctl(dev, IOCTL_REMOVE, rp_bytes, &mut none) }.map(|_| ()) - } - - unsafe fn ping(&self, dev: HANDLE) -> Result<()> { - let mut none: [u8; 0] = []; - unsafe { ioctl(dev, IOCTL_DRIVER_PING, &[], &mut none) }.map(|_| ()) - } -} - -/// The Windows SudoVDA virtual-display backend. A marker — the lifecycle lives in the shared -/// [`VirtualDisplayManager`](super::manager::VirtualDisplayManager). -pub struct SudoVdaDisplay; - -impl SudoVdaDisplay { - pub fn new() -> Result { - super::manager::init(Box::new(SudoVdaDriver)).open_backend()?; - Ok(Self) - } -} - -impl VirtualDisplay for SudoVdaDisplay { - fn name(&self) -> &'static str { - "sudovda" - } - - fn create(&mut self, mode: Mode) -> Result { - super::manager::vdm().acquire(mode) - } -} - -/// Readiness probe: can we open the SudoVDA control device? -pub fn probe() -> Result<()> { - let h = unsafe { open_device()? }; - unsafe { - let _ = CloseHandle(h); - } - Ok(()) -} - -/// Is the SudoVDA driver present (device interface enumerable)? -pub fn is_available() -> bool { - unsafe { open_device().map(|h| CloseHandle(h)).is_ok() } -} - -#[cfg(test)] -mod tests { - use super::*; - use std::thread; - use std::time::Duration; - - /// Live hardware round trip — skipped unless `PUNKTFUNK_SUDOVDA_LIVE=1` (needs the SudoVDA - /// driver installed). Exercises the real trait path: open -> create -> hold -> drop (REMOVE). - #[test] - fn live_create_drop() { - if std::env::var("PUNKTFUNK_SUDOVDA_LIVE").is_err() { - return; - } - let mut vd = SudoVdaDisplay::new().expect("open SudoVDA"); - let vout = vd - .create(Mode { - width: 1920, - height: 1080, - refresh_hz: 60, - }) - .expect("create virtual display"); - assert_eq!(vout.preferred_mode, Some((1920, 1080, 60))); - thread::sleep(Duration::from_secs(3)); - drop(vout); // triggers REMOVE + stops the pinger - } -} diff --git a/crates/punktfunk-host/src/windows/win_adapter.rs b/crates/punktfunk-host/src/windows/win_adapter.rs index ecf8472..912d1d3 100644 --- a/crates/punktfunk-host/src/windows/win_adapter.rs +++ b/crates/punktfunk-host/src/windows/win_adapter.rs @@ -3,8 +3,8 @@ //! The discrete render-GPU LUID picker used to live in the SudoVDA backend (`vdisplay::sudovda`) — a //! historical accident, since it is display-utility, not SudoVDA-specific. It lives here so the capturers //! (IDD-push) and the pf-vdisplay backend depend on it as a *peer* instead of reaching into the SudoVDA -//! module — breaking that circular reach-in so SudoVDA can eventually be dropped without losing this -//! helper (audit §9 / Goal 2). This is the plan's `windows/adapter.rs`. +//! module — breaking that circular reach-in, which let the SudoVDA backend be dropped without losing this +//! helper (audit §9 / Goal 2 — done). This is the plan's `windows/adapter.rs`. use windows::Win32::Foundation::LUID; diff --git a/crates/punktfunk-host/src/windows/win_display.rs b/crates/punktfunk-host/src/windows/win_display.rs index dcd158e..bdd6cd8 100644 --- a/crates/punktfunk-host/src/windows/win_display.rs +++ b/crates/punktfunk-host/src/windows/win_display.rs @@ -5,8 +5,8 @@ //! These are display-utility, NOT SudoVDA-specific (a pf-vdisplay monitor's target_id is a real OS target //! id, so they operate identically), so they live here rather than in the SudoVDA backend — breaking the //! circular reach-in where the capturers + the pf-vdisplay backend reached into `vdisplay::sudovda` for -//! them, so SudoVDA can eventually be dropped without losing them (audit §9 / Goal 2). The plan's -//! `windows/display_ccd.rs`. Moved verbatim from `vdisplay::sudovda`. +//! them, which let the SudoVDA backend be dropped without losing them (audit §9 / Goal 2 — done). The +//! plan's `windows/display_ccd.rs`. Extracted verbatim from the former SudoVDA backend before its removal. use std::mem::size_of; diff --git a/docs/windows-host-rewrite.md b/docs/windows-host-rewrite.md index 15453c2..7d2c84a 100644 --- a/docs/windows-host-rewrite.md +++ b/docs/windows-host-rewrite.md @@ -33,7 +33,7 @@ which kept the live-validated host working at every step. The driver, by contras | Item | Status | Evidence | |---|---|---| | **Goal 1** — clean, layered host architecture | ✅ **DONE** | `config.rs` (`HostConfig`), `session_plan.rs` (`SessionPlan`), `SessionContext`, `windows/`+`linux/` confinement (`38c68c3`), `VirtualDisplayManager` (§2.5), `EncoderCaps` (`0ccd0fe`) | -| **Goal 2** — drop every trace of SudoVDA | 🟡 **PARTIAL** | reach-in **decoupled** (F1: `d638a93`/`e60cda3` → `win_adapter`/`win_display`); `sudovda.rs` still present as a fallback backend — deletable now, not yet deleted | +| **Goal 2** — drop every trace of SudoVDA | ✅ **DONE** | reach-in decoupled (F1: `d638a93`/`e60cda3` → `win_adapter`/`win_display`), then the `sudovda.rs` backend + the dual-backend select **deleted** (this branch) — pf-vdisplay is the sole Windows virtual-display backend | | **Goal 3** — minimize `unsafe` + P0 lints | 🟡 **PARTIAL** | driver `deny(unsafe_op_in_unsafe_fn)` (`a755d6e`); host crate has **no** P0 lints yet; `OwnedHandle` adopted in `manager.rs`/`pf_vdisplay.rs`/`sudovda.rs`, **not** `idd_push.rs` | | **M0** — proto ABI + driver toolchain + `/INTEGRITYCHECK` + `iddcx` | ✅ **DONE** | `pf-vdisplay-proto`; vendored `windows-drivers-rs` 0.5.1; `clear-force-integrity.ps1`; CI-green | | **M1** — new IddCx driver, first light + HDR | ✅ **DONE (on-glass)** | STEP 0–8 (`d7a9fbf`…`cd59151`); HDR live ("Mac connects WITH HDR", `6399d28`) | @@ -220,21 +220,33 @@ These are expensive empirical wins; keep them intact when touching the code: sustained churn on the RTX box, so this needs an **on-glass reconnect-storm A/B** to confirm (the box is ephemeral). Keep `packaging/windows/reset-pf-vdisplay.ps1` as the recovery until validated. -**P2 — hygiene / architecture completion** +**P2 — hygiene / architecture completion** (the unsafe-reduction + stability priority) 4. **D1-host — host-crate P0 lints.** Add `#![deny(unsafe_op_in_unsafe_fn)]` + - `#![warn(clippy::undocumented_unsafe_blocks)]` to the host crate and fix the fallout (large, mechanical, - touches Linux + Windows `unsafe`). Do it incrementally per-subsystem. The driver already has the deny. -5. **D2 — `OwnedHandle` in `idd_push.rs`.** `map`/`event`/`dbg_map` are still raw `HANDLE` closed in `Drop`; - wrap in `std::os::windows::io::OwnedHandle` (RAII close, fixes leak-on-error). `manager.rs` already shows - the pattern. -6. **Goal 2 — delete `sudovda.rs`.** The reach-in is fully decoupled (F1); the backend is now a thin, - deletable fallback. Retire it (and the `PUNKTFUNK_VDISPLAY=sudovda` path) to finish "drop SudoVDA." -7. **E1 — finish the driver ownership refactor.** Move the process-globals - (`MONITOR_MODES`/`NEXT_ID`/`ADAPTER`/`DEVICE_POOL`) into a `DeviceContext`; wire `EvtCleanupCallback` on - the `IDDCX_MONITOR` object (today only the WDFDEVICE has it); collapse the 3-key monitor identity. This - is the prerequisite to `max_concurrent>1` on Windows + removes the host-side preempt dance. **On-glass - gated** (must instrument that `MonitorContext::Drop` actually fires on this UMDF/IddCx stack; keep the - explicit REMOVE path as fallback if it doesn't). + `#![warn(clippy::undocumented_unsafe_blocks)]` to the host crate and fix the fallout (~30 of the 52 + `unsafe fn`s need an inner `unsafe {}`). Stage it **per-module, Linux-first** (item-level `#[deny]` on + `linux/zerocopy/cuda.rs`/`egl.rs`, `encode/linux/vaapi.rs` — locally verifiable), then the Windows + modules (CI-gated), then promote to crate-level. The driver already has the deny. +5. **D2 — `OwnedHandle` rollout.** Highest-impact first: `windows/service.rs` (the `AtomicIsize` + STOP/SESSION event smuggling + the Job + the `PROCESS_INFORMATION` handles across 5 cleanup arms — deletes + ~15 manual `CloseHandle`), then `capture/windows/idd_push.rs` (`map`/`event`/`dbg_map`), then the gamepad + shm handles. `manager.rs`/`pf_vdisplay.rs` already use `OwnedHandle` (the pattern). RAII close + fixes + leak-on-error. +6. **Driver unsafe levers** (the driver is already `deny`-clean with per-site SAFETY; these *reduce count*): + a `pod_init!` macro for the ~11 `mem::zeroed()` POD inits (Linux-verifiable as a macro), one audited + `ThreadBound`/`SendPtr` replacing the 8 scattered `unsafe impl Send`, a generic IOCTL dispatch + helper in `control.rs`, and a `KeyedMutexGuard`/`AcquiredSurface` RAII for the frame-transport hot loop + (needs an on-glass latency check). ~157 → ~105. +7. **M6 scaffolding cleanup** — delete the bring-up diagnostics (`spawn_observer`/`DebugBlock` in + `idd_push.rs`) and, once full parity is proven on glass, the host monoliths. + +**Explicitly NOT doing (stability decision): E1 — driver `DeviceContext` ownership + per-`IDDCX_MONITOR` +`EvtCleanupCallback`.** The current process-global design is *sound*: IddCx DDIs receive only an +`IDDCX_MONITOR` handle (never the WDFDEVICE/context), and `ProcessSharingDisabled` makes one devnode = one +host process that dies with the device. A "device-owned" variant would *add* a use-after-free window (the +watchdog races device cleanup) for no gain, and the per-monitor cleanup callback isn't reliably reachable +on this UMDF/IddCx stack. Cleanup is already deterministic (WDFDEVICE `EvtCleanupCallback` + +`cleanup_for_device_removal` + the host-gone watchdog). **Revisit only if `max_concurrent>1` on Windows is +actually needed.** (`monitor.rs` documents this rationale at the `MONITOR_MODES` static.) 8. **M6 scaffolding cleanup** — delete the bring-up diagnostics (`spawn_observer`/`DebugBlock` in `idd_push.rs`) and, once full parity is proven on glass, the host monoliths. diff --git a/scripts/windows/host.env.example b/scripts/windows/host.env.example index a86cd39..2549fb9 100644 --- a/scripts/windows/host.env.example +++ b/scripts/windows/host.env.example @@ -19,8 +19,8 @@ PUNKTFUNK_ENCODER=auto # refresh — the flagship mode. Requires the bundled pf-vdisplay indirect display driver installed. PUNKTFUNK_VIDEO_SOURCE=virtual -# Virtual-display backend: `pf` = the all-Rust pf-vdisplay IddCx driver the installer bundles (the -# shipped driver; leave as the default). `sudovda` selects the legacy backend if one is present. +# Virtual-display backend: the all-Rust pf-vdisplay IddCx driver the installer bundles is the only +# backend now (the legacy SudoVDA backend was removed). This is informational; leave it as `pf`. PUNKTFUNK_VDISPLAY=pf # Capture straight from the pf-vdisplay driver's shared ring — the validated zero-copy path (incl. the