From 261766a7083892eb3acc86b343e6d6974eb7e778 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Wed, 15 Oct 2025 15:48:19 +1030 Subject: [PATCH 01/20] wire: flags: Use debug representation for flag value Signed-off-by: Andrew Jeffery --- src/wire/flags.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wire/flags.rs b/src/wire/flags.rs index da80a67..a28ebd7 100644 --- a/src/wire/flags.rs +++ b/src/wire/flags.rs @@ -35,7 +35,7 @@ where { let val = <::Type>::from_reader_with_ctx(reader, ctx)?; let fs = FlagSet::new(val) - .map_err(|_| deku_error!(DekuError::Parse, "Found invalid flag set", "{}", val))?; + .map_err(|_| deku_error!(DekuError::Parse, "Found invalid flag set", "{:?}", val))?; Ok(WireFlagSet(fs)) } } From 2dfee824b6eb9614be1fc4cecd40c59e84811a8f Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Thu, 23 Oct 2025 10:32:12 +1030 Subject: [PATCH 02/20] nvme: mi: Restore license and copyright to top of file Signed-off-by: Andrew Jeffery --- src/nvme/mi.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/nvme/mi.rs b/src/nvme/mi.rs index 6ff4f68..debcb9b 100644 --- a/src/nvme/mi.rs +++ b/src/nvme/mi.rs @@ -1,3 +1,8 @@ +// SPDX-License-Identifier: GPL-3.0-only +/* + * Copyright (c) 2025 Code Construct + */ + use deku::ctx::Endian; use deku::{DekuError, DekuRead, DekuWrite}; use flagset::{FlagSet, flags}; @@ -11,10 +16,6 @@ use crate::{CommandEffectError, Discriminant, Encode, MAX_CONTROLLERS}; use super::{AdminGetLogPageLidRequestType, AdminIdentifyCnsRequestType}; -// SPDX-License-Identifier: GPL-3.0-only -/* - * Copyright (c) 2025 Code Construct - */ pub mod dev; // MI v2.0, 3.1.1, Figure 20, NMIMT From 1b51248c38384c87b83f670d97da59e9008768a8 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Wed, 22 Oct 2025 15:17:33 +1030 Subject: [PATCH 03/20] cargo: Bump log dependency to align with deku Signed-off-by: Andrew Jeffery --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0801480..2a65167 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -446,9 +446,9 @@ checksum = "d26c52dbd32dccf2d10cac7725f8eae5296885fb5703b261f7d0a0739ec807ab" [[package]] name = "log" -version = "0.4.27" +version = "0.4.28" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "13dc2df351e3202783a1fe0d44375f7295ffb4049267b0f3018346dc122a1d94" +checksum = "34080505efa8e45a4b816c349525ebe327ceaa8559756f0356cba97ef3bf7432" [[package]] name = "mctp" diff --git a/Cargo.toml b/Cargo.toml index 0f1197e..8f43871 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,7 @@ deku = { version = "0.20.0", default-features = false } flagset = { version = "0.4.7", default-features = false } heapless = "0.8.0" hmac = { version = "0.12.1", default-features = false } -log = "0.4.22" +log = "0.4.28" mctp = { version = "0.2.0", default-features = false } sha2 = { version = "0.10.9", default-features = false } uuid = { version = "1.17.0", default-features = false } From 6da8f9b6087e1bb3abd81451fc0832cfe2051389 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Wed, 15 Oct 2025 10:36:26 +1030 Subject: [PATCH 04/20] nvme: mi: Convert MessageHeader to deku bits Signed-off-by: Andrew Jeffery --- Cargo.lock | 46 ++++++++++++++++++++++++++++++++++++---- Cargo.toml | 2 +- src/nvme/mi.rs | 53 ++++++++++++++++------------------------------ src/nvme/mi/dev.rs | 29 +++++++++++-------------- 4 files changed, 73 insertions(+), 57 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2a65167..ab08b89 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -141,6 +141,18 @@ version = "2.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1b8e56985ec62d17e9c1001dc89c88ecd7dc08e47eba5ec7c29c7b5eeecde967" +[[package]] +name = "bitvec" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1bc2832c24239b0141d5674bb9174f9d68a8b5b3f2753311927c172ca46f7e9c" +dependencies = [ + "funty", + "radium", + "tap", + "wyz", +] + [[package]] name = "block-buffer" version = "0.10.4" @@ -262,9 +274,9 @@ dependencies = [ [[package]] name = "deku" version = "0.20.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8bdbd42cb0450604984c8135ced826886d7e146994ad265b52eefcbf717c7be5" +source = "git+https://github.com/codeconstruct/deku.git?tag=cc%2Fdeku-v0.19.1%2Fbit-precise-no-alloc-1#036c221dc47aca4890db20e08d381bb32bc788f6" dependencies = [ + "bitvec", "deku_derive", "no_std_io2", "rustversion", @@ -273,8 +285,7 @@ dependencies = [ [[package]] name = "deku_derive" version = "0.20.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2a27e999281c31ca063da429182b6055a07745564912c925036e2b7e67170646" +source = "git+https://github.com/codeconstruct/deku.git?tag=cc%2Fdeku-v0.19.1%2Fbit-precise-no-alloc-1#036c221dc47aca4890db20e08d381bb32bc788f6" dependencies = [ "darling", "proc-macro2", @@ -351,6 +362,12 @@ version = "1.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" +[[package]] +name = "funty" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6d5a32815ae3f33302d95fdcb2ce17862f8c65363dcfd29360480ba1001fc9c" + [[package]] name = "futures-core" version = "0.3.31" @@ -565,6 +582,12 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "radium" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc33ff2d4973d518d823d61aa239014831e521c75da58e3df4840d3f47749d09" + [[package]] name = "rustix" version = "0.38.44" @@ -690,6 +713,12 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "tap" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369" + [[package]] name = "termcolor" version = "1.4.1" @@ -853,3 +882,12 @@ name = "windows_x86_64_msvc" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" + +[[package]] +name = "wyz" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "05f360fc0b24296329c78fda852a1e9ae82de9cf7b27dae4b7f62f118f77b9ed" +dependencies = [ + "tap", +] diff --git a/Cargo.toml b/Cargo.toml index 8f43871..1bd6c1d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,7 +6,7 @@ license = "GPL-3.0-only" [dependencies] crc = "3.2.1" -deku = { version = "0.20.0", default-features = false } +deku = { git = "https://github.com/codeconstruct/deku.git", tag = "cc/deku-v0.19.1/bit-precise-no-alloc-1", default-features = false, features = ["bits"] } flagset = { version = "0.4.7", default-features = false } heapless = "0.8.0" hmac = { version = "0.12.1", default-features = false } diff --git a/src/nvme/mi.rs b/src/nvme/mi.rs index debcb9b..184c61e 100644 --- a/src/nvme/mi.rs +++ b/src/nvme/mi.rs @@ -3,7 +3,7 @@ * Copyright (c) 2025 Code Construct */ -use deku::ctx::Endian; +use deku::ctx::{Endian, Order}; use deku::{DekuError, DekuRead, DekuWrite}; use flagset::{FlagSet, flags}; use log::debug; @@ -19,7 +19,14 @@ use super::{AdminGetLogPageLidRequestType, AdminIdentifyCnsRequestType}; pub mod dev; // MI v2.0, 3.1.1, Figure 20, NMIMT -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, DekuRead, DekuWrite, PartialEq, Eq)] +#[deku( + bits = "4", + bit_order = "order", + ctx = "endian: Endian, order: Order", + endian = "endian", + id_type = "u8" +)] #[repr(u8)] enum MessageType { ControlPrimitive = 0x00, @@ -28,52 +35,28 @@ enum MessageType { PcieCommand = 0x04, AsynchronousEvent = 0x05, } -unsafe impl Discriminant for MessageType {} - -impl TryFrom for MessageType { - type Error = u8; - - fn try_from(value: u8) -> Result { - match value { - 0x00 => Ok(Self::ControlPrimitive), - 0x01 => Ok(Self::NvmeMiCommand), - 0x02 => Ok(Self::NvmeAdminCommand), - 0x04 => Ok(Self::PcieCommand), - 0x05 => Ok(Self::AsynchronousEvent), - _ => Err(value), - } - } -} // MI v2.0, 3.1.1, Figure 20 #[derive(Debug, DekuRead, DekuWrite)] -#[deku(endian = "little")] +#[deku(bit_order = "lsb", endian = "little")] struct MessageHeader { - #[deku(pad_bytes_after = "2")] - nmimt: u8, + #[deku(bits = "1", pad_bits_after = "2")] + csi: u8, + nmimt: MessageType, + #[deku(bits = "1", pad_bytes_after = "2")] + ror: bool, } impl Encode<3> for MessageHeader {} impl MessageHeader { fn respond(nmimt: MessageType) -> Self { Self { - nmimt: ((true as u8) << 7) | ((nmimt.id() & 0xf) << 3), + csi: 0, + nmimt, + ror: true, } } - - fn nmimt(&self) -> Result { - ((self.nmimt >> 3) & 0xf).try_into() - } - - fn csi(&self) -> bool { - (self.nmimt & 0x01) != 0 - } - - fn ror(&self) -> bool { - (self.nmimt & 0x80) != 0 - } } - // MI v2.0, 4.1.2, Figure 29 #[derive(Debug, DekuRead, DekuWrite, PartialEq)] #[deku(endian = "endian", ctx = "endian: Endian", id_type = "u8")] diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index 28c184b..925b65f 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -97,11 +97,7 @@ impl RequestHandler for MessageHeader { debug!("{self:x?}"); // TODO: Command and Feature Lockdown handling // TODO: Handle subsystem reset, section 8.1, v2.0 - let Ok(nmimt) = ctx.nmimt() else { - return Err(ResponseStatus::InvalidCommandOpcode); - }; - - match nmimt { + match ctx.nmimt { MessageType::NvmeMiCommand => { match &NvmeMiCommandRequestHeader::from_bytes((rest, 0)) { Ok(((rest, _), ch)) => ch.handle(ch, mep, subsys, rest, resp, app).await, @@ -135,7 +131,7 @@ impl RequestHandler for MessageHeader { } } _ => { - debug!("Unimplemented NMINT: {:?}", ctx.nmimt()); + debug!("Unimplemented NMINT: {:?}", ctx.nmimt); Err(ResponseStatus::InternalError) } } @@ -2170,31 +2166,30 @@ impl crate::ManagementEndpoint { return; } - let Ok(((rest, _), mh)) = MessageHeader::from_bytes((msg, 0)) else { - debug!("Message too short to extract NVMeMIMessageHeader"); + let res = MessageHeader::from_bytes((msg, 0)); + let Ok(((rest, _), mh)) = &res else { + debug!( + "Message too short to extract NVMeMIMessageHeader: {:?}", + res + ); return; }; - if mh.csi() { + if mh.csi != 0 { debug!("Support second command slot"); return; } - if mh.ror() { - debug!("NVMe-MI message was not a request: {:?}", mh.ror()); + if mh.ror { + debug!("NVMe-MI message was not a request: {:?}", mh.ror); return; } - let Ok(nmimt) = mh.nmimt() else { - debug!("Message contains unrecognised NMIMT: {mh:x?}"); - return; - }; - if let Err(status) = mh.handle(&mh, self, subsys, rest, &mut resp, app).await { let mut digest = ISCSI.digest(); digest.update(&[0x80 | 0x04]); - let Ok(mh) = MessageHeader::respond(nmimt).encode() else { + let Ok(mh) = MessageHeader::respond(mh.nmimt).encode() else { debug!("Failed to encode MessageHeader for error response"); return; }; From 5ac22e413cec0f9876801bb3b0a63f523d90a873 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Tue, 21 Oct 2025 10:52:58 +1030 Subject: [PATCH 05/20] nvme: mi: Convert SmbusFrequency to deku bits Signed-off-by: Andrew Jeffery --- src/lib.rs | 30 ++++++++++++------ src/nvme/mi.rs | 77 ++++++++++++++++++++++++++++++---------------- src/nvme/mi/dev.rs | 29 ++++++++--------- tests/nvme_mi.rs | 4 +-- 4 files changed, 88 insertions(+), 52 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index df45577..52fb00c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -26,6 +26,16 @@ const MAX_NAMESPACES: usize = 4; const MAX_PORTS: usize = 2; const MAX_NIDTS: usize = 2; +pub mod smbus { + #[derive(Debug, Clone, Copy, Eq, PartialEq)] + pub enum BusFrequency { + NotSupported, + Freq100Khz, + Freq400Khz, + Freq1Mhz, + } +} + #[derive(Debug)] pub enum CommandEffect { SetMtu { @@ -34,7 +44,7 @@ pub enum CommandEffect { }, SetSmbusFreq { port_id: PortId, - freq: nvme::mi::SmbusFrequency, + freq: smbus::BusFrequency, }, } @@ -115,25 +125,25 @@ impl Default for PciePort { pub struct TwoWirePort { // MI v2.0, 5.7.2, Figure 116 cvpdaddr: u8, - mvpdfreq: nvme::mi::SmbusFrequency, + mvpdfreq: smbus::BusFrequency, cmeaddr: u8, i3csprt: bool, - msmbfreq: nvme::mi::SmbusFrequency, + msmbfreq: smbus::BusFrequency, nvmebms: bool, // Local state - smbfreq: nvme::mi::SmbusFrequency, + smbfreq: smbus::BusFrequency, } impl TwoWirePort { pub fn new() -> Self { Self { cvpdaddr: 0, - mvpdfreq: nvme::mi::SmbusFrequency::FreqNotSupported, + mvpdfreq: smbus::BusFrequency::NotSupported, cmeaddr: 0x1d, i3csprt: false, - msmbfreq: nvme::mi::SmbusFrequency::Freq100Khz, + msmbfreq: smbus::BusFrequency::Freq100Khz, nvmebms: false, - smbfreq: nvme::mi::SmbusFrequency::Freq100Khz, + smbfreq: smbus::BusFrequency::Freq100Khz, } } @@ -149,17 +159,17 @@ impl Default for TwoWirePort { } pub struct TwoWirePortBuilder { - msmbfreq: nvme::mi::SmbusFrequency, + msmbfreq: smbus::BusFrequency, } impl TwoWirePortBuilder { pub fn new() -> Self { Self { - msmbfreq: nvme::mi::SmbusFrequency::Freq100Khz, + msmbfreq: smbus::BusFrequency::Freq100Khz, } } - pub fn msmbfreq(&mut self, freq: nvme::mi::SmbusFrequency) -> &mut Self { + pub fn msmbfreq(&mut self, freq: smbus::BusFrequency) -> &mut Self { self.msmbfreq = freq; self } diff --git a/src/nvme/mi.rs b/src/nvme/mi.rs index 184c61e..341095a 100644 --- a/src/nvme/mi.rs +++ b/src/nvme/mi.rs @@ -3,7 +3,7 @@ * Copyright (c) 2025 Code Construct */ -use deku::ctx::{Endian, Order}; +use deku::ctx::{BitSize, Endian, Order}; use deku::{DekuError, DekuRead, DekuWrite}; use flagset::{FlagSet, flags}; use log::debug; @@ -161,13 +161,51 @@ enum NvmeMiConfigurationIdentifierRequestType { AsynchronousEvent = 0x04, } +// MI v2.0, 5.1.1, Figure 77, SFREQ +#[derive(Debug, Clone, Copy, DekuRead, DekuWrite, Eq, PartialEq, PartialOrd)] +#[deku( + bits = "bits.0", + id_type = "u8", + ctx = "endian: Endian, bits: BitSize", + endian = "endian" +)] +#[repr(u8)] +enum SmbusFrequency { + Reserved = 0x00, + Freq100Khz = 0x01, + Freq400Khz = 0x02, + Freq1Mhz = 0x03, +} + +impl From for crate::smbus::BusFrequency { + fn from(value: SmbusFrequency) -> Self { + match value { + SmbusFrequency::Reserved => Self::NotSupported, + SmbusFrequency::Freq100Khz => Self::Freq100Khz, + SmbusFrequency::Freq400Khz => Self::Freq400Khz, + SmbusFrequency::Freq1Mhz => Self::Freq1Mhz, + } + } +} + +impl From for SmbusFrequency { + fn from(value: crate::smbus::BusFrequency) -> Self { + match value { + crate::smbus::BusFrequency::NotSupported => Self::Reserved, + crate::smbus::BusFrequency::Freq100Khz => Self::Freq100Khz, + crate::smbus::BusFrequency::Freq400Khz => Self::Freq400Khz, + crate::smbus::BusFrequency::Freq1Mhz => Self::Freq1Mhz, + } + } +} + // MI v2.0, 5.1.1, Figure 77 #[derive(Debug, DekuWrite, PartialEq)] #[deku(endian = "little")] struct GetSmbusI2cFrequencyResponse { status: ResponseStatus, - #[deku(pad_bytes_after = "2")] - mr_sfreq: crate::nvme::mi::SmbusFrequency, + #[deku(bits = "4", pad_bits_before = "4", pad_bytes_after = "2")] + sfreq: SmbusFrequency, } impl Encode<4> for GetSmbusI2cFrequencyResponse {} @@ -201,13 +239,10 @@ struct NvmeMiConfigurationSetRequest { #[derive(Debug, DekuRead, DekuWrite, Eq, PartialEq)] #[deku(ctx = "endian: Endian", endian = "endian")] struct SmbusI2cFrequencyRequest { - // XXX: This is inaccurate as SFREQ is specified as 4 bits, not 8 - // TODO: Support deku/bits feature without deku/alloc - dw0_sfreq: crate::nvme::mi::SmbusFrequency, - // Skip intermediate bytes in DWORD 0 - #[deku(seek_from_current = "1")] - dw0_portid: u8, - _dw1: u32, + #[deku(bits = "4", pad_bits_before = "4", pad_bytes_after = "1")] + sfreq: SmbusFrequency, + #[deku(pad_bytes_after = "4")] + portid: u8, } // MI v2.0, 5.2.2, Figure 87 @@ -670,28 +705,18 @@ struct PciePortDataResponse { } impl Encode<24> for PciePortDataResponse {} -// MI v2.0, Figure 116, MVPDFREQ -#[allow(dead_code)] -#[derive(Clone, Copy, Debug, DekuRead, DekuWrite, Eq, Ord, PartialEq, PartialOrd)] -#[deku(endian = "endian", ctx = "endian: Endian")] -#[deku(id_type = "u8")] -#[repr(u8)] -pub enum SmbusFrequency { - FreqNotSupported = 0x00, - Freq100Khz = 0x01, - Freq400Khz = 0x02, - Freq1Mhz = 0x03, -} -unsafe impl Discriminant for SmbusFrequency {} - // MI v2.0, 5.7.2, Figure 116 #[derive(Debug, DekuWrite)] #[deku(endian = "little")] struct TwoWirePortDataResponse { cvpdaddr: u8, - mvpdfreq: u8, + #[deku(bits = "8")] + mvpdfreq: SmbusFrequency, cmeaddr: u8, - twprt: u8, + #[deku(bits = "1", pad_bits_after = "5")] + twprt_i3csprt: bool, + #[deku(bits = "2")] + twprt_msmbfreq: SmbusFrequency, nvmebm: u8, } impl Encode<24> for TwoWirePortDataResponse {} diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index 925b65f..ff09c37 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -397,27 +397,27 @@ impl RequestHandler for NvmeMiConfigurationSetRequest { return Err(ResponseStatus::InvalidCommandSize); } - let Some(port) = subsys.ports.get_mut(sifr.dw0_portid as usize) else { - debug!("Unrecognised port ID: {}", sifr.dw0_portid); + let Some(port) = subsys.ports.get_mut(sifr.portid as usize) else { + debug!("Unrecognised port ID: {}", sifr.portid); return Err(ResponseStatus::InvalidParameter); }; let crate::PortType::TwoWire(twprt) = &mut port.typ else { - debug!("Port {} is not a TwoWire port: {:?}", sifr.dw0_portid, port); + debug!("Port {} is not a TwoWire port: {:?}", sifr.portid, port); return Err(ResponseStatus::InvalidParameter); }; - if sifr.dw0_sfreq > twprt.msmbfreq { - debug!("Unsupported SMBus frequency: {:?}", sifr.dw0_sfreq); + if sifr.sfreq > twprt.msmbfreq.into() { + debug!("Unsupported SMBus frequency: {:?}", sifr.sfreq); return Err(ResponseStatus::InvalidParameter); } app(CommandEffect::SetSmbusFreq { port_id: port.id, - freq: sifr.dw0_sfreq, + freq: sifr.sfreq.into(), }) .await?; - twprt.smbfreq = sifr.dw0_sfreq; + twprt.smbfreq = sifr.sfreq.into(); let mh = MessageHeader::respond(MessageType::NvmeMiCommand).encode()?; @@ -510,13 +510,13 @@ impl RequestHandler for NvmeMiConfigurationGetRequest { return Err(ResponseStatus::InvalidCommandSize); } - let Some(port) = subsys.ports.get(sifr.dw0_portid as usize) else { - debug!("Unrecognised port ID: {}", sifr.dw0_portid); + let Some(port) = subsys.ports.get(sifr.portid as usize) else { + debug!("Unrecognised port ID: {}", sifr.portid); return Err(ResponseStatus::InvalidParameter); }; let crate::PortType::TwoWire(twprt) = port.typ else { - debug!("Port {} is not a TwoWire port: {:?}", sifr.dw0_portid, port); + debug!("Port {} is not a TwoWire port: {:?}", sifr.portid, port); return Err(ResponseStatus::InvalidParameter); }; @@ -524,7 +524,7 @@ impl RequestHandler for NvmeMiConfigurationGetRequest { let fr = GetSmbusI2cFrequencyResponse { status: ResponseStatus::Success, - mr_sfreq: twprt.smbfreq, + sfreq: twprt.smbfreq.into(), } .encode()?; @@ -668,9 +668,10 @@ impl RequestHandler for NvmeMiDataStructureRequest { crate::PortType::TwoWire(twprt) => { let twpd = TwoWirePortDataResponse { cvpdaddr: twprt.cvpdaddr, - mvpdfreq: twprt.mvpdfreq.id(), + mvpdfreq: twprt.mvpdfreq.into(), cmeaddr: twprt.cmeaddr, - twprt: (twprt.i3csprt as u8) << 7 | twprt.msmbfreq.id() & 3, + twprt_i3csprt: twprt.i3csprt, + twprt_msmbfreq: twprt.msmbfreq.into(), nvmebm: twprt.nvmebms.into(), } .encode()?; @@ -2185,7 +2186,7 @@ impl crate::ManagementEndpoint { return; } - if let Err(status) = mh.handle(&mh, self, subsys, rest, &mut resp, app).await { + if let Err(status) = mh.handle(mh, self, subsys, rest, &mut resp, app).await { let mut digest = ISCSI.digest(); digest.update(&[0x80 | 0x04]); diff --git a/tests/nvme_mi.rs b/tests/nvme_mi.rs index 9ff8708..a3eebdb 100644 --- a/tests/nvme_mi.rs +++ b/tests/nvme_mi.rs @@ -224,7 +224,7 @@ mod read_nvme_mi_data_structure { let mut subsys = Subsystem::new(SubsystemInfo::invalid()); let _ = subsys.add_port(PortType::Pcie(PciePort::new())).unwrap(); let twp = TwoWirePort::builder() - .msmbfreq(nvme_mi_dev::nvme::mi::SmbusFrequency::Freq400Khz) + .msmbfreq(nvme_mi_dev::smbus::BusFrequency::Freq400Khz) .build(); let twpid = subsys.add_port(PortType::TwoWire(twp)).unwrap(); let mut mep = ManagementEndpoint::new(twpid); @@ -1281,7 +1281,7 @@ mod configuration_set { let mut subsys = Subsystem::new(SubsystemInfo::invalid()); let _ = subsys.add_port(PortType::Pcie(PciePort::new())).unwrap(); let twp = TwoWirePort::builder() - .msmbfreq(nvme_mi_dev::nvme::mi::SmbusFrequency::Freq400Khz) + .msmbfreq(nvme_mi_dev::smbus::BusFrequency::Freq400Khz) .build(); let twpid = subsys.add_port(PortType::TwoWire(twp)).unwrap(); let mut mep = ManagementEndpoint::new(twpid); From 3a4569c8357aa124f1339e861e2476f4fa17a2e8 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Tue, 21 Oct 2025 12:33:34 +1030 Subject: [PATCH 06/20] nvme: mi: Tidy PCIe Link attributes Signed-off-by: Andrew Jeffery --- src/lib.rs | 16 +++--- src/nvme/mi.rs | 125 ++++++++++++++++++++++++++++++++++++--------- src/nvme/mi/dev.rs | 14 +++-- src/pcie.rs | 32 ++++++++++++ 4 files changed, 152 insertions(+), 35 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 52fb00c..10e63ed 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -94,10 +94,10 @@ pub struct PciePort { d: u16, f: u16, seg: u8, - mps: nvme::mi::PciePayloadSize, - cls: nvme::mi::PcieLinkSpeed, - mlw: nvme::mi::PcieLinkWidth, - nlw: nvme::mi::PcieLinkWidth, + mps: pcie::PayloadSize, + cls: pcie::LinkSpeed, + mlw: pcie::LinkWidth, + nlw: pcie::LinkWidth, } impl PciePort { @@ -107,10 +107,10 @@ impl PciePort { d: 0, f: 0, seg: 0, - mps: nvme::mi::PciePayloadSize::Payload128B, - cls: nvme::mi::PcieLinkSpeed::Gts2p5, - mlw: nvme::mi::PcieLinkWidth::X2, - nlw: nvme::mi::PcieLinkWidth::X1, + mps: pcie::PayloadSize::Payload128B, + cls: pcie::LinkSpeed::Gts2p5, + mlw: pcie::LinkWidth::X2, + nlw: pcie::LinkWidth::X1, } } } diff --git a/src/nvme/mi.rs b/src/nvme/mi.rs index 341095a..d686389 100644 --- a/src/nvme/mi.rs +++ b/src/nvme/mi.rs @@ -634,10 +634,10 @@ struct PortInformationResponse { impl Encode<8> for PortInformationResponse {} // MI v2.0, 5.7.2, Figure 115, PCIEMPS -#[allow(dead_code)] -#[derive(Clone, Copy, Debug, Eq, PartialEq)] +#[derive(Clone, Copy, DekuRead, DekuWrite, Debug, Eq, PartialEq)] +#[deku(ctx = "endian: Endian", endian = "endian", id_type = "u8")] #[repr(u8)] -pub enum PciePayloadSize { +enum PciePayloadSize { Payload128B = 0x00, Payload256B = 0x01, Payload512B = 0x02, @@ -646,17 +646,50 @@ pub enum PciePayloadSize { Payload4Kb = 0x05, } -impl From for u8 { - fn from(pps: PciePayloadSize) -> Self { - pps as Self +impl From for PciePayloadSize { + fn from(value: crate::pcie::PayloadSize) -> Self { + match value { + crate::pcie::PayloadSize::Payload128B => Self::Payload128B, + crate::pcie::PayloadSize::Payload256B => Self::Payload256B, + crate::pcie::PayloadSize::Payload512B => Self::Payload512B, + crate::pcie::PayloadSize::Payload1Kb => Self::Payload1Kb, + crate::pcie::PayloadSize::Payload2Kb => Self::Payload2Kb, + crate::pcie::PayloadSize::Payload4Kb => Self::Payload4Kb, + } + } +} + +impl From for crate::pcie::PayloadSize { + fn from(value: PciePayloadSize) -> Self { + match value { + PciePayloadSize::Payload128B => Self::Payload128B, + PciePayloadSize::Payload256B => Self::Payload256B, + PciePayloadSize::Payload512B => Self::Payload512B, + PciePayloadSize::Payload1Kb => Self::Payload1Kb, + PciePayloadSize::Payload2Kb => Self::Payload2Kb, + PciePayloadSize::Payload4Kb => Self::Payload4Kb, + } + } +} + +// MI v2.0, 5.7.2, Figure 115, PCIESLSV +flags! { + #[repr(u8)] + enum PcieSupportedLinkSpeeds: u8 { + Gts2p5, + Gts5, + Gts8, + Gts16, + Gts32, + Gts64, } } // MI v2.0, 5.7.2, Figure 115, PCIECLS -#[allow(dead_code)] -#[derive(Clone, Copy, Debug, Eq, PartialEq)] +#[derive(Clone, Copy, Debug, DekuRead, DekuWrite, Eq, PartialEq)] +#[deku(ctx = "endian: Endian", endian = "endian", id_type = "u8")] #[repr(u8)] -pub enum PcieLinkSpeed { +enum PcieLinkSpeed { Inactive = 0x00, Gts2p5 = 0x01, Gts5 = 0x02, @@ -666,17 +699,39 @@ pub enum PcieLinkSpeed { Gts64 = 0x06, } -impl From for u8 { - fn from(pls: PcieLinkSpeed) -> Self { - pls as Self +impl From for PcieLinkSpeed { + fn from(value: crate::pcie::LinkSpeed) -> Self { + match value { + crate::pcie::LinkSpeed::Inactive => Self::Inactive, + crate::pcie::LinkSpeed::Gts2p5 => Self::Gts2p5, + crate::pcie::LinkSpeed::Gts5 => Self::Gts5, + crate::pcie::LinkSpeed::Gts8 => Self::Gts8, + crate::pcie::LinkSpeed::Gts16 => Self::Gts16, + crate::pcie::LinkSpeed::Gts32 => Self::Gts32, + crate::pcie::LinkSpeed::Gts64 => Self::Gts64, + } + } +} + +impl From for crate::pcie::LinkSpeed { + fn from(value: PcieLinkSpeed) -> Self { + match value { + PcieLinkSpeed::Inactive => Self::Inactive, + PcieLinkSpeed::Gts2p5 => Self::Gts2p5, + PcieLinkSpeed::Gts5 => Self::Gts5, + PcieLinkSpeed::Gts8 => Self::Gts8, + PcieLinkSpeed::Gts16 => Self::Gts16, + PcieLinkSpeed::Gts32 => Self::Gts32, + PcieLinkSpeed::Gts64 => Self::Gts64, + } } } // MI v2.0, 5.7.2, Figure 115, PCIEMLW -#[allow(dead_code)] -#[derive(Clone, Copy, Debug, Eq, PartialEq)] +#[derive(Clone, Copy, Debug, DekuRead, DekuWrite, Eq, PartialEq)] +#[deku(ctx = "endian: Endian", endian = "endian", id_type = "u8")] #[repr(u8)] -pub enum PcieLinkWidth { +enum PcieLinkWidth { X1 = 1, X2 = 2, X4 = 4, @@ -686,21 +741,43 @@ pub enum PcieLinkWidth { X32 = 32, } -impl From for u8 { - fn from(plw: PcieLinkWidth) -> Self { - plw as Self +impl From for PcieLinkWidth { + fn from(value: crate::pcie::LinkWidth) -> Self { + match value { + crate::pcie::LinkWidth::X1 => Self::X1, + crate::pcie::LinkWidth::X2 => Self::X2, + crate::pcie::LinkWidth::X4 => Self::X4, + crate::pcie::LinkWidth::X8 => Self::X8, + crate::pcie::LinkWidth::X12 => Self::X12, + crate::pcie::LinkWidth::X16 => Self::X16, + crate::pcie::LinkWidth::X32 => Self::X32, + } + } +} + +impl From for crate::pcie::LinkWidth { + fn from(value: PcieLinkWidth) -> Self { + match value { + PcieLinkWidth::X1 => Self::X1, + PcieLinkWidth::X2 => Self::X2, + PcieLinkWidth::X4 => Self::X4, + PcieLinkWidth::X8 => Self::X8, + PcieLinkWidth::X12 => Self::X12, + PcieLinkWidth::X16 => Self::X16, + PcieLinkWidth::X32 => Self::X32, + } } } // MI v2.0, 5.7.2, Figure 115 -#[derive(Debug, DekuWrite)] +#[derive(Debug, DekuRead, DekuWrite)] #[deku(endian = "little")] struct PciePortDataResponse { - pciemps: u8, - pcieslsv: u8, - pciecls: u8, - pciemlw: u8, - pcienlw: u8, + pciemps: PciePayloadSize, + pcieslsv: WireFlagSet, + pciecls: PcieLinkSpeed, + pciemlw: PcieLinkWidth, + pcienlw: PcieLinkWidth, pciepn: u8, } impl Encode<24> for PciePortDataResponse {} diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index ff09c37..8ed5dc9 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -33,7 +33,7 @@ use crate::{ NvmSubsystemInformationResponse, NvmeManagementResponse, NvmeMiCommandRequestHeader, NvmeMiCommandRequestType, NvmeMiDataStructureManagementResponse, NvmeMiDataStructureRequestType, PcieCommandRequestHeader, PciePortDataResponse, - PortInformationResponse, TwoWirePortDataResponse, + PcieSupportedLinkSpeeds, PortInformationResponse, TwoWirePortDataResponse, }, }, pcie::PciDeviceFunctionConfigurationSpace, @@ -227,7 +227,7 @@ impl RequestHandler for NvmeMiCommandRequestHeader { | (subsys.health.nss.sfm as u8) << 6 | (subsys.health.nss.df as u8) << 5 | (subsys.health.nss.rnr as u8) << 4 - | ((pprt.cls != crate::nvme::mi::PcieLinkSpeed::Inactive) as u8) << 3 // P0LA + | ((pprt.cls != crate::pcie::LinkSpeed::Inactive) as u8) << 3 // P0LA | (false as u8) << 2, // P1LA #[allow(clippy::nonminimal_bool)] sw: (!false as u8) << 5 // PMRRO @@ -647,7 +647,15 @@ impl RequestHandler for NvmeMiDataStructureRequest { crate::PortType::Pcie(pprt) => { let ppd = PciePortDataResponse { pciemps: pprt.mps.into(), - pcieslsv: 0x3fu8, + pcieslsv: { + PcieSupportedLinkSpeeds::Gts2p5 + | PcieSupportedLinkSpeeds::Gts5 + | PcieSupportedLinkSpeeds::Gts8 + | PcieSupportedLinkSpeeds::Gts16 + | PcieSupportedLinkSpeeds::Gts32 + | PcieSupportedLinkSpeeds::Gts64 + } + .into(), pciecls: pprt.cls.into(), pciemlw: pprt.mlw.into(), pcienlw: pprt.nlw.into(), diff --git a/src/pcie.rs b/src/pcie.rs index 746d197..ef1692a 100644 --- a/src/pcie.rs +++ b/src/pcie.rs @@ -5,6 +5,38 @@ use deku::ctx::Endian; use deku::{DekuRead, DekuWrite}; +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum PayloadSize { + Payload128B, + Payload256B, + Payload512B, + Payload1Kb, + Payload2Kb, + Payload4Kb, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum LinkSpeed { + Inactive, + Gts2p5, + Gts5, + Gts8, + Gts16, + Gts32, + Gts64, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum LinkWidth { + X1, + X2, + X4, + X8, + X12, + X16, + X32, +} + // PCIe Base 4.0r1.0, 7.5.1.2, Figure 7-10 #[derive(Debug, DekuRead, DekuWrite)] #[deku(endian = "little")] From 67fcb71347d38ad68b10fe4a5793408d86636345 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Tue, 21 Oct 2025 15:05:44 +1030 Subject: [PATCH 07/20] nvme: mi: Convert PortInformationResponse to FlagSet Signed-off-by: Andrew Jeffery --- src/nvme/mi.rs | 16 ++++++++++++---- src/nvme/mi/dev.rs | 19 ++++++++++++++++--- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/nvme/mi.rs b/src/nvme/mi.rs index d686389..284344b 100644 --- a/src/nvme/mi.rs +++ b/src/nvme/mi.rs @@ -603,14 +603,14 @@ struct NvmSubsystemInformationResponse { impl Encode<32> for NvmSubsystemInformationResponse {} // MI v2.0, 5.7.2, Figure 114, PRTTYP -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, DekuRead, DekuWrite, PartialEq, Eq)] +#[deku(ctx = "endian: Endian", endian = "endian", id_type = "u8")] #[repr(u8)] pub enum PortType { Inactive = 0x00, Pcie = 0x01, TwoWire = 0x02, } -unsafe impl Discriminant for PortType {} impl From<&crate::PortType> for PortType { fn from(value: &crate::PortType) -> Self { @@ -622,12 +622,20 @@ impl From<&crate::PortType> for PortType { } } +// MI v2.0, 5.7.2, Figure 114, PRTCAP +flags! { + enum PortCapabilityFlags: u8 { + Ciaps, + Aems, + } +} + // MI v2.0, 5.7.2, Figure 114 #[derive(Debug, DekuWrite)] #[deku(endian = "little")] struct PortInformationResponse { - prttyp: u8, - prtcap: u8, + prttyp: PortType, + prtcap: WireFlagSet, mmtus: u16, mebs: u32, } diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index 8ed5dc9..44fbec6 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -8,6 +8,7 @@ use heapless::Vec; use log::debug; use mctp::{AsyncRespChannel, MsgIC}; +use crate::nvme::mi::PortCapabilityFlags; use crate::{ CommandEffect, CommandEffectError, Controller, ControllerError, ControllerType, Discriminant, MAX_CONTROLLERS, MAX_NAMESPACES, NamespaceId, NamespaceIdDisposition, SubsystemError, @@ -635,9 +636,21 @@ impl RequestHandler for NvmeMiDataStructureRequest { return Err(ResponseStatus::InvalidParameter); }; let pi = PortInformationResponse { - // FIXME: Change prttyp to crate::nvme::mi::PortType - prttyp: Into::::into(&port.typ).id(), - prtcap: (port.caps.aems as u8) << 1 | (port.caps.ciaps as u8), + prttyp: Into::::into(&port.typ), + prtcap: { + let mut flags = FlagSet::empty(); + + if port.caps.ciaps { + flags |= PortCapabilityFlags::Ciaps; + } + + if port.caps.aems { + flags |= PortCapabilityFlags::Aems; + } + + flags + } + .into(), mmtus: port.mmtus, mebs: port.mebs, } From 208d7541e76f184e386475866213467f81b8cc3e Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Tue, 21 Oct 2025 15:58:05 +1030 Subject: [PATCH 08/20] nvme: mi: Convert NvmSubsystemStatus to FlagSet Signed-off-by: Andrew Jeffery --- src/lib.rs | 5 +++-- src/nvme/mi.rs | 35 +++++++++-------------------------- src/nvme/mi/dev.rs | 20 ++++++++++++-------- 3 files changed, 24 insertions(+), 36 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 10e63ed..c348417 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -439,13 +439,14 @@ impl Controller { #[derive(Debug)] struct SubsystemHealth { - nss: nvme::mi::NvmSubsystemStatus, + nss: FlagSet, } impl SubsystemHealth { fn new() -> Self { Self { - nss: nvme::mi::NvmSubsystemStatus::new(), + nss: crate::nvme::mi::NvmSubsystemStatusFlags::Rnr + | crate::nvme::mi::NvmSubsystemStatusFlags::Df, } } } diff --git a/src/nvme/mi.rs b/src/nvme/mi.rs index 284344b..b6bbb08 100644 --- a/src/nvme/mi.rs +++ b/src/nvme/mi.rs @@ -496,31 +496,14 @@ struct CompositeControllerStatusDataStructureResponse { impl Encode<4> for CompositeControllerStatusDataStructureResponse {} // MI v2.0, 5.6, Figure 108, NSS -// TODO: Convert to Flags/FlagSet -#[derive(Debug)] -pub struct NvmSubsystemStatus { - atf: bool, - sfm: bool, - df: bool, - rnr: bool, - rd: bool, -} - -impl Default for NvmSubsystemStatus { - fn default() -> Self { - Self::new() - } -} - -impl NvmSubsystemStatus { - pub fn new() -> Self { - Self { - atf: false, - sfm: false, - df: true, - rnr: true, - rd: false, - } +flags! { + pub enum NvmSubsystemStatusFlags: u8 { + P1la = 1 << 2, + P0la = 1 << 3, + Rnr = 1 << 4, + Df = 1 << 5, + Sfm = 1 << 6, + Atf = 1 << 7, } } @@ -528,7 +511,7 @@ impl NvmSubsystemStatus { #[derive(Debug, DekuRead, DekuWrite)] #[deku(endian = "little")] struct NvmSubsystemHealthDataStructureResponse { - nss: u8, + nss: WireFlagSet, sw: u8, ctemp: u8, pldu: u8, diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index 44fbec6..a86f529 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -8,7 +8,7 @@ use heapless::Vec; use log::debug; use mctp::{AsyncRespChannel, MsgIC}; -use crate::nvme::mi::PortCapabilityFlags; +use crate::nvme::mi::{NvmSubsystemStatusFlags, PortCapabilityFlags}; use crate::{ CommandEffect, CommandEffectError, Controller, ControllerError, ControllerType, Discriminant, MAX_CONTROLLERS, MAX_NAMESPACES, NamespaceId, NamespaceIdDisposition, SubsystemError, @@ -224,17 +224,21 @@ impl RequestHandler for NvmeMiCommandRequestHeader { let pdlu = core::cmp::min(255, 100 * ctlr.write_age / ctlr.write_lifespan); let nvmshds = NvmSubsystemHealthDataStructureResponse { - nss: (subsys.health.nss.atf as u8) << 7 - | (subsys.health.nss.sfm as u8) << 6 - | (subsys.health.nss.df as u8) << 5 - | (subsys.health.nss.rnr as u8) << 4 - | ((pprt.cls != crate::pcie::LinkSpeed::Inactive) as u8) << 3 // P0LA - | (false as u8) << 2, // P1LA + nss: { + let mut flags = subsys.health.nss; + + if pprt.cls != crate::pcie::LinkSpeed::Inactive { + flags |= NvmSubsystemStatusFlags::P0la; + } + + flags + } + .into(), #[allow(clippy::nonminimal_bool)] sw: (!false as u8) << 5 // PMRRO | (!false as u8) << 4 // VMBF | (!ctlr.ro as u8) << 3 // AMRO - | (!subsys.health.nss.rd as u8) << 2 // NDR + | (!ctlr.ro as u8) << 2 // NDR | (!(ctlr.temp_range.lower <= ctlr.temp && ctlr.temp <= ctlr.temp_range.upper) as u8) << 1 // TTC | (!((100 * ctlr.spare / ctlr.capacity) < ctlr.spare_range.lower) as u8), ctemp: ctemp as u8, From 6a3b29fa62ca98c9492e2b68a8626376a70bb049 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Tue, 21 Oct 2025 16:38:40 +1030 Subject: [PATCH 09/20] nvme: mi: Convert NvmSubsystemHealthDataStructureResponse to FlagSet Signed-off-by: Andrew Jeffery --- src/nvme/mi.rs | 22 +++++++++++++++++++++- src/nvme/mi/dev.rs | 27 ++++++++++++++++++++------- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/nvme/mi.rs b/src/nvme/mi.rs index b6bbb08..4d35166 100644 --- a/src/nvme/mi.rs +++ b/src/nvme/mi.rs @@ -507,12 +507,32 @@ flags! { } } +// MI v2.0, 5.6, Figure 108, SW +flags! { + pub enum SmartWarningFlags: u8 { + Ascbt, + Ttc, + Ndr, + Amro, + Vmbf, + Pmrro, + } +} + +impl From> for WireFlagSet { + fn from(value: FlagSet) -> Self { + FlagSet::::new((!value.bits()) & 0x3f) + .expect("Undefined bits set") + .into() + } +} + // MI v2.0, 5.6, Figure 108 #[derive(Debug, DekuRead, DekuWrite)] #[deku(endian = "little")] struct NvmSubsystemHealthDataStructureResponse { nss: WireFlagSet, - sw: u8, + sw: WireFlagSet, ctemp: u8, pldu: u8, } diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index a86f529..f1d0611 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -234,13 +234,26 @@ impl RequestHandler for NvmeMiCommandRequestHeader { flags } .into(), - #[allow(clippy::nonminimal_bool)] - sw: (!false as u8) << 5 // PMRRO - | (!false as u8) << 4 // VMBF - | (!ctlr.ro as u8) << 3 // AMRO - | (!ctlr.ro as u8) << 2 // NDR - | (!(ctlr.temp_range.lower <= ctlr.temp && ctlr.temp <= ctlr.temp_range.upper) as u8) << 1 // TTC - | (!((100 * ctlr.spare / ctlr.capacity) < ctlr.spare_range.lower) as u8), + sw: { + let mut flags = FlagSet::full(); + + if ctlr.ro { + flags -= super::SmartWarningFlags::Amro; + flags -= super::SmartWarningFlags::Ndr; + } + + if ctlr.temp_range.lower <= ctlr.temp && ctlr.temp <= ctlr.temp_range.upper + { + flags -= super::SmartWarningFlags::Ttc; + } + + if (100 * ctlr.spare / ctlr.capacity) < ctlr.spare_range.lower { + flags -= super::SmartWarningFlags::Ascbt; + } + + flags + } + .into(), ctemp: ctemp as u8, pldu: pdlu as u8, } From 2fb98bb565926988d098c0e9022bf0af1bb2bd97 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Tue, 21 Oct 2025 16:49:08 +1030 Subject: [PATCH 10/20] nvme: mi: Convert NvmSubsystemHealthStatusPollRequest to deku bits Signed-off-by: Andrew Jeffery --- src/nvme/mi.rs | 4 ++-- src/nvme/mi/dev.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/nvme/mi.rs b/src/nvme/mi.rs index 4d35166..22a1980 100644 --- a/src/nvme/mi.rs +++ b/src/nvme/mi.rs @@ -419,8 +419,8 @@ flags! { #[derive(Debug, DekuRead, DekuWrite, Eq, PartialEq)] #[deku(ctx = "endian: Endian", endian = "endian")] struct NvmSubsystemHealthStatusPollRequest { - dword0: u32, - dword1: u32, + #[deku(bits = "1", pad_bits_after = "7", pad_bytes_before = "7")] + cs: bool, } // MI v2.0, 5.6, Figure 107 diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index f1d0611..2fdb928 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -265,7 +265,7 @@ impl RequestHandler for NvmeMiCommandRequestHeader { .encode()?; // CS: See Figure 106, NVMe MI v2.0 - if (shsp.dword1 & (1u32 << 31)) != 0 { + if shsp.cs { mep.ccsf.0.clear(); } From 62f2316e4f076470918b4fdf97e82646ccc04ffe Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Tue, 21 Oct 2025 16:56:48 +1030 Subject: [PATCH 11/20] nvme: mi: Convert ControllerInformationResponse to deku bits Signed-off-by: Andrew Jeffery --- src/nvme/mi.rs | 10 ++++++++-- src/nvme/mi/dev.rs | 6 ++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/nvme/mi.rs b/src/nvme/mi.rs index 22a1980..181d842 100644 --- a/src/nvme/mi.rs +++ b/src/nvme/mi.rs @@ -815,8 +815,14 @@ impl Encode<24> for TwoWirePortDataResponse {} struct ControllerInformationResponse { #[deku(pad_bytes_after = "4")] portid: u8, - prii: u8, - pri: u16, + #[deku(bits = "1", pad_bits_before = "7")] + prii_pcieriv: bool, + #[deku(bits = "8")] + pri_pcibn: u16, + #[deku(bits = "5")] + pri_pcidn: u16, + #[deku(bits = "3")] + pri_pcifn: u16, pcivid: u16, pcidid: u16, pcisvid: u16, diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index 2fdb928..6bf27d8 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -797,8 +797,10 @@ impl RequestHandler for NvmeMiDataStructureRequest { let ci = ControllerInformationResponse { portid: ctlr.port.0, - prii: 1, - pri: pprt.b << 8 | pprt.d << 4 | pprt.f, + prii_pcieriv: true, + pri_pcibn: pprt.b, + pri_pcidn: pprt.d, + pri_pcifn: pprt.f, pcivid: subsys.info.pci_vid, pcidid: subsys.info.pci_did, pcisvid: subsys.info.pci_svid, From 2807f6036f930ddb6882fec1c842ac7a8397d02f Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Tue, 21 Oct 2025 17:23:14 +1030 Subject: [PATCH 12/20] nvme: Convert AdminIdentifyControllerResponse to FlagSet Signed-off-by: Andrew Jeffery --- src/nvme.rs | 60 ++++++++++++++++++++++++++++++++++++++++--- src/nvme/mi/dev.rs | 64 ++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 110 insertions(+), 14 deletions(-) diff --git a/src/nvme.rs b/src/nvme.rs index 0f38f92..c40238d 100644 --- a/src/nvme.rs +++ b/src/nvme.rs @@ -448,6 +448,42 @@ pub enum CommandSetIdentifier { ComputationalPrograms = 0x04, } +// Base v2.1, 5.1.13.2.1, Figure 312, CMIC +flags! { + enum ControllerMultipathIoNamespaceSharingCapabilityFlags: u8 { + Mports, + Mctrs, + Ft, + Anars, + } +} + +// Base v2.1, 5.1.13.2.1, Figure 312, CTRATT +flags! { + enum ControllerAttributeFlags: u32 { + Hids, + Nopspm, + Nsets, + Rrlvls, + Egs, + Plm, + Tbkas, + Ng, + Sqa, + Ulist, + Mds, + Fcm, + Vcm, + Deg, + Dnvms, + Elbas, + Mem, + Hmbr, + Rhii, + Fdps, + } +} + // Base v2.1, 5.1.13.2.1, Figure 312, CNTRLTYPE #[derive(Clone, Copy, Debug, DekuRead, DekuWrite, PartialEq)] #[deku(id_type = "u8", endian = "endian", ctx = "endian: Endian")] @@ -469,6 +505,22 @@ impl From for ControllerType { } } +// Base v2.1, 5.1.13.2.1, Figure 312, NVMSR +flags! { + enum NvmSubsystemReportFlags: u8 { + Nvmesd, + Nvmee, + } +} + +// Base v2.1, 5.1.13.2.1, Figure 312, MEC +flags! { + enum ManagementEndpointCapabilityFlags: u8 { + Twpme, + Pcieme, + } +} + // Base v2.1, 5.1.13.2.1, Figure 312, LPA flags! { #[repr(u8)] @@ -551,20 +603,20 @@ struct AdminIdentifyControllerResponse { fr: WireString<8>, rab: u8, ieee: [u8; 3], - cmic: u8, + cmic: WireFlagSet, mdts: u8, cntlid: u16, ver: u32, rtd3r: u32, rtd3e: u32, oaes: u32, - ctratt: u32, + ctratt: WireFlagSet, #[deku(seek_from_current = "11")] cntrltype: crate::nvme::ControllerType, #[deku(seek_from_current = "141")] - nvmsr: u8, + nvmsr: WireFlagSet, vwci: u8, - mec: u8, + mec: WireFlagSet, ocas: u16, acl: u8, aerl: u8, diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index 6bf27d8..9dc5468 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -9,6 +9,10 @@ use log::debug; use mctp::{AsyncRespChannel, MsgIC}; use crate::nvme::mi::{NvmSubsystemStatusFlags, PortCapabilityFlags}; +use crate::nvme::{ + ControllerAttributeFlags, ControllerMultipathIoNamespaceSharingCapabilityFlags, + ManagementEndpointCapabilityFlags, NvmSubsystemReportFlags, +}; use crate::{ CommandEffect, CommandEffectError, Controller, ControllerError, ControllerType, Discriminant, MAX_CONTROLLERS, MAX_NAMESPACES, NamespaceId, NamespaceIdDisposition, SubsystemError, @@ -1385,8 +1389,22 @@ impl RequestHandler for AdminIdentifyRequest { fixup.reverse(); fixup }, - cmic: ((subsys.ctlrs.len() > 1) as u8) << 1 // MCTRS - | ((subsys.ports.len() > 1) as u8), // MPORTS + cmic: { + let mut flags = FlagSet::empty(); + + if subsys.ctlrs.len() > 1 { + flags |= + ControllerMultipathIoNamespaceSharingCapabilityFlags::Mctrs; + } + + if subsys.ports.len() > 1 { + flags |= + ControllerMultipathIoNamespaceSharingCapabilityFlags::Mports; + } + + flags + } + .into(), mdts: 0, cntlid: ctlr.id.0, ver: 0, @@ -1394,17 +1412,43 @@ impl RequestHandler for AdminIdentifyRequest { rtd3e: 0, oaes: 0, // TODO: Tie to data model - ctratt: ((false as u32) << 14) // DNVMS - | ((false as u32) << 13) // DEG - | ((false as u32) << 4) // EGS - | ((false as u32) << 2), // NSETS + ctratt: { + let mut flags = FlagSet::empty(); + + flags -= ControllerAttributeFlags::Nsets; + flags -= ControllerAttributeFlags::Egs; + flags -= ControllerAttributeFlags::Deg; + flags -= ControllerAttributeFlags::Dnvms; + + flags + } + .into(), cntrltype: ctlr.cntrltype.into(), // TODO: Tie to data model - nvmsr: ((false as u8) << 1) // NVMEE - | (true as u8), // NVMESD + nvmsr: { FlagSet::empty() | NvmSubsystemReportFlags::Nvmesd }.into(), vwci: 0, - mec: ((subsys.ports.iter().any(|p| matches!(p.typ, crate::PortType::Pcie(_)))) as u8) << 1 // PCIEME - | (subsys.ports.iter().any(|p| matches!(p.typ, crate::PortType::TwoWire(_)))) as u8, // TWPME + mec: { + let mut flags = FlagSet::empty(); + + if subsys + .ports + .iter() + .any(|p| matches!(p.typ, crate::PortType::Pcie(_))) + { + flags |= ManagementEndpointCapabilityFlags::Pcieme; + } + + if subsys + .ports + .iter() + .any(|p| matches!(p.typ, crate::PortType::TwoWire(_))) + { + flags |= ManagementEndpointCapabilityFlags::Twpme; + } + + flags + } + .into(), ocas: 0, acl: 0, aerl: 0, From 9d7566580ec7e0e8274215ea266ea8708be00ea3 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Wed, 22 Oct 2025 15:03:29 +1030 Subject: [PATCH 13/20] nvme: mi: Convert AdminIoCqeStatus handling to deku bits Signed-off-by: Andrew Jeffery --- src/nvme.rs | 64 +++++++++++++++++++++++++++++++--------------- src/nvme/mi.rs | 2 +- src/nvme/mi/dev.rs | 62 ++++++++------------------------------------ 3 files changed, 55 insertions(+), 73 deletions(-) diff --git a/src/nvme.rs b/src/nvme.rs index c40238d..ee021db 100644 --- a/src/nvme.rs +++ b/src/nvme.rs @@ -4,7 +4,7 @@ */ pub mod mi; -use deku::ctx::Endian; +use deku::ctx::{BitSize, Endian, Order}; use deku::{DekuError, DekuRead, DekuWrite, deku_derive}; use flagset::flags; use log::debug; @@ -43,40 +43,65 @@ flags! { } // Base v2.1, 4.2.1, Figure 98 +// +// Switch to LSB order because the packing of these fields is just messy, +// largely thanks to the position of P. However, the SC / SCT ordering also +// throws ergonomic deku representations straight out the window. +#[derive(Debug, DekuRead, DekuWrite)] +#[deku(bit_order = "lsb", ctx = "endian: Endian", endian = "endian")] struct AdminIoCqeStatus { cid: u16, + #[deku(bits = 1)] p: bool, - status: AdminIoCqeStatusType, + #[deku(bits = 8)] + sc: u8, + #[deku(bits = 3)] + sct: u8, + #[deku(bits = 2)] crd: CommandRetryDelay, + #[deku(bits = 1)] m: bool, + #[deku(bits = 1)] dnr: bool, } -impl From for u32 { - fn from(value: AdminIoCqeStatus) -> Self { - let dnr: u32 = value.dnr.into(); - let m: u32 = value.m.into(); - let crd: u32 = value.crd.id().into(); - debug_assert_eq!((crd & !3), 0); - let sct: u32 = value.status.id().into(); - debug_assert_eq!((sct & !7), 0); - let sc: u32 = match value.status { - AdminIoCqeStatusType::GenericCommandStatus(s) => s.id(), - AdminIoCqeStatusType::CommandSpecificStatus(v) => v, +impl From for AdminIoCqeStatus { + fn from(value: AdminIoCqeStatusType) -> Self { + match value { + AdminIoCqeStatusType::GenericCommandStatus(sts) => AdminIoCqeStatus { + cid: 0, + sc: sts as u8, + p: true, + dnr: sts != AdminIoCqeGenericCommandStatus::SuccessfulCompletion, + m: false, + crd: CommandRetryDelay::None, + sct: value.id(), + }, + AdminIoCqeStatusType::CommandSpecificStatus(sts) => AdminIoCqeStatus { + cid: 0, + sc: sts, + p: true, + dnr: true, + m: false, + crd: CommandRetryDelay::None, + sct: value.id(), + }, AdminIoCqeStatusType::MediaAndDataIntegrityErrors => todo!(), AdminIoCqeStatusType::PathRelatedStatus => todo!(), AdminIoCqeStatusType::VendorSpecific => todo!(), } - .into(); - debug_assert_eq!((sc & !0xff), 0); - let p: u32 = value.p.into(); - let cid: u32 = value.cid.into(); - (dnr << 31) | (m << 30) | (crd << 28) | (sct << 25) | (sc << 17) | (p << 16) | cid } } // Base v2.1, 4.2.3, Figure 100, CRD -#[expect(dead_code)] +#[derive(Debug, DekuRead, DekuWrite)] +#[deku( + bits = "bits.0", + bit_order = "order", + ctx = "endian: Endian, bits: BitSize, order: Order", + endian = "endian", + id_type = "u8" +)] #[repr(u8)] enum CommandRetryDelay { None = 0x00, @@ -84,7 +109,6 @@ enum CommandRetryDelay { Time2 = 0x02, Time3 = 0x03, } -unsafe impl Discriminant for CommandRetryDelay {} // Base v2.1, 4.2.3, Figure 101 #[derive(Clone, Copy, Debug, Eq, PartialEq)] diff --git a/src/nvme/mi.rs b/src/nvme/mi.rs index 181d842..6254f92 100644 --- a/src/nvme/mi.rs +++ b/src/nvme/mi.rs @@ -1009,7 +1009,7 @@ struct AdminCommandResponseHeader { #[deku(seek_from_start = "4")] cqedw0: u32, cqedw1: u32, - cqedw3: u32, + cqedw3: super::AdminIoCqeStatus, } impl Encode<16> for AdminCommandResponseHeader {} diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index 9dc5468..814f48d 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -23,7 +23,7 @@ use crate::{ AdminIdentifyControllerResponse, AdminIdentifyNamespaceIdentificationDescriptorListResponse, AdminIdentifyNvmIdentifyNamespaceResponse, AdminIoCqeGenericCommandStatus, - AdminIoCqeStatus, AdminIoCqeStatusType, AdminSanitizeConfiguration, ControllerListResponse, + AdminIoCqeStatusType, AdminSanitizeConfiguration, ControllerListResponse, LidSupportedAndEffectsDataStructure, LidSupportedAndEffectsFlags, LogPageAttributes, NamespaceIdentifierType, SanitizeAction, SanitizeOperationStatus, SanitizeState, SanitizeStateInformation, SanitizeStatus, SanitizeStatusLogPageResponse, @@ -971,16 +971,9 @@ where status: ResponseStatus::Success, cqedw0: 0, cqedw1: 0, - cqedw3: AdminIoCqeStatus { - cid: 0, - p: true, - status: AdminIoCqeStatusType::GenericCommandStatus( - AdminIoCqeGenericCommandStatus::SuccessfulCompletion, - ), - crd: crate::nvme::CommandRetryDelay::None, - m: false, - dnr: false, - } + cqedw3: AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::SuccessfulCompletion, + ) .into(), } .encode()?; @@ -1003,15 +996,7 @@ where status: ResponseStatus::Success, cqedw0: 0, cqedw1: 0, - cqedw3: AdminIoCqeStatus { - cid: 0, - p: true, - status, - crd: crate::nvme::CommandRetryDelay::None, - m: false, - dnr: true, - } - .into(), + cqedw3: status.into(), } .encode()?; @@ -1745,16 +1730,9 @@ impl RequestHandler for AdminNamespaceManagementRequest { status: ResponseStatus::Success, cqedw0: nsid.0, cqedw1: 0, - cqedw3: AdminIoCqeStatus { - cid: 0, - p: true, - status: AdminIoCqeStatusType::GenericCommandStatus( - AdminIoCqeGenericCommandStatus::SuccessfulCompletion, - ), - crd: crate::nvme::CommandRetryDelay::None, - m: false, - dnr: false, - } + cqedw3: AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::SuccessfulCompletion, + ) .into(), } .encode()?; @@ -1782,15 +1760,7 @@ impl RequestHandler for AdminNamespaceManagementRequest { status: ResponseStatus::Success, cqedw0: self.nsid, // TODO: Base v2.1, 5.1.21 unclear, test against hardware cqedw1: 0, - cqedw3: AdminIoCqeStatus { - cid: 0, - p: true, - status, - crd: crate::nvme::CommandRetryDelay::None, - m: false, - dnr: res.is_err(), - } - .into(), + cqedw3: status.into(), } .encode()?; @@ -1918,19 +1888,7 @@ impl RequestHandler for AdminNamespaceAttachmentRequest { status: ResponseStatus::Success, cqedw0: self.nsid, cqedw1: 0, - cqedw3: AdminIoCqeStatus { - cid: 0, - p: true, - status, - crd: crate::nvme::CommandRetryDelay::None, - m: false, - dnr: { - AdminIoCqeStatusType::GenericCommandStatus( - AdminIoCqeGenericCommandStatus::SuccessfulCompletion, - ) != status - }, - } - .into(), + cqedw3: status.into(), } .encode()?; From a7f06eff9100eaff61f7a0c17295758f085a2a8b Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Wed, 22 Oct 2025 15:42:21 +1030 Subject: [PATCH 14/20] nvme: mi: Convert AdminFormatNvmConfiguration to deku bits Signed-off-by: Andrew Jeffery --- src/nvme.rs | 53 +++++++++++++++++++--------------------------- src/nvme/mi.rs | 5 +++-- src/nvme/mi/dev.rs | 22 +++++-------------- tests/admin.rs | 3 ++- 4 files changed, 32 insertions(+), 51 deletions(-) diff --git a/src/nvme.rs b/src/nvme.rs index ee021db..958ef07 100644 --- a/src/nvme.rs +++ b/src/nvme.rs @@ -154,50 +154,41 @@ struct ControllerListRequest { } // Base v2.1, 5.1.10, Figure 189, SES -#[derive(Debug)] +#[derive(Debug, DekuRead, DekuWrite, Eq, PartialEq)] +#[deku( + bits = "bits.0", + ctx = "endian: Endian, bits: BitSize", + endian = "endian", + id_type = "u8" +)] #[repr(u8)] enum SecureEraseSettings { NoOperation = 0b000, UserDataErase = 0b001, CryptographicErase = 0b010, } -unsafe impl Discriminant for SecureEraseSettings {} - -impl TryFrom for SecureEraseSettings { - type Error = (); - - fn try_from(value: u32) -> Result { - match value { - 0b000 => Ok(Self::NoOperation), - 0b001 => Ok(Self::UserDataErase), - 0b010 => Ok(Self::CryptographicErase), - _ => Err(()), - } - } -} // Base v2.1, 5.1.10, Figure 189 -#[derive(Debug)] -#[expect(dead_code)] +#[derive(Debug, DekuRead, DekuWrite, Eq, PartialEq)] +#[deku(ctx = "endian: Endian", endian = "endian")] struct AdminFormatNvmConfiguration { - lbafi: u8, - mset: bool, + #[deku(bits = "3")] pi: u8, - pil: bool, + #[deku(bits = "1")] + mset: bool, + #[deku(bits = "4")] + lbafl: u8, + #[deku(bits = "2", pad_bits_before = "2")] + lbafu: u8, + #[deku(bits = "3")] ses: SecureEraseSettings, + #[deku(bits = "1", pad_bytes_after = "2")] + pil: bool, } -impl TryFrom for AdminFormatNvmConfiguration { - type Error = (); - - fn try_from(value: u32) -> Result { - Ok(Self { - lbafi: ((((value >> 12) & 0x3) << 4) | (value & 0xf)) as u8, - mset: ((value >> 4) & 1) == 1, - pi: ((value >> 5) & 0x3) as u8, - pil: ((value >> 6) & 1) == 1, - ses: TryFrom::try_from((value >> 9) & 0x7)?, - }) +impl AdminFormatNvmConfiguration { + fn lbafi(&self) -> u8 { + self.lbafu << 4 | self.lbafl } } diff --git a/src/nvme/mi.rs b/src/nvme/mi.rs index 6254f92..8305659 100644 --- a/src/nvme/mi.rs +++ b/src/nvme/mi.rs @@ -9,7 +9,8 @@ use flagset::{FlagSet, flags}; use log::debug; use crate::nvme::{ - AdminNamespaceAttachmentSelect, AdminNamespaceManagementSelect, ControllerListRequest, + AdminFormatNvmConfiguration, AdminNamespaceAttachmentSelect, AdminNamespaceManagementSelect, + ControllerListRequest, }; use crate::wire::{WireFlagSet, WireVec}; use crate::{CommandEffectError, Discriminant, Encode, MAX_CONTROLLERS}; @@ -901,7 +902,7 @@ struct AdminFormatNvmRequest { dlen: u32, #[deku(seek_from_current = "8")] #[deku(pad_bytes_after = "20")] - config: u32, + config: AdminFormatNvmConfiguration, } // MI v2.0, 6, Figure 136 diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index 814f48d..0bbc525 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -17,10 +17,9 @@ use crate::{ CommandEffect, CommandEffectError, Controller, ControllerError, ControllerType, Discriminant, MAX_CONTROLLERS, MAX_NAMESPACES, NamespaceId, NamespaceIdDisposition, SubsystemError, nvme::{ - AdminFormatNvmConfiguration, AdminGetLogPageLidRequestType, - AdminGetLogPageSupportedLogPagesResponse, AdminIdentifyActiveNamespaceIdListResponse, - AdminIdentifyAllocatedNamespaceIdListResponse, AdminIdentifyCnsRequestType, - AdminIdentifyControllerResponse, + AdminGetLogPageLidRequestType, AdminGetLogPageSupportedLogPagesResponse, + AdminIdentifyActiveNamespaceIdListResponse, AdminIdentifyAllocatedNamespaceIdListResponse, + AdminIdentifyCnsRequestType, AdminIdentifyControllerResponse, AdminIdentifyNamespaceIdentificationDescriptorListResponse, AdminIdentifyNvmIdentifyNamespaceResponse, AdminIoCqeGenericCommandStatus, AdminIoCqeStatusType, AdminSanitizeConfiguration, ControllerListResponse, @@ -2016,19 +2015,8 @@ impl RequestHandler for AdminFormatNvmRequest { .await; }; - let Ok(config) = TryInto::::try_into(self.config) else { - debug!("Invalid configuration for Admin Format NVM"); - return admin_send_status( - resp, - AdminIoCqeStatusType::GenericCommandStatus( - AdminIoCqeGenericCommandStatus::InvalidFieldInCommand, - ), - ) - .await; - }; - - if config.lbafi != 0 { - debug!("Unsupported LBA format index: {}", config.lbafi); + if self.config.lbafi() != 0 { + debug!("Unsupported LBA format index: {}", self.config.lbafi()); return admin_send_status( resp, AdminIoCqeStatusType::GenericCommandStatus( diff --git a/tests/admin.rs b/tests/admin.rs index e25f76c..ba4b0ac 100644 --- a/tests/admin.rs +++ b/tests/admin.rs @@ -4990,7 +4990,8 @@ mod format_nvm { 0x7a, 0x05, 0xbd, 0xb8 ]; - let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_FIELD); + // FIXME: Poor error response code for the problem at hand + let resp = ExpectedRespChannel::new(&RESP_INVALID_COMMAND_SIZE); smol::block_on(async { mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) .await From 15fcafc0f8945fb164aca9e650f3ec5f412b6796 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Wed, 22 Oct 2025 16:10:52 +1030 Subject: [PATCH 15/20] nvme: Convert SanitizeStatus to deku bits Signed-off-by: Andrew Jeffery --- src/nvme.rs | 30 ++++++++++++++++-------------- src/nvme/mi/dev.rs | 10 +++++----- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/nvme.rs b/src/nvme.rs index 958ef07..b8e97a3 100644 --- a/src/nvme.rs +++ b/src/nvme.rs @@ -309,7 +309,13 @@ flags! { } // Base v2.1, 5.1.12.1.33, Figure 291, SSTAT, SOS -#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +#[derive(Clone, Copy, Debug, Default, DekuRead, DekuWrite, Eq, PartialEq)] +#[deku( + bits = "bits.0", + ctx = "endian: Endian, bits: BitSize", + endian = "endian", + id_type = "u8" +)] #[repr(u8)] pub enum SanitizeOperationStatus { #[default] @@ -322,21 +328,17 @@ pub enum SanitizeOperationStatus { unsafe impl crate::Discriminant for SanitizeOperationStatus {} // Base v2.1, 5.1.12.1.33, Figure 291, SSTAT -#[derive(Clone, Copy, Debug, Default)] +#[derive(Clone, Copy, Debug, Default, DekuRead, DekuWrite)] +#[deku(ctx = "endian: Endian", endian = "endian")] pub struct SanitizeStatus { - sos: SanitizeOperationStatus, + #[deku(bits = "5")] opc: u8, - gde: bool, + #[deku(bits = "3")] + sos: SanitizeOperationStatus, + #[deku(bits = "1", pad_bits_before = "6")] mvcncled: bool, -} - -impl From for u16 { - fn from(value: SanitizeStatus) -> Self { - ((value.mvcncled as u16) << 9) - | ((value.gde as u16) << 8) - | ((value.opc & ((1 << 6) - 1)) << 3) as u16 - | value.sos.id() as u16 - } + #[deku(bits = "1")] + gde: bool, } // Base v2.1, 5.12.1.33, Fgure 291, SSI, SANS @@ -373,7 +375,7 @@ impl From for u8 { #[deku(endian = "little")] struct SanitizeStatusLogPageResponse { sprog: u16, - sstat: u16, + sstat: SanitizeStatus, scdw10: u32, eto: u32, etbe: u32, diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index 0bbc525..61ce323 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -1276,7 +1276,7 @@ impl RequestHandler for AdminGetLogPageRequest { let sslpr = SanitizeStatusLogPageResponse { sprog: u16::MAX, - sstat: subsys.sstat.into(), + sstat: subsys.sstat, scdw10: { if let Some(sconf) = subsys.sconf { sconf.into() @@ -1955,10 +1955,10 @@ impl RequestHandler for AdminSanitizeRequest { fails: 0, }; subsys.sstat = SanitizeStatus { - sos: SanitizeOperationStatus::Sanitized, opc: 0, - gde: true, + sos: SanitizeOperationStatus::Sanitized, mvcncled: false, + gde: true, }; subsys.sconf = Some(self.config.try_into()?); @@ -1970,10 +1970,10 @@ impl RequestHandler for AdminSanitizeRequest { fails: 0, }; subsys.sstat = SanitizeStatus { - sos: SanitizeOperationStatus::Sanitized, opc: config.owpass, - gde: true, + sos: SanitizeOperationStatus::Sanitized, mvcncled: false, + gde: true, }; subsys.sconf = Some(self.config.try_into()?); From 964c327ead326c25024848ee3dfa78682a93d541 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Wed, 22 Oct 2025 16:58:43 +1030 Subject: [PATCH 16/20] nvme: Convert SanitizeStateInformation to deku bits Signed-off-by: Andrew Jeffery --- src/nvme.rs | 24 +++++++++++++----------- src/nvme/mi/dev.rs | 2 +- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/nvme.rs b/src/nvme.rs index b8e97a3..59febbb 100644 --- a/src/nvme.rs +++ b/src/nvme.rs @@ -342,9 +342,14 @@ pub struct SanitizeStatus { } // Base v2.1, 5.12.1.33, Fgure 291, SSI, SANS -#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +#[derive(Clone, Copy, Debug, Default, DekuRead, DekuWrite, Eq, PartialEq)] +#[deku( + bits = "bits.0", + ctx = "endian: Endian, bits: BitSize", + endian = "endian", + id_type = "u8" +)] #[repr(u8)] -#[expect(dead_code)] enum SanitizeState { #[default] Idle = 0x00, @@ -358,16 +363,13 @@ enum SanitizeState { unsafe impl crate::Discriminant for SanitizeState {} // Base v2.1, 5.1.12.1.33, Figure 291, SSI -#[derive(Clone, Copy, Debug, Default)] +#[derive(Clone, Copy, Debug, Default, DekuRead, DekuWrite)] +#[deku(ctx = "endian: Endian", endian = "endian")] pub struct SanitizeStateInformation { - sans: SanitizeState, + #[deku(bits = "4")] fails: u8, -} - -impl From for u8 { - fn from(value: SanitizeStateInformation) -> Self { - (value.fails << 4) | (value.sans.id()) - } + #[deku(bits = "4")] + sans: SanitizeState, } // Base v2.1, 5.1.12.1.33, Figure 291 @@ -384,7 +386,7 @@ struct SanitizeStatusLogPageResponse { etbenmm: u32, etcenmm: u32, etpvds: u32, - ssi: u8, + ssi: SanitizeStateInformation, } impl Encode<512> for SanitizeStatusLogPageResponse {} diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index 61ce323..80d707d 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -1291,7 +1291,7 @@ impl RequestHandler for AdminGetLogPageRequest { etbenmm: u32::MAX, etcenmm: u32::MAX, etpvds: u32::MAX, - ssi: subsys.ssi.into(), + ssi: subsys.ssi, } .encode()?; From 5d7fe5d6224c281017a88ef063ec70eea07d7003 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Wed, 22 Oct 2025 17:54:55 +1030 Subject: [PATCH 17/20] nvme: Convert Sanitize structures to deku bits, FlagSet Signed-off-by: Andrew Jeffery --- src/lib.rs | 9 +++- src/nvme.rs | 111 ++++++++++++++++++--------------------------- src/nvme/mi.rs | 4 +- src/nvme/mi/dev.rs | 45 ++++++------------ 4 files changed, 69 insertions(+), 100 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c348417..44c321b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -638,7 +638,8 @@ pub struct Subsystem { nsids: u32, nss: heapless::Vec, health: SubsystemHealth, - sanicap: nvme::SanitizeCapabilities, + sanicap: FlagSet, + nodmmas: nvme::NoDeallocateModifiesMediaAfterSanitize, ssi: nvme::SanitizeStateInformation, sstat: nvme::SanitizeStatus, sconf: Option, @@ -665,7 +666,11 @@ impl Subsystem { sstat: Default::default(), sconf: None, ssi: Default::default(), - sanicap: Default::default(), + sanicap: nvme::SanitizeCapabilityFlags::Ces + | nvme::SanitizeCapabilityFlags::Bes + | nvme::SanitizeCapabilityFlags::Ows + | nvme::SanitizeCapabilityFlags::Ndi, + nodmmas: Default::default(), } } diff --git a/src/nvme.rs b/src/nvme.rs index 59febbb..c4c81f7 100644 --- a/src/nvme.rs +++ b/src/nvme.rs @@ -378,7 +378,7 @@ pub struct SanitizeStateInformation { struct SanitizeStatusLogPageResponse { sprog: u16, sstat: SanitizeStatus, - scdw10: u32, + scdw10: AdminSanitizeConfiguration, eto: u32, etbe: u32, etce: u32, @@ -555,7 +555,14 @@ flags! { } // Base v2.1, 5.1.13.2.1, Figure 312, SANICAP, NODMMAS -#[derive(Clone, Copy, Debug, Default)] +#[derive(Clone, Copy, Debug, Default, DekuRead, DekuWrite)] +#[deku( + bits = "bits.0", + bit_order = "order", + ctx = "endian: Endian, bits: BitSize, order: Order", + endian = "endian", + id_type = "u8" +)] #[repr(u8)] pub enum NoDeallocateModifiesMediaAfterSanitize { Undefined = 0b00, @@ -564,41 +571,26 @@ pub enum NoDeallocateModifiesMediaAfterSanitize { Modified = 0b10, Reserved = 0b11, } -unsafe impl Discriminant for NoDeallocateModifiesMediaAfterSanitize {} // Base v2.1, 5.1.13.2.1, Figure 312, SANICAP -#[derive(Clone, Copy, Debug)] -pub struct SanitizeCapabilities { - ces: bool, - bes: bool, - ows: bool, - vers: bool, - ndi: bool, - nodmmas: NoDeallocateModifiesMediaAfterSanitize, -} - -impl From for u32 { - fn from(value: SanitizeCapabilities) -> Self { - (((value.nodmmas.id() & 0b11) as u32) << 30) - | ((value.ndi as u32) << 29) - | ((value.vers as u32) << 3) - | ((value.ows as u32) << 2) - | ((value.bes as u32) << 1) - | (value.ces as u32) +flags! { + pub enum SanitizeCapabilityFlags: u32 { + Ces = 1 << 0, + Bes = 1 << 1, + Ows = 1 << 2, + Vers = 1 << 3, + Ndi = 1 << 29, } } -impl Default for SanitizeCapabilities { - fn default() -> Self { - Self { - ces: true, - bes: true, - ows: true, - vers: false, - ndi: true, - nodmmas: NoDeallocateModifiesMediaAfterSanitize::Unmodified, - } - } +// Base v2.1, 5.1.13.2.1, Figure 312, SANICAP +#[derive(Clone, Copy, Debug, DekuRead, DekuWrite)] +#[deku(bit_order = "lsb", ctx = "endian: Endian", endian = "endian")] +pub struct SanitizeCapabilities { + #[deku(bits = 30)] + caps: WireFlagSet, + #[deku(bits = 2)] + nodmmas: NoDeallocateModifiesMediaAfterSanitize, } // Base v2.1, 5.1.13.2.1, Figure 312, FNA @@ -651,7 +643,7 @@ struct AdminIdentifyControllerResponse { fwug: u8, kas: u16, #[deku(seek_from_current = "6")] - sanicap: u32, + sanicap: SanitizeCapabilities, #[deku(seek_from_current = "54")] cqt: u16, #[deku(seek_from_current = "124")] @@ -800,9 +792,16 @@ struct NvmNamespaceManagementCreate { } // Base v2.1, 5.1.22, Figure 372, SANACT -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, Default, DekuRead, DekuWrite, Eq, PartialEq)] +#[deku( + bits = "bits.0", + ctx = "endian: Endian, bits: BitSize", + endian = "endian", + id_type = "u8" +)] #[repr(u8)] enum SanitizeAction { + #[default] Reserved = 0x00, ExitFailureMode = 0x01, StartBlockErase = 0x02, @@ -810,7 +809,6 @@ enum SanitizeAction { StartCryptoErase = 0x04, ExitMediaVerificationState = 0x05, } -unsafe impl Discriminant for SanitizeAction {} impl TryFrom for SanitizeAction { type Error = (); @@ -829,40 +827,21 @@ impl TryFrom for SanitizeAction { } // Base v2.1, 5.1.22, Figure 372 -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, Default, DekuRead, DekuWrite, Eq, PartialEq)] +#[deku(ctx = "endian: Endian", endian = "endian")] pub struct AdminSanitizeConfiguration { - sanact: SanitizeAction, - ause: bool, + #[deku(bits = "4")] owpass: u8, - oipbp: bool, - ndas: bool, + #[deku(bits = "1")] + ause: bool, + #[deku(bits = "3")] + sanact: SanitizeAction, + #[deku(bits = "1", pad_bits_before = "5")] emvs: bool, -} - -impl TryFrom for AdminSanitizeConfiguration { - type Error = (); - - fn try_from(value: u32) -> Result { - Ok(Self { - sanact: TryInto::try_into(value & 0x7)?, - ause: ((value >> 3) & 1) == 1, - owpass: ((value >> 4) & 0xf) as u8, - oipbp: ((value >> 8) & 1) == 1, - ndas: ((value >> 9) & 1) == 1, - emvs: ((value >> 10) & 1) == 1, - }) - } -} - -impl From for u32 { - fn from(value: AdminSanitizeConfiguration) -> Self { - ((value.emvs as u32) << 10) - | ((value.ndas as u32) << 9) - | ((value.oipbp as u32) << 8) - | (((value.owpass & 0xf) as u32) << 4) - | ((value.ause as u32) << 3) - | value.sanact.id() as u32 - } + #[deku(bits = "1")] + ndas: bool, + #[deku(bits = "1", pad_bytes_after = "2")] + oipbp: bool, } // Base v2.1, 5.1.25, Figure 385 diff --git a/src/nvme/mi.rs b/src/nvme/mi.rs index 8305659..eb42ae2 100644 --- a/src/nvme/mi.rs +++ b/src/nvme/mi.rs @@ -10,7 +10,7 @@ use log::debug; use crate::nvme::{ AdminFormatNvmConfiguration, AdminNamespaceAttachmentSelect, AdminNamespaceManagementSelect, - ControllerListRequest, + AdminSanitizeConfiguration, ControllerListRequest, }; use crate::wire::{WireFlagSet, WireVec}; use crate::{CommandEffectError, Discriminant, Encode, MAX_CONTROLLERS}; @@ -997,7 +997,7 @@ struct AdminSanitizeRequest { dofst: u32, dlen: u32, #[deku(seek_from_current = "8")] - config: u32, + config: AdminSanitizeConfiguration, #[deku(pad_bytes_after = "16")] ovrpat: u32, } diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index 80d707d..a189640 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -11,7 +11,7 @@ use mctp::{AsyncRespChannel, MsgIC}; use crate::nvme::mi::{NvmSubsystemStatusFlags, PortCapabilityFlags}; use crate::nvme::{ ControllerAttributeFlags, ControllerMultipathIoNamespaceSharingCapabilityFlags, - ManagementEndpointCapabilityFlags, NvmSubsystemReportFlags, + ManagementEndpointCapabilityFlags, NvmSubsystemReportFlags, SanitizeCapabilityFlags, }; use crate::{ CommandEffect, CommandEffectError, Controller, ControllerError, ControllerType, Discriminant, @@ -22,11 +22,10 @@ use crate::{ AdminIdentifyCnsRequestType, AdminIdentifyControllerResponse, AdminIdentifyNamespaceIdentificationDescriptorListResponse, AdminIdentifyNvmIdentifyNamespaceResponse, AdminIoCqeGenericCommandStatus, - AdminIoCqeStatusType, AdminSanitizeConfiguration, ControllerListResponse, - LidSupportedAndEffectsDataStructure, LidSupportedAndEffectsFlags, LogPageAttributes, - NamespaceIdentifierType, SanitizeAction, SanitizeOperationStatus, SanitizeState, - SanitizeStateInformation, SanitizeStatus, SanitizeStatusLogPageResponse, - SmartHealthInformationLogPageResponse, + AdminIoCqeStatusType, ControllerListResponse, LidSupportedAndEffectsDataStructure, + LidSupportedAndEffectsFlags, LogPageAttributes, NamespaceIdentifierType, SanitizeAction, + SanitizeOperationStatus, SanitizeState, SanitizeStateInformation, SanitizeStatus, + SanitizeStatusLogPageResponse, SmartHealthInformationLogPageResponse, mi::{ AdminCommandRequestHeader, AdminCommandResponseHeader, AdminFormatNvmRequest, AdminNamespaceAttachmentRequest, AdminNamespaceManagementRequest, AdminSanitizeRequest, @@ -1277,13 +1276,7 @@ impl RequestHandler for AdminGetLogPageRequest { let sslpr = SanitizeStatusLogPageResponse { sprog: u16::MAX, sstat: subsys.sstat, - scdw10: { - if let Some(sconf) = subsys.sconf { - sconf.into() - } else { - 0 - } - }, + scdw10: subsys.sconf.unwrap_or_default(), eto: u32::MAX, etbe: u32::MAX, etce: u32::MAX, @@ -1464,7 +1457,10 @@ impl RequestHandler for AdminIdentifyRequest { msdbd: 0, ofcs: 0, apsta: 0, - sanicap: subsys.sanicap.into(), + sanicap: crate::nvme::SanitizeCapabilities { + caps: subsys.sanicap.into(), + nodmmas: subsys.nodmmas, + }, } .encode() .map_err(AdminIoCqeGenericCommandStatus::from) @@ -1918,18 +1914,7 @@ impl RequestHandler for AdminSanitizeRequest { return Err(ResponseStatus::InvalidCommandSize); } - let Ok(config) = TryInto::::try_into(self.config) else { - debug!("Invalid sanitize configuration: {}", self.config); - return admin_send_status( - resp, - AdminIoCqeStatusType::GenericCommandStatus( - AdminIoCqeGenericCommandStatus::InvalidFieldInCommand, - ), - ) - .await; - }; - - if subsys.sanicap.ndi && config.ndas { + if subsys.sanicap.contains(SanitizeCapabilityFlags::Ndi) && self.config.ndas { debug!("Request for No-Deallocate After Sanitize when No-Deallocate is inhibited"); return admin_send_status( resp, @@ -1941,7 +1926,7 @@ impl RequestHandler for AdminSanitizeRequest { } // TODO: Implement action latency, progress state machine, error states - match config.sanact { + match self.config.sanact { SanitizeAction::Reserved => Err(ResponseStatus::InvalidParameter), SanitizeAction::ExitFailureMode | SanitizeAction::ExitMediaVerificationState => { if subsys.ssi.sans != SanitizeState::Idle { @@ -1960,7 +1945,7 @@ impl RequestHandler for AdminSanitizeRequest { mvcncled: false, gde: true, }; - subsys.sconf = Some(self.config.try_into()?); + subsys.sconf = Some(self.config); admin_send_response_body(resp, &[]).await } @@ -1970,12 +1955,12 @@ impl RequestHandler for AdminSanitizeRequest { fails: 0, }; subsys.sstat = SanitizeStatus { - opc: config.owpass, + opc: self.config.owpass, sos: SanitizeOperationStatus::Sanitized, mvcncled: false, gde: true, }; - subsys.sconf = Some(self.config.try_into()?); + subsys.sconf = Some(self.config); admin_send_response_body(resp, &[]).await } From a227202df2be76161d608191890881f3b18fbbc1 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Thu, 23 Oct 2025 10:15:43 +1030 Subject: [PATCH 18/20] pci: Convert PowerManagementCapabilities to deku bits Signed-off-by: Andrew Jeffery --- src/nvme.rs | 1 - src/pcie.rs | 29 ++++++++++++----------------- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/nvme.rs b/src/nvme.rs index c4c81f7..8840749 100644 --- a/src/nvme.rs +++ b/src/nvme.rs @@ -134,7 +134,6 @@ enum AdminIoCqeGenericCommandStatus { InternalError = 0x06, InvalidNamespaceOrFormat = 0x0b, } -unsafe impl Discriminant for AdminIoCqeGenericCommandStatus {} impl From for AdminIoCqeGenericCommandStatus { fn from(err: DekuError) -> Self { diff --git a/src/pcie.rs b/src/pcie.rs index ef1692a..ded4189 100644 --- a/src/pcie.rs +++ b/src/pcie.rs @@ -104,8 +104,7 @@ impl PciDeviceFunctionConfigurationSpace { d2: false, pme: 0, } - } - .into(), + }, pmcsr: 0, data: 0, }), @@ -175,36 +174,32 @@ impl PciDeviceFunctionConfigurationSpaceBuilder { } } -#[derive(Debug)] +#[derive(Debug, DekuRead, DekuWrite)] +#[deku(bit_order = "lsb", ctx = "endian: Endian", endian = "endian")] pub struct PowerManagementCapabilities { + #[deku(bits = "3")] version: u8, + #[deku(bits = "1")] pme_clock: bool, + #[deku(bits = "1")] ready_d0: bool, + #[deku(bits = "1")] dsi: bool, + #[deku(bits = "3")] aux_current: u8, + #[deku(bits = "1")] d1: bool, + #[deku(bits = "1")] d2: bool, + #[deku(bits = "5")] pme: u8, } -impl From for u16 { - fn from(value: PowerManagementCapabilities) -> Self { - ((value.pme as u16 & 0xf) << 11) - | ((value.d2 as u16) << 10) - | ((value.d1 as u16) << 9) - | ((value.aux_current as u16 & 0x7) << 6) - | ((value.dsi as u16) << 5) - | ((value.ready_d0 as u16) << 4) - | ((value.pme_clock as u16) << 3) - | (value.version as u16 & 0x7) - } -} - #[derive(Debug, DekuRead, DekuWrite)] #[deku(ctx = "endian: Endian", endian = "endian")] pub struct PciPowerManagementCapability { next: u8, - pmc: u16, + pmc: PowerManagementCapabilities, pmcsr: u16, #[deku(seek_from_current = "1")] data: u8, From 85cc791b89e10e11c4f81d6bff7e1fd608878956 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Thu, 23 Oct 2025 10:18:24 +1030 Subject: [PATCH 19/20] pcie: Drop Discriminant trait for PciCapabilityType Signed-off-by: Andrew Jeffery --- src/pcie.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pcie.rs b/src/pcie.rs index ded4189..7f0f07d 100644 --- a/src/pcie.rs +++ b/src/pcie.rs @@ -240,4 +240,3 @@ pub enum PciCapabilityType { #[deku(id = "0x10")] Pcie(PcieCapability), } -unsafe impl crate::Discriminant for PciCapabilityType {} From 0233985290d8cc6df07695b3cb16107d8a82b493 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Thu, 23 Oct 2025 10:26:20 +1030 Subject: [PATCH 20/20] nvme: mi: PcieCommandRequestType: Drop Discriminant impl Signed-off-by: Andrew Jeffery --- src/nvme/mi.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/nvme/mi.rs b/src/nvme/mi.rs index eb42ae2..a165306 100644 --- a/src/nvme/mi.rs +++ b/src/nvme/mi.rs @@ -1039,7 +1039,6 @@ enum PcieCommandRequestType { IoRead = 0x04, IoWrite = 0x05, } -unsafe impl Discriminant for PcieCommandRequestType {} // MI v2.0, 7, Figure 151-152 #[derive(Debug, DekuRead, DekuWrite, Eq, PartialEq)]