On 27.10.20 11:49, Klaus Jensen wrote: > From: Klaus Jensen <k.jen...@samsung.com> > > For now, support the Data Block, Segment and Last Segment descriptor > types. > > See NVM Express 1.3d, Section 4.4 ("Scatter Gather List (SGL)"). > > Signed-off-by: Klaus Jensen <k.jen...@samsung.com> > Reviewed-by: Keith Busch <kbu...@kernel.org> > --- > include/block/nvme.h | 6 +- > hw/block/nvme.c | 329 ++++++++++++++++++++++++++++++++++-------- > hw/block/trace-events | 4 + > 3 files changed, 279 insertions(+), 60 deletions(-)
[...] > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index c0f1f8ccd473..63d0a17bc5f0 100644 > --- a/hw/block/nvme.c > +++ b/hw/block/nvme.c > @@ -413,13 +413,262 @@ static uint16_t nvme_map_prp(NvmeCtrl *n, uint64_t > prp1, uint64_t prp2, [...] > + if (*len == 0) { > + /* > + * All data has been mapped, but the SGL contains additional > + * segments and/or descriptors. The controller might accept > + * ignoring the rest of the SGL. > + */ > + uint16_t sgls = le16_to_cpu(n->id_ctrl.sgls); > + if (sgls & NVME_CTRL_SGLS_EXCESS_LENGTH) { Hi, Coverity reports that this condition is always false because NVME_CTRL_SGLS_EXCESS_LENGTH is (1 << 18) and thus won’t fit in a uint16_t. I think the problem is that n->id_ctrl.sgls is a uint32_t, so sgls should be uint32_t and we should use le32_to_cpu(). (Which technically is a second problem, namely to access a 32 bit LE field as a 16 bit field. That will give wrong results on big endian hosts.) Max