From c94a81d523f1abda13d29a6de18882f73d716e5e Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Wed, 24 Jun 2026 23:16:25 +0000 Subject: [PATCH] chore(windows-drivers): clean up STEP-3 debugging artifacts; restore device interface Verified on-glass after cleanup: adapter still inits (IddCxAdapterInitAsync 0x0, Status OK) and WdfDeviceCreateDeviceInterface 0x0. - RESTORE WdfDeviceCreateDeviceInterface (regression from debugging): the proto control plane sends IOCTLs via EvtIddCxDeviceIoControl, which needs the device interface for the host to open. Upstream omits it only because it uses a socket; ours is IOCTL-based. - Drop the framework_struct_size / version-table machinery + size.rs: size_of suffices (these are IddCx 1.10 structs on a 1.10 framework, matching upstream). The version-table reads were added chasing a size mismatch that was never the bug (GammaSupport was). - Drop /OPT:NOICF (ICF folding was a non-issue) + fix the stale stub-pick comment (the 1.10 stub is needed for the dispatch table, not size.rs symbols). - Debug-wait/PID-file/go-file gate already removed. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../windows/drivers/pf-vdisplay/build.rs | 14 ++--- .../drivers/pf-vdisplay/src/adapter.rs | 33 +++-------- .../windows/drivers/pf-vdisplay/src/entry.rs | 38 +++++++----- .../windows/drivers/pf-vdisplay/src/lib.rs | 1 - .../windows/drivers/pf-vdisplay/src/size.rs | 59 ------------------- 5 files changed, 34 insertions(+), 111 deletions(-) delete mode 100644 packaging/windows/drivers/pf-vdisplay/src/size.rs diff --git a/packaging/windows/drivers/pf-vdisplay/build.rs b/packaging/windows/drivers/pf-vdisplay/build.rs index e2ae2cc..be62abe 100644 --- a/packaging/windows/drivers/pf-vdisplay/build.rs +++ b/packaging/windows/drivers/pf-vdisplay/build.rs @@ -4,19 +4,13 @@ fn main() -> Result<(), wdk_build::ConfigError> { wdk_build::configure_wdk_binary_build()?; link_iddcx_stub(); - // wdk-build emits `/OPT:REF,ICF`. ICF (Identical COMDAT Folding) merges functions with identical - // bodies into ONE address — our many identical stub IddCx callbacks (`return STATUS_SUCCESS`) collapse - // to the same pointer (and even CRT EH handlers fold, hence the dumpbin `__CxxFrameHandler4 = DllMain` - // alias). The working virtual-display-rs links with NO `/OPT`, and IddCxAdapterInitAsync rejects a - // config whose distinct callbacks alias each other. Disable ICF (REF stays on); last `/OPT` wins. - println!("cargo::rustc-cdylib-link-arg=/OPT:NOICF"); Ok(()) } -/// Link `IddCxStub.lib`. It ships under `Lib\\um\\iddcx\\`, and the iddcx -/// versions are NOT equivalent: the `1.0`/`1.2` stubs lack the versioned-struct-size table symbols -/// (`IddStructures`/`IddStructureCount`/`IddClientVersionHigherThanFramework`) that `size.rs` needs — -/// `1.3`+ and `1.10` have them. So pick the HIGHEST `iddcx\` dir that has the lib (version-aware, +/// Link `IddCxStub.lib`. It ships under `Lib\\um\\iddcx\\`, and the iddcx versions +/// are NOT interchangeable: the stub's `IddFunctions` dispatch table must match the running framework +/// (linking the `1.0` stub made even IddCxDeviceInitConfig fail; the box framework is 1.10, and upstream +/// virtual-display-rs pins 1.10). So pick the HIGHEST `iddcx\` dir that has the lib (version-aware, /// since "1.10" < "1.2" lexically). x64 only. fn link_iddcx_stub() { const ARCH: &str = "x64"; diff --git a/packaging/windows/drivers/pf-vdisplay/src/adapter.rs b/packaging/windows/drivers/pf-vdisplay/src/adapter.rs index 5c6b54a..852746e 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/adapter.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/adapter.rs @@ -40,10 +40,9 @@ unsafe impl Sync for SendAdapter {} static ADAPTER: OnceLock = OnceLock::new(); -/// A WDF context type for the adapter object. SudoVDA/the oracle create the adapter via -/// `WDF_OBJECT_ATTRIBUTES::init_context_type(..)`; passing attributes with NO `ContextTypeInfo` is the one -/// structural difference left vs the working SudoVDA driver. `WDF_OBJECT_CONTEXT_TYPE_INFO` holds raw -/// pointers (so a Sync wrapper to allow a `static`); `UniqueType` self-references per `WDF_DECLARE_CONTEXT_TYPE`. +/// A WDF context type for the adapter object (matches the upstream's `init_context_type`); STEP 4 stores +/// adapter state here. `WDF_OBJECT_CONTEXT_TYPE_INFO` holds raw pointers (so a Sync wrapper to allow a +/// `static`); `UniqueType` self-references per `WDF_DECLARE_CONTEXT_TYPE`. #[repr(transparent)] struct CtxTypeInfo(wdk_sys::WDF_OBJECT_CONTEXT_TYPE_INFO); // SAFETY: immutable 'static type metadata; the inner raw pointers are 'static and never written. @@ -64,27 +63,11 @@ pub fn init_adapter(device: WDFDEVICE) -> NTSTATUS { } dbglog!("[pf-vd] init_adapter"); - // The framework binds an IddCx version (the INF's UmdfExtensions) that may be OLDER than our 1.10 - // headers, so use ITS expected struct sizes — newer fields (e.g. IDDCX_ADAPTER_CAPS's - // StaticDesktopReencodeFrameCount) make size_of too big and IddCxAdapterInitAsync rejects it. The - // table is readable now (post-IddCxDeviceInitialize). Falls back to size_of if unavailable. - let ver_size = crate::size::framework_struct_size( - iddcx::_IDDSTRUCTENUM::INDEX_IDDCX_ENDPOINT_VERSION as u32, - ) - .unwrap_or(core::mem::size_of::() as u32); - let diag_size = crate::size::framework_struct_size( - iddcx::_IDDSTRUCTENUM::INDEX_IDDCX_ENDPOINT_DIAGNOSTIC_INFO as u32, - ) - .unwrap_or(core::mem::size_of::() as u32); - let caps_size = crate::size::framework_struct_size( - iddcx::_IDDSTRUCTENUM::INDEX_IDDCX_ADAPTER_CAPS as u32, - ) - .unwrap_or(core::mem::size_of::() as u32); - // Firmware/hardware version (telemetry). The oracle points BOTH at one IDDCX_ENDPOINT_VERSION. - // `version` is a stack local read synchronously by IddCxAdapterInitAsync (same as the oracle). + // `version` is a stack local read synchronously by IddCxAdapterInitAsync (same as the oracle). `.Size` + // is `size_of` throughout — these are the IddCx 1.10 structs and the framework here is 1.10 (= upstream). let mut version: iddcx::IDDCX_ENDPOINT_VERSION = unsafe { core::mem::zeroed() }; - version.Size = ver_size; + version.Size = core::mem::size_of::() as u32; version.MajorVer = env!("CARGO_PKG_VERSION_MAJOR").parse().unwrap_or(0); version.MinorVer = env!("CARGO_PKG_VERSION_MINOR").parse().unwrap_or(0); version.Build = env!("CARGO_PKG_VERSION_PATCH").parse().unwrap_or(0); @@ -94,7 +77,7 @@ pub fn init_adapter(device: WDFDEVICE) -> NTSTATUS { // rejects with INVALID_PARAMETER (ddivalidation.cpp:797) — set it to NONE (1) like upstream. THIS was // the on-glass adapter-init blocker. let mut diag: iddcx::IDDCX_ENDPOINT_DIAGNOSTIC_INFO = unsafe { core::mem::zeroed() }; - diag.Size = diag_size; + diag.Size = core::mem::size_of::() as u32; diag.GammaSupport = iddcx::IDDCX_FEATURE_IMPLEMENTATION::IDDCX_FEATURE_IMPLEMENTATION_NONE; diag.TransmissionType = iddcx::IDDCX_TRANSMISSION_TYPE::IDDCX_TRANSMISSION_TYPE_WIRED_OTHER; diag.pEndPointFriendlyName = wstr!("punktfunk Virtual Display Adapter"); @@ -104,7 +87,7 @@ pub fn init_adapter(device: WDFDEVICE) -> NTSTATUS { diag.pHardwareVersion = (&raw mut version).cast(); let mut caps: iddcx::IDDCX_ADAPTER_CAPS = unsafe { core::mem::zeroed() }; - caps.Size = caps_size; + caps.Size = core::mem::size_of::() as u32; // Flags = NONE (SDR). The working upstream virtual-display-rs sets NO Flags. CAN_PROCESS_FP16 requires a // newer IddCx contract than the INF's UmdfExtensions=IddCx0102 grants, so the framework's adapter-caps // Validate (ddivalidation.cpp:797) rejects it with STATUS_INVALID_PARAMETER. HDR/FP16 is deferred to diff --git a/packaging/windows/drivers/pf-vdisplay/src/entry.rs b/packaging/windows/drivers/pf-vdisplay/src/entry.rs index 23cf3a3..a150b9f 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/entry.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/entry.rs @@ -10,7 +10,7 @@ use wdk_sys::{ WDF_NO_OBJECT_ATTRIBUTES, WDF_PNPPOWER_EVENT_CALLBACKS, }; -use crate::{callbacks, size, STATUS_NOT_FOUND}; +use crate::callbacks; /// A WDF device context, attached to the WDFDEVICE at WdfDeviceCreate. The working virtual-display-rs + /// oracle both create the device with a context-typed `DeviceContext` (we previously passed @@ -68,13 +68,9 @@ extern "C" fn driver_add(_driver: WDFDRIVER, mut init: PWDFDEVICE_INIT) -> NTSTA call_unsafe_wdf_function_binding!(WdfDeviceInitSetPnpPowerEventCallbacks, init, &mut pnp); } - // Build + size the IddCx client config (versioned size) and wire the 14 callbacks. - let Some(cfg_size) = size::idd_cx_client_config_size() else { - dbglog!("[pf-vd] config size unavailable"); - return STATUS_NOT_FOUND; - }; + // Build the IddCx client config and wire the SDR callbacks. `.Size` = size_of (1.10 structs, 1.10 fw). let mut cfg: iddcx::IDD_CX_CLIENT_CONFIG = unsafe { core::mem::zeroed() }; - cfg.Size = cfg_size; + cfg.Size = core::mem::size_of::() as u32; cfg.EvtIddCxAdapterInitFinished = Some(callbacks::adapter_init_finished); cfg.EvtIddCxParseMonitorDescription = Some(callbacks::parse_monitor_description); cfg.EvtIddCxMonitorGetDefaultDescriptionModes = Some(callbacks::monitor_get_default_modes); @@ -112,17 +108,27 @@ extern "C" fn driver_add(_driver: WDFDRIVER, mut init: PWDFDEVICE_INIT) -> NTSTA return status; } - // IddCx must be initialized on the device BEFORE other device setup (the canonical IddCx sample order). - // We previously created the device interface first — which can leave IddCx not fully ready by D0Entry, - // making IddCxAdapterInitAsync reject (INVALID_PARAMETER) despite byte-perfect caps. - // DIAGNOSTIC (STEP 3): the WdfDeviceCreateDeviceInterface call is REMOVED. Upstream virtual-display-rs - // and the MS IddCx sample create NO device interface — IddCx's control channel is EvtIddCxDeviceIoControl - // (already registered in the config). A non-IddCx device interface registered on the IddCx/IndirectKmd - // stack is a prime suspect for IddCxAdapterInitAsync -> INVALID_PARAMETER. If removing it fixes adapter - // init, the proto control plane moves to EvtIddCxDeviceIoControl (STEP 4) instead of a device interface. - let _ = pf_vdisplay_proto::interface_guid_fields; // keep the dep referenced // SAFETY: device is the just-created WDFDEVICE. let status = unsafe { wdk_iddcx::IddCxDeviceInitialize(device) }; dbglog!("[pf-vd] IddCxDeviceInitialize -> {status:#x}"); + if !nt_success(status) { + return status; + } + + // Expose the owned pf-vdisplay control interface: the host opens this GUID and drives the proto control + // plane (IOCTL_ADD/REMOVE/PING/…) which arrives at EvtIddCxDeviceIoControl. NOT SudoVDA's GUID. (The + // upstream uses a socket instead, so it has no interface; ours is IOCTL-based.) + let (d1, d2, d3, d4) = pf_vdisplay_proto::interface_guid_fields(); + let guid = GUID { + Data1: d1, + Data2: d2, + Data3: d3, + Data4: d4, + }; + // SAFETY: device is the just-created WDFDEVICE; guid lives for the call; no reference string. + let status = unsafe { + call_unsafe_wdf_function_binding!(WdfDeviceCreateDeviceInterface, device, &guid, core::ptr::null()) + }; + dbglog!("[pf-vd] WdfDeviceCreateDeviceInterface -> {status:#x}"); status } diff --git a/packaging/windows/drivers/pf-vdisplay/src/lib.rs b/packaging/windows/drivers/pf-vdisplay/src/lib.rs index 7ad0442..8faf35e 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/lib.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/lib.rs @@ -17,7 +17,6 @@ mod callbacks; #[allow(dead_code)] // salvaged verbatim; wired into the mode callbacks in STEP 4 mod edid; mod entry; -mod size; use wdk_sys::NTSTATUS; diff --git a/packaging/windows/drivers/pf-vdisplay/src/size.rs b/packaging/windows/drivers/pf-vdisplay/src/size.rs deleted file mode 100644 index ac5193f..0000000 --- a/packaging/windows/drivers/pf-vdisplay/src/size.rs +++ /dev/null @@ -1,59 +0,0 @@ -//! `.Size` for `IDD_CX_CLIENT_CONFIG` — the oracle's `IDD_STRUCTURE_SIZE!`, ported. -//! -//! IddCx structs are versioned. If the client (our headers) is NEWER than the running framework -//! (`IddClientVersionHigherThanFramework != 0`), `size_of` of our struct is too big, so the framework's -//! own `IddStructures[INDEX]` size must be used. Otherwise `size_of` is correct. `IddStructures` is null -//! until IddCx initialises, so it must ONLY be read in the `higher` branch (the framework populates it -//! exactly then). build.rs links the `iddcx>=1.3` stub that exports these symbols. Logged so we can see -//! which branch + value the box actually takes. - -use wdk_sys::iddcx; - -/// The correct `.Size` for `IDD_CX_CLIENT_CONFIG`, or `None` if the struct is unusable on this framework. -#[must_use] -pub fn idd_cx_client_config_size() -> Option { - let local = core::mem::size_of::(); - // SAFETY: BOOLEAN static, read-only. - let higher = unsafe { (&raw const iddcx::IddClientVersionHigherThanFramework).read() } != 0; - dbglog!("[pf-vd] cfg size: higher={higher} size_of={local}"); - if !higher { - return u32::try_from(local).ok(); - } - // SAFETY: read-only; the framework populates the size table exactly when `higher` is true. - let count = unsafe { (&raw const iddcx::IddStructureCount).read() }; - let index = iddcx::_IDDSTRUCTENUM::INDEX_IDD_CX_CLIENT_CONFIG as u32; - if index >= count { - dbglog!("[pf-vd] cfg size: index {index} >= count {count} -> None"); - return None; - } - // SAFETY: `IddStructures` is the framework's size table; `index` validated `< count`. - let table = unsafe { (&raw const iddcx::IddStructures).read() }; - let fw = unsafe { table.add(index as usize).read() }; - dbglog!("[pf-vd] cfg size: framework={fw}"); - u32::try_from(fw).ok() -} - -/// The framework's expected `.Size` for the `_IDDSTRUCTENUM` struct at `index`, or `None` if the size -/// table isn't usable (index out of range / `IddStructures` not yet populated). The framework binds a -/// specific IddCx version (the INF's `UmdfExtensions`), which can be OLDER than our headers — newer -/// fields (e.g. `IDDCX_ADAPTER_CAPS::StaticDesktopReencodeFrameCount`) make `size_of` too big and -/// `IddCxAdapterInitAsync` rejects it; this returns the size the framework actually expects. -/// -/// Only call once the IddCx context is initialised (e.g. from `EvtDeviceD0Entry`, after -/// `IddCxDeviceInitialize`) — `IddStructures` may be null before that. -#[must_use] -pub fn framework_struct_size(index: u32) -> Option { - // SAFETY: read-only. - let count = unsafe { (&raw const iddcx::IddStructureCount).read() }; - if index >= count { - return None; - } - // SAFETY: read-only; `IddStructures` is the framework size table. - let table = unsafe { (&raw const iddcx::IddStructures).read() }; - if table.is_null() { - return None; - } - // SAFETY: `index` validated `< count`; `table` non-null. - let size = unsafe { table.add(index as usize).read() }; - u32::try_from(size).ok() -}