From 0a7ae5ef094a38635ca0bdd58143a9f4787355a2 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Thu, 25 Jun 2026 12:49:49 +0000 Subject: [PATCH] feat(windows-drivers): host-gone watchdog, SET_RENDER_ADAPTER, log gate, mode bounds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit §4.1: implement the host-gone watchdog — it was dead code (WATCHDOG_PINGS bumped but never sampled, no thread). Every IOCTL now bumps a liveness counter; a watchdog thread reap_orphaned()s monitors (created_at grace) if no IOCTL arrives within WATCHDOG_TIMEOUT_S, so a crashed/TerminateProcess'd host no longer leaves its virtual monitor + swap-chain worker + pooled D3D device wedged until the next CLEAR_ALL. Removes the false 'watchdog thread' comments. Audit §4.2: implement SET_RENDER_ADAPTER (was STATUS_NOT_IMPLEMENTED) via IddCxAdapterSetRenderAdapter, so the host can pin the IDD render to the NVENC GPU on a hybrid iGPU+dGPU box (else the OS-picked iGPU makes the host ring textures un-openable -> DRV_STATUS_TEX_FAIL). Audit §4.4: gate the world-writable C:\Users\Public\pfvd-driver.log behind debug builds / PFVD_DEBUG_LOG (a release build never writes it). Audit §4.5: bounds-check the requested mode in IOCTL_ADD; compute display_info clock_rate in u64 + saturate (the old u32 refresh*(h+4)^2 overflowed/aborted the mode DDI for large modes). Verified: driver workspace builds clean on the RTX box (WDK 26100 + LLVM 21.1.2, MSVC). On-glass functional validation of the watchdog/render-pin is a follow-up (needs a driver reinstall + session). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../drivers/pf-vdisplay/src/adapter.rs | 23 +++++ .../drivers/pf-vdisplay/src/callbacks.rs | 1 + .../drivers/pf-vdisplay/src/control.rs | 94 +++++++++++++++++-- .../windows/drivers/pf-vdisplay/src/log.rs | 19 +++- .../drivers/pf-vdisplay/src/monitor.rs | 58 +++++++++++- 5 files changed, 177 insertions(+), 18 deletions(-) diff --git a/packaging/windows/drivers/pf-vdisplay/src/adapter.rs b/packaging/windows/drivers/pf-vdisplay/src/adapter.rs index bbb3601..6d98ada 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/adapter.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/adapter.rs @@ -142,3 +142,26 @@ pub fn set_adapter(adapter: iddcx::IDDCX_ADAPTER) { pub(crate) fn adapter() -> Option { ADAPTER.get().map(|a| a.0) } + +/// Honor the host's `IOCTL_SET_RENDER_ADAPTER`: pin the GPU the IddCx swap-chain renders on. On a hybrid +/// iGPU+dGPU box the OS may otherwise pick the iGPU to render the virtual monitor, so the host's shared +/// ring textures (created on the NVENC dGPU) can't be opened → `DRV_STATUS_TEX_FAIL` → the host's 20 s +/// black bail. Pinning the render adapter to the encode GPU fixes that. Unconditional — NOT the +/// SudoVDA-parity default-off branch (`docs/windows-host-rewrite-audit.md` §4.2). Returns +/// `STATUS_NOT_FOUND` if called before the adapter exists. +pub fn set_render_adapter(luid_low: u32, luid_high: i32) -> NTSTATUS { + let Some(adapter) = adapter() else { + return crate::STATUS_NOT_FOUND; + }; + // SAFETY: building a C POD — the all-zero bit pattern is a valid IDARG_IN_ADAPTERSETRENDERADAPTER; + // the one meaningful field is assigned below. + let mut in_args: iddcx::IDARG_IN_ADAPTERSETRENDERADAPTER = unsafe { core::mem::zeroed() }; + in_args.PreferredRenderAdapter = wdk_sys::LUID { + LowPart: luid_low, + HighPart: luid_high, + }; + dbglog!("[pf-vd] set_render_adapter -> {luid_high:08x}:{luid_low:08x}"); + // SAFETY: `adapter` is the stashed IddCx adapter; `in_args` is valid local storage read synchronously. + unsafe { wdk_iddcx::IddCxAdapterSetRenderAdapter(adapter, &in_args) }; + STATUS_SUCCESS +} diff --git a/packaging/windows/drivers/pf-vdisplay/src/callbacks.rs b/packaging/windows/drivers/pf-vdisplay/src/callbacks.rs index 1d067a0..3d1788f 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/callbacks.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/callbacks.rs @@ -33,6 +33,7 @@ pub unsafe extern "C" fn adapter_init_finished( ) -> NTSTATUS { dbglog!("[pf-vd] adapter_init_finished"); crate::adapter::set_adapter(adapter); + crate::control::start_watchdog(); STATUS_SUCCESS } diff --git a/packaging/windows/drivers/pf-vdisplay/src/control.rs b/packaging/windows/drivers/pf-vdisplay/src/control.rs index 51bc90d..7aa5d39 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/control.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/control.rs @@ -3,25 +3,76 @@ //! (watchdog keepalive), ADD/REMOVE/CLEAR_ALL (virtual monitors), and SET_RENDER_ADAPTER (next). Every //! path completes the `WDFREQUEST` exactly once (the `EVT_IDD_CX_DEVICE_IO_CONTROL` shape returns `()`). -use core::sync::atomic::{AtomicU64, Ordering}; +use core::sync::atomic::{AtomicBool, AtomicU64, Ordering}; +use std::time::{Duration, Instant}; use pf_vdisplay_proto::control; use wdk_iddcx::nt_success; use wdk_sys::{NTSTATUS, WDFREQUEST, call_unsafe_wdf_function_binding}; -use crate::{STATUS_INVALID_PARAMETER, STATUS_NOT_FOUND, STATUS_NOT_IMPLEMENTED, STATUS_SUCCESS}; +use crate::{STATUS_INVALID_PARAMETER, STATUS_NOT_FOUND, STATUS_SUCCESS}; -/// The host must PING within this window or the watchdog reaps all monitors (STEP 4: the watchdog thread). +/// The host must send an IOCTL within this window (it PINGs on a `timeout/3` timer) or the watchdog +/// treats it as gone and reaps every monitor. Reported to the host via [`control::IOCTL_GET_INFO`]. const WATCHDOG_TIMEOUT_S: u32 = 10; -/// Keepalive counter — PING bumps it; STEP 4's watchdog thread samples it to detect a gone host. +/// Host-liveness counter — EVERY inbound IOCTL bumps it; [`start_watchdog`]'s thread samples it. static WATCHDOG_PINGS: AtomicU64 = AtomicU64::new(0); +/// Spawns the watchdog thread exactly once (idempotent across re-entrant adapter inits). +static WATCHDOG_STARTED: AtomicBool = AtomicBool::new(false); + +/// Start the host-liveness watchdog (once, from `adapter_init_finished`). +/// +/// Previously [`WATCHDOG_PINGS`] was bumped but NEVER sampled (no thread existed) — so a host that died +/// without a cooperative REMOVE (crash / `TerminateProcess`) left its virtual monitor + swap-chain +/// worker + pooled D3D device wedged in WUDFHost until the next host start's CLEAR_ALL, and a +/// not-restarted host left the orphan monitor in the desktop topology indefinitely +/// (`docs/windows-host-rewrite-audit.md` §4.1). This thread closes that: if no IOCTL arrives for +/// `WATCHDOG_TIMEOUT_S` while monitors exist, it departs them all. +/// +/// (A WDF `EvtFileClose` on the control handle would be more immediate — the plan's preferred §3.4 +/// option — but the polling watchdog matches the proven oracle and needs no IddCx file-object plumbing.) +pub fn start_watchdog() { + if WATCHDOG_STARTED.swap(true, Ordering::SeqCst) { + return; + } + let tick = Duration::from_secs(u64::from((WATCHDOG_TIMEOUT_S / 3).max(1))); + let timeout = Duration::from_secs(u64::from(WATCHDOG_TIMEOUT_S)); + std::thread::spawn(move || { + let mut last = WATCHDOG_PINGS.load(Ordering::Relaxed); + let mut last_change = Instant::now(); + loop { + std::thread::sleep(tick); + let cur = WATCHDOG_PINGS.load(Ordering::Relaxed); + if cur != last { + last = cur; + last_change = Instant::now(); + continue; + } + // No IOCTL since `last_change`. A live host PINGs every `timeout/3`, so this only trips once + // the host is truly gone; only reap when there's something to reap. + if last_change.elapsed() >= timeout && crate::monitor::has_monitors() { + let n = crate::monitor::reap_orphaned(Duration::from_secs(3)); + if n > 0 { + dbglog!( + "[pf-vd] watchdog: no host IOCTL in {WATCHDOG_TIMEOUT_S}s — host gone, departed {n} monitor(s)" + ); + } + last_change = Instant::now(); // don't re-reap every tick + } + } + }); +} /// Dispatch one control IOCTL and complete the request. /// /// # Safety /// `request` is the framework-provided `WDFREQUEST` for an `EvtIddCxDeviceIoControl` call. pub unsafe fn dispatch(request: WDFREQUEST, ioctl_code: u32) { + // Every inbound IOCTL is host liveness (the host PINGs on a timer, plus ADD/REMOVE/GET_INFO/…) — + // bump the watchdog at the top so it only fires once the host has gone truly silent. See + // [`start_watchdog`]. + WATCHDOG_PINGS.fetch_add(1, Ordering::Relaxed); match ioctl_code { control::IOCTL_GET_INFO => { let reply = control::InfoReply { @@ -31,10 +82,7 @@ pub unsafe fn dispatch(request: WDFREQUEST, ioctl_code: u32) { // SAFETY: `request` is the framework WDFREQUEST. unsafe { write_output_complete(request, &reply) }; } - control::IOCTL_PING => { - WATCHDOG_PINGS.fetch_add(1, Ordering::Relaxed); - complete(request, STATUS_SUCCESS); - } + control::IOCTL_PING => complete(request, STATUS_SUCCESS), // SAFETY: `request` is the framework WDFREQUEST. control::IOCTL_ADD => unsafe { add(request) }, // SAFETY: `request` is the framework WDFREQUEST. @@ -43,12 +91,34 @@ pub unsafe fn dispatch(request: WDFREQUEST, ioctl_code: u32) { crate::monitor::clear_all(); complete(request, STATUS_SUCCESS); } - // SET_RENDER_ADAPTER (hybrid-GPU render pin): STEP 4 (next). - control::IOCTL_SET_RENDER_ADAPTER => complete(request, STATUS_NOT_IMPLEMENTED), + // SAFETY: `request` is the framework WDFREQUEST. + control::IOCTL_SET_RENDER_ADAPTER => unsafe { set_render_adapter(request) }, _ => complete(request, STATUS_NOT_FOUND), } } +/// Sanity bounds for a requested mode — generous (covers any real client) but rejects zero/absurd +/// values that would otherwise feed the EDID/mode math unchecked. +fn valid_mode(width: u32, height: u32, refresh_hz: u32) -> bool { + (1..=16384).contains(&width) + && (1..=16384).contains(&height) + && (1..=1000).contains(&refresh_hz) +} + +/// `IOCTL_SET_RENDER_ADAPTER`: pin the IddCx render adapter (hybrid-GPU IDD-push). +/// +/// # Safety +/// `request` is the framework `WDFREQUEST`. +unsafe fn set_render_adapter(request: WDFREQUEST) { + // SAFETY: `request` is the framework WDFREQUEST. + let Some(req) = (unsafe { read_input::(request) }) else { + complete(request, STATUS_INVALID_PARAMETER); + return; + }; + let st = crate::adapter::set_render_adapter(req.luid_low, req.luid_high); + complete(request, st); +} + /// `IOCTL_ADD`: create a virtual monitor at the requested mode → reply with the OS target id + LUID. /// /// # Safety @@ -59,6 +129,10 @@ unsafe fn add(request: WDFREQUEST) { complete(request, STATUS_INVALID_PARAMETER); return; }; + if !valid_mode(req.width, req.height, req.refresh_hz) { + complete(request, STATUS_INVALID_PARAMETER); + return; + } let Some((target_id, luid_low, luid_high)) = crate::monitor::create_monitor(req.session_id, req.width, req.height, req.refresh_hz) else { diff --git a/packaging/windows/drivers/pf-vdisplay/src/log.rs b/packaging/windows/drivers/pf-vdisplay/src/log.rs index add0975..617bd36 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/log.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/log.rs @@ -1,16 +1,29 @@ -//! Minimal driver logger (matches the DualSense driver). DebugView can't capture the UMDF host across -//! session 0, so besides `OutputDebugStringA` we append to a world-writable file readable over SSH. Used -//! only for bring-up/diagnostics; cheap and best-effort (ignores all errors). +//! Minimal driver logger. `OutputDebugStringA` always (ETW/DebugView); the optional world-writable file +//! (`C:\Users\Public\pfvd-driver.log`, readable over SSH) is now OPT-IN — debug builds, or the +//! `PFVD_DEBUG_LOG` env var, only — so a RELEASE build never writes it (audit §4.4: it was an +//! info-leak/DoS surface). Best-effort; ignores all errors. Production driver-state visibility is the +//! SharedHeader `driver_status` channel, not this file. unsafe extern "system" { fn OutputDebugStringA(s: *const u8); } +/// Whether the world-writable bring-up file log is enabled (resolved once). Off in release builds unless +/// `PFVD_DEBUG_LOG` is set. +fn file_log_enabled() -> bool { + use std::sync::OnceLock; + static ON: OnceLock = OnceLock::new(); + *ON.get_or_init(|| cfg!(debug_assertions) || std::env::var_os("PFVD_DEBUG_LOG").is_some()) +} + pub fn log(s: &str) { if let Ok(c) = std::ffi::CString::new(s) { // SAFETY: `c` is a valid NUL-terminated string for the duration of the call. unsafe { OutputDebugStringA(c.as_ptr().cast()) }; } + if !file_log_enabled() { + return; + } use std::io::Write; if let Ok(mut f) = std::fs::OpenOptions::new() .create(true) diff --git a/packaging/windows/drivers/pf-vdisplay/src/monitor.rs b/packaging/windows/drivers/pf-vdisplay/src/monitor.rs index b1d0d24..3917cb6 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/monitor.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/monitor.rs @@ -6,7 +6,7 @@ use std::sync::Mutex; use std::sync::atomic::{AtomicU32, Ordering}; -use std::time::Instant; +use std::time::{Duration, Instant}; use wdk_sys::iddcx; @@ -64,6 +64,50 @@ pub static MONITOR_MODES: Mutex> = Mutex::new(Vec::new()); /// Monitor id / EDID-serial counter (unique per created monitor). static NEXT_ID: AtomicU32 = AtomicU32::new(1); +/// True if any virtual monitor currently exists — the host-gone watchdog only reaps when there's +/// something to reap (see [`crate::control::start_watchdog`]). +pub fn has_monitors() -> bool { + MONITOR_MODES.lock().map(|l| !l.is_empty()).unwrap_or(false) +} + +/// Depart every monitor that has existed at least `grace` — the host-gone watchdog reap +/// ([`crate::control::start_watchdog`]). The grace skips a just-created monitor (the host adds it, then +/// starts pinging) so a momentarily-stale ping timer can't nuke a brand-new monitor. Returns the count +/// departed. Same lock discipline as [`remove_monitor`]: drop each worker (which RAII-joins its thread) +/// OUTSIDE the `MONITOR_MODES` lock, then depart. +pub fn reap_orphaned(grace: Duration) -> usize { + let mut drained: Vec<( + Option, + Option, + )> = { + let Ok(mut lock) = MONITOR_MODES.lock() else { + return 0; + }; + let mut taken = Vec::new(); + let mut i = 0; + while i < lock.len() { + if lock[i].created_at.elapsed() >= grace { + let mut m = lock.remove(i); + taken.push((m.object, m.swap_chain_processor.take())); + } else { + i += 1; + } + } + taken + }; + let n = drained.len(); + for (_, processor) in &mut drained { + drop(processor.take()); + } + for (object, _) in drained { + if let Some(m) = object { + // SAFETY: `m` is a live IddCx monitor handle; departure tears it down. + unsafe { wdk_iddcx::IddCxMonitorDeparture(m) }; + } + } + n +} + /// Fallback modes appended after the requested mode, so a topology change still has options. fn default_modes() -> Vec { vec![ @@ -86,17 +130,21 @@ pub fn display_info( height: u32, refresh_rate: u32, ) -> wdk_sys::DISPLAYCONFIG_VIDEO_SIGNAL_INFO { - let clock_rate = refresh_rate * (height + 4) * (height + 4) + 1000; + // Compute in u64 then saturate the u32 rational numerators: the old u32 `refresh*(h+4)^2` overflows + // for a large mode (e.g. 8K@240), which panics→aborts the extern-"C" mode DDI in a debug build. + // Identical for every real mode; only an absurd (also now bounds-rejected) mode saturates. + let clock_rate: u64 = u64::from(refresh_rate) * u64::from(height + 4) * u64::from(height + 4) + 1000; + let clock_rate_u32 = u32::try_from(clock_rate).unwrap_or(u32::MAX); // SAFETY: building a C POD — the all-zero bit pattern is a valid uninitialized // DISPLAYCONFIG_VIDEO_SIGNAL_INFO; every meaningful field is assigned below. let mut si: wdk_sys::DISPLAYCONFIG_VIDEO_SIGNAL_INFO = unsafe { core::mem::zeroed() }; - si.pixelRate = u64::from(clock_rate); + si.pixelRate = clock_rate; si.hSyncFreq = wdk_sys::DISPLAYCONFIG_RATIONAL { - Numerator: clock_rate, + Numerator: clock_rate_u32, Denominator: height + 4, }; si.vSyncFreq = wdk_sys::DISPLAYCONFIG_RATIONAL { - Numerator: clock_rate, + Numerator: clock_rate_u32, Denominator: (height + 4) * (height + 4), }; si.activeSize = wdk_sys::DISPLAYCONFIG_2DREGION {