| Message ID | 20210126050248.9077-3-dmitry.fomichev@wdc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix zone write validation | expand |
On Jan 26 14:02, Dmitry Fomichev wrote: > The code in nvme_check_zone_write() first checks for zone boundary > condition violation and only after that it proceeds to verify that the > zone state is suitable the write to happen. This is not consistent with > nvme_check_zone_read() flow - in that function, zone state is checked > before making any boundary checks. Besides, checking that ZSLBA + NLB > does not cross the write boundary is now redundant for Zone Append and > only needs to be done for writes. > > Move the check in the code to be performed for Write and Write Zeroes > commands only and to occur after zone state checks. > > Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com> > --- > hw/block/nvme.c | 21 ++++++++++----------- > 1 file changed, 10 insertions(+), 11 deletions(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index 67538010ef..b712634c27 100644 > --- a/hw/block/nvme.c > +++ b/hw/block/nvme.c > @@ -1138,13 +1138,8 @@ static uint16_t nvme_check_zone_write(NvmeCtrl *n, NvmeNamespace *ns, > uint64_t bndry = nvme_zone_wr_boundary(zone); > uint16_t status; > > - if (unlikely(slba + nlb > bndry)) { > - status = NVME_ZONE_BOUNDARY_ERROR; > - } else { > - status = nvme_check_zone_state_for_write(zone); > - } Alright. The double check on boundary that I pointed out in the previous patch is fixed here. > - > - if (status != NVME_SUCCESS) { > + status = nvme_check_zone_state_for_write(zone); > + if (status) { > trace_pci_nvme_err_zone_write_not_ok(slba, nlb, status); > } else { > assert(nvme_wp_is_valid(zone)); > @@ -1158,10 +1153,14 @@ static uint16_t nvme_check_zone_write(NvmeCtrl *n, NvmeNamespace *ns, > trace_pci_nvme_err_append_too_large(slba, nlb, n->zasl); > status = NVME_INVALID_FIELD; > } > - } else if (unlikely(slba != zone->w_ptr)) { > - trace_pci_nvme_err_write_not_at_wp(slba, zone->d.zslba, > - zone->w_ptr); > - status = NVME_ZONE_INVALID_WRITE; > + } else { > + if (unlikely(slba != zone->w_ptr)) { > + trace_pci_nvme_err_write_not_at_wp(slba, zone->d.zslba, > + zone->w_ptr); > + status = NVME_ZONE_INVALID_WRITE; > + } else if (unlikely(slba + nlb > bndry)) { > + status = NVME_ZONE_BOUNDARY_ERROR; > + } > } > } > > -- > 2.28.0 > >
diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 67538010ef..b712634c27 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -1138,13 +1138,8 @@ static uint16_t nvme_check_zone_write(NvmeCtrl *n, NvmeNamespace *ns, uint64_t bndry = nvme_zone_wr_boundary(zone); uint16_t status; - if (unlikely(slba + nlb > bndry)) { - status = NVME_ZONE_BOUNDARY_ERROR; - } else { - status = nvme_check_zone_state_for_write(zone); - } - - if (status != NVME_SUCCESS) { + status = nvme_check_zone_state_for_write(zone); + if (status) { trace_pci_nvme_err_zone_write_not_ok(slba, nlb, status); } else { assert(nvme_wp_is_valid(zone)); @@ -1158,10 +1153,14 @@ static uint16_t nvme_check_zone_write(NvmeCtrl *n, NvmeNamespace *ns, trace_pci_nvme_err_append_too_large(slba, nlb, n->zasl); status = NVME_INVALID_FIELD; } - } else if (unlikely(slba != zone->w_ptr)) { - trace_pci_nvme_err_write_not_at_wp(slba, zone->d.zslba, - zone->w_ptr); - status = NVME_ZONE_INVALID_WRITE; + } else { + if (unlikely(slba != zone->w_ptr)) { + trace_pci_nvme_err_write_not_at_wp(slba, zone->d.zslba, + zone->w_ptr); + status = NVME_ZONE_INVALID_WRITE; + } else if (unlikely(slba + nlb > bndry)) { + status = NVME_ZONE_BOUNDARY_ERROR; + } } }
The code in nvme_check_zone_write() first checks for zone boundary condition violation and only after that it proceeds to verify that the zone state is suitable the write to happen. This is not consistent with nvme_check_zone_read() flow - in that function, zone state is checked before making any boundary checks. Besides, checking that ZSLBA + NLB does not cross the write boundary is now redundant for Zone Append and only needs to be done for writes. Move the check in the code to be performed for Write and Write Zeroes commands only and to occur after zone state checks. Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com> --- hw/block/nvme.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)