From ba5956997f1ee7842519de5149c25ffb02611278 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Thu, 18 Sep 2025 20:37:41 +0930 Subject: [PATCH 01/12] tests: common: Require lengths match in ExpectedRespChannel Signed-off-by: Andrew Jeffery --- tests/common/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/common/mod.rs b/tests/common/mod.rs index d2a1f1e..5e9e1e3 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -105,6 +105,7 @@ impl mctp::AsyncRespChannel for ExpectedRespChannel<'_> { "Failed emptiness consensus:\n\tExpected: {:02x?}\n\tFound: {bufs:02x?}", self.resp ); + assert_eq!(bufs.iter().map(|b| b.len()).sum::(), self.resp.len()); assert!( core::iter::zip(self.resp, bufs.iter().flat_map(|b| b.iter())).all(|(e, f)| e == f), "Expected: {:02x?}, found: {:02x?}", From f2b24900611e3ad2c59f937cb4226d1fb2d4a984 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Thu, 18 Sep 2025 20:39:56 +0930 Subject: [PATCH 02/12] tests: common: Prefer assert_eq!() in ExpectedRespChannel Signed-off-by: Andrew Jeffery --- tests/common/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 5e9e1e3..26ddca7 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -100,10 +100,10 @@ impl mctp::AsyncRespChannel for ExpectedRespChannel<'_> { async fn send_vectored(&mut self, _integrity_check: MsgIC, bufs: &[&[u8]]) -> mctp::Result<()> { self.sent = true; - assert!( - self.resp.is_empty() == bufs.iter().all(|b| b.is_empty()), - "Failed emptiness consensus:\n\tExpected: {:02x?}\n\tFound: {bufs:02x?}", - self.resp + assert_eq!( + self.resp.is_empty(), + bufs.iter().all(|b| b.is_empty()), + "Failed emptiness consensus" ); assert_eq!(bufs.iter().map(|b| b.len()).sum::(), self.resp.len()); assert!( From e407cbb72b8d0c3fc507ebcd33ae17e38cc509de Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Thu, 18 Sep 2025 12:58:09 +0930 Subject: [PATCH 03/12] nvme: mi: dev: s/inset/insert/ in debug log Signed-off-by: Andrew Jeffery --- src/nvme/mi/dev.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index 9340896..5940e89 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -1454,7 +1454,7 @@ impl RequestHandler for AdminIdentifyRequest { let mut vec = WireVec::new(); for nsid in start..=end { if vec.push(nsid).is_err() { - debug!("Failed to inset NSID {nsid}"); + debug!("Failed to insert NSID {nsid}"); return Err(ResponseStatus::InternalError); }; } From c496425ed5c72a0ec557f8909ef1863cf82fe0e2 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Thu, 18 Sep 2025 12:56:43 +0930 Subject: [PATCH 04/12] nvme: mi: dev: Rework admin_send_invalid_field() Increase utility by requiring the status to be sent as an argument, and renaming it to admin_send_status(). Signed-off-by: Andrew Jeffery --- src/nvme/mi/dev.rs | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index 5940e89..3f89a6c 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -936,7 +936,10 @@ where Ok(()) } -async fn admin_send_invalid_field(resp: &mut C) -> Result<(), ResponseStatus> +async fn admin_send_status( + resp: &mut C, + status: AdminIoCqeStatusType, +) -> Result<(), ResponseStatus> where C: AsyncRespChannel, { @@ -949,9 +952,7 @@ where cqedw3: AdminIoCqeStatus { cid: 0, p: true, - status: AdminIoCqeStatusType::GenericCommandStatus( - AdminIoCqeGenericCommandStatus::InvalidFieldInCommand, - ), + status, crd: crate::nvme::CommandRetryDelay::None, m: false, dnr: true, @@ -1019,7 +1020,13 @@ impl RequestHandler for AdminGetLogPageRequest { if flags.contains(LidSupportedAndEffectsFlags::Ios) { todo!("Add OT support"); } else { - return admin_send_invalid_field(resp).await; + return admin_send_status( + resp, + AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::InvalidFieldInCommand, + ), + ) + .await; } } @@ -1079,14 +1086,26 @@ impl RequestHandler for AdminGetLogPageRequest { // Base v2.1, 5.1.2, Figure 199 let lpol = self.lpo & !3u64; if lpol > 512 { - return admin_send_invalid_field(resp).await; + return admin_send_status( + resp, + AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::InvalidFieldInCommand, + ), + ) + .await; } if self.nsid != 0 && self.nsid != u32::MAX { if ctlr.lpa.contains(LogPageAttributes::Smarts) { todo!(); } else { - return admin_send_invalid_field(resp).await; + return admin_send_status( + resp, + AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::InvalidFieldInCommand, + ), + ) + .await; } } From 7b8a695ac1deb0a955da38f41374b7339e280b5e Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Thu, 18 Sep 2025 14:11:50 +0930 Subject: [PATCH 05/12] nvme: Fix mismatched spec reference for AdminIoCqeStatusType Signed-off-by: Andrew Jeffery --- src/nvme.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nvme.rs b/src/nvme.rs index 10c9cae..d672758 100644 --- a/src/nvme.rs +++ b/src/nvme.rs @@ -85,7 +85,7 @@ enum CommandRetryDelay { } unsafe impl Discriminant for CommandRetryDelay {} -// Base v2.1, 4.3.2, Figure 101 +// Base v2.1, 4.2.3, Figure 101 #[derive(Clone, Copy, Debug, Eq, PartialEq)] #[repr(u8)] enum AdminIoCqeStatusType { From ea3a4e347548b7097015fc2168f556b269022d99 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Thu, 18 Sep 2025 14:13:30 +0930 Subject: [PATCH 06/12] nvme: Implement From for AdminIoCqeGenericCommandStatus This will help tidy up the error handling using .map_err(). Signed-off-by: Andrew Jeffery --- src/nvme.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/nvme.rs b/src/nvme.rs index d672758..74c5bdf 100644 --- a/src/nvme.rs +++ b/src/nvme.rs @@ -5,8 +5,9 @@ pub mod mi; use deku::ctx::Endian; -use deku::{DekuRead, DekuWrite, deku_derive}; +use deku::{DekuError, DekuRead, DekuWrite, deku_derive}; use flagset::flags; +use log::debug; use crate::wire::WireFlagSet; use crate::wire::WireString; @@ -106,9 +107,17 @@ unsafe impl Discriminant for AdminIoCqeStatusType {} enum AdminIoCqeGenericCommandStatus { SuccessfulCompletion = 0x00, InvalidFieldInCommand = 0x02, + InternalError = 0x06, } unsafe impl Discriminant for AdminIoCqeGenericCommandStatus {} +impl From for AdminIoCqeGenericCommandStatus { + fn from(err: DekuError) -> Self { + debug!("Codec operation failed: {err}"); + Self::InternalError + } +} + // Base v2.1, 4.6.1, Figure 137 // TODO: Unify with ControllerListResponse #[derive(Debug, DekuRead, Eq, PartialEq)] From 123155fb2eb5971c3861e96bfb3c81b79205d6a2 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Wed, 17 Sep 2025 18:22:15 +0930 Subject: [PATCH 07/12] lib: Introduce NamespaceId::disposition() and NamespaceIdDisposition Signed-off-by: Andrew Jeffery --- src/lib.rs | 59 ++++++++++++++-- src/nvme/mi/dev.rs | 172 +++++++++++++++++++++------------------------ 2 files changed, 137 insertions(+), 94 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index b6beb0a..d253c37 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -413,6 +413,61 @@ pub enum NamespaceIdentifierType { Csi(nvme::CommandSetIdentifier), } +// Base v2.1, 3.2.1 +// Base v2.1, 3.2.1.5, Figure 71 +#[derive(Clone, Copy, Debug)] +enum NamespaceIdDisposition<'a> { + Invalid, + Broadcast, + Unallocated, + Inactive(&'a Namespace), + Active(&'a Namespace), +} + +// NSID +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct NamespaceId(u32); + +impl NamespaceId { + fn disposition<'a>(&self, subsys: &'a Subsystem) -> NamespaceIdDisposition<'a> { + if self.0 == 0 { + return NamespaceIdDisposition::Invalid; + } + + if self.0 == u32::MAX { + return NamespaceIdDisposition::Broadcast; + } + + assert!(subsys.nss.capacity() <= u32::MAX.try_into().unwrap()); + if self.0 > subsys.nss.capacity() as u32 { + return NamespaceIdDisposition::Invalid; + } + + let Some(ns) = subsys.nss.iter().find(|nsid| self.0 == nsid.id.0) else { + return NamespaceIdDisposition::Unallocated; + }; + + if !subsys + .ctlrs + .iter() + .flat_map(|c| c.active_ns.iter()) + .any(|&nsid| nsid.0 == self.0) + { + return NamespaceIdDisposition::Inactive(ns); + } + + NamespaceIdDisposition::Active(ns) + } + + fn max(subsys: &Subsystem) -> u32 { + subsys + .nss + .capacity() + .try_into() + .expect("Too many namespaces") + } +} + #[derive(Debug)] pub struct Namespace { id: NamespaceId, @@ -423,10 +478,6 @@ pub struct Namespace { nids: [NamespaceIdentifierType; 2], } -// NSID -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct NamespaceId(u32); - impl Namespace { fn generate_uuid(seed: &[u8], nsid: NamespaceId) -> Uuid { let mut hasher = hmac::Hmac::::new_from_slice(seed).unwrap(); diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index 3f89a6c..cb1e934 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -10,7 +10,7 @@ use mctp::{AsyncRespChannel, MsgIC}; use crate::{ CommandEffect, CommandEffectError, Controller, ControllerError, ControllerType, Discriminant, - MAX_CONTROLLERS, MAX_NAMESPACES, NamespaceId, SubsystemError, + MAX_CONTROLLERS, MAX_NAMESPACES, NamespaceId, NamespaceIdDisposition, SubsystemError, nvme::{ AdminFormatNvmConfiguration, AdminGetLogPageLidRequestType, AdminGetLogPageSupportedLogPagesResponse, AdminIdentifyActiveNamespaceIdListResponse, @@ -1242,59 +1242,43 @@ impl RequestHandler for AdminIdentifyRequest { match &self.req { AdminIdentifyCnsRequestType::NvmIdentifyNamespace => { - assert!(subsys.nss.len() <= u32::MAX.try_into().unwrap()); - - if self.nsid == u32::MAX { - let ainvminr = AdminIdentifyNvmIdentifyNamespaceResponse { - lbaf0_lbads: 9, // TODO: Tie to controller model - ..Default::default() + let ainvminr = match NamespaceId(self.nsid).disposition(subsys) { + NamespaceIdDisposition::Invalid => { + debug!("Invalid NSID: {}", self.nsid); + Err(ResponseStatus::InvalidParameter) } - .encode()?; - - return admin_send_response_body( - resp, - admin_constrain_body(self.dofst, self.dlen, &ainvminr.0)?, - ) - .await; - } - - if self.nsid == 0 || self.nsid > subsys.nss.capacity() as u32 { - debug!("Invalid NSID: {}", self.nsid); - return Err(ResponseStatus::InvalidParameter); - } - - let Some(ns) = subsys.nss.get(self.nsid as usize - 1) else { - debug!("Unallocated NSID: {}", self.nsid); - return Err(ResponseStatus::InvalidParameter); - }; - - // 4.1.5.1 NVM Command Set Spec, v1.0c - // TODO: Ensure the associated controller is an IO controller - // FIXME: Improve determination algo - let active = subsys - .ctlrs - .iter() - .flat_map(|c| c.active_ns.iter()) - .any(|&nsid| nsid.0 == self.nsid); - let ainvminr = if active { - AdminIdentifyNvmIdentifyNamespaceResponse { - nsze: ns.size, - ncap: ns.capacity, - nuse: ns.used, - nsfeat: ((ns.size == ns.capacity) as u8), - nlbaf: 0, - flbas: 0, - mc: 0, - dpc: 0, - dps: 0, - nvmcap: 2_u128.pow(ns.block_order as u32) * ns.size as u128, - lbaf0: 0, - lbaf0_lbads: ns.block_order, - lbaf0_rp: 0, + NamespaceIdDisposition::Broadcast => { + Ok(AdminIdentifyNvmIdentifyNamespaceResponse { + lbaf0_lbads: 9, // TODO: Tie to controller model + ..Default::default() + }) } - } else { - AdminIdentifyNvmIdentifyNamespaceResponse::default() - } + NamespaceIdDisposition::Unallocated => { + debug!("Unallocated NSID: {}", self.nsid); + Err(ResponseStatus::InvalidParameter) + } + NamespaceIdDisposition::Inactive(_) => { + Ok(AdminIdentifyNvmIdentifyNamespaceResponse::default()) + } + // 4.1.5.1 NVM Command Set Spec, v1.0c + NamespaceIdDisposition::Active(ns) => { + Ok(AdminIdentifyNvmIdentifyNamespaceResponse { + nsze: ns.size, + ncap: ns.capacity, + nuse: ns.used, + nsfeat: ((ns.size == ns.capacity) as u8), + nlbaf: 0, + flbas: 0, + mc: 0, + dpc: 0, + dps: 0, + nvmcap: 2_u128.pow(ns.block_order as u32) * ns.size as u128, + lbaf0: 0, + lbaf0_lbads: ns.block_order, + lbaf0_rp: 0, + }) + } + }? .encode()?; admin_send_response_body( @@ -1358,11 +1342,7 @@ impl RequestHandler for AdminIdentifyRequest { sqes: 0, cqes: 0, maxcmd: 0, - nn: subsys - .nss - .capacity() - .try_into() - .expect("Too many namespaces"), + nn: NamespaceId::max(subsys), oncs: 0, fuses: 0, fna: ctlr.fna.into(), @@ -1420,36 +1400,43 @@ impl RequestHandler for AdminIdentifyRequest { } AdminIdentifyCnsRequestType::NamespaceIdentificationDescriptorList => { // 5.1.13.2.3, Base v2.1 - if self.nsid >= u32::MAX - 1 { - debug!("Unacceptable NSID for Namespace Identification Descriptor List"); - return Err(ResponseStatus::InvalidParameter); - } - - if self.nsid == 0 || self.nsid > subsys.nss.capacity() as u32 { - debug!("Invalid NSID: {}", self.nsid); - return Err(ResponseStatus::InvalidParameter); - } - - let Some(ns) = subsys.nss.get(self.nsid as usize - 1) else { - debug!("Unallocated NSID: {}", self.nsid); - return Err(ResponseStatus::InvalidParameter); - }; - - let ainsidlr = AdminIdentifyNamespaceIdentificationDescriptorListResponse { - nids: { - let mut vec = WireVec::new(); - for nid in &ns.nids { - if vec - .push(Into::::into(*nid)) - .is_err() - { - debug!("Failed to push NID {nid:?}"); - return Err(ResponseStatus::InternalError); - } + let ainsidlr = match NamespaceId(self.nsid).disposition(subsys) { + NamespaceIdDisposition::Invalid => { + if self.nsid == u32::MAX - 1 { + debug!( + "Unacceptable NSID for Namespace Identification Descriptor List" + ); + } else { + debug!("Invalid NSID: {}", self.nsid); } - vec - }, - } + Err(ResponseStatus::InvalidParameter) + } + NamespaceIdDisposition::Broadcast => { + debug!("Invalid NSID: {}", self.nsid); + Err(ResponseStatus::InvalidParameter) + } + NamespaceIdDisposition::Unallocated => { + debug!("Unallocated NSID: {}", self.nsid); + Err(ResponseStatus::InvalidParameter) + } + NamespaceIdDisposition::Inactive(ns) | NamespaceIdDisposition::Active(ns) => { + Ok(AdminIdentifyNamespaceIdentificationDescriptorListResponse { + nids: { + let mut vec = WireVec::new(); + for nid in &ns.nids { + if vec + .push(Into::::into(*nid)) + .is_err() + { + debug!("Failed to push NID {nid:?}"); + return Err(ResponseStatus::InternalError); + } + } + vec + }, + }) + } + }? .encode()?; admin_send_response_body( @@ -1465,13 +1452,18 @@ impl RequestHandler for AdminIdentifyRequest { return Err(ResponseStatus::InvalidParameter); } - assert!(subsys.nss.len() < 4096 / core::mem::size_of::()); + assert!(NamespaceId::max(subsys) < (4096 / core::mem::size_of::()) as u32); let aiansidl = AdminIdentifyAllocatedNamespaceIdListResponse { nsid: { - let start = self.nsid + 1; - let end = subsys.nss.len() as u32; + let mut allocated: heapless::Vec = subsys + .nss + .iter() + .map(|ns| ns.id.0) + .filter(|nsid| *nsid > self.nsid) + .collect(); + allocated.sort_unstable(); let mut vec = WireVec::new(); - for nsid in start..=end { + for nsid in allocated { if vec.push(nsid).is_err() { debug!("Failed to insert NSID {nsid}"); return Err(ResponseStatus::InternalError); From 6e422cf4f24db023d23fcd79d455dda60693fa41 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Thu, 18 Sep 2025 12:59:21 +0930 Subject: [PATCH 08/12] nvme: mi: dev: Implement Admin / Identify / Identify Namespace for Allocated Namespace ID Signed-off-by: Andrew Jeffery --- src/nvme.rs | 22 ++++ src/nvme/mi/dev.rs | 49 +++++--- tests/admin.rs | 289 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 343 insertions(+), 17 deletions(-) diff --git a/src/nvme.rs b/src/nvme.rs index 74c5bdf..7179da2 100644 --- a/src/nvme.rs +++ b/src/nvme.rs @@ -108,6 +108,7 @@ enum AdminIoCqeGenericCommandStatus { SuccessfulCompletion = 0x00, InvalidFieldInCommand = 0x02, InternalError = 0x06, + InvalidNamespaceOrFormat = 0x0b, } unsafe impl Discriminant for AdminIoCqeGenericCommandStatus {} @@ -384,6 +385,7 @@ enum AdminIdentifyCnsRequestType { IoActiveNamespaceIdList = 0x07, IdentifyNamespace = 0x08, AllocatedNamespaceIdList = 0x10, + IdentifyNamespaceForAllocatedNamespaceId = 0x11, NvmSubsystemControllerList = 0x13, SecondaryControllerList = 0x15, } @@ -413,6 +415,26 @@ pub struct AdminIdentifyNvmIdentifyNamespaceResponse { } impl Encode<4096> for AdminIdentifyNvmIdentifyNamespaceResponse {} +impl From<&crate::Namespace> for AdminIdentifyNvmIdentifyNamespaceResponse { + fn from(value: &crate::Namespace) -> Self { + Self { + nsze: value.size, + ncap: value.capacity, + nuse: value.used, + nsfeat: ((value.size == value.capacity) as u8), + nlbaf: 0, + flbas: 0, + mc: 0, + dpc: 0, + dps: 0, + nvmcap: 2_u128.pow(value.block_order as u32) * value.size as u128, + lbaf0: 0, + lbaf0_lbads: value.block_order, + lbaf0_rp: 0, + } + } +} + // Base v2.1, 5.1.13.1, Figure 311 #[derive(Clone, Copy, Debug, DekuRead, DekuWrite)] #[deku(id_type = "u8", endian = "endian", ctx = "endian: Endian")] diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index cb1e934..a8d43d0 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -1261,23 +1261,7 @@ impl RequestHandler for AdminIdentifyRequest { Ok(AdminIdentifyNvmIdentifyNamespaceResponse::default()) } // 4.1.5.1 NVM Command Set Spec, v1.0c - NamespaceIdDisposition::Active(ns) => { - Ok(AdminIdentifyNvmIdentifyNamespaceResponse { - nsze: ns.size, - ncap: ns.capacity, - nuse: ns.used, - nsfeat: ((ns.size == ns.capacity) as u8), - nlbaf: 0, - flbas: 0, - mc: 0, - dpc: 0, - dps: 0, - nvmcap: 2_u128.pow(ns.block_order as u32) * ns.size as u128, - lbaf0: 0, - lbaf0_lbads: ns.block_order, - lbaf0_rp: 0, - }) - } + NamespaceIdDisposition::Active(ns) => Ok(ns.into()), }? .encode()?; @@ -1480,6 +1464,37 @@ impl RequestHandler for AdminIdentifyRequest { ) .await } + AdminIdentifyCnsRequestType::IdentifyNamespaceForAllocatedNamespaceId => { + // Base v2.1, 5.1.13.2.10 + match match NamespaceId(self.nsid).disposition(subsys) { + NamespaceIdDisposition::Invalid | NamespaceIdDisposition::Broadcast => { + Err(AdminIoCqeGenericCommandStatus::InvalidNamespaceOrFormat) + } + NamespaceIdDisposition::Unallocated => { + AdminIdentifyNvmIdentifyNamespaceResponse::default() + .encode() + .map_err(AdminIoCqeGenericCommandStatus::from) + } + NamespaceIdDisposition::Inactive(ns) | NamespaceIdDisposition::Active(ns) => { + let ainvminr: AdminIdentifyNvmIdentifyNamespaceResponse = ns.into(); + ainvminr + .encode() + .map_err(AdminIoCqeGenericCommandStatus::from) + } + } { + Ok(response) => { + admin_send_response_body( + resp, + admin_constrain_body(self.dofst, self.dlen, &response.0)?, + ) + .await + } + Err(err) => { + admin_send_status(resp, AdminIoCqeStatusType::GenericCommandStatus(err)) + .await + } + } + } AdminIdentifyCnsRequestType::NvmSubsystemControllerList => { assert!( subsys.ctlrs.len() <= 2047, diff --git a/tests/admin.rs b/tests/admin.rs index 3577298..0d72e5c 100644 --- a/tests/admin.rs +++ b/tests/admin.rs @@ -98,6 +98,16 @@ mod identify { use crate::common::setup; use mctp::MsgIC; + #[rustfmt::skip] + const RESP_ADMIN_STATUS_INVALID_NAMESPACE: [u8; 23] = [ + 0x90, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x17, 0x80, + 0xfb, 0x4e, 0x5e, 0x4f + ]; + #[test] fn controller_short() { setup(); @@ -1561,6 +1571,285 @@ mod identify { }); } + #[test] + fn identify_namespace_for_allocated_namespace_id_invalid_nsid() { + setup(); + + let (mut mep, mut subsys) = new_device(DeviceType::P1p1tC1iN1a1a); + + #[rustfmt::skip] + const REQ: [u8; 71] = [ + 0x10, 0x00, 0x00, + 0x06, 0x00, 0x00, 0x00, + + // SQE DWORD 1 + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // DOFST + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x10, 0x00, 0x00, + + // Reserved + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // SQE DWORD 10 + 0x11, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // MIC + 0xe7, 0xd2, 0xf8, 0x68 + ]; + + let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_NAMESPACE); + smol::block_on(async { + mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) + .await + }); + } + + #[test] + fn identify_namespace_for_allocated_namespace_id_broadcast_nsid() { + setup(); + + let (mut mep, mut subsys) = new_device(DeviceType::P1p1tC1iN1a1a); + + #[rustfmt::skip] + const REQ: [u8; 71] = [ + 0x10, 0x00, 0x00, + 0x06, 0x00, 0x00, 0x00, + + // SQE DWORD 1 + 0xff, 0xff, 0xff, 0xff, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // DOFST + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x10, 0x00, 0x00, + + // Reserved + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // SQE DWORD 10 + 0x11, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // MIC + 0x29, 0x28, 0x0c, 0xcd + ]; + + let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_NAMESPACE); + smol::block_on(async { + mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) + .await + }); + } + + #[test] + fn identify_namespace_for_allocated_namespace_id_unallocated_nsid() { + setup(); + + let (mut mep, mut subsys) = new_device(DeviceType::P1p1tC1iN1a1a); + + #[rustfmt::skip] + const REQ: [u8; 71] = [ + 0x10, 0x00, 0x00, + 0x06, 0x00, 0x00, 0x00, + + // SQE DWORD 1 + 0x02, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // DOFST + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x10, 0x00, 0x00, + + // Reserved + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // SQE DWORD 10 + 0x11, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // MIC + 0x51, 0x9a, 0x8f, 0x83 + ]; + + #[rustfmt::skip] + const RESP_DATA: [u8; 19] = [ + 0x90, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x01, 0x00 + ]; + + const RESP_LEN: usize = 4119; + let mut resp: [u8; RESP_LEN] = [0; RESP_LEN]; + resp[..RESP_DATA.len()].copy_from_slice(&RESP_DATA); + resp[(RESP_LEN - 4)..].copy_from_slice(&[0x87, 0xd7, 0x1f, 0xaa]); + + let resp = ExpectedRespChannel::new(&resp); + smol::block_on(async { + mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) + .await + }); + } + + #[test] + fn identify_namespace_for_allocated_namespace_id_inactive_nsid() { + setup(); + + let lbads = 9u8; + let blocks = 1024u64; + let nvmcap = (blocks as u128) * 2_u128.pow(lbads as u32); + + let (mut mep, mut subsys) = new_device(DeviceType::P1p1tC1iN1a0a); + + #[rustfmt::skip] + const REQ: [u8; 71] = [ + 0x10, 0x00, 0x00, + 0x06, 0x00, 0x00, 0x00, + + // SQE DWORD 1 + 0x01, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // DOFST + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x10, 0x00, 0x00, + + // Reserved + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // SQE DWORD 10 + 0x11, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // MIC + 0xbc, 0x76, 0x43, 0x1d + ]; + + let blocks_repr = blocks.to_le_bytes(); + let nvmcap_repr = nvmcap.to_le_bytes(); + let lbads_repr = [lbads]; + + #[rustfmt::skip] + let resp_fields: Vec = vec![ + // NSZE + (19, &blocks_repr), + // NCAP + (19+8, &blocks_repr), + // NVMCAP + (19+48, &nvmcap_repr), + // LBAF0_LBADS + (19+130, &lbads_repr) + ]; + + let resp = RelaxedRespChannel::new(resp_fields); + smol::block_on(async { + mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) + .await + }); + } + + #[test] + fn identify_namespace_for_allocated_namespace_id_active_nsid() { + setup(); + + let lbads = 9u8; + let blocks = 1024u64; + let nvmcap = (blocks as u128) * 2_u128.pow(lbads as u32); + + let (mut mep, mut subsys) = new_device(DeviceType::P1p1tC1iN1a1a); + + #[rustfmt::skip] + const REQ: [u8; 71] = [ + 0x10, 0x00, 0x00, + 0x06, 0x00, 0x00, 0x00, + + // SQE DWORD 1 + 0x01, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // DOFST + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x10, 0x00, 0x00, + + // Reserved + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // SQE DWORD 10 + 0x11, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // MIC + 0xbc, 0x76, 0x43, 0x1d + ]; + + let blocks_repr = blocks.to_le_bytes(); + let nvmcap_repr = nvmcap.to_le_bytes(); + let lbads_repr = [lbads]; + + #[rustfmt::skip] + let resp_fields: Vec = vec![ + // NSZE + (19, &blocks_repr), + // NCAP + (19+8, &blocks_repr), + // NVMCAP + (19+48, &nvmcap_repr), + // LBAF0_LBADS + (19+130, &lbads_repr) + ]; + + let resp = RelaxedRespChannel::new(resp_fields); + smol::block_on(async { + mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) + .await + }); + } + #[test] fn secondary_controller_list() { setup(); From e42413b50abfee5b8ebddab5d5a8a6236dd2f24f Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Thu, 18 Sep 2025 13:45:26 +0930 Subject: [PATCH 09/12] nvme: mi: dev: Implement Admin / Identify / Namespace Attached Controller List Signed-off-by: Andrew Jeffery --- src/nvme.rs | 1 + src/nvme/mi/dev.rs | 44 ++++++ tests/admin.rs | 350 +++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 382 insertions(+), 13 deletions(-) diff --git a/src/nvme.rs b/src/nvme.rs index 7179da2..0f38f92 100644 --- a/src/nvme.rs +++ b/src/nvme.rs @@ -386,6 +386,7 @@ enum AdminIdentifyCnsRequestType { IdentifyNamespace = 0x08, AllocatedNamespaceIdList = 0x10, IdentifyNamespaceForAllocatedNamespaceId = 0x11, + NamespaceAttachedControllerList = 0x12, NvmSubsystemControllerList = 0x13, SecondaryControllerList = 0x15, } diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index a8d43d0..4abdcc7 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -1495,6 +1495,50 @@ impl RequestHandler for AdminIdentifyRequest { } } } + AdminIdentifyCnsRequestType::NamespaceAttachedControllerList => { + match match NamespaceId(self.nsid).disposition(subsys) { + NamespaceIdDisposition::Invalid => ControllerListResponse::new() + .encode() + .map_err(AdminIoCqeGenericCommandStatus::from), + NamespaceIdDisposition::Broadcast => { + Err(AdminIoCqeGenericCommandStatus::InvalidFieldInCommand) + } + NamespaceIdDisposition::Unallocated | NamespaceIdDisposition::Inactive(_) => { + ControllerListResponse::new() + .encode() + .map_err(AdminIoCqeGenericCommandStatus::from) + } + NamespaceIdDisposition::Active(ns) => { + let mut clr = ControllerListResponse::new(); + for cid in subsys.ctlrs.iter().filter_map(|c| { + if c.id.0 >= self.cntid && c.active_ns.contains(&ns.id) { + Some(c.id) + } else { + None + } + }) { + if let Err(id) = clr.ids.push(cid.0) { + debug!("Failed to push controller ID {id}"); + return Err(ResponseStatus::InternalError); + } + } + clr.update()?; + clr.encode().map_err(AdminIoCqeGenericCommandStatus::from) + } + } { + Ok(response) => { + admin_send_response_body( + resp, + admin_constrain_body(self.dofst, self.dlen, &response.0)?, + ) + .await + } + Err(status) => { + admin_send_status(resp, AdminIoCqeStatusType::GenericCommandStatus(status)) + .await + } + } + } AdminIdentifyCnsRequestType::NvmSubsystemControllerList => { assert!( subsys.ctlrs.len() <= 2047, diff --git a/tests/admin.rs b/tests/admin.rs index 0d72e5c..dc5c0ae 100644 --- a/tests/admin.rs +++ b/tests/admin.rs @@ -35,6 +35,16 @@ const RESP_ADMIN_SUCCESS: [u8; 23] = [ 0x30, 0xd5, 0xa2, 0x9b ]; +#[rustfmt::skip] +const RESP_ADMIN_STATUS_INVALID_FIELD: [u8; 23] = [ + 0x90, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x05, 0x80, + 0x94, 0x8f, 0xde, 0x57, +]; + mod prohibited { use super::RESP_INVALID_COMMAND; use crate::common::{DeviceType, ExpectedRespChannel, new_device, setup}; @@ -89,6 +99,7 @@ mod prohibited { mod identify { use super::RESP_INVALID_COMMAND_SIZE; use super::RESP_INVALID_PARAMETER; + use crate::RESP_ADMIN_STATUS_INVALID_FIELD; use crate::common::DeviceType; use crate::common::ExpectedField; use crate::common::ExpectedRespChannel; @@ -1850,6 +1861,331 @@ mod identify { }); } + #[test] + fn namespace_attached_controller_list_invalid_nsid() { + setup(); + + let (mut mep, mut subsys) = new_device(DeviceType::P1p1tC1iN1a1a); + + #[rustfmt::skip] + const REQ: [u8; 71] = [ + 0x10, 0x00, 0x00, + 0x06, 0x00, 0x00, 0x00, + + // SQE DWORD 1 + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // DOFST + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x10, 0x00, 0x00, + + // Reserved + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // SQE DWORD 10 + 0x12, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // MIC + 0x84, 0xe3, 0xc4, 0xa3 + ]; + + #[rustfmt::skip] + const RESP_DATA: [u8; 19] = [ + 0x90, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x01, 0x00 + ]; + + const RESP_LEN: usize = 4119; + let mut resp: [u8; RESP_LEN] = [0; RESP_LEN]; + resp[..RESP_DATA.len()].copy_from_slice(&RESP_DATA); + resp[(RESP_LEN - 4)..].copy_from_slice(&[0x87, 0xd7, 0x1f, 0xaa]); + + let resp = ExpectedRespChannel::new(&resp); + smol::block_on(async { + mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) + .await + }); + } + + #[test] + fn namespace_attached_controller_list_broadcast_nsid() { + setup(); + + let (mut mep, mut subsys) = new_device(DeviceType::P1p1tC1iN1a1a); + + #[rustfmt::skip] + const REQ: [u8; 71] = [ + 0x10, 0x00, 0x00, + 0x06, 0x00, 0x00, 0x00, + + // SQE DWORD 1 + 0xff, 0xff, 0xff, 0xff, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // DOFST + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x10, 0x00, 0x00, + + // Reserved + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // SQE DWORD 10 + 0x12, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // MIC + 0x4a, 0x19, 0x30, 0x06 + ]; + + let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_FIELD); + smol::block_on(async { + mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) + .await + }); + } + + #[test] + fn namespace_attached_controller_list_unallocated_nsid() { + setup(); + + let (mut mep, mut subsys) = new_device(DeviceType::P1p1tC1iN1a1a); + + #[rustfmt::skip] + const REQ: [u8; 71] = [ + 0x10, 0x00, 0x00, + 0x06, 0x00, 0x00, 0x00, + + // SQE DWORD 1 + 0x02, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // DOFST + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x10, 0x00, 0x00, + + // Reserved + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // SQE DWORD 10 + 0x12, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // MIC + 0x32, 0xab, 0xb3, 0x48 + ]; + + #[rustfmt::skip] + const RESP_DATA: [u8; 19] = [ + 0x90, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x01, 0x00 + ]; + + const RESP_LEN: usize = 4119; + let mut resp: [u8; RESP_LEN] = [0; RESP_LEN]; + resp[..RESP_DATA.len()].copy_from_slice(&RESP_DATA); + resp[(RESP_LEN - 4)..].copy_from_slice(&[0x87, 0xd7, 0x1f, 0xaa]); + + let resp = ExpectedRespChannel::new(&resp); + smol::block_on(async { + mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) + .await + }); + } + + #[test] + fn namespace_attached_controller_list_inactive_nsid() { + setup(); + + let (mut mep, mut subsys) = new_device(DeviceType::P1p1tC1iN1a0a); + + #[rustfmt::skip] + const REQ: [u8; 71] = [ + 0x10, 0x00, 0x00, + 0x06, 0x00, 0x00, 0x00, + + // SQE DWORD 1 + 0x01, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // DOFST + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x10, 0x00, 0x00, + + // Reserved + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // SQE DWORD 10 + 0x12, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // MIC + 0xdf, 0x47, 0x7f, 0xd6 + ]; + + #[rustfmt::skip] + const RESP_DATA: [u8; 19] = [ + 0x90, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x01, 0x00 + ]; + + const RESP_LEN: usize = 4119; + let mut resp: [u8; RESP_LEN] = [0; RESP_LEN]; + resp[..RESP_DATA.len()].copy_from_slice(&RESP_DATA); + resp[(RESP_LEN - 4)..].copy_from_slice(&[0x87, 0xd7, 0x1f, 0xaa]); + + let resp = ExpectedRespChannel::new(&resp); + smol::block_on(async { + mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) + .await + }); + } + + #[test] + fn namespace_attached_controller_list_active_nsid_unconstrained() { + setup(); + + let (mut mep, mut subsys) = new_device(DeviceType::P1p1tC1iN1a1a); + + #[rustfmt::skip] + const REQ: [u8; 71] = [ + 0x10, 0x00, 0x00, + 0x06, 0x00, 0x00, 0x00, + + // SQE DWORD 1 + 0x01, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // DOFST + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x10, 0x00, 0x00, + + // Reserved + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // SQE DWORD 10 + 0x12, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // MIC + 0xdf, 0x47, 0x7f, 0xd6 + ]; + + #[rustfmt::skip] + let resp_fields: Vec = vec![ + (0, &[0x90]), + (19, &[1, 0]), + (19 + 2, &[0, 0]), + ]; + + let resp = RelaxedRespChannel::new(resp_fields); + smol::block_on(async { + mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) + .await + }); + } + + #[test] + fn namespace_attached_controller_list_active_nsid_constrained() { + setup(); + + let (mut mep, mut subsys) = new_device(DeviceType::P1p1tC1iN1a1a); + + #[rustfmt::skip] + const REQ: [u8; 71] = [ + 0x10, 0x00, 0x00, + 0x06, 0x00, 0x00, 0x00, + + // SQE DWORD 1 + 0x01, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // DOFST + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x10, 0x00, 0x00, + + // Reserved + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // SQE DWORD 10 + 0x12, 0x00, 0x01, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // MIC + 0x6b, 0xa9, 0x6a, 0x8a + ]; + + #[rustfmt::skip] + let resp_fields: Vec = vec![ + (0, &[0x90]), + (19, &[0, 0]), + ]; + + let resp = RelaxedRespChannel::new(resp_fields); + smol::block_on(async { + mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) + .await + }); + } + #[test] fn secondary_controller_list() { setup(); @@ -1909,24 +2245,12 @@ mod get_log_page { }; use crate::{ - RESP_INVALID_COMMAND_SIZE, RESP_INVALID_PARAMETER, + RESP_ADMIN_STATUS_INVALID_FIELD, RESP_INVALID_COMMAND_SIZE, RESP_INVALID_PARAMETER, common::{ DeviceType, ExpectedField, ExpectedRespChannel, RelaxedRespChannel, new_device, setup, }, }; - // Base v2.1, 5.1.12 - // Admin command response with SCT / SC set - #[rustfmt::skip] - const RESP_ADMIN_STATUS_INVALID_FIELD: [u8; 23] = [ - 0x90, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x05, 0x80, - 0x94, 0x8f, 0xde, 0x57, - ]; - #[test] fn get_supported_log_pages_short() { setup(); From 2dd5c48e14255596d4c1b5f5a248fe9d22bd95b6 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Fri, 19 Sep 2025 15:58:45 +0930 Subject: [PATCH 10/12] nvme: mi: dev: Use Admin status responses for Admin / Identify Signed-off-by: Andrew Jeffery --- src/nvme/mi/dev.rs | 159 ++++++++++++++++++++++++++++++--------------- tests/admin.rs | 14 ++-- 2 files changed, 115 insertions(+), 58 deletions(-) diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index 4abdcc7..deaca59 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -1242,34 +1242,47 @@ impl RequestHandler for AdminIdentifyRequest { match &self.req { AdminIdentifyCnsRequestType::NvmIdentifyNamespace => { - let ainvminr = match NamespaceId(self.nsid).disposition(subsys) { + match match NamespaceId(self.nsid).disposition(subsys) { NamespaceIdDisposition::Invalid => { debug!("Invalid NSID: {}", self.nsid); - Err(ResponseStatus::InvalidParameter) + Err(AdminIoCqeGenericCommandStatus::InvalidNamespaceOrFormat) } NamespaceIdDisposition::Broadcast => { - Ok(AdminIdentifyNvmIdentifyNamespaceResponse { + AdminIdentifyNvmIdentifyNamespaceResponse { lbaf0_lbads: 9, // TODO: Tie to controller model ..Default::default() - }) + } + .encode() + .map_err(AdminIoCqeGenericCommandStatus::from) } NamespaceIdDisposition::Unallocated => { debug!("Unallocated NSID: {}", self.nsid); - Err(ResponseStatus::InvalidParameter) + Err(AdminIoCqeGenericCommandStatus::InvalidNamespaceOrFormat) } NamespaceIdDisposition::Inactive(_) => { - Ok(AdminIdentifyNvmIdentifyNamespaceResponse::default()) + AdminIdentifyNvmIdentifyNamespaceResponse::default() + .encode() + .map_err(AdminIoCqeGenericCommandStatus::from) } // 4.1.5.1 NVM Command Set Spec, v1.0c - NamespaceIdDisposition::Active(ns) => Ok(ns.into()), - }? - .encode()?; - - admin_send_response_body( - resp, - admin_constrain_body(self.dofst, self.dlen, &ainvminr.0)?, - ) - .await + NamespaceIdDisposition::Active(ns) => { + Into::::into(ns) + .encode() + .map_err(AdminIoCqeGenericCommandStatus::from) + } + } { + Ok(response) => { + admin_send_response_body( + resp, + admin_constrain_body(self.dofst, self.dlen, &response.0)?, + ) + .await + } + Err(err) => { + admin_send_status(resp, AdminIoCqeStatusType::GenericCommandStatus(err)) + .await + } + } } AdminIdentifyCnsRequestType::IdentifyController => { let Some(ctlr) = subsys.ctlrs.get(ctx.ctlid as usize) else { @@ -1343,13 +1356,22 @@ impl RequestHandler for AdminIdentifyRequest { apsta: 0, sanicap: subsys.sanicap.into(), } - .encode()?; + .encode() + .map_err(AdminIoCqeGenericCommandStatus::from); - admin_send_response_body( - resp, - admin_constrain_body(self.dofst, self.dlen, &aicr.0)?, - ) - .await + match aicr { + Ok(response) => { + admin_send_response_body( + resp, + admin_constrain_body(self.dofst, self.dlen, &response.0)?, + ) + .await + } + Err(err) => { + admin_send_status(resp, AdminIoCqeStatusType::GenericCommandStatus(err)) + .await + } + } } AdminIdentifyCnsRequestType::ActiveNamespaceIDList => { // 5.1.13.2.2, Base v2.1 @@ -1374,17 +1396,27 @@ impl RequestHandler for AdminIdentifyRequest { return Err(ResponseStatus::InternalError); }; } - let aianidlr = aianidlr.encode()?; + let aianidlr = aianidlr + .encode() + .map_err(AdminIoCqeGenericCommandStatus::from); - admin_send_response_body( - resp, - admin_constrain_body(self.dofst, self.dlen, &aianidlr.0)?, - ) - .await + match aianidlr { + Ok(response) => { + admin_send_response_body( + resp, + admin_constrain_body(self.dofst, self.dlen, &response.0)?, + ) + .await + } + Err(err) => { + admin_send_status(resp, AdminIoCqeStatusType::GenericCommandStatus(err)) + .await + } + } } AdminIdentifyCnsRequestType::NamespaceIdentificationDescriptorList => { // 5.1.13.2.3, Base v2.1 - let ainsidlr = match NamespaceId(self.nsid).disposition(subsys) { + match match NamespaceId(self.nsid).disposition(subsys) { NamespaceIdDisposition::Invalid => { if self.nsid == u32::MAX - 1 { debug!( @@ -1393,18 +1425,18 @@ impl RequestHandler for AdminIdentifyRequest { } else { debug!("Invalid NSID: {}", self.nsid); } - Err(ResponseStatus::InvalidParameter) + Err(AdminIoCqeGenericCommandStatus::InvalidNamespaceOrFormat) } NamespaceIdDisposition::Broadcast => { debug!("Invalid NSID: {}", self.nsid); - Err(ResponseStatus::InvalidParameter) + Err(AdminIoCqeGenericCommandStatus::InvalidNamespaceOrFormat) } NamespaceIdDisposition::Unallocated => { debug!("Unallocated NSID: {}", self.nsid); - Err(ResponseStatus::InvalidParameter) + Err(AdminIoCqeGenericCommandStatus::InvalidNamespaceOrFormat) } NamespaceIdDisposition::Inactive(ns) | NamespaceIdDisposition::Active(ns) => { - Ok(AdminIdentifyNamespaceIdentificationDescriptorListResponse { + AdminIdentifyNamespaceIdentificationDescriptorListResponse { nids: { let mut vec = WireVec::new(); for nid in &ns.nids { @@ -1418,16 +1450,23 @@ impl RequestHandler for AdminIdentifyRequest { } vec }, - }) + } + .encode() + .map_err(AdminIoCqeGenericCommandStatus::from) } - }? - .encode()?; - - admin_send_response_body( - resp, - admin_constrain_body(self.dofst, self.dlen, &ainsidlr.0)?, - ) - .await + } { + Ok(response) => { + admin_send_response_body( + resp, + admin_constrain_body(self.dofst, self.dlen, &response.0)?, + ) + .await + } + Err(err) => { + admin_send_status(resp, AdminIoCqeStatusType::GenericCommandStatus(err)) + .await + } + } } AdminIdentifyCnsRequestType::AllocatedNamespaceIdList => { // 5.1.13.2.9, Base v2.1 @@ -1456,13 +1495,22 @@ impl RequestHandler for AdminIdentifyRequest { vec }, } - .encode()?; + .encode() + .map_err(AdminIoCqeGenericCommandStatus::from); - admin_send_response_body( - resp, - admin_constrain_body(self.dofst, self.dlen, &aiansidl.0)?, - ) - .await + match aiansidl { + Ok(response) => { + admin_send_response_body( + resp, + admin_constrain_body(self.dofst, self.dlen, &response.0)?, + ) + .await + } + Err(err) => { + admin_send_status(resp, AdminIoCqeStatusType::GenericCommandStatus(err)) + .await + } + } } AdminIdentifyCnsRequestType::IdentifyNamespaceForAllocatedNamespaceId => { // Base v2.1, 5.1.13.2.10 @@ -1553,10 +1601,19 @@ impl RequestHandler for AdminIdentifyRequest { }; } cl.update()?; - let cl = cl.encode()?; - - admin_send_response_body(resp, admin_constrain_body(self.dofst, self.dlen, &cl.0)?) - .await + match cl.encode().map_err(AdminIoCqeGenericCommandStatus::from) { + Ok(response) => { + admin_send_response_body( + resp, + admin_constrain_body(self.dofst, self.dlen, &response.0)?, + ) + .await + } + Err(status) => { + admin_send_status(resp, AdminIoCqeStatusType::GenericCommandStatus(status)) + .await + } + } } AdminIdentifyCnsRequestType::SecondaryControllerList => { let Some(ctlr) = subsys.ctlrs.get(ctx.ctlid as usize) else { diff --git a/tests/admin.rs b/tests/admin.rs index dc5c0ae..3612805 100644 --- a/tests/admin.rs +++ b/tests/admin.rs @@ -633,7 +633,7 @@ mod identify { 0x12, 0x14, 0x1c, 0x57 ]; - let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER); + let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_NAMESPACE); smol::block_on(async { mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) .await @@ -724,7 +724,7 @@ mod identify { 0x49, 0xb0, 0xa7, 0x22 ]; - let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER); + let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_NAMESPACE); smol::block_on(async { mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) .await @@ -1078,7 +1078,7 @@ mod identify { 0xe4, 0x7b, 0x6f, 0x4c ]; - let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER); + let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_NAMESPACE); smol::block_on(async { mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) .await @@ -1123,7 +1123,7 @@ mod identify { 0xbf, 0xdf, 0xd4, 0x39 ]; - let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER); + let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_NAMESPACE); smol::block_on(async { mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) .await @@ -1168,7 +1168,7 @@ mod identify { 0x71, 0x25, 0x20, 0x9c ]; - let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER); + let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_NAMESPACE); smol::block_on(async { mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) .await @@ -1213,7 +1213,7 @@ mod identify { 0x9c, 0xc9, 0xec, 0x02 ]; - let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER); + let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_NAMESPACE); smol::block_on(async { mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) .await @@ -1258,7 +1258,7 @@ mod identify { 0xc7, 0x6d, 0x57, 0x77 ]; - let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER); + let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_NAMESPACE); smol::block_on(async { mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) .await From c636d9b2f8be924df72b7f51f6a3cd6b402942ef Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Fri, 19 Sep 2025 16:53:16 +0930 Subject: [PATCH 11/12] nvme: mi: dev: Consolidate response handling for Admin / Identify Signed-off-by: Andrew Jeffery --- src/nvme/mi/dev.rs | 269 +++++++++++++++------------------------------ 1 file changed, 88 insertions(+), 181 deletions(-) diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index deaca59..4d4cf2a 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -1240,9 +1240,9 @@ impl RequestHandler for AdminIdentifyRequest { return Err(ResponseStatus::InvalidCommandSize); } - match &self.req { + let res = match &self.req { AdminIdentifyCnsRequestType::NvmIdentifyNamespace => { - match match NamespaceId(self.nsid).disposition(subsys) { + match NamespaceId(self.nsid).disposition(subsys) { NamespaceIdDisposition::Invalid => { debug!("Invalid NSID: {}", self.nsid); Err(AdminIoCqeGenericCommandStatus::InvalidNamespaceOrFormat) @@ -1270,107 +1270,81 @@ impl RequestHandler for AdminIdentifyRequest { .encode() .map_err(AdminIoCqeGenericCommandStatus::from) } - } { - Ok(response) => { - admin_send_response_body( - resp, - admin_constrain_body(self.dofst, self.dlen, &response.0)?, - ) - .await - } - Err(err) => { - admin_send_status(resp, AdminIoCqeStatusType::GenericCommandStatus(err)) - .await - } } } AdminIdentifyCnsRequestType::IdentifyController => { - let Some(ctlr) = subsys.ctlrs.get(ctx.ctlid as usize) else { - debug!("No such CTLID: {}", ctx.ctlid); - return Err(ResponseStatus::InvalidParameter); - }; - - let aicr = AdminIdentifyControllerResponse { - vid: subsys.info.pci_vid, - ssvid: subsys.info.pci_svid, - sn: WireString::from(subsys.sn)?, - mn: WireString::from(subsys.mn)?, - fr: WireString::from(subsys.fr)?, - rab: 0, - ieee: { - // 4.5.3, Base v2.1 - let mut fixup = subsys.info.ieee_oui; - fixup.reverse(); - fixup - }, - cmic: ((subsys.ctlrs.len() > 1) as u8) << 1 // MCTRS + if let Some(ctlr) = subsys.ctlrs.get(ctx.ctlid as usize) { + AdminIdentifyControllerResponse { + vid: subsys.info.pci_vid, + ssvid: subsys.info.pci_svid, + sn: WireString::from(subsys.sn)?, + mn: WireString::from(subsys.mn)?, + fr: WireString::from(subsys.fr)?, + rab: 0, + ieee: { + // 4.5.3, Base v2.1 + let mut fixup = subsys.info.ieee_oui; + fixup.reverse(); + fixup + }, + cmic: ((subsys.ctlrs.len() > 1) as u8) << 1 // MCTRS | ((subsys.ports.len() > 1) as u8), // MPORTS - mdts: 0, - cntlid: ctlr.id.0, - ver: 0, - rtd3r: 0, - rtd3e: 0, - oaes: 0, - // TODO: Tie to data model - ctratt: ((false as u32) << 14) // DNVMS + mdts: 0, + cntlid: ctlr.id.0, + ver: 0, + rtd3r: 0, + 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 - cntrltype: ctlr.cntrltype.into(), - // TODO: Tie to data model - nvmsr: ((false as u8) << 1) // NVMEE + cntrltype: ctlr.cntrltype.into(), + // TODO: Tie to data model + nvmsr: ((false as u8) << 1) // NVMEE | (true as u8), // NVMESD - vwci: 0, - mec: ((subsys.ports.iter().any(|p| matches!(p.typ, crate::PortType::Pcie(_)))) as u8) << 1 // PCIEME + 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 - ocas: 0, - acl: 0, - aerl: 0, - frmw: 0, - lpa: ctlr.lpa.into(), - elpe: 0, - npss: 0, - avscc: 0, - wctemp: 0x157, - cctemp: 0x157, - fwug: 0, - kas: 0, - cqt: 0, - sqes: 0, - cqes: 0, - maxcmd: 0, - nn: NamespaceId::max(subsys), - oncs: 0, - fuses: 0, - fna: ctlr.fna.into(), - vwc: 0, - awun: 0, - awupf: 0, - icsvscc: 0, - nwpc: 0, - mnan: 0, - subnqn: WireString::new(), - fcatt: 0, - msdbd: 0, - ofcs: 0, - apsta: 0, - sanicap: subsys.sanicap.into(), - } - .encode() - .map_err(AdminIoCqeGenericCommandStatus::from); - - match aicr { - Ok(response) => { - admin_send_response_body( - resp, - admin_constrain_body(self.dofst, self.dlen, &response.0)?, - ) - .await - } - Err(err) => { - admin_send_status(resp, AdminIoCqeStatusType::GenericCommandStatus(err)) - .await + ocas: 0, + acl: 0, + aerl: 0, + frmw: 0, + lpa: ctlr.lpa.into(), + elpe: 0, + npss: 0, + avscc: 0, + wctemp: 0x157, + cctemp: 0x157, + fwug: 0, + kas: 0, + cqt: 0, + sqes: 0, + cqes: 0, + maxcmd: 0, + nn: NamespaceId::max(subsys), + oncs: 0, + fuses: 0, + fna: ctlr.fna.into(), + vwc: 0, + awun: 0, + awupf: 0, + icsvscc: 0, + nwpc: 0, + mnan: 0, + subnqn: WireString::new(), + fcatt: 0, + msdbd: 0, + ofcs: 0, + apsta: 0, + sanicap: subsys.sanicap.into(), } + .encode() + .map_err(AdminIoCqeGenericCommandStatus::from) + } else { + debug!("No such CTLID: {}", ctx.ctlid); + Err(AdminIoCqeGenericCommandStatus::InvalidFieldInCommand) } } AdminIdentifyCnsRequestType::ActiveNamespaceIDList => { @@ -1396,27 +1370,13 @@ impl RequestHandler for AdminIdentifyRequest { return Err(ResponseStatus::InternalError); }; } - let aianidlr = aianidlr + aianidlr .encode() - .map_err(AdminIoCqeGenericCommandStatus::from); - - match aianidlr { - Ok(response) => { - admin_send_response_body( - resp, - admin_constrain_body(self.dofst, self.dlen, &response.0)?, - ) - .await - } - Err(err) => { - admin_send_status(resp, AdminIoCqeStatusType::GenericCommandStatus(err)) - .await - } - } + .map_err(AdminIoCqeGenericCommandStatus::from) } AdminIdentifyCnsRequestType::NamespaceIdentificationDescriptorList => { // 5.1.13.2.3, Base v2.1 - match match NamespaceId(self.nsid).disposition(subsys) { + match NamespaceId(self.nsid).disposition(subsys) { NamespaceIdDisposition::Invalid => { if self.nsid == u32::MAX - 1 { debug!( @@ -1454,18 +1414,6 @@ impl RequestHandler for AdminIdentifyRequest { .encode() .map_err(AdminIoCqeGenericCommandStatus::from) } - } { - Ok(response) => { - admin_send_response_body( - resp, - admin_constrain_body(self.dofst, self.dlen, &response.0)?, - ) - .await - } - Err(err) => { - admin_send_status(resp, AdminIoCqeStatusType::GenericCommandStatus(err)) - .await - } } } AdminIdentifyCnsRequestType::AllocatedNamespaceIdList => { @@ -1476,7 +1424,7 @@ impl RequestHandler for AdminIdentifyRequest { } assert!(NamespaceId::max(subsys) < (4096 / core::mem::size_of::()) as u32); - let aiansidl = AdminIdentifyAllocatedNamespaceIdListResponse { + AdminIdentifyAllocatedNamespaceIdListResponse { nsid: { let mut allocated: heapless::Vec = subsys .nss @@ -1496,25 +1444,11 @@ impl RequestHandler for AdminIdentifyRequest { }, } .encode() - .map_err(AdminIoCqeGenericCommandStatus::from); - - match aiansidl { - Ok(response) => { - admin_send_response_body( - resp, - admin_constrain_body(self.dofst, self.dlen, &response.0)?, - ) - .await - } - Err(err) => { - admin_send_status(resp, AdminIoCqeStatusType::GenericCommandStatus(err)) - .await - } - } + .map_err(AdminIoCqeGenericCommandStatus::from) } AdminIdentifyCnsRequestType::IdentifyNamespaceForAllocatedNamespaceId => { // Base v2.1, 5.1.13.2.10 - match match NamespaceId(self.nsid).disposition(subsys) { + match NamespaceId(self.nsid).disposition(subsys) { NamespaceIdDisposition::Invalid | NamespaceIdDisposition::Broadcast => { Err(AdminIoCqeGenericCommandStatus::InvalidNamespaceOrFormat) } @@ -1529,22 +1463,10 @@ impl RequestHandler for AdminIdentifyRequest { .encode() .map_err(AdminIoCqeGenericCommandStatus::from) } - } { - Ok(response) => { - admin_send_response_body( - resp, - admin_constrain_body(self.dofst, self.dlen, &response.0)?, - ) - .await - } - Err(err) => { - admin_send_status(resp, AdminIoCqeStatusType::GenericCommandStatus(err)) - .await - } } } AdminIdentifyCnsRequestType::NamespaceAttachedControllerList => { - match match NamespaceId(self.nsid).disposition(subsys) { + match NamespaceId(self.nsid).disposition(subsys) { NamespaceIdDisposition::Invalid => ControllerListResponse::new() .encode() .map_err(AdminIoCqeGenericCommandStatus::from), @@ -1573,18 +1495,6 @@ impl RequestHandler for AdminIdentifyRequest { clr.update()?; clr.encode().map_err(AdminIoCqeGenericCommandStatus::from) } - } { - Ok(response) => { - admin_send_response_body( - resp, - admin_constrain_body(self.dofst, self.dlen, &response.0)?, - ) - .await - } - Err(status) => { - admin_send_status(resp, AdminIoCqeStatusType::GenericCommandStatus(status)) - .await - } } } AdminIdentifyCnsRequestType::NvmSubsystemControllerList => { @@ -1601,19 +1511,7 @@ impl RequestHandler for AdminIdentifyRequest { }; } cl.update()?; - match cl.encode().map_err(AdminIoCqeGenericCommandStatus::from) { - Ok(response) => { - admin_send_response_body( - resp, - admin_constrain_body(self.dofst, self.dlen, &response.0)?, - ) - .await - } - Err(status) => { - admin_send_status(resp, AdminIoCqeStatusType::GenericCommandStatus(status)) - .await - } - } + cl.encode().map_err(AdminIoCqeGenericCommandStatus::from) } AdminIdentifyCnsRequestType::SecondaryControllerList => { let Some(ctlr) = subsys.ctlrs.get(ctx.ctlid as usize) else { @@ -1625,15 +1523,24 @@ impl RequestHandler for AdminIdentifyRequest { todo!("Support listing secondary controllers"); } + Ok(([0u8; 4096], 4096usize)) + } + _ => { + debug!("Unimplemented CNS: {self:?}"); + return Err(ResponseStatus::InternalError); + } + }; + + match res { + Ok(response) => { admin_send_response_body( resp, - admin_constrain_body(self.dofst, self.dlen, &[0u8; 4096])?, + admin_constrain_body(self.dofst, self.dlen, &response.0)?, ) .await } - _ => { - debug!("Unimplemented CNS: {self:?}"); - Err(ResponseStatus::InternalError) + Err(err) => { + admin_send_status(resp, AdminIoCqeStatusType::GenericCommandStatus(err)).await } } } From eb5765bdab64329fdc13184db76c3369961b03f8 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Mon, 22 Sep 2025 10:18:55 +0930 Subject: [PATCH 12/12] nvme: mi: dev: Admin commands respond with Admin errors where appropriate Signed-off-by: Andrew Jeffery --- src/nvme/mi/dev.rs | 136 +++++++++++++++++++++++++++++++++++++++------ tests/admin.rs | 26 ++++----- 2 files changed, 132 insertions(+), 30 deletions(-) diff --git a/src/nvme/mi/dev.rs b/src/nvme/mi/dev.rs index 4d4cf2a..02726b1 100644 --- a/src/nvme/mi/dev.rs +++ b/src/nvme/mi/dev.rs @@ -993,7 +993,13 @@ impl RequestHandler for AdminGetLogPageRequest { | AdminGetLogPageLidRequestType::FeatureIdentifiersSupportedAndEffects => { if self.csi != 0 { debug!("Support CSI"); - return Err(ResponseStatus::InternalError); + return admin_send_status( + resp, + AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::InternalError, + ), + ) + .await; } } AdminGetLogPageLidRequestType::ErrorInformation @@ -1003,7 +1009,13 @@ impl RequestHandler for AdminGetLogPageRequest { let Some(ctlr) = subsys.ctlrs.get(ctx.ctlid as usize) else { debug!("Unrecognised CTLID: {}", ctx.ctlid); - return Err(ResponseStatus::InvalidParameter); + return admin_send_status( + resp, + AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::InvalidFieldInCommand, + ), + ) + .await; }; let Some(flags) = ctlr.lsaes.get(self.req.id() as usize) else { @@ -1011,7 +1023,13 @@ impl RequestHandler for AdminGetLogPageRequest { "LSAE mismatch with known LID {:?} on controller {}", self.req, ctlr.id.0 ); - return Err(ResponseStatus::InternalError); + return admin_send_status( + resp, + AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::InvalidFieldInCommand, + ), + ) + .await; }; // Base v2.1, 5.1.12 @@ -1043,7 +1061,13 @@ impl RequestHandler for AdminGetLogPageRequest { AdminGetLogPageLidRequestType::SupportedLogPages => { if (self.numdw + 1) * 4 != 1024 { debug!("Implement support for NUMDL / NUMDU"); - return Err(ResponseStatus::InternalError); + return admin_send_status( + resp, + AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::InternalError, + ), + ) + .await; } let mut lsids = WireVec::new(); @@ -1069,7 +1093,13 @@ impl RequestHandler for AdminGetLogPageRequest { AdminGetLogPageLidRequestType::ErrorInformation => { if (self.numdw + 1) * 4 != 64 { debug!("Implement support for NUMDL / NUMDU"); - return Err(ResponseStatus::InternalError); + return admin_send_status( + resp, + AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::InternalError, + ), + ) + .await; } admin_send_response_body( resp, @@ -1080,7 +1110,13 @@ impl RequestHandler for AdminGetLogPageRequest { AdminGetLogPageLidRequestType::SmartHealthInformation => { if (self.numdw + 1) * 4 != 512 { debug!("Implement support for NUMDL / NUMDU"); - return Err(ResponseStatus::InternalError); + return admin_send_status( + resp, + AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::InternalError, + ), + ) + .await; } // Base v2.1, 5.1.2, Figure 199 @@ -1168,7 +1204,13 @@ impl RequestHandler for AdminGetLogPageRequest { AdminGetLogPageLidRequestType::FeatureIdentifiersSupportedAndEffects => { if (self.numdw + 1) * 4 != 1024 { debug!("Implement support for NUMDL / NUMDU"); - return Err(ResponseStatus::InternalError); + return admin_send_status( + resp, + AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::InternalError, + ), + ) + .await; } admin_send_response_body( @@ -1185,7 +1227,13 @@ impl RequestHandler for AdminGetLogPageRequest { AdminGetLogPageLidRequestType::SanitizeStatus => { if (self.numdw + 1) * 4 != 512 { debug!("Implement support for NUMDL / NUMDU"); - return Err(ResponseStatus::InternalError); + return admin_send_status( + resp, + AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::InternalError, + ), + ) + .await; } let sslpr = SanitizeStatusLogPageResponse { @@ -1577,13 +1625,25 @@ impl RequestHandler for AdminNamespaceManagementRequest { crate::nvme::mi::AdminNamespaceManagementSelect::Create(req) => { if self.csi != 0 { debug!("Support CSI {}", self.csi); - return Err(ResponseStatus::InternalError); + return admin_send_status( + resp, + AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::InternalError, + ), + ) + .await; } let Ok(nsid) = subsys.add_namespace(req.ncap) else { debug!("Failed to create namespace"); // TODO: Implement Base v2.1, 5.1.21.1, Figure 370 - return Err(ResponseStatus::InternalError); + return admin_send_status( + resp, + AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::InternalError, + ), + ) + .await; }; let mh = MessageHeader::respond(MessageType::NvmeAdminCommand).encode()?; @@ -1700,7 +1760,13 @@ impl RequestHandler for AdminNamespaceAttachmentRequest { if self.nsid == u32::MAX { debug!("Refusing to perform {:?} for broadcast NSID", self.sel); - return Err(ResponseStatus::InvalidParameter); + return admin_send_status( + resp, + AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::InvalidFieldInCommand, + ), + ) + .await; } // TODO: Handle MAXCNA @@ -1803,12 +1869,24 @@ impl RequestHandler for AdminSanitizeRequest { let Ok(config) = TryInto::::try_into(self.config) else { debug!("Invalid sanitize configuration: {}", self.config); - return Err(ResponseStatus::InvalidParameter); + return admin_send_status( + resp, + AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::InvalidFieldInCommand, + ), + ) + .await; }; if subsys.sanicap.ndi && config.ndas { debug!("Request for No-Deallocate After Sanitize when No-Deallocate is inhibited"); - return Err(ResponseStatus::InvalidParameter); + return admin_send_status( + resp, + AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::InvalidFieldInCommand, + ), + ) + .await; } // TODO: Implement action latency, progress state machine, error states @@ -1877,22 +1955,46 @@ impl RequestHandler for AdminFormatNvmRequest { let Some(ctlr) = subsys.ctlrs.iter().find(|c| c.id.0 == ctx.ctlid) else { debug!("Unrecognised CTLID: {}", ctx.ctlid); - return Err(ResponseStatus::InvalidParameter); + return admin_send_status( + resp, + AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::InvalidFieldInCommand, + ), + ) + .await; }; let Ok(config) = TryInto::::try_into(self.config) else { debug!("Invalid configuration for Admin Format NVM"); - return Err(ResponseStatus::InvalidParameter); + return admin_send_status( + resp, + AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::InvalidFieldInCommand, + ), + ) + .await; }; if config.lbafi != 0 { debug!("Unsupported LBA format index: {}", config.lbafi); - return Err(ResponseStatus::InvalidParameter); + return admin_send_status( + resp, + AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::InvalidFieldInCommand, + ), + ) + .await; } if !ctlr.active_ns.iter().any(|ns| ns.0 == self.nsid) && self.nsid != u32::MAX { debug!("Unrecognised NSID: {}", self.nsid); - return Err(ResponseStatus::InvalidParameter); + return admin_send_status( + resp, + AdminIoCqeStatusType::GenericCommandStatus( + AdminIoCqeGenericCommandStatus::InvalidFieldInCommand, + ), + ) + .await; } // TODO: handle config.ses diff --git a/tests/admin.rs b/tests/admin.rs index 3612805..e25f76c 100644 --- a/tests/admin.rs +++ b/tests/admin.rs @@ -2245,7 +2245,7 @@ mod get_log_page { }; use crate::{ - RESP_ADMIN_STATUS_INVALID_FIELD, RESP_INVALID_COMMAND_SIZE, RESP_INVALID_PARAMETER, + RESP_ADMIN_STATUS_INVALID_FIELD, RESP_INVALID_COMMAND_SIZE, common::{ DeviceType, ExpectedField, ExpectedRespChannel, RelaxedRespChannel, new_device, setup, }, @@ -2380,7 +2380,7 @@ mod get_log_page { 0x29, 0xe2, 0x53, 0x0a ]; - let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER); + let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_FIELD); smol::block_on(async { mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) .await @@ -2568,7 +2568,7 @@ mod get_log_page { 0x80, 0x60, 0xc4, 0x3b ]; - let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER); + let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_FIELD); smol::block_on(async { mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) .await @@ -3713,7 +3713,7 @@ mod namespace_attachment { use mctp::MsgIC; use crate::{ - RESP_INVALID_COMMAND_SIZE, RESP_INVALID_PARAMETER, + RESP_ADMIN_STATUS_INVALID_FIELD, RESP_INVALID_COMMAND_SIZE, common::{DeviceType, ExpectedRespChannel, new_device, setup}, }; @@ -3910,7 +3910,7 @@ mod namespace_attachment { req[..REQ_DATA.len()].copy_from_slice(&REQ_DATA); req[{ len - REQ_MIC.len() }..].copy_from_slice(&REQ_MIC); - let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER); + let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_FIELD); smol::block_on(async { mep.handle_async(&mut subsys, &req, MsgIC(true), resp, async |_| Ok(())) .await @@ -4293,7 +4293,7 @@ mod namespace_attachment { req[..REQ_DATA.len()].copy_from_slice(&REQ_DATA); req[{ len - REQ_MIC.len() }..].copy_from_slice(&REQ_MIC); - let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER); + let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_FIELD); smol::block_on(async { mep.handle_async(&mut subsys, &req, MsgIC(true), resp, async |_| Ok(())) .await @@ -4488,7 +4488,7 @@ mod sanitize { use mctp::MsgIC; use crate::{ - RESP_ADMIN_SUCCESS, RESP_INVALID_COMMAND_SIZE, RESP_INVALID_PARAMETER, + RESP_ADMIN_STATUS_INVALID_FIELD, RESP_ADMIN_SUCCESS, RESP_INVALID_COMMAND_SIZE, common::{DeviceType, ExpectedRespChannel, new_device, setup}, }; @@ -4756,7 +4756,7 @@ mod sanitize { 0xc8, 0x47, 0x8d, 0x88 ]; - let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER); + let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_FIELD); smol::block_on(async { mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) .await @@ -4813,7 +4813,7 @@ mod format_nvm { use mctp::MsgIC; use crate::{ - RESP_ADMIN_SUCCESS, RESP_INVALID_COMMAND_SIZE, RESP_INVALID_PARAMETER, + RESP_ADMIN_STATUS_INVALID_FIELD, RESP_ADMIN_SUCCESS, RESP_INVALID_COMMAND_SIZE, common::{DeviceType, ExpectedRespChannel, new_device, setup}, }; @@ -4945,7 +4945,7 @@ mod format_nvm { 0x0f, 0x14, 0xe6, 0x14 ]; - let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER); + let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_FIELD); smol::block_on(async { mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) .await @@ -4990,7 +4990,7 @@ mod format_nvm { 0x7a, 0x05, 0xbd, 0xb8 ]; - let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER); + let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_FIELD); smol::block_on(async { mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) .await @@ -5035,7 +5035,7 @@ mod format_nvm { 0x27, 0xdd, 0x59, 0xa2 ]; - let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER); + let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_FIELD); smol::block_on(async { mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) .await @@ -5080,7 +5080,7 @@ mod format_nvm { 0xd1, 0xad, 0x95, 0x56 ]; - let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER); + let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_FIELD); smol::block_on(async { mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(())) .await