firmware: floor vendor-command allocation to MIN_ALLOC_LEN — drive aborts sub-16-byte data-in (HW-confirmed)

This commit is contained in:
Matthew Jackson
2026-09-15 17:20:51 -07:00
parent 7da4b3bba4
commit f1801351e2
3 changed files with 52 additions and 17 deletions
+23 -6
View File
@@ -66,6 +66,18 @@ pub const CDB_ALLOC_LEN: usize = 7;
/// Bytes returned by one [`Verb::DumpAll`] read (fixed 64-byte window). /// Bytes returned by one [`Verb::DumpAll`] read (fixed 64-byte window).
pub const MEMREAD_LEN: usize = 64; 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 /// Feature state: **passthrough** — firmware does not touch this subsystem
/// (OEM behaviour). The boot default of every feature and what [`Verb::Reset`] /// (OEM behaviour). The boot default of every feature and what [`Verb::Reset`]
/// restores. `0xFF`. /// restores. `0xFF`.
@@ -197,19 +209,24 @@ pub fn build_cdb(
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] { 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] { 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] { 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. /// Build an `IDENTITY` CDB. `alloc_len` sizes the magic+version+state reply.
+22 -5
View File
@@ -34,6 +34,7 @@ fn state_and_frame_constants_match_abi() {
assert_eq!(RESP_MAGIC, b"freemkv"); assert_eq!(RESP_MAGIC, b"freemkv");
assert_eq!(CDB_LEN, 10); assert_eq!(CDB_LEN, 10);
assert_eq!(MEMREAD_LEN, 64); assert_eq!(MEMREAD_LEN, 64);
assert_eq!(MIN_ALLOC_LEN, 64);
assert_eq!(STATE_PASSTHROUGH, 0xFF); assert_eq!(STATE_PASSTHROUGH, 0xFF);
assert_eq!(STATE_OFF, 0x00); assert_eq!(STATE_OFF, 0x00);
assert_eq!(STATE_ON, 0x01); assert_eq!(STATE_ON, 0x01);
@@ -47,27 +48,43 @@ fn state_and_frame_constants_match_abi() {
#[test] #[test]
fn build_set_cdb_exact_bytes() { 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!( assert_eq!(
build_set_cdb(Feature::Ake, STATE_ON), 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] #[test]
fn build_get_cdb_exact_bytes() { 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!( assert_eq!(
build_get_cdb(Feature::Bus), 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] #[test]
fn build_reset_and_identity_cdb_exact_bytes() { 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!( assert_eq!(
build_reset_cdb(), 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]). // IDENTITY with a 64-byte allocation (0x0040 big-endian at cdb[7..9]).
assert_eq!( assert_eq!(
+7 -6
View File
@@ -313,24 +313,25 @@ mod tests {
} }
/// Each unlock SET is `Verb::Set` (02) on the right feature id (cdb[5]) with /// 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] #[test]
fn unlock_set_cdbs_have_the_new_grammar_shape() { fn unlock_set_cdbs_have_the_new_grammar_shape() {
assert_eq!( assert_eq!(
build_set_cdb(Feature::Region, STATE_ON), 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!( assert_eq!(
build_set_cdb(Feature::Speed, SPEED_MAX), 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!( assert_eq!(
build_set_cdb(Feature::Ake, STATE_ON), 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!( assert_eq!(
build_set_cdb(Feature::Bus, STATE_ON), 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() { fn reset_cdb_shape() {
assert_eq!( assert_eq!(
build_reset_cdb(), build_reset_cdb(),
[0x3C, 0x0E, 0xC0, 0xDE, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00] [0x3C, 0x0E, 0xC0, 0xDE, 0x04, 0x00, 0x00, 0x00, 0x40, 0x00]
); );
} }