fix(win-display): punktfunk's own display is not one of the operator's panels

The IddCx driver declares `DISPLAYCONFIG_OUTPUT_TECHNOLOGY_HDMI` for its monitors
(`packaging/windows/drivers/pf-vdisplay/src/monitor.rs`, `IDDCX_MONITOR_INFO::MonitorType`),
and `output_tech_class`'s allowlist reads HDMI as a real external panel. So every consumer of
`target_inventory` counted OUR OWN virtual display among the operator's physical monitors.
Measured on .173: both targets came back `external_physical`, `[(4352, "LG TV SSCR2", HDMI),
(257, "punktfunk", HDMI)]`.

That is not cosmetic. `restore_displays_ccd` ends with the last-resort guarantee that the desk
is never left all-dark:

    let (connected, lit) = target_inventory().filter(external_physical) …
    if connected > 0 && lit == 0 { force_extend_topology() }

The restore runs BEFORE the virtual is REMOVEd, so our own still-active display kept `lit >= 1`
and the backstop could NEVER fire — in exactly the situation it was written for. It also made
our display a candidate "physical suspect" in the disturbance-attribution inventory, which
`TargetInventory`'s own doc rules out ("only indirect/virtual targets (our own IDD included)").

Targets carrying our EDID manufacturer id ("PNK", stamped by the driver's edid.rs) are now
classified `external_physical = false`, tech `punktfunk-virtual`. Matching on the monitor device
path rather than the friendly name — the path carries the manufacturer id the OS itself derived
— and on both spellings, since the CCD device-interface path uses `#`
(`\\?\DISPLAY#PNK0000#…`) while the PnP instance id for the same monitor uses `\`
(`DISPLAY\PNK0000\1&15ecd195&5&UID264`, observed on .173). Allowlist-shaped like
`output_tech_class`, so a third-party virtual display is never silently adopted.

ON GLASS, read-only, on .173 — and it caught the backstop's trigger live:

  target  264  active=true  external_physical=false tech=punktfunk-virtual "punktfunk"
  target 4352  active=false external_physical=true  tech=HDMI              "LG TV SSCR2"

The physical is dark while our virtual holds the desktop. Post-fix that is `connected=1, lit=0`
— the backstop fires. Pre-fix it was `connected=2, lit=1` — suppressed. The new hardware case is
deliberately READ-ONLY (creates and destroys nothing) so it is safe against a live host;
repeated IddCx create/destroy churn is what wedges the driver's slot pool.

Also adds `manager::FAIL_NEXT_ISOLATES`, a `#[cfg(test)]` seam that fails the next N
`isolate_displays_ccd` calls, with every call site in manager.rs routed through it. The Phase-3
recovery legs fire only on a failed isolate, which real hardware does not produce, so they
cannot otherwise be exercised. `#[cfg(test)]` rather than an env knob: this crate's live tests
compile under `cfg(test)`, so no production switch exists that could disable display isolation
on an operator's box. The §5 3.2 case built on it is included but is NOT yet a passing on-glass
result — see the plan for where it got to.

Verified on .173: clippy -p punktfunk-host -p pf-vdisplay --all-targets and -p pf-win-display
--all-targets clean; pf-vdisplay 56 pass, pf-win-display 2 pass + hardware cases; both xcheck
targets clean.
This commit is contained in:
2026-07-28 22:17:26 +02:00
parent 2b2c6f045d
commit 21eda37aa3
3 changed files with 309 additions and 22 deletions
@@ -221,6 +221,42 @@ fn poll_gdi_name(target_id: u32) -> Option<String> {
None
}
/// Test-only fault injection for the CCD isolate.
///
/// Every Phase-3 recovery leg in this file fires only when [`isolate_displays_ccd`] returns `None`,
/// and on healthy hardware it never does — which is exactly why those legs shipped unexercised on
/// glass. This counter lets a live test fail the next N isolates against the REAL driver and a REAL
/// panel, so the recovery is observed rather than reasoned about.
///
/// `#[cfg(test)]`-only on purpose: this crate's live hardware tests compile under `cfg(test)`, so
/// the seam is reachable where it is needed WITHOUT shipping a production knob that could leave
/// display isolation silently disabled on an operator's box.
#[cfg(test)]
pub(crate) static FAIL_NEXT_ISOLATES: std::sync::atomic::AtomicU32 =
std::sync::atomic::AtomicU32::new(0);
/// [`isolate_displays_ccd`] with the test seam in front of it. Every call site in this file goes
/// through here so an injected failure exercises the same gates a real one would.
fn isolate_displays_ccd_seam(keep_target_ids: &[u32]) -> Option<SavedConfig> {
#[cfg(test)]
{
use std::sync::atomic::Ordering;
if FAIL_NEXT_ISOLATES
.fetch_update(Ordering::Relaxed, Ordering::Relaxed, |n| {
(n > 0).then(|| n - 1)
})
.is_ok()
{
tracing::warn!(
keep = ?keep_target_ids,
"TEST fault injection: forcing isolate_displays_ccd -> None"
);
return None;
}
}
isolate_displays_ccd(keep_target_ids)
}
fn shrink_action(ccd_exclusive: bool, has_saved: bool) -> ShrinkAction {
if ccd_exclusive {
ShrinkAction::Reisolate
@@ -975,7 +1011,7 @@ impl VirtualDisplayManager {
"re-asserting exclusive topology"
),
}
let _ = isolate_displays_ccd(&keep);
let _ = isolate_displays_ccd_seam(&keep);
// That same forced re-commit hands the live IDD path a fresh swap-chain,
// orphaning the session's capture ring — announce it so the session rebuilds
// its capture attachment (same-mode ring recreate + driver re-attach + fresh
@@ -1198,7 +1234,7 @@ impl VirtualDisplayManager {
if crate::policy::prefs().ddc_power_off() {
inner.group.ddc_panels_off = crate::ddc::panel_off_except(n);
}
inner.group.ccd_saved = isolate_displays_ccd(&keep);
inner.group.ccd_saved = isolate_displays_ccd_seam(&keep);
// EXPERIMENTAL `pnp_disable_monitors` policy axis: AFTER the isolate took,
// additionally disable the deactivated monitors' PnP devnodes (persistent
// across hot-plug re-arrival) so a standby monitor/TV's periodic wake
@@ -1223,7 +1259,7 @@ impl VirtualDisplayManager {
// Grown set: re-isolate so the fresh member joins the composited set
// (its auto-activate may have lit nothing extra to deactivate, but the
// re-commit also drives COMMIT_MODES for the new path).
let snap = isolate_displays_ccd(&keep);
let snap = isolate_displays_ccd_seam(&keep);
// Normally DISCARDED — the group restores the FIRST member's snapshot.
// But if the first member's isolate FAILED, there is no first snapshot,
// and this one just deactivated the physicals with nothing able to put
@@ -1643,7 +1679,7 @@ impl VirtualDisplayManager {
// snapshot is DISCARDED — the group keeps the first member's (design §6.1).
let mut keep = inner.target_ids();
keep.push(new_target);
let _ = isolate_displays_ccd(&keep);
let _ = isolate_displays_ccd_seam(&keep);
}
Topology::Primary => {
// Make the new target primary again (its predecessor held primary), preserving the
@@ -1740,7 +1776,7 @@ impl VirtualDisplayManager {
// the group keeps the first member's.
ShrinkAction::Reisolate => {
let keep = inner.target_ids();
let _ = isolate_displays_ccd(&keep);
let _ = isolate_displays_ccd_seam(&keep);
}
// Re-promote a survivor rather than leaving the desktop's primary on a target that
// is about to be REMOVEd. Same save/restore-the-snapshot dance as
@@ -924,6 +924,161 @@ mod tests {
drop(vout); // triggers REMOVE + stops the pinger
}
/// Forces `Topology::Exclusive` for the duration of a case and puts the operator's real policy
/// back on drop — including when the case panics.
///
/// The isolate branch this file's Phase-3 cases exercise runs ONLY under `Exclusive`, and a real
/// install is usually configured otherwise (.173 is `"topology": "extend"`, which is why the
/// first attempt at these cases silently never ran an isolate at all — `topology_action()`
/// returns `effective_topology()` as soon as ANY policy is configured). Note this writes the
/// host's `display-settings.json`; the guard is what makes that safe to do on a real box.
struct ExclusiveTopology(crate::policy::DisplayPolicy);
impl ExclusiveTopology {
fn force() -> Self {
let original = crate::policy::prefs().get();
let mut forced = original.clone();
forced.preset = crate::policy::Preset::Custom; // explicit fields are ignored otherwise
forced.topology = crate::policy::Topology::Exclusive;
crate::policy::prefs()
.set(forced)
.expect("force Topology::Exclusive for this case");
assert_eq!(
crate::effective_topology(),
crate::policy::Topology::Exclusive,
"the forced policy did not resolve to Exclusive"
);
Self(original)
}
}
impl Drop for ExclusiveTopology {
fn drop(&mut self) {
if let Err(e) = crate::policy::prefs().set(self.0.clone()) {
eprintln!("WARNING: could not restore the display policy: {e}");
}
}
}
/// §5 3.2 on glass: when the FIRST member's isolate fails, a later member's isolate must be
/// ADOPTED as the group's restore snapshot — otherwise it deactivates the operator's panels
/// with nothing able to put them back.
///
/// This leg only fires on a FAILED `isolate_displays_ccd`, which real hardware does not
/// produce, so it shipped unexercised. `manager::FAIL_NEXT_ISOLATES` (a `#[cfg(test)]` seam)
/// fails exactly the first isolate, against the real driver and a real panel; the second member
/// then isolates for real and the physical genuinely goes dark mid-test.
///
/// The assertion is the user-visible one: after both members are torn down, the operator's
/// external panel is ACTIVE again. Without the adoption the group holds no snapshot,
/// `teardown_removed`'s restore is gated on it and never runs, and the panel stays deactivated.
///
/// ⚠️ Two members means two SLOTS, which is what `slot_id_for(client_fp, …)` keys on — hence the
/// two distinct client fingerprints. Needs `Topology::Exclusive`, which is the default when no
/// policy is configured and `PUNKTFUNK_NO_ISOLATE` is unset; the test asserts an isolate really
/// happened rather than trusting that.
///
/// ⚠️ If this test leaves the desk dark, recover from the CONSOLE session with
/// `SetDisplayConfig(0,null,0,null, SDC_USE_DATABASE_CURRENT|SDC_APPLY)` — measured rc=0 on
/// .173. `SDC_TOPOLOGY_EXTEND` will NOT do it with a single connected display (rc=31).
#[test]
#[ignore = "needs the pf-vdisplay driver on real hardware; run with --ignored"]
fn live_a_failed_first_isolate_is_recovered_by_adopting_the_next() {
assert!(
std::env::var("PUNKTFUNK_NO_ISOLATE").is_err(),
"PUNKTFUNK_NO_ISOLATE forces Topology::Extend — this case needs Exclusive"
);
let _topology = ExclusiveTopology::force();
let physicals_before = active_physicals();
assert!(
!physicals_before.is_empty(),
"no external physical panel is active, so 'the panel came back' cannot be observed — \
power the display on first (a TV in standby reads as Code 45 / zero CCD paths)"
);
println!("physicals before : {physicals_before:?}");
// Fail EXACTLY the first member's isolate.
super::super::manager::FAIL_NEXT_ISOLATES.store(1, std::sync::atomic::Ordering::Relaxed);
let mut vd1 = PfVdisplayDisplay::new().expect("open pf-vdisplay (member 1)");
vd1.set_client_identity(Some([0xA1; 32]));
let out1 = vd1
.create(Mode {
width: 1920,
height: 1080,
refresh_hz: 60,
})
.expect("create member 1");
thread::sleep(Duration::from_secs(2));
println!(
"after member 1 (isolate INJECTED to fail): {:?}",
active_targets()
);
let mut vd2 = PfVdisplayDisplay::new().expect("open pf-vdisplay (member 2)");
vd2.set_client_identity(Some([0xB2; 32]));
let out2 = vd2
.create(Mode {
width: 1280,
height: 720,
refresh_hz: 60,
})
.expect("create member 2");
thread::sleep(Duration::from_secs(2));
let during = active_physicals();
println!(
"after member 2 (isolate REAL) : {:?}",
active_targets()
);
println!("physicals during : {during:?}");
// The seam must have been consumed — otherwise the injection never took and a pass here
// would prove nothing about the recovery.
assert_eq!(
super::super::manager::FAIL_NEXT_ISOLATES.load(std::sync::atomic::Ordering::Relaxed),
0,
"the injected isolate failure was never consumed — no isolate ran, so this run proves \
nothing (is the topology really Exclusive?)"
);
drop(out2);
drop(out1);
thread::sleep(Duration::from_secs(6)); // async PnP removal + the restore settling
let physicals_after = active_physicals();
println!("physicals after teardown : {physicals_after:?}");
assert!(
!physicals_after.is_empty(),
"the operator's physical panel was left DEACTIVATED after teardown. The first \
member's isolate failed, so the group held no restore snapshot; the second member's \
isolate deactivated the physicals and its snapshot was discarded (sweep §5 3.2). \
Active targets now: {:?}",
active_targets()
);
}
/// The ACTIVE display targets, as `(target_id, friendly)` — not just a count.
///
/// Counting alone cannot tell "the physical is still lit" from "the physical was deactivated
/// and the virtual took its place", which on a single-panel box are both `1`. Every on-glass
/// claim in this module about panels going dark rests on the identities, so read them.
fn active_targets() -> Vec<(u32, String)> {
pf_win_display::win_display::target_inventory()
.into_iter()
.filter(|t| t.active)
.map(|t| (t.target_id, format!("{} [{}]", t.friendly, t.tech)))
.collect()
}
/// The active targets that are EXTERNAL PHYSICAL panels — the operator's actual desk.
fn active_physicals() -> Vec<(u32, String)> {
pf_win_display::win_display::target_inventory()
.into_iter()
.filter(|t| t.active && t.external_physical)
.map(|t| (t.target_id, format!("{} [{}]", t.friendly, t.tech)))
.collect()
}
/// `SDC_TOPOLOGY_EXTEND` needs something to extend ACROSS — and that is the state its callers
/// are in, which is why this looked like a defect and is not.
///
@@ -953,10 +1108,7 @@ mod tests {
#[test]
#[ignore = "needs the pf-vdisplay driver on real hardware; run with --ignored"]
fn live_force_extend_with_a_virtual_display_present() {
fn active_paths() -> Option<u32> {
pf_win_display::win_display::count_other_active(&[])
}
let before = active_paths();
let before = active_targets();
let mut vd = PfVdisplayDisplay::new().expect("open pf-vdisplay");
let vout = vd
.create(Mode {
@@ -966,22 +1118,27 @@ mod tests {
})
.expect("create virtual display");
thread::sleep(Duration::from_secs(2));
let with_virtual = active_paths();
let with_virtual = active_targets();
let physicals_with_virtual = active_physicals();
pf_win_display::win_display::force_extend_topology();
thread::sleep(Duration::from_secs(2));
let after_extend = active_paths();
let after_extend = active_targets();
drop(vout);
thread::sleep(Duration::from_secs(3));
let after_drop = active_paths();
println!(
"force-EXTEND on glass: active paths {before:?} -> (virtual up) {with_virtual:?} -> \
(after force-EXTEND) {after_extend:?} -> (virtual dropped) {after_drop:?}"
);
assert_ne!(
after_drop,
Some(0),
thread::sleep(Duration::from_secs(6)); // PnP removal is async — a short wait reads a ghost
let after_drop = active_targets();
println!("force-EXTEND on glass, ACTIVE TARGETS at each step:");
println!(" before : {before:?}");
println!(" virtual up : {with_virtual:?} (physicals: {physicals_with_virtual:?})");
println!(" after force-EXT : {after_extend:?}");
println!(" virtual dropped : {after_drop:?}");
assert!(
!after_drop.is_empty(),
"the desk was left with NO active display path after the teardown"
);
assert!(
!active_physicals().is_empty(),
"the operator's physical panel was left DEACTIVATED after teardown: {after_drop:?}"
);
}
/// Live in-place resize spike — `#[ignore]`d (needs a v4 pf-vdisplay driver installed + the host
+96 -2
View File
@@ -1000,6 +1000,37 @@ pub struct TargetInventory {
pub monitor_device_path: String,
}
/// EDID manufacturer id of punktfunk's own IddCx monitors, as it appears in the PnP hardware id and
/// therefore in the CCD monitor device path (`\\?\DISPLAY#PNK….`). The driver stamps `"PNK"` into
/// EDID bytes 8-9 (`packaging/windows/drivers/pf-vdisplay/src/edid.rs`).
const PF_EDID_MANUFACTURER: &str = "PNK";
/// Is this monitor device path one of OUR virtual displays?
///
/// It has to be asked, because the connector class cannot answer it: our IddCx monitor declares
/// `DISPLAYCONFIG_OUTPUT_TECHNOLOGY_HDMI` (driver `monitor.rs`, `IDDCX_MONITOR_INFO::MonitorType`),
/// which [`output_tech_class`]'s allowlist reads as a real external panel — so without this check
/// punktfunk's own display counts as one of the operator's physical monitors. Measured on .173:
/// `target_inventory()` returned `[(4352, "LG TV SSCR2", HDMI), (257, "punktfunk", HDMI)]` with
/// BOTH flagged `external_physical`.
///
/// That is not cosmetic. [`restore_displays_ccd`]'s last-resort "the desk is not left dark"
/// backstop fires on `connected > 0 && lit == 0` over exactly this set, and the restore runs BEFORE
/// the virtual is REMOVEd — so our own still-active display kept `lit >= 1` and the backstop could
/// never fire, in precisely the situation it was written for. It also made our display a candidate
/// "physical suspect" in the disturbance-attribution inventory, which [`TargetInventory`]'s own doc
/// says can never happen ("only indirect/virtual targets (our own IDD included)").
///
/// Matching on the device path rather than the friendly name: the name comes from the EDID's 0xFC
/// descriptor and is what a user sees, while the path carries the manufacturer id the OS itself
/// derived. Allowlist-shaped like [`output_tech_class`] — anything unrecognised stays "not ours",
/// so a third-party virtual display is never silently adopted.
fn is_our_virtual_display(monitor_device_path: &str) -> bool {
monitor_device_path
.to_ascii_uppercase()
.contains(PF_EDID_MANUFACTURER)
}
/// Classify a CCD output technology: `(external physical?, log label)`. Allowlist, not blocklist:
/// new/unknown/indirect technologies read as non-external, so a co-installed third-party virtual
/// display can never be mistaken for a physical suspect (same precision rule as `monitor_devnode`).
@@ -1096,7 +1127,14 @@ pub fn target_inventory() -> Vec<TargetInventory> {
if unsafe { DisplayConfigGetDeviceInfo(&mut req.header) } != 0 {
continue; // target with no queryable monitor — nothing to attribute to
}
let (external_physical, tech) = output_tech_class(req.outputTechnology);
let monitor_device_path = utf16z_str(&req.monitorDevicePath);
let (mut external_physical, mut tech) = output_tech_class(req.outputTechnology);
// Our own IddCx monitor claims HDMI, so the connector class alone would call it one of the
// operator's panels — see `is_our_virtual_display` for what that broke.
if is_our_virtual_display(&monitor_device_path) {
external_physical = false;
tech = "punktfunk-virtual";
}
out.push(TargetInventory {
target_id: t.id,
active: active.contains(&key),
@@ -1104,7 +1142,7 @@ pub fn target_inventory() -> Vec<TargetInventory> {
internal_panel: tech == "internal-panel",
tech,
friendly: utf16z_str(&req.monitorFriendlyDeviceName),
monitor_device_path: utf16z_str(&req.monitorDevicePath),
monitor_device_path,
});
}
out
@@ -1685,6 +1723,62 @@ mod live_tests {
/// where the query returned 0x57 ERROR_INVALID_PARAMETER (and 0x5 ERROR_ACCESS_DENIED from
/// session 0). This case FAILS there without the zero-path short-circuit and passes with it,
/// while a box that does have a display lit passes either way.
/// Pure, so it runs everywhere — the identification rule itself needs no hardware.
#[test]
fn our_own_virtual_display_is_never_an_external_physical() {
assert!(super::is_our_virtual_display(
r"\\?\DISPLAY#PNK0000#5&1234abcd&0&UID257#{e6f07b5f-ee97-4a90-b076-33f57bf4eaa7}"
));
// Case-insensitive: the OS is not consistent about the path's case.
assert!(super::is_our_virtual_display(
r"\\?\display#pnk0000#5&1&0&uid257#{guid}"
));
// A real panel, and a third-party virtual display, both stay physical suspects.
assert!(!super::is_our_virtual_display(
r"\\?\DISPLAY#GSM83CD#5&367fb4cb&0&UID4352#{e6f07b5f-ee97-4a90-b076-33f57bf4eaa7}"
));
assert!(!super::is_our_virtual_display(
r"\\?\DISPLAY#SMVD0001#5&1&0&UID999#{guid}"
));
}
/// Read-only: prove on real hardware that punktfunk's own display is not counted among the
/// operator's physical panels. Creates and destroys nothing, so it is safe to run against a
/// live host — which matters, because repeated IddCx create/destroy cycles are exactly what
/// wedges the driver's slot pool.
///
/// Measured on .173 BEFORE the fix: `[(4352, "LG TV SSCR2", HDMI), (257, "punktfunk", HDMI)]`
/// with both flagged `external_physical`, because the driver declares
/// `DISPLAYCONFIG_OUTPUT_TECHNOLOGY_HDMI`.
#[test]
#[ignore = "hardware: reads the live display topology"]
fn our_own_display_is_excluded_from_the_operators_physicals_on_real_hardware() {
let inv = target_inventory();
for t in &inv {
println!(
"target {:>5} active={:<5} external_physical={:<5} tech={:<18} {:?} {}",
t.target_id,
t.active,
t.external_physical,
t.tech,
t.friendly,
t.monitor_device_path
);
}
for t in inv
.iter()
.filter(|t| is_our_virtual_display(&t.monitor_device_path))
{
assert!(
!t.external_physical,
"our own display {} ({:?}) is still counted as one of the operator's physical \
panels — `restore_displays_ccd`'s dark-desk backstop keys on exactly this set and \
would never fire",
t.target_id, t.friendly
);
}
}
#[test]
#[ignore = "hardware: reads the live display topology"]
fn a_host_with_nothing_lit_reports_zero_actives_rather_than_a_failed_query() {