Skip to content

Commit eb5765b

Browse files
amboaramboar
committed
nvme: mi: dev: Admin commands respond with Admin errors where appropriate
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
1 parent c636d9b commit eb5765b

File tree

2 files changed

+132
-30
lines changed

2 files changed

+132
-30
lines changed

‎src/nvme/mi/dev.rs‎

Lines changed: 119 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -993,7 +993,13 @@ impl RequestHandler for AdminGetLogPageRequest {
993993
| AdminGetLogPageLidRequestType::FeatureIdentifiersSupportedAndEffects => {
994994
if self.csi != 0 {
995995
debug!("Support CSI");
996-
return Err(ResponseStatus::InternalError);
996+
return admin_send_status(
997+
resp,
998+
AdminIoCqeStatusType::GenericCommandStatus(
999+
AdminIoCqeGenericCommandStatus::InternalError,
1000+
),
1001+
)
1002+
.await;
9971003
}
9981004
}
9991005
AdminGetLogPageLidRequestType::ErrorInformation
@@ -1003,15 +1009,27 @@ impl RequestHandler for AdminGetLogPageRequest {
10031009

10041010
let Some(ctlr) = subsys.ctlrs.get(ctx.ctlid as usize) else {
10051011
debug!("Unrecognised CTLID: {}", ctx.ctlid);
1006-
return Err(ResponseStatus::InvalidParameter);
1012+
return admin_send_status(
1013+
resp,
1014+
AdminIoCqeStatusType::GenericCommandStatus(
1015+
AdminIoCqeGenericCommandStatus::InvalidFieldInCommand,
1016+
),
1017+
)
1018+
.await;
10071019
};
10081020

10091021
let Some(flags) = ctlr.lsaes.get(self.req.id() as usize) else {
10101022
debug!(
10111023
"LSAE mismatch with known LID {:?} on controller {}",
10121024
self.req, ctlr.id.0
10131025
);
1014-
return Err(ResponseStatus::InternalError);
1026+
return admin_send_status(
1027+
resp,
1028+
AdminIoCqeStatusType::GenericCommandStatus(
1029+
AdminIoCqeGenericCommandStatus::InvalidFieldInCommand,
1030+
),
1031+
)
1032+
.await;
10151033
};
10161034

10171035
// Base v2.1, 5.1.12
@@ -1043,7 +1061,13 @@ impl RequestHandler for AdminGetLogPageRequest {
10431061
AdminGetLogPageLidRequestType::SupportedLogPages => {
10441062
if (self.numdw + 1) * 4 != 1024 {
10451063
debug!("Implement support for NUMDL / NUMDU");
1046-
return Err(ResponseStatus::InternalError);
1064+
return admin_send_status(
1065+
resp,
1066+
AdminIoCqeStatusType::GenericCommandStatus(
1067+
AdminIoCqeGenericCommandStatus::InternalError,
1068+
),
1069+
)
1070+
.await;
10471071
}
10481072

10491073
let mut lsids = WireVec::new();
@@ -1069,7 +1093,13 @@ impl RequestHandler for AdminGetLogPageRequest {
10691093
AdminGetLogPageLidRequestType::ErrorInformation => {
10701094
if (self.numdw + 1) * 4 != 64 {
10711095
debug!("Implement support for NUMDL / NUMDU");
1072-
return Err(ResponseStatus::InternalError);
1096+
return admin_send_status(
1097+
resp,
1098+
AdminIoCqeStatusType::GenericCommandStatus(
1099+
AdminIoCqeGenericCommandStatus::InternalError,
1100+
),
1101+
)
1102+
.await;
10731103
}
10741104
admin_send_response_body(
10751105
resp,
@@ -1080,7 +1110,13 @@ impl RequestHandler for AdminGetLogPageRequest {
10801110
AdminGetLogPageLidRequestType::SmartHealthInformation => {
10811111
if (self.numdw + 1) * 4 != 512 {
10821112
debug!("Implement support for NUMDL / NUMDU");
1083-
return Err(ResponseStatus::InternalError);
1113+
return admin_send_status(
1114+
resp,
1115+
AdminIoCqeStatusType::GenericCommandStatus(
1116+
AdminIoCqeGenericCommandStatus::InternalError,
1117+
),
1118+
)
1119+
.await;
10841120
}
10851121

10861122
// Base v2.1, 5.1.2, Figure 199
@@ -1168,7 +1204,13 @@ impl RequestHandler for AdminGetLogPageRequest {
11681204
AdminGetLogPageLidRequestType::FeatureIdentifiersSupportedAndEffects => {
11691205
if (self.numdw + 1) * 4 != 1024 {
11701206
debug!("Implement support for NUMDL / NUMDU");
1171-
return Err(ResponseStatus::InternalError);
1207+
return admin_send_status(
1208+
resp,
1209+
AdminIoCqeStatusType::GenericCommandStatus(
1210+
AdminIoCqeGenericCommandStatus::InternalError,
1211+
),
1212+
)
1213+
.await;
11721214
}
11731215

11741216
admin_send_response_body(
@@ -1185,7 +1227,13 @@ impl RequestHandler for AdminGetLogPageRequest {
11851227
AdminGetLogPageLidRequestType::SanitizeStatus => {
11861228
if (self.numdw + 1) * 4 != 512 {
11871229
debug!("Implement support for NUMDL / NUMDU");
1188-
return Err(ResponseStatus::InternalError);
1230+
return admin_send_status(
1231+
resp,
1232+
AdminIoCqeStatusType::GenericCommandStatus(
1233+
AdminIoCqeGenericCommandStatus::InternalError,
1234+
),
1235+
)
1236+
.await;
11891237
}
11901238

11911239
let sslpr = SanitizeStatusLogPageResponse {
@@ -1577,13 +1625,25 @@ impl RequestHandler for AdminNamespaceManagementRequest {
15771625
crate::nvme::mi::AdminNamespaceManagementSelect::Create(req) => {
15781626
if self.csi != 0 {
15791627
debug!("Support CSI {}", self.csi);
1580-
return Err(ResponseStatus::InternalError);
1628+
return admin_send_status(
1629+
resp,
1630+
AdminIoCqeStatusType::GenericCommandStatus(
1631+
AdminIoCqeGenericCommandStatus::InternalError,
1632+
),
1633+
)
1634+
.await;
15811635
}
15821636

15831637
let Ok(nsid) = subsys.add_namespace(req.ncap) else {
15841638
debug!("Failed to create namespace");
15851639
// TODO: Implement Base v2.1, 5.1.21.1, Figure 370
1586-
return Err(ResponseStatus::InternalError);
1640+
return admin_send_status(
1641+
resp,
1642+
AdminIoCqeStatusType::GenericCommandStatus(
1643+
AdminIoCqeGenericCommandStatus::InternalError,
1644+
),
1645+
)
1646+
.await;
15871647
};
15881648
let mh = MessageHeader::respond(MessageType::NvmeAdminCommand).encode()?;
15891649

@@ -1700,7 +1760,13 @@ impl RequestHandler for AdminNamespaceAttachmentRequest {
17001760

17011761
if self.nsid == u32::MAX {
17021762
debug!("Refusing to perform {:?} for broadcast NSID", self.sel);
1703-
return Err(ResponseStatus::InvalidParameter);
1763+
return admin_send_status(
1764+
resp,
1765+
AdminIoCqeStatusType::GenericCommandStatus(
1766+
AdminIoCqeGenericCommandStatus::InvalidFieldInCommand,
1767+
),
1768+
)
1769+
.await;
17041770
}
17051771

17061772
// TODO: Handle MAXCNA
@@ -1803,12 +1869,24 @@ impl RequestHandler for AdminSanitizeRequest {
18031869

18041870
let Ok(config) = TryInto::<AdminSanitizeConfiguration>::try_into(self.config) else {
18051871
debug!("Invalid sanitize configuration: {}", self.config);
1806-
return Err(ResponseStatus::InvalidParameter);
1872+
return admin_send_status(
1873+
resp,
1874+
AdminIoCqeStatusType::GenericCommandStatus(
1875+
AdminIoCqeGenericCommandStatus::InvalidFieldInCommand,
1876+
),
1877+
)
1878+
.await;
18071879
};
18081880

18091881
if subsys.sanicap.ndi && config.ndas {
18101882
debug!("Request for No-Deallocate After Sanitize when No-Deallocate is inhibited");
1811-
return Err(ResponseStatus::InvalidParameter);
1883+
return admin_send_status(
1884+
resp,
1885+
AdminIoCqeStatusType::GenericCommandStatus(
1886+
AdminIoCqeGenericCommandStatus::InvalidFieldInCommand,
1887+
),
1888+
)
1889+
.await;
18121890
}
18131891

18141892
// TODO: Implement action latency, progress state machine, error states
@@ -1877,22 +1955,46 @@ impl RequestHandler for AdminFormatNvmRequest {
18771955

18781956
let Some(ctlr) = subsys.ctlrs.iter().find(|c| c.id.0 == ctx.ctlid) else {
18791957
debug!("Unrecognised CTLID: {}", ctx.ctlid);
1880-
return Err(ResponseStatus::InvalidParameter);
1958+
return admin_send_status(
1959+
resp,
1960+
AdminIoCqeStatusType::GenericCommandStatus(
1961+
AdminIoCqeGenericCommandStatus::InvalidFieldInCommand,
1962+
),
1963+
)
1964+
.await;
18811965
};
18821966

18831967
let Ok(config) = TryInto::<AdminFormatNvmConfiguration>::try_into(self.config) else {
18841968
debug!("Invalid configuration for Admin Format NVM");
1885-
return Err(ResponseStatus::InvalidParameter);
1969+
return admin_send_status(
1970+
resp,
1971+
AdminIoCqeStatusType::GenericCommandStatus(
1972+
AdminIoCqeGenericCommandStatus::InvalidFieldInCommand,
1973+
),
1974+
)
1975+
.await;
18861976
};
18871977

18881978
if config.lbafi != 0 {
18891979
debug!("Unsupported LBA format index: {}", config.lbafi);
1890-
return Err(ResponseStatus::InvalidParameter);
1980+
return admin_send_status(
1981+
resp,
1982+
AdminIoCqeStatusType::GenericCommandStatus(
1983+
AdminIoCqeGenericCommandStatus::InvalidFieldInCommand,
1984+
),
1985+
)
1986+
.await;
18911987
}
18921988

18931989
if !ctlr.active_ns.iter().any(|ns| ns.0 == self.nsid) && self.nsid != u32::MAX {
18941990
debug!("Unrecognised NSID: {}", self.nsid);
1895-
return Err(ResponseStatus::InvalidParameter);
1991+
return admin_send_status(
1992+
resp,
1993+
AdminIoCqeStatusType::GenericCommandStatus(
1994+
AdminIoCqeGenericCommandStatus::InvalidFieldInCommand,
1995+
),
1996+
)
1997+
.await;
18961998
}
18971999

18982000
// TODO: handle config.ses

‎tests/admin.rs‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2245,7 +2245,7 @@ mod get_log_page {
22452245
};
22462246

22472247
use crate::{
2248-
RESP_ADMIN_STATUS_INVALID_FIELD, RESP_INVALID_COMMAND_SIZE, RESP_INVALID_PARAMETER,
2248+
RESP_ADMIN_STATUS_INVALID_FIELD, RESP_INVALID_COMMAND_SIZE,
22492249
common::{
22502250
DeviceType, ExpectedField, ExpectedRespChannel, RelaxedRespChannel, new_device, setup,
22512251
},
@@ -2380,7 +2380,7 @@ mod get_log_page {
23802380
0x29, 0xe2, 0x53, 0x0a
23812381
];
23822382

2383-
let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER);
2383+
let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_FIELD);
23842384
smol::block_on(async {
23852385
mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(()))
23862386
.await
@@ -2568,7 +2568,7 @@ mod get_log_page {
25682568
0x80, 0x60, 0xc4, 0x3b
25692569
];
25702570

2571-
let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER);
2571+
let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_FIELD);
25722572
smol::block_on(async {
25732573
mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(()))
25742574
.await
@@ -3713,7 +3713,7 @@ mod namespace_attachment {
37133713
use mctp::MsgIC;
37143714

37153715
use crate::{
3716-
RESP_INVALID_COMMAND_SIZE, RESP_INVALID_PARAMETER,
3716+
RESP_ADMIN_STATUS_INVALID_FIELD, RESP_INVALID_COMMAND_SIZE,
37173717
common::{DeviceType, ExpectedRespChannel, new_device, setup},
37183718
};
37193719

@@ -3910,7 +3910,7 @@ mod namespace_attachment {
39103910
req[..REQ_DATA.len()].copy_from_slice(&REQ_DATA);
39113911
req[{ len - REQ_MIC.len() }..].copy_from_slice(&REQ_MIC);
39123912

3913-
let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER);
3913+
let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_FIELD);
39143914
smol::block_on(async {
39153915
mep.handle_async(&mut subsys, &req, MsgIC(true), resp, async |_| Ok(()))
39163916
.await
@@ -4293,7 +4293,7 @@ mod namespace_attachment {
42934293
req[..REQ_DATA.len()].copy_from_slice(&REQ_DATA);
42944294
req[{ len - REQ_MIC.len() }..].copy_from_slice(&REQ_MIC);
42954295

4296-
let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER);
4296+
let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_FIELD);
42974297
smol::block_on(async {
42984298
mep.handle_async(&mut subsys, &req, MsgIC(true), resp, async |_| Ok(()))
42994299
.await
@@ -4488,7 +4488,7 @@ mod sanitize {
44884488
use mctp::MsgIC;
44894489

44904490
use crate::{
4491-
RESP_ADMIN_SUCCESS, RESP_INVALID_COMMAND_SIZE, RESP_INVALID_PARAMETER,
4491+
RESP_ADMIN_STATUS_INVALID_FIELD, RESP_ADMIN_SUCCESS, RESP_INVALID_COMMAND_SIZE,
44924492
common::{DeviceType, ExpectedRespChannel, new_device, setup},
44934493
};
44944494

@@ -4756,7 +4756,7 @@ mod sanitize {
47564756
0xc8, 0x47, 0x8d, 0x88
47574757
];
47584758

4759-
let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER);
4759+
let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_FIELD);
47604760
smol::block_on(async {
47614761
mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(()))
47624762
.await
@@ -4813,7 +4813,7 @@ mod format_nvm {
48134813
use mctp::MsgIC;
48144814

48154815
use crate::{
4816-
RESP_ADMIN_SUCCESS, RESP_INVALID_COMMAND_SIZE, RESP_INVALID_PARAMETER,
4816+
RESP_ADMIN_STATUS_INVALID_FIELD, RESP_ADMIN_SUCCESS, RESP_INVALID_COMMAND_SIZE,
48174817
common::{DeviceType, ExpectedRespChannel, new_device, setup},
48184818
};
48194819

@@ -4945,7 +4945,7 @@ mod format_nvm {
49454945
0x0f, 0x14, 0xe6, 0x14
49464946
];
49474947

4948-
let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER);
4948+
let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_FIELD);
49494949
smol::block_on(async {
49504950
mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(()))
49514951
.await
@@ -4990,7 +4990,7 @@ mod format_nvm {
49904990
0x7a, 0x05, 0xbd, 0xb8
49914991
];
49924992

4993-
let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER);
4993+
let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_FIELD);
49944994
smol::block_on(async {
49954995
mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(()))
49964996
.await
@@ -5035,7 +5035,7 @@ mod format_nvm {
50355035
0x27, 0xdd, 0x59, 0xa2
50365036
];
50375037

5038-
let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER);
5038+
let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_FIELD);
50395039
smol::block_on(async {
50405040
mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(()))
50415041
.await
@@ -5080,7 +5080,7 @@ mod format_nvm {
50805080
0xd1, 0xad, 0x95, 0x56
50815081
];
50825082

5083-
let resp = ExpectedRespChannel::new(&RESP_INVALID_PARAMETER);
5083+
let resp = ExpectedRespChannel::new(&RESP_ADMIN_STATUS_INVALID_FIELD);
50845084
smol::block_on(async {
50855085
mep.handle_async(&mut subsys, &REQ, MsgIC(true), resp, async |_| Ok(()))
50865086
.await

0 commit comments

Comments
 (0)