Skip to content

Commit 99d2194

Browse files
committed
femu/zns: refuse the commands that rewrite a zone behind its back
Dataset Management, Write Zeroes and Write Uncorrectable are dispatched before the namespace's own mode sees them, so on a zoned namespace they reached the plain block paths. A deallocate over a full sequential zone zeroed its data while the descriptor still reported the zone full with its write pointer at capacity: a rewrite out of order that the host is given no way to know about. Write Uncorrectable set bits the zoned read path never consults, so it silently did nothing at all. Refuse them on a zoned namespace, and drop the two the zoned command effects log claimed so a host does not try.
1 parent 3721250 commit 99d2194

3 files changed

Lines changed: 53 additions & 5 deletions

File tree

hw/femu/nvme-admin.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,15 @@ static const uint32_t nvme_cse_iocs_nvm[256] = {
6868
[NVME_CMD_COMPARE] = NVME_CMD_EFF_CSUPP,
6969
};
7070

71+
/*
72+
* Write Zeroes and Dataset Management are left out: they change logical blocks
73+
* without going through the zone state machine, so a zoned namespace refuses
74+
* them and this log says so.
75+
*/
7176
static const uint32_t nvme_cse_iocs_zoned[256] = {
7277
[NVME_CMD_FLUSH] = NVME_CMD_EFF_CSUPP | NVME_CMD_EFF_LBCC,
73-
[NVME_CMD_WRITE_ZEROES] = NVME_CMD_EFF_CSUPP | NVME_CMD_EFF_LBCC,
7478
[NVME_CMD_WRITE] = NVME_CMD_EFF_CSUPP | NVME_CMD_EFF_LBCC,
7579
[NVME_CMD_READ] = NVME_CMD_EFF_CSUPP,
76-
[NVME_CMD_DSM] = NVME_CMD_EFF_CSUPP | NVME_CMD_EFF_LBCC,
7780
[NVME_CMD_COMPARE] = NVME_CMD_EFF_CSUPP,
7881
[NVME_CMD_ZONE_APPEND] = NVME_CMD_EFF_CSUPP | NVME_CMD_EFF_LBCC,
7982
[NVME_CMD_ZONE_MGMT_SEND] = NVME_CMD_EFF_CSUPP | NVME_CMD_EFF_LBCC,

hw/femu/nvme-io.c

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,7 +1270,17 @@ static uint16_t nvme_io_cmd(FemuCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
12701270
}
12711271
return nvme_flush(n, ns, cmd, req);
12721272
case NVME_CMD_DSM:
1273-
if ((NVME_ONCS_DSM & n->oncs) && nvme_ns_has_nvm_cmd_set(ns)) {
1273+
/*
1274+
* This and the two below change logical blocks without going through
1275+
* the zone state machine. A deallocate over a full sequential zone
1276+
* zeroed its data while the descriptor still reported the zone full
1277+
* with its write pointer at capacity -- a rewrite out of order that
1278+
* the host is told nothing about. Refuse them on a zoned namespace
1279+
* until they honour the zone state; the zoned command effects log no
1280+
* longer claims them either.
1281+
*/
1282+
if ((NVME_ONCS_DSM & n->oncs) && nvme_ns_has_nvm_cmd_set(ns) &&
1283+
!NS_ZNSSD(ns)) {
12741284
return nvme_dsm(n, ns, cmd, req);
12751285
}
12761286
return NVME_INVALID_OPCODE | NVME_DNR;
@@ -1280,12 +1290,14 @@ static uint16_t nvme_io_cmd(FemuCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
12801290
}
12811291
return NVME_INVALID_OPCODE | NVME_DNR;
12821292
case NVME_CMD_WRITE_ZEROES:
1283-
if ((NVME_ONCS_WRITE_ZEROS & n->oncs) && nvme_ns_has_nvm_cmd_set(ns)) {
1293+
if ((NVME_ONCS_WRITE_ZEROS & n->oncs) && nvme_ns_has_nvm_cmd_set(ns) &&
1294+
!NS_ZNSSD(ns)) {
12841295
return nvme_write_zeros(n, ns, cmd, req);
12851296
}
12861297
return NVME_INVALID_OPCODE | NVME_DNR;
12871298
case NVME_CMD_WRITE_UNCOR:
1288-
if ((NVME_ONCS_WRITE_UNCORR & n->oncs) && nvme_ns_has_nvm_cmd_set(ns)) {
1299+
if ((NVME_ONCS_WRITE_UNCORR & n->oncs) && nvme_ns_has_nvm_cmd_set(ns) &&
1300+
!NS_ZNSSD(ns)) {
12891301
return nvme_write_uncor(n, ns, cmd, req);
12901302
}
12911303
return NVME_INVALID_OPCODE | NVME_DNR;

hw/femu/tests/qtest/femu-test.c

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#define FEMU_CNS_CS_NS_FMT 0x0a /* command-set NS for a format index */
2828
#define FEMU_CSI_KV 0x01 /* key-value command set */
2929
#define FEMU_ZONE_ACTION_RESET 0x04
30+
#define FEMU_DSM_AD 0x04 /* Dataset Management: deallocate */
3031
#define FEMU_CNS_CS_CTRL 0x06 /* command-set controller identify */
3132
#define FEMU_CSI_ZONED 0x02 /* zoned namespace command set */
3233
#define FEMU_CQ_IEN 0x02 /* Create CQ: interrupts enabled */
@@ -744,6 +745,38 @@ static void femu_test_zone_reset(void *obj, void *data,
744745
g_assert_cmpint(out[0], ==, 0x5a);
745746
g_assert_cmpint(out[FEMU_DATA_SIZE - 1], ==, 0x5a);
746747

748+
/*
749+
* Deallocate is the other way to make blocks read as zeros, and it does not
750+
* go through the zone state machine: accepted, it zeroed a sequential
751+
* zone's data while the descriptor still reported the write pointer.
752+
* It has to be refused, and the data has to survive the refusal.
753+
*/
754+
{
755+
uint64_t ranges = guest_alloc(alloc, 4096);
756+
uint8_t desc[16] = { 0 };
757+
758+
stl_le_p(desc + 4, FEMU_DATA_SIZE / c.lba_size);
759+
qtest_memwrite(femu->dev.bus->qts, ranges, desc, sizeof(desc));
760+
761+
memset(&cmd, 0, sizeof(cmd));
762+
cmd.opcode = NVME_CMD_DSM;
763+
cmd.nsid = cpu_to_le32(1);
764+
cmd.dptr.prp1 = cpu_to_le64(ranges);
765+
cmd.cdw11 = cpu_to_le32(FEMU_DSM_AD);
766+
want = c.cid;
767+
femu_submit(&c, &c.io, &cmd);
768+
g_assert_cmpint(FEMU_SC(femu_complete(&c, &c.io, &got, NULL)), !=,
769+
NVME_SUCCESS);
770+
g_assert_cmpint(got, ==, want);
771+
guest_free(alloc, ranges);
772+
}
773+
774+
qtest_memset(femu->dev.bus->qts, buf, 0, FEMU_DATA_SIZE);
775+
g_assert_cmpint(FEMU_SC(femu_rw(&c, NVME_CMD_READ, 0, buf)), ==,
776+
NVME_SUCCESS);
777+
qtest_memread(femu->dev.bus->qts, buf, out, sizeof(out));
778+
g_assert_cmpint(out[0], ==, 0x5a);
779+
747780
memset(&cmd, 0, sizeof(cmd));
748781
cmd.opcode = NVME_CMD_ZONE_MGMT_SEND;
749782
cmd.nsid = cpu_to_le32(1);

0 commit comments

Comments
 (0)