diff --git a/src/firmware/mod.rs b/src/firmware/mod.rs index fbcb4a5..9d1ceee 100644 --- a/src/firmware/mod.rs +++ b/src/firmware/mod.rs @@ -66,6 +66,18 @@ pub const CDB_ALLOC_LEN: usize = 7; /// Bytes returned by one [`Verb::DumpAll`] read (fixed 64-byte window). pub const MEMREAD_LEN: usize = 64; +/// Minimum data-in allocation length any freemkv vendor command may request. +/// +/// **Hardware-confirmed (LG BU40N on freemkv firmware):** the drive's `READ +/// BUFFER` hijack ABORTS (Check Condition, sense key Aborted Command) any vendor +/// command whose data-in allocation length is under ~16 bytes (0/1/2 all abort; +/// 16 and 64 both succeed). Every builder floors its allocation at `64` — it +/// matches [`MEMREAD_LEN`] / the IDENTITY caller and is safely above the minimum. +/// The verb/feature/state ride in the CDB, so a larger data-in is harmless; a +/// [`Verb::Get`] still reads its state byte from data offset 0. Mirrors the +/// firmware ABI `MIN_ALLOC_LEN`. +pub const MIN_ALLOC_LEN: u16 = 64; + /// Feature state: **passthrough** — firmware does not touch this subsystem /// (OEM behaviour). The boot default of every feature and what [`Verb::Reset`] /// restores. `0xFF`. @@ -197,19 +209,24 @@ pub fn build_cdb( cdb } -/// Build a `SET feature = state` CDB (no data-in). +/// Build a `SET feature = state` CDB. Requests a [`MIN_ALLOC_LEN`]-byte data-in +/// (the drive aborts sub-16-byte transfers — HW-confirmed, see [`MIN_ALLOC_LEN`]); +/// the feature/state ride in the CDB, so the returned payload is ignored. pub fn build_set_cdb(feature: Feature, state: u8) -> [u8; CDB_LEN] { - build_cdb(Verb::Set, Some(feature), Some(state), 0) + build_cdb(Verb::Set, Some(feature), Some(state), MIN_ALLOC_LEN) } -/// Build a `GET feature` CDB (reads the state byte back in a 1-byte data-in). +/// Build a `GET feature` CDB. Requests a [`MIN_ALLOC_LEN`]-byte data-in (the +/// drive aborts a 1-byte transfer — HW-confirmed, see [`MIN_ALLOC_LEN`]); the +/// current state byte is read back from data offset 0. pub fn build_get_cdb(feature: Feature) -> [u8; CDB_LEN] { - build_cdb(Verb::Get, Some(feature), None, 1) + build_cdb(Verb::Get, Some(feature), None, MIN_ALLOC_LEN) } -/// Build a `RESET` CDB (all features → passthrough). +/// Build a `RESET` CDB (all features → passthrough). Floors its data-in at +/// [`MIN_ALLOC_LEN`] for the same HW min-transfer reason as [`build_set_cdb`]. pub fn build_reset_cdb() -> [u8; CDB_LEN] { - build_cdb(Verb::Reset, None, None, 0) + build_cdb(Verb::Reset, None, None, MIN_ALLOC_LEN) } /// Build an `IDENTITY` CDB. `alloc_len` sizes the magic+version+state reply. diff --git a/src/firmware/mod_tests.rs b/src/firmware/mod_tests.rs index 781f5b3..43511a5 100644 --- a/src/firmware/mod_tests.rs +++ b/src/firmware/mod_tests.rs @@ -34,6 +34,7 @@ fn state_and_frame_constants_match_abi() { assert_eq!(RESP_MAGIC, b"freemkv"); assert_eq!(CDB_LEN, 10); assert_eq!(MEMREAD_LEN, 64); + assert_eq!(MIN_ALLOC_LEN, 64); assert_eq!(STATE_PASSTHROUGH, 0xFF); assert_eq!(STATE_OFF, 0x00); assert_eq!(STATE_ON, 0x01); @@ -47,27 +48,43 @@ fn state_and_frame_constants_match_abi() { #[test] fn build_set_cdb_exact_bytes() { - // SET Ake = ON: verb 02, feature 06, state 01, no alloc. + // SET Ake = ON: verb 02, feature 06, state 01, alloc MIN_ALLOC_LEN (0x0040) + // at cdb[7..9] big-endian — the drive aborts a sub-16-byte data-in. assert_eq!( build_set_cdb(Feature::Ake, STATE_ON), - [0x3C, 0x0E, 0xC0, 0xDE, 0x02, 0x06, 0x01, 0x00, 0x00, 0x00] + [0x3C, 0x0E, 0xC0, 0xDE, 0x02, 0x06, 0x01, 0x00, 0x40, 0x00] ); } #[test] fn build_get_cdb_exact_bytes() { - // GET Bus: verb 03, feature 07, alloc 1 at cdb[7..9] big-endian. + // GET Bus: verb 03, feature 07, alloc MIN_ALLOC_LEN (0x0040) at cdb[7..9] + // big-endian — the drive aborts a 1-byte data-in; state read from offset 0. assert_eq!( build_get_cdb(Feature::Bus), - [0x3C, 0x0E, 0xC0, 0xDE, 0x03, 0x07, 0x00, 0x00, 0x01, 0x00] + [0x3C, 0x0E, 0xC0, 0xDE, 0x03, 0x07, 0x00, 0x00, 0x40, 0x00] ); } +#[test] +fn vendor_commands_floor_alloc_len_at_min_for_hw() { + // HW-confirmed: the drive aborts a vendor command with a sub-16-byte data-in + // allocation, so SET/GET/RESET all request at least MIN_ALLOC_LEN (= 64, + // comfortably above the ~16-byte hardware floor). + assert_eq!(MIN_ALLOC_LEN, 64); + let alloc = + |cdb: &[u8; CDB_LEN]| u16::from_be_bytes([cdb[CDB_ALLOC_LEN], cdb[CDB_ALLOC_LEN + 1]]); + assert_eq!(alloc(&build_set_cdb(Feature::Ake, STATE_ON)), MIN_ALLOC_LEN); + assert_eq!(alloc(&build_get_cdb(Feature::Ake)), MIN_ALLOC_LEN); + assert_eq!(alloc(&build_reset_cdb()), MIN_ALLOC_LEN); +} + #[test] fn build_reset_and_identity_cdb_exact_bytes() { + // RESET floors its data-in at MIN_ALLOC_LEN (0x0040) for the same HW reason. assert_eq!( build_reset_cdb(), - [0x3C, 0x0E, 0xC0, 0xDE, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00] + [0x3C, 0x0E, 0xC0, 0xDE, 0x04, 0x00, 0x00, 0x00, 0x40, 0x00] ); // IDENTITY with a 64-byte allocation (0x0040 big-endian at cdb[7..9]). assert_eq!( diff --git a/src/freemkv/mod.rs b/src/freemkv/mod.rs index f6cecf3..7c39054 100644 --- a/src/freemkv/mod.rs +++ b/src/freemkv/mod.rs @@ -313,24 +313,25 @@ mod tests { } /// Each unlock SET is `Verb::Set` (02) on the right feature id (cdb[5]) with - /// the state at cdb[6]; SET carries no data-in (alloc 0). + /// the state at cdb[6]; SET floors its data-in allocation at MIN_ALLOC_LEN + /// (0x0040 at cdb[7..9]) — the drive aborts a sub-16-byte transfer (HW-confirmed). #[test] fn unlock_set_cdbs_have_the_new_grammar_shape() { assert_eq!( build_set_cdb(Feature::Region, STATE_ON), - [0x3C, 0x0E, 0xC0, 0xDE, 0x02, 0x02, 0x01, 0x00, 0x00, 0x00] + [0x3C, 0x0E, 0xC0, 0xDE, 0x02, 0x02, 0x01, 0x00, 0x40, 0x00] ); assert_eq!( build_set_cdb(Feature::Speed, SPEED_MAX), - [0x3C, 0x0E, 0xC0, 0xDE, 0x02, 0x01, 0x01, 0x00, 0x00, 0x00] + [0x3C, 0x0E, 0xC0, 0xDE, 0x02, 0x01, 0x01, 0x00, 0x40, 0x00] ); assert_eq!( build_set_cdb(Feature::Ake, STATE_ON), - [0x3C, 0x0E, 0xC0, 0xDE, 0x02, 0x06, 0x01, 0x00, 0x00, 0x00] + [0x3C, 0x0E, 0xC0, 0xDE, 0x02, 0x06, 0x01, 0x00, 0x40, 0x00] ); assert_eq!( build_set_cdb(Feature::Bus, STATE_ON), - [0x3C, 0x0E, 0xC0, 0xDE, 0x02, 0x07, 0x01, 0x00, 0x00, 0x00] + [0x3C, 0x0E, 0xC0, 0xDE, 0x02, 0x07, 0x01, 0x00, 0x40, 0x00] ); } @@ -572,7 +573,7 @@ mod tests { fn reset_cdb_shape() { assert_eq!( build_reset_cdb(), - [0x3C, 0x0E, 0xC0, 0xDE, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00] + [0x3C, 0x0E, 0xC0, 0xDE, 0x04, 0x00, 0x00, 0x00, 0x40, 0x00] ); }