From b20c99f141c6a941c978af06b9755e07e7f8c973 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Fri, 12 Jun 2026 14:57:38 +0200 Subject: [PATCH] [Subsystem-fixes] 6 verified ours-vs-canary divergence fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From the 2026-06-12 5-subsystem differential audit. All verified against canary as oracle; 660/660 workspace tests green (655 + 5 new). 1. nt_create_event polarity (exports.rs) — `manual_reset = gpr[5] != 0` was INVERTED. Canary xboxkrnl_threading.cc:668 `Initialize(!event_type,..)` + xevent.cc:41 (type 0 = NotificationEvent = manual, type 1 = Sync = auto). Now `== 0`. Was the dormant 2.AI fix on chore/portable-snapshot, never merged. The Ke-path was already correct; only the Nt-path was wrong. 2. 2.AF deadline drain (main.rs coord_pre_round) — expired KeWait/KeDelay deadlines never fired under load because advance_to_next_wake_if_due was only called in coord_idle_advance (no-Ready-threads path). Added a per-round drain loop; covers BOTH lockstep and parallel outer loops since both call coord_pre_round. Was the dormant 2.AF fix, never merged. 3. handle slab-recycle ABA guard (state.rs + scheduler.rs) — release_handle_slot (my round-34 regression) recycled a closed slot even with a thread still parked on it, risking a stale-waiter wake when the slot is re-minted. Added Scheduler::any_thread_waiting_on; decline to recycle a still-waited slot. 4. vpkpx pixel-pack (vmx.rs) — wrong field mapping (~100% mismatch). Now exact canary ppc_emit_altivec.cc:1795 shift/mask (red 6b out[15:10] from w[24:19], green out[9:5] from w[14:10], blue out[4:0] from w[7:3]; no fabricated alpha bit). +unit test. 5. VFS GDFX attribute plumbing (vfs/*, exports.rs query fns) — VfsEntry now carries the real on-disc attribute byte (GDFX dirent +12, canary disc_image_device.cc:136/154) instead of inferring directory-ness from path shape. Query exports report the real FILE_ATTRIBUTE_* bits. Candidate driver of the XamShowDirtyDiscErrorUI gate. +tests. 6. MmGetPhysicalAddress region-aware mirror (exports.rs) — flat 0x1FFFFFFF mask missed canary's +0x1000 host_address_offset for 0xE0000000+ mirror (memory.cc:2317). Read-only query; proven byte-identical 50M digest. +test. Investigated and intentionally NOT changed: - zero-on-recommit: no-op; ours has no region-reuse path (bump allocators, free is a stub). - 32-bit ALU writeback truncation (PPCBUG-020): documented-deliberate; premise (MSR.SF=0) is questionable but flipping it is out of scope here. - KeSetEvent/NtSetEvent return value: ours returns true previous state (hardware-faithful); canary returns constant 1 — NOT an ours bug. sylpheed_n50m golden will need re-baselining (legit behavior change). Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/xenia-app/src/main.rs | 21 +++++ crates/xenia-cpu/src/scheduler.rs | 22 +++++ crates/xenia-cpu/src/vmx.rs | 70 +++++++++----- crates/xenia-kernel/src/exports.rs | 146 ++++++++++++++++++++++++----- crates/xenia-kernel/src/state.rs | 10 ++ crates/xenia-vfs/src/device.rs | 4 + crates/xenia-vfs/src/disc_image.rs | 80 ++++++++++++++++ crates/xenia-vfs/src/lib.rs | 10 ++ 8 files changed, 319 insertions(+), 44 deletions(-) diff --git a/crates/xenia-app/src/main.rs b/crates/xenia-app/src/main.rs index 6e3abfe..5a2fa01 100644 --- a/crates/xenia-app/src/main.rs +++ b/crates/xenia-app/src/main.rs @@ -2124,6 +2124,27 @@ fn coord_pre_round( } kernel.fire_due_timers(); + // 2.AF — fire expired wait-deadlines under load. Without this drain, + // `advance_to_next_wake_if_due` only runs in `coord_idle_advance` (the + // no-Ready-threads path), so a thread whose `KeWait*`/`KeDelay` deadline + // expires while other threads keep the scheduler busy sits Blocked + // forever (observed: tid=5's 42.95ms deadline unfired 29s+). Drain every + // entry whose deadline `<=` the current guest timebase — the same `now` + // basis `fire_due_timers` uses, so the two stay in lock-step — and let + // `handle_timeout_wake` stamp `STATUS_TIMEOUT` and scrub the waiter from + // each handle. `advance_to_next_wake_if_due` pops at most one due wake + // per call and returns `None` once the earliest remaining deadline is in + // the future, so this loop terminates. Deterministic: `ctx(0).timebase` + // is the guest-cycle timebase, not host_ns. This runs in `coord_pre_round` + // which both the lockstep and parallel outer loops call every round. + loop { + let now = kernel.scheduler.ctx(0).timebase; + let Some((r, reason)) = kernel.scheduler.advance_to_next_wake_if_due(now) + else { + break; + }; + kernel.handle_timeout_wake(r, reason); + } // Graphics-interrupt delivery is no longer done here — see // `dispatch_graphics_interrupts`, called from the outer loop with // `mem` and `&mut stats` in scope. The audio path still uses the diff --git a/crates/xenia-cpu/src/scheduler.rs b/crates/xenia-cpu/src/scheduler.rs index 17a5456..4cf926f 100644 --- a/crates/xenia-cpu/src/scheduler.rs +++ b/crates/xenia-cpu/src/scheduler.rs @@ -1184,6 +1184,28 @@ impl Scheduler { }) } + /// True if any thread is currently `Blocked` on a `WaitAny`/`WaitAll` + /// whose handle set contains `handle`. Used by the handle-slab recycler + /// (AUDIT-059 R34) to avoid an ABA hazard: if a closed handle's slot is + /// returned to the free list while a thread is still parked on it, a + /// later `alloc_handle` could hand the same slot to a NEW object, and a + /// signal on that new object would wake the stale waiter that was + /// waiting on the OLD (closed) object. Canary sidesteps this by keeping + /// the object alive via an object_ref while waiters hold references; we + /// instead simply decline to recycle a still-waited slot (leaking it, + /// matching the pre-R34 bump-only behaviour for that rare case). + pub fn any_thread_waiting_on(&self, handle: u32) -> bool { + self.slots.iter().any(|slot| { + slot.runqueue.iter().any(|t| match &t.state { + HwState::Blocked(BlockReason::WaitAny { handles, .. }) + | HwState::Blocked(BlockReason::WaitAll { handles, .. }) => { + handles.contains(&handle) + } + _ => false, + }) + }) + } + /// Snapshot thread states for diagnostic logging. One entry per live /// guest thread (Exited are included so post-mortem can see exit codes). pub fn diagnostic_snapshot(&self) -> Vec<(ThreadRef, Option, HwState)> { diff --git a/crates/xenia-cpu/src/vmx.rs b/crates/xenia-cpu/src/vmx.rs index 4be636f..5fc56a4 100644 --- a/crates/xenia-cpu/src/vmx.rs +++ b/crates/xenia-cpu/src/vmx.rs @@ -293,28 +293,23 @@ pub fn store_vector_right(mem: &dyn MemoryAccess, ea: u32, v: Vec128) { } } -// ─── 5-6-5 pixel pack (vpkpx / vupkhpx / vupklpx) ───────────────────────── -// PPC vpkpx takes a 32-bit RGB lane and packs it into a 16-bit 1-5-5-5 pixel. -// vupkhpx / vupklpx reverse the operation. -// -// Format: input 32-bit word holds -// bits 0-6: unused (0) -// bit 7: alpha-select (→ bit 15 of output) -// bits 8-15: R (top 5 bits kept) -// bits 16-23: G (top 5 bits kept) -// bits 24-31: B (top 5 bits kept) -// Output 16-bit word: -// bit 15: A (from input bit 7) -// bits 10-14: R -// bits 5-9: G -// bits 0-4: B +// ─── pixel pack (vpkpx / vupkhpx / vupklpx) ─────────────────────────────── +// PPC vpkpx packs each 32-bit lane into a 16-bit 1-5-5-5 pixel. +// Mapping transcribed EXACTLY from xenia-canary +// `ppc_emit_altivec.cc::vkpkx_in_low` (lines 1795-1808): +// tmp1 = (input >> 9) & 0xFC00 // out bits 15:10 = in bits 24:19 +// tmp2 = (input >> 6) & 0x3E0 // out bits 9:5 = in bits 14:10 +// tmp3 = (input >> 3) & 0x1F // out bits 4:0 = in bits 7:3 +// result = tmp1 | tmp2 | tmp3 +// This is a pure shift/mask: there is NO standalone alpha select. Output +// bit 15 is simply input bit 24 (the top of the 6-bit field masked by +// 0xFC00) — NOT input bit 7. The red field is 6 bits wide here. #[inline] pub fn pack_pixel_555(input: u32) -> u16 { - let a = (input >> 7) & 0x1; - let r = (input >> 8) & 0xFF; - let g = (input >> 16) & 0xFF; - let b = (input >> 24) & 0xFF; - ((a << 15) | ((r & 0xF8) << 7) | ((g & 0xF8) << 2) | ((b & 0xF8) >> 3)) as u16 + let tmp1 = (input >> 9) & 0xFC00; + let tmp2 = (input >> 6) & 0x3E0; + let tmp3 = (input >> 3) & 0x1F; + (tmp1 | tmp2 | tmp3) as u16 } #[inline] pub fn unpack_pixel_555(input: u16) -> u32 { @@ -801,9 +796,38 @@ mod tests { } #[test] - fn pack_unpack_pixel_555() { - let encoded = pack_pixel_555(0x80_F8_F8_F8); - assert_eq!(encoded & 0x8000, 0x8000); + fn pack_pixel_555_matches_canary() { + // Mapping (canary ppc_emit_altivec.cc::vkpkx_in_low): + // out[15:10] = in[24:19], out[9:5] = in[14:10], out[4:0] = in[7:3] + // Pure shift/mask, NO standalone alpha bit. + + // All three colour fields exercised. Expected (hand-computed): + // (0x018844C0 >> 9)&0xFC00 = 0xC400 + // (0x018844C0 >> 6)&0x3E0 = 0x100 + // (0x018844C0 >> 3)&0x1F = 0x18 + // => 0xC518 + assert_eq!(pack_pixel_555(0x01_88_44_C0), 0xC518); + + // Boundary the audit flagged: low byte 0xF8 has bit 7 set. Canary does + // NOT turn that into output bit 15 (alpha). Output bit 15 = in bit 24, + // which is 0 here => high bit clear. (Old impl wrongly produced 0x8000.) + assert_eq!(pack_pixel_555(0x80_F8_F8_F8), 0x7FFF); + assert_eq!(pack_pixel_555(0x80_F8_F8_F8) & 0x8000, 0); + + // Lone source bit 7 (0x80) lands in the blue field, not in bit 15. + assert_eq!(pack_pixel_555(0x00_00_00_80), 0x0010); + + // Output bit 15 is sourced from input bit 24, not bit 7. + assert_eq!(pack_pixel_555(0x01_00_00_00), 0x8000); + + // Saturated input -> all field bits set. + assert_eq!(pack_pixel_555(0xFF_FF_FF_FF), 0xFFFF); + } + + #[test] + fn unpack_pixel_555_roundtrip() { + // vupkhpx/vupklpx are NOTIMPLEMENTED in canary, so unpack_pixel_555 is + // unchanged; just sanity-check the alpha-replicate path still holds. let w = unpack_pixel_555(0x8000 | (0x1F << 10) | (0x1F << 5) | 0x1F); assert_eq!(w & 0xFF000000, 0xFF000000); } diff --git a/crates/xenia-kernel/src/exports.rs b/crates/xenia-kernel/src/exports.rs index 84fb0de..5243688 100644 --- a/crates/xenia-kernel/src/exports.rs +++ b/crates/xenia-kernel/src/exports.rs @@ -696,9 +696,36 @@ fn mm_create_kernel_stack(ctx: &mut PpcContext, mem: &GuestMemory, state: &mut K } } +/// Region-aware guest-virtual → physical translation, matching canary's +/// `Memory::GetPhysicalAddress` + `PhysicalHeap::GetPhysicalAddress` +/// (`xenia-canary/src/xenia/memory.cc:528-545` and `:2317-2326`). +/// +/// Canary `PhysicalHeap::GetPhysicalAddress`: +/// ```c +/// address -= heap_base_; +/// if (heap_base_ >= 0xE0000000) { address += 0x1000; } +/// return address; +/// ``` +/// The three physical heap bases (0xA0000000 / 0xC0000000 / 0xE0000000) all +/// alias the same 512 MB physical window, so `address - heap_base == +/// address & 0x1FFFFFFF` for each. The only region-specific delta is the +/// `+0x1000` host-address-offset for the 0xE0000000+ 4 KB mirror — see +/// `memory.h:368-372` (`host_address_offset` for `heap_base >= 0xE0000000`). +/// For non-physical / sub-0x1FFFFFFF virtual addresses canary returns the +/// address unchanged, which equals `address & 0x1FFFFFFF` there too. +pub(crate) fn translate_physical_address(virt: u32) -> u32 { + let phys = virt & 0x1FFF_FFFF; + if virt >= 0xE000_0000 { + phys + 0x1000 + } else { + phys + } +} + fn mm_get_physical_address(ctx: &mut PpcContext, _mem: &GuestMemory, _state: &mut KernelState) { - // r3 = virtual address -> return physical address - ctx.gpr[3] &= 0x1FFF_FFFF; // Mask to 512MB physical + // r3 = virtual address -> return physical address. + // Region-aware, mirroring canary (see `translate_physical_address`). + ctx.gpr[3] = translate_physical_address(ctx.gpr[3] as u32) as u64; } fn mm_query_address_protect(ctx: &mut PpcContext, _mem: &GuestMemory, _state: &mut KernelState) { @@ -1480,20 +1507,35 @@ fn nt_query_information_file(ctx: &mut PpcContext, mem: &GuestMemory, state: &mu *size }; - // Root-of-device opens (`game:\`, `cache:\`, `partition0`) strip to - // an empty string post-prefix — see `open_vfs_file`'s synth path. - // Games query these as directories (DirectoryObject probe), and - // reporting `Directory=0` makes Sylpheed treat the open as "found a - // non-directory where I expected a directory" and call - // `XamShowDirtyDiscErrorUI`. Canary's `NtQueryInformationFile` pulls - // the real file-system entry's kind; we key on path shape since we - // don't model directory entries. - let is_directory = path.is_empty() - || path.ends_with('/') - || path.ends_with(':'); + // Snapshot what we need from the handle, then drop the borrow so we can + // re-resolve the path against the VFS for its real attribute byte. + let path = path.clone(); let size = live_size; let position = *position; + // Pull the REAL GDFX attribute byte (canary `disc_image_device.cc:154`) + // for disc-backed handles by re-resolving the stored path. Root-of-device + // opens (`game:\`, `cache:\`, `partition0`) strip to an empty string and + // synth-stub opens have no VFS entry — for those we fall back to the + // path-shape heuristic. Games query these as directories (DirectoryObject + // probe), and reporting `Directory=0` makes Sylpheed treat the open as + // "found a non-directory where I expected a directory" and call + // `XamShowDirtyDiscErrorUI`. + let vfs_attributes: Option = if path.is_empty() { + None + } else { + state + .vfs + .as_ref() + .and_then(|vfs| vfs.stat(&path).ok()) + .map(|e| e.attributes) + .filter(|&a| a != 0) + }; + let is_directory = match vfs_attributes { + Some(a) => (a & 0x10) != 0, + None => path.is_empty() || path.ends_with('/') || path.ends_with(':'), + }; + // `FILE_ATTRIBUTE_DIRECTORY` (NT / Xbox) — advertised in // `FileNetworkOpenInformation.FileAttributes`; Sylpheed's async-I/O // worker queries with class=34 and the calling code checks this bit @@ -1532,10 +1574,13 @@ fn nt_query_information_file(ctx: &mut PpcContext, mem: &GuestMemory, state: &mu } mem.write_u64(file_info + 32, size); mem.write_u64(file_info + 40, size); - let attrs = if is_directory { - FILE_ATTRIBUTE_DIRECTORY - } else { - FILE_ATTRIBUTE_NORMAL + // Prefer the real GDFX attribute byte; fall back to the + // DIRECTORY/NORMAL split for root-of-device and synth-stub + // handles that have no VFS entry. + let attrs = match vfs_attributes { + Some(a) => a, + None if is_directory => FILE_ATTRIBUTE_DIRECTORY, + None => FILE_ATTRIBUTE_NORMAL, }; mem.write_u32(file_info + 48, attrs); mem.write_u32(file_info + 52, 0); // pad @@ -1738,7 +1783,18 @@ fn nt_query_full_attributes_file(ctx: &mut PpcContext, mem: &GuestMemory, state: mem.write_u32(out + 28, filetime as u32); mem.write_u64(out + 32, entry.size); mem.write_u64(out + 40, entry.size); - let attrs: u32 = if entry.is_directory { 0x10 } else { 0x80 }; + // Use the REAL GDFX attribute byte forwarded by the VFS + // (canary `disc_image_device.cc:154`) instead of a + // path-shape guess. Disc rips never carry a 0-attribute + // entry, but guard anyway so a synthesised/legacy entry + // still advertises a sane DIRECTORY/NORMAL split. + let attrs: u32 = if entry.attributes != 0 { + entry.attributes + } else if entry.is_directory { + 0x10 + } else { + 0x80 + }; mem.write_u32(out + 48, attrs); mem.write_u32(out + 52, 0); } @@ -1859,6 +1915,7 @@ fn nt_query_directory_file(ctx: &mut PpcContext, mem: &GuestMemory, state: &mut is_directory: e.is_directory, size: e.size, offset: e.offset, + attributes: e.attributes, }) }) .collect(), @@ -1909,7 +1966,12 @@ fn nt_query_directory_file(ctx: &mut PpcContext, mem: &GuestMemory, state: &mut mem.write_u64(base + 0x20, 0); mem.write_u64(base + 0x28, entry.size); mem.write_u64(base + 0x30, entry.size); - let attrs = if entry.is_directory { + // Real GDFX attribute byte (canary `disc_image_device.cc:154`); + // fall back to the directory/normal split only for legacy entries + // that carry no attribute bits. + let attrs = if entry.attributes != 0 { + entry.attributes + } else if entry.is_directory { FILE_ATTRIBUTE_DIRECTORY } else { FILE_ATTRIBUTE_NORMAL @@ -1985,9 +2047,21 @@ fn nt_close(ctx: &mut PpcContext, _mem: &GuestMemory, state: &mut KernelState) { } fn nt_create_event(ctx: &mut PpcContext, mem: &GuestMemory, state: &mut KernelState) { - // r3 = handle_ptr, r4 = obj_attrs, r5 = event_type, r6 = initial_state + // r3 = handle_ptr, r4 = obj_attrs, r5 = event_type, r6 = initial_state. + // + // Xenon DISPATCHER_HEADER `Type` (NT convention): + // 0 = NotificationEvent (manual-reset) + // 1 = SynchronizationEvent (auto-reset) + // Canary: `xboxkrnl_threading.cc:668` `ev->Initialize(!event_type, !!initial_state)` + // with `XEvent::Initialize(bool manual_reset, ...)` (xevent.cc:25) and + // `InitializeNative` (xevent.cc:41 `case 0x00: manual_reset_ = true`). + // So `manual_reset = (event_type == 0)`. The Ke-path + // (`ensure_dispatcher_object`) was already correct; the Nt-path here was + // inverted, mis-classifying Sylpheed's per-frame VSync gate (type=1 auto + + // initial=1) as manual-reset+signaled → it stayed signaled forever and + // tid=1's main loop spun ~2800x canary's 60Hz. let handle_ptr = ctx.gpr[3] as u32; - let manual_reset = ctx.gpr[5] != 0; + let manual_reset = ctx.gpr[5] == 0; let signaled = ctx.gpr[6] != 0; let handle = state.alloc_handle_for(KernelObject::Event { manual_reset, @@ -4823,12 +4897,14 @@ mod tests { is_directory: false, size: 0x1000, offset: 0, + attributes: 0x81, // NORMAL | READONLY }, xenia_vfs::VfsEntry { name: "dat".into(), is_directory: true, size: 0, offset: 0, + attributes: 0x11, // DIRECTORY | READONLY }, // A grandchild — must NOT appear in root enumeration. xenia_vfs::VfsEntry { @@ -4836,6 +4912,7 @@ mod tests { is_directory: false, size: 0x2000, offset: 0, + attributes: 0x81, }, ], })); @@ -4862,9 +4939,11 @@ mod tests { // NextEntryOffset. let mut cursor: u32 = 0; let mut names: Vec = Vec::new(); + let mut attrs: Vec = Vec::new(); loop { let entry_base = buf + cursor; let name_len = mem.read_u32(entry_base + 0x3C) as usize; + attrs.push(mem.read_u32(entry_base + 0x38)); let mut bytes = Vec::with_capacity(name_len); for i in 0..name_len as u32 { bytes.push(mem.read_u8(entry_base + 0x40 + i)); @@ -4877,6 +4956,12 @@ mod tests { cursor += next; } assert_eq!(names, vec!["default.xex", "dat"]); + // The real GDFX attribute byte must be forwarded verbatim: the file + // reports NORMAL|READONLY (no DIRECTORY bit), the directory reports + // DIRECTORY|READONLY. + assert_eq!(attrs, vec![0x81, 0x11]); + assert_eq!(attrs[0] & 0x10, 0, "file must not advertise DIRECTORY"); + assert_ne!(attrs[1] & 0x10, 0, "dir must advertise DIRECTORY"); // A second call on the same handle must return NO_MORE_FILES — // the cursor has advanced past the end. ctx.gpr[3] = handle as u64; @@ -6396,4 +6481,23 @@ mod tests { assert!(resolved.ends_with("etc/foo")); std::fs::remove_dir_all(&dir).ok(); } + + /// `MmGetPhysicalAddress` must be region-aware, matching canary's + /// `PhysicalHeap::GetPhysicalAddress`: the 0xE0000000+ 4 KB mirror gets a + /// `+0x1000` host-address-offset; every other region is a flat + /// `& 0x1FFFFFFF` mask. + #[test] + fn mm_get_physical_address_region_aware() { + // 0xE0000000 mirror: canary `address - heap_base (==addr & 0x1FFFFFFF)` + // then `+ 0x1000`. + assert_eq!(translate_physical_address(0xE000_0000), 0x0000_1000); + assert_eq!(translate_physical_address(0xE000_5000), 0x0000_6000); + assert_eq!(translate_physical_address(0xFFFF_F000), 0x1FFF_F000 + 0x1000); + // 0xA0000000 / 0xC0000000 physical heaps: flat mask, no offset. + assert_eq!(translate_physical_address(0xA000_0000), 0x0000_0000); + assert_eq!(translate_physical_address(0xC012_3000), 0x0012_3000); + // Virtual / already-physical (< 0x20000000): unchanged. + assert_eq!(translate_physical_address(0x0012_3000), 0x0012_3000); + assert_eq!(translate_physical_address(0x4012_3000), 0x0012_3000); + } } diff --git a/crates/xenia-kernel/src/state.rs b/crates/xenia-kernel/src/state.rs index f28739c..290818c 100644 --- a/crates/xenia-kernel/src/state.rs +++ b/crates/xenia-kernel/src/state.rs @@ -721,10 +721,20 @@ impl KernelState { /// recycle queue. No-op for the synthetic XAudio range (`>= 0xF000_0000`, /// AUDIT-048) and the reserved `< 0x1000` band. Call site: `nt_close`'s /// `objects.remove` branch when refcount reaches zero. + /// + /// ABA guard (subsystem-audit 2026-06-12): never recycle a slot that a + /// thread is still parked on. Without this, a closed slot could be + /// re-minted for a new object and a signal on that new object would wake + /// the stale waiter that was blocked on the OLD object at the same slot. + /// Such a slot is simply leaked (it stays out of `free_handles`), + /// reproducing the pre-R34 bump-only behaviour for that rare case. pub fn release_handle_slot(&mut self, handle: u32) { if handle < 0x1000 || handle >= 0xF000_0000 { return; } + if self.scheduler.any_thread_waiting_on(handle) { + return; + } self.free_handles.push_back(handle); } diff --git a/crates/xenia-vfs/src/device.rs b/crates/xenia-vfs/src/device.rs index 3a83bc8..15b3686 100644 --- a/crates/xenia-vfs/src/device.rs +++ b/crates/xenia-vfs/src/device.rs @@ -31,6 +31,9 @@ impl VfsDevice for HostPathDevice { is_directory: metadata.is_dir(), size: metadata.len(), offset: 0, + // Host FS carries no Xbox attribute byte; synthesise the + // DIRECTORY/NORMAL split like canary's HostPathDevice. + attributes: if metadata.is_dir() { 0x10 } else { 0x80 }, }); } Ok(entries) @@ -49,6 +52,7 @@ impl VfsDevice for HostPathDevice { is_directory: metadata.is_dir(), size: metadata.len(), offset: 0, + attributes: if metadata.is_dir() { 0x10 } else { 0x80 }, }) } } diff --git a/crates/xenia-vfs/src/disc_image.rs b/crates/xenia-vfs/src/disc_image.rs index 80b541b..a44d58b 100644 --- a/crates/xenia-vfs/src/disc_image.rs +++ b/crates/xenia-vfs/src/disc_image.rs @@ -29,6 +29,11 @@ const GDFX_MAGIC: &[u8; 20] = b"MICROSOFT*XBOX*MEDIA"; /// File attribute: directory const FILE_ATTRIBUTE_DIRECTORY: u8 = 0x10; +/// File attribute: read-only. Canary OR's this into every GDFX entry's +/// attribute byte because a pressed disc is inherently read-only +/// (`disc_image_device.cc:154`: `attributes | kFileAttributeReadOnly`). +const FILE_ATTRIBUTE_READONLY: u8 = 0x01; + /// Known game partition offsets to try const LIKELY_OFFSETS: &[u64] = &[ 0x0000_0000, @@ -131,6 +136,11 @@ impl DiscImageDevice { let name = String::from_utf8_lossy(&buffer[p + 14..p + 14 + name_length]).to_string(); let is_directory = (attributes & FILE_ATTRIBUTE_DIRECTORY) != 0; + // Match canary: the on-disc attribute byte (DIRECTORY/HIDDEN/SYSTEM/ + // ARCHIVE/NORMAL bits as authored) OR the implicit READONLY bit for + // pressed media. We forward the FULL byte, not a path-shape guess, so + // attribute queries report exactly what the disc records. + let attributes = (attributes | FILE_ATTRIBUTE_READONLY) as u32; let file_offset = self.game_offset + sector * SECTOR_SIZE; let full_path = if prefix.is_empty() { name.clone() @@ -143,6 +153,7 @@ impl DiscImageDevice { is_directory, size: length, offset: file_offset, + attributes, }); // Descend into subdirectories. Zero-length directory entries exist @@ -260,4 +271,73 @@ mod tests { .expect("read_file on nested path"); assert!(!bytes.is_empty(), "nested read returned empty buffer"); } + + /// Build a one-node GDFX directory buffer in memory and parse it with + /// `collect_entries`, asserting the real on-disc attribute byte is + /// forwarded into `VfsEntry.attributes` (with READONLY OR'd in, matching + /// canary `disc_image_device.cc:154`) rather than synthesised from the + /// path shape. + fn parse_single_entry(name: &str, on_disc_attr: u8) -> VfsEntry { + // GDFX dirent: node_l(u16) node_r(u16) sector(u32) length(u32) + // attributes(u8) name_length(u8) name(bytes). The directory bit + // gates subdirectory descent; use length=0 so a "directory" entry + // is treated as an empty leaf and we don't recurse off the buffer. + let mut buf = Vec::new(); + buf.extend_from_slice(&0u16.to_le_bytes()); // node_l + buf.extend_from_slice(&0u16.to_le_bytes()); // node_r + buf.extend_from_slice(&0u32.to_le_bytes()); // sector + buf.extend_from_slice(&0u32.to_le_bytes()); // length (0 => leaf) + buf.push(on_disc_attr); // attributes + buf.push(name.len() as u8); // name_length + buf.extend_from_slice(name.as_bytes()); + + let mut dev = DiscImageDevice { + name: "test".into(), + path: std::path::PathBuf::new(), + game_offset: 0, + entries: Vec::new(), + }; + // `file` is only touched when descending into a non-empty directory; + // our length=0 entries never recurse, so a dummy handle is fine. + let mut file = std::fs::File::open("/dev/null").expect("open /dev/null"); + dev.collect_entries(&mut file, &buf, 0, "").expect("parse"); + assert_eq!(dev.entries.len(), 1); + dev.entries.into_iter().next().unwrap() + } + + #[test] + fn directory_entry_reports_directory_attribute() { + // On-disc 0x10 (DIRECTORY) -> attributes carries 0x10 and READONLY. + let e = parse_single_entry("dat", FILE_ATTRIBUTE_DIRECTORY); + assert!(e.is_directory, "directory bit not decoded"); + assert_ne!( + e.attributes & 0x10, + 0, + "FILE_ATTRIBUTE_DIRECTORY must be set for a directory entry" + ); + assert_ne!(e.attributes & 0x01, 0, "READONLY must be OR'd in (canary)"); + } + + #[test] + fn file_entry_has_no_directory_attribute() { + // On-disc 0x80 (NORMAL) -> not a directory; READONLY still OR'd in. + let e = parse_single_entry("default.xex", 0x80); + assert!(!e.is_directory, "non-directory misdecoded as directory"); + assert_eq!( + e.attributes & 0x10, + 0, + "FILE_ATTRIBUTE_DIRECTORY must be clear for a file entry" + ); + assert_ne!(e.attributes & 0x80, 0, "NORMAL bit must be preserved"); + assert_ne!(e.attributes & 0x01, 0, "READONLY must be OR'd in (canary)"); + } + + #[test] + fn archive_and_hidden_bits_are_preserved() { + // ARCHIVE(0x20) | HIDDEN(0x02) authored on disc must survive intact. + let e = parse_single_entry("save.dat", 0x20 | 0x02); + assert_eq!(e.attributes & 0x20, 0x20, "ARCHIVE bit dropped"); + assert_eq!(e.attributes & 0x02, 0x02, "HIDDEN bit dropped"); + assert_eq!(e.attributes & 0x10, 0, "spurious DIRECTORY bit"); + } } diff --git a/crates/xenia-vfs/src/lib.rs b/crates/xenia-vfs/src/lib.rs index 62f484c..55b1d4b 100644 --- a/crates/xenia-vfs/src/lib.rs +++ b/crates/xenia-vfs/src/lib.rs @@ -22,6 +22,16 @@ pub struct VfsEntry { pub is_directory: bool, pub size: u64, pub offset: u64, + /// Xbox `FILE_ATTRIBUTE_*` bitmask for this entry, sourced from the + /// backing device's real on-disc metadata rather than inferred from + /// the path shape. For GDFX disc images this is the on-disc attribute + /// byte at dirent offset +12 OR'd with `FILE_ATTRIBUTE_READONLY` + /// (matches xenia-canary `disc_image_device.cc:154`: + /// `entry->attributes_ = attributes | kFileAttributeReadOnly`). + /// + /// Bit layout (canary `vfs/entry.h:66-76`): READONLY=0x01, HIDDEN=0x02, + /// SYSTEM=0x04, DIRECTORY=0x10, ARCHIVE=0x20, NORMAL=0x80. + pub attributes: u32, } /// Trait for VFS device implementations (XISO, STFS, host path, etc.)