From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7D0D9C3A5A3 for ; Tue, 27 Aug 2019 22:24:03 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 440022064A for ; Tue, 27 Aug 2019 22:24:03 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 440022064A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:59296 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1i2jsg-0006YP-Fh for qemu-devel@archiver.kernel.org; Tue, 27 Aug 2019 18:24:02 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:42501) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1i2jrq-0005zH-R5 for qemu-devel@nongnu.org; Tue, 27 Aug 2019 18:23:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1i2jrp-00047z-0O for qemu-devel@nongnu.org; Tue, 27 Aug 2019 18:23:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36456) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1i2jrg-0003uX-OE; Tue, 27 Aug 2019 18:23:00 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 39DA410881CB; Tue, 27 Aug 2019 22:22:58 +0000 (UTC) Received: from [10.18.17.187] (dhcp-17-187.bos.redhat.com [10.18.17.187]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4EA295D9E1; Tue, 27 Aug 2019 22:22:57 +0000 (UTC) To: Maxim Levitsky , qemu-devel@nongnu.org References: <20190825071541.10389-1-mlevitsk@redhat.com> <20190825071541.10389-2-mlevitsk@redhat.com> From: John Snow Openpgp: preference=signencrypt Autocrypt: addr=jsnow@redhat.com; prefer-encrypt=mutual; keydata= mQINBFTKefwBEAChvwqYC6saTzawbih87LqBYq0d5A8jXYXaiFMV/EvMSDqqY4EY6whXliNO IYzhgrPEe7ZmPxbCSe4iMykjhwMh5byIHDoPGDU+FsQty2KXuoxto+ZdrP9gymAgmyqdk3aV vzzmCa3cOppcqKvA0Kqr10UeX/z4OMVV390V+DVWUvzXpda45/Sxup57pk+hyY52wxxjIqef rj8u5BN93s5uCVTus0oiVA6W+iXYzTvVDStMFVqnTxSxlpZoH5RGKvmoWV3uutByQyBPHW2U 1Y6n6iEZ9MlP3hcDqlo0S8jeP03HaD4gOqCuqLceWF5+2WyHzNfylpNMFVi+Hp0H/nSDtCvQ ua7j+6Pt7q5rvqgHvRipkDDVsjqwasuNc3wyoHexrBeLU/iJBuDld5iLy+dHXoYMB3HmjMxj 3K5/8XhGrDx6BDFeO3HIpi3u2z1jniB7RtyVEtdupED6lqsDj0oSz9NxaOFZrS3Jf6z/kHIf h42mM9Sx7+s4c07N2LieUxcfqhFTaa/voRibF4cmkBVUhOD1AKXNfhEsTvmcz9NbUchCkcvA T9119CrsxfVsE7bXiGvdXnzyGLXdsoosjzwacKdOrVaDmN3Uy+SHiQXo6TlkSdV0XH2PUxTM LsBFIO9qXO43Ai6J6iPAP/01l8fuZfpJE0/L/c25yyaND7xA3wARAQABtCpKb2huIFNub3cg KEpvaG4gSHVzdG9uKSA8anNub3dAcmVkaGF0LmNvbT6JAlQEEwECAD4CGwMCHgECF4AFCwkI BwMFFQoJCAsFFgIDAQAWIQT665cRoSz0dYEvGPKIqQZNGDVh6wUCXF392gUJC1Xq3gAKCRCI qQZNGDVh6558D/9pM4pu4njX5aT6uUW3vAmbWLF1jfPxiTQgSHAnm9EBMZED/fsvkzj97clo LN7JKmbYZNgJmR01A7flG45V4iOR/249qAfaVuD+ZzZi1R4jFzr13WS+IEdn0hYp9ITndb7R ezW+HGu6/rP2PnfmDnNowgJu6Dp6IUEabq8SXXwGHXZPuMIrsXJxUdKJdGnh1o2u7271yNO7 J9PEMuMDsgjsdnaGtv7aQ9CECtXvBleAc06pLW2HU10r5wQyBMZGITemJdBhhdzGmbHAL0M6 vKi/bafHRWqfMqOAdDkv3Jg4arl2NCG/uNateR1z5e529+UlB4XVAQT+f5T/YyI65DFTY940 il3aZhA8u788jZEPMXmt94u7uPZbEYp7V0jt68SrTaOgO7NaXsboXFjwEa42Ug5lB5d5/Qdp 1AITUv0NJ51kKwhHL1dEagGeloIsGVQILmpS0MLdtitBHqZLsnJkRvtMaxo47giyBlv2ewmq tIGTlVLxHx9xkc9aVepOuiGlZaZB72c9AvZs9rKaAjgU2UfJHlB/Hr4uSk/1EY0IgMv4vnsG 1sA5gvS7A4T4euu0PqHtn2sZEWDrk5RDbw0yIb53JYdXboLFmFXKzVASfKh2ZVeXRBlQQSJi 3PBR1GzzqORlfryby7mkY857xzCI2NkIkD2eq+HhzFTfFOTdGrkCDQRUynn8ARAAwbhP45BE d/zAMBPV2dk2WwIwKRSKULElP3kXpcuiDWYQob3UODUUqClO+3aXVRndaNmZX9WbzGYexVo3 5j+CVBCGr3DlU8AL9pp3KQ3SJihWcDed1LSmUf8tS+10d6mdGxDqgnd/OWU214isvhgWZtZG MM/Xj7cx5pERIiP+jqu7PT1cibcfcEKhPjYdyV1QnLtKNGrTg/UMKaL+qkWBUI/8uBoa0HLs NH63bXsRtNAG8w6qG7iiueYZUIXKc4IHINUguqYQJVdSe+u8b2N5XNhDSEUhdlqFYraJvX6d TjxMTW5lzVG2KjztfErRNSUmu2gezbw1/CV0ztniOKDA7mkQi6UIUDRh4LxRm5mflfKiCyDQ L6P/jxHBxFv+sIgjuLrfNhIC1p3z9rvCh+idAVJgtHtYl8p6GAVrF+4xQV2zZH45tgmHo2+S JsLPjXZtWVsWANpepXnesyabWtNAV4qQB7/SfC77zZwsVX0OOY2Qc+iohmXo8U7DgXVDgl/R /5Qgfnlv0/3rOdMt6ZPy5LJr8D9LJmcP0RvX98jyoBOf06Q9QtEwJsNLCOCo2LKNL71DNjZr nXEwjUH66CXiRXDbDKprt71BiSTitkFhGGU88XCtrp8R9yArXPf4MN+wNYBjfT7K29gWTzxt 9DYQIvEf69oZD5Z5qHYGp031E90AEQEAAYkCPAQYAQIAJgIbDBYhBPrrlxGhLPR1gS8Y8oip Bk0YNWHrBQJcXf3JBQkLVerNAAoJEIipBk0YNWHrU1AP/1FOK2SBGbyhHa5vDHuf47fgLipC e0/h1E0vdSonzlhPxuZoQ47FjzG9uOhqqQG6/PqtWs/FJIyz8aGG4aV+pSA/9Ko3/2ND8MSY ZflWs7Y8Peg08Ro01GTHFITjEUgHpTpHiT6TNcZB5aZNJ8jqCtW5UlqvXXbVeSTmO70ZiVtc vUJbpvSxYmzhFfZWaXIPcNcKWL1rnmnzs67lDhMLdkYVf91aml/XtyMUlfB8Iaejzud9Ht3r C0pA9MG57pLblX7okEshxAC0+tUdY2vANWFeX0mgqRt1GSuG9XM9H/cKP1czfUV/FgaWo/Ya fM4eMhUAlL/y+/AJxxumPhBXftM4yuiktp2JMezoIMJI9fmhjfWDw7+2jVrx9ze1joLakFD1 rVAoHxVJ7ORfQ4Ni/qWbQm3T6qQkSMt4N/scNsMczibdTPxU7qtwQwIeFOOc3wEwmJ9Qe3ox TODQ0agXiWVj0OXYCHJ6MxTDswtyTGQW+nUHpKBgHGwUaR6d1kr/LK9+5LpOfRlK9VRfEu7D PGNiRkr8Abp8jHsrBqQWfUS1bAf62bq6XUel0kUCtb7qCq024aOczXYWPFpJFX+nhp4d7NeH Edq+wlC13sBSiSHC7T5yssJ+7JPa2ATLlSKhEvBsLe2TsSTTtFlA0nBclqhfJXzimiuge9qU E40lvMWBuQINBFTKimUBEADDbJ+pQ5M4QBMWkaWImRj7c598xIZ37oKM6rGaSnuB1SVb7YCr Ci2MTwQcrQscA2jm80O8VFqWk+/XsEp62dty47GVwSfdGje/3zv3VTH2KhOCKOq3oPP5ZXWY rz2d2WnTvx++o6lU7HLHDEC3NGLYNLkL1lyVxLhnhvcMxkf1EGA1DboEcMgnJrNB1pGP27ww cSfvdyPGseV+qZZa8kuViDga1oxmnYDxFKMGLxrClqHrRt8geQL1Wj5KFM5hFtGTK4da5lPn wGNd6/CINMeCT2AWZY5ySz7/tSZe5F22vPvVZGoPgQicYWdNc3ap7+7IKP86JNjmec/9RJcz jvrYjJdiqBVldXou72CtDydKVLVSKv8c2wBDJghYZitfYIaL8cTvQfUHRYTfo0n5KKSec8Vo vjDuxmdbOUBA+SkRxqmneP5OxGoZ92VusrwWCjry8HRsNdR+2T+ClDCO6Wpihu4V3CPkQwTy eCuMHPAT0ka5paTwLrnZIxsdfnjUa96T10vzmQgAxpbbiaLvgKJ8+76OPdDnhddyxd2ldYfw RkF5PEGg3mqZnYKNNBtwjvX49SAvgETQvLzQ8IKVgZS0m4z9qHHvtc1BsQnFfe+LJOFjzZr7 CrDNJMqk1JTHYsSi2JcN3vY32WMezXSQ0TzeMK4kdnclSQyp/h23GWod5QARAQABiQRbBBgB AgAmAhsCFiEE+uuXEaEs9HWBLxjyiKkGTRg1YesFAlxd/coFCQtV2mQCKcFdIAQZAQIABgUC VMqKZQAKCRB974EGqvw5DiJoEACLmuiRq9ifvOh5DyBFwRS7gvA14DsGQngmC57EzV0EFcfM XVi1jX5OtwUyUe0Az5r6lHyyHDsDsIpLKBlWrYCeLpUhRR3oy181T7UNxvujGFeTkzvLAOo6 Hs3b8Wv9ARg+7acRYkQRNY7k0GIJ6YZz149tRyRKAy/vSjsaB9Lt0NOd1wf2EQMKwRVELwJD y0AazGn+0PRP7Bua2YbtxaBmhBBDb2tPpwn8U9xdckB4Vlft9lcWNsC/18Gi9bpjd9FSbdH/ sOUI+3ToWYENeoT4IP09wn6EkgWaJS3nAUN/MOycNej2i4Yhy2wDDSKyTAnVkSSSoXk+tK91 HfqtokbDanB8daP+K5LgoiWHzjfWzsxA2jKisI4YCGjrYQzTyGOT6P6u6SEeoEx10865B/zc 8/vN50kncdjYz2naacIDEKQNZlnGLsGkpCbfmfdi3Zg4vuWKNdWr0wGUzDUcpqW0y/lUXna+ 6uyQShX5e4JD2UPuf9WAQ9HtgSAkaDd4O1I2J41sleePzZOVB3DmYgy+ECRJJ5nw3ihdxpgc y/v3lfcJaqiyCv0PF+K/gSOvwhH7CbVqARmptT7yhhxqFdaYWo2Z2ksuKyoKSRMFCXQY5oac uTmyPIT4STFyUQFeqSCWDum/NFNoSKhmItw2Td+4VSJHShRVbg39KNFPZ7mXYAkQiKkGTRg1 YesWJA/+PV3qDUtPNEGwjVvjQqHSbrBy94tu6gJvPHgGPtRDYvxnCaJsmgiC0pGB2KFRsnfl 2zBNBEWF/XwsI081jQE5UO60GKmHTputChLXpVobyuc+lroG2YhknXRBAV969SLnZR4BS/1s Gi046gOXfaKYatve8BiZr5it5Foq3FMPDNgZMit1H9Dk8rkKFfDMRf8EGS/Z+TmyEsIf99H7 TH3n7lco8qO81fSFwkh4pvo2kWRFYTC5vsIVQ+GqVUp+W1DZJHxX8LwWuF1AzUt4MUTtNAvy TXl5EgsmoY9mpNNL7ZnW65oG63nEP5KNiybvuQJzXVxR8eqzOh2Mod4nHg3PE7UCd3DvLNsn GXFRo44WyT/G2lArBtjpkut7bDm0i1nENABy2UgS+1QvdmgNu6aEZxdNthwRjUhuuvCCDMA4 rCDQYyakH2tJNQgkXkeLodBKF4bHiBbuwj0E39S9wmGgg+q4OTnAO/yhQGknle7a7G5xHBwE i0HjnLoJP5jDcoMTabZTIazXmJz3pKM11HYJ5/ZsTIf3ZRJJKIvXJpbmcAPVwTZII6XxiJdh RSSX4Mvd5pL/+5WI6NTdW6DMfigTtdd85fe6PwBNVJL2ZvBfsBJZ5rxg1TOH3KLsYBqBTgW2 glQofxhkJhDEcvjLhe3Y2BlbCWKOmvM8XS9TRt0OwUs= Message-ID: <80305e48-fedf-a014-2ccc-c3fcf3a92ee9@redhat.com> Date: Tue, 27 Aug 2019 18:22:56 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <20190825071541.10389-2-mlevitsk@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.6.2 (mx1.redhat.com [10.5.110.65]); Tue, 27 Aug 2019 22:22:58 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: Re: [Qemu-devel] [PATCH 1/2] block/nvme: add support for write zeros X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Fam Zheng , Kevin Wolf , Paolo Bonzini , qemu-block@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" Without a commit message, I have no real hope of reviewing this. I was CC'd, though, so I'll give it a blind shot. We want to add write_zeroes support for block/nvme, but I can't really verify any of that is correct or working without a unit test, a spec, or some instructions to help me verify any of this does what it looks like it does. On 8/25/19 3:15 AM, Maxim Levitsky wrote: > Signed-off-by: Maxim Levitsky > --- > block/nvme.c | 72 +++++++++++++++++++++++++++++++++++++++++++- > block/trace-events | 1 + > include/block/nvme.h | 19 +++++++++++- > 3 files changed, 90 insertions(+), 2 deletions(-) > > diff --git a/block/nvme.c b/block/nvme.c > index 5be3a39b63..f8bd11e19a 100644 > --- a/block/nvme.c > +++ b/block/nvme.c > @@ -111,6 +111,8 @@ typedef struct { > uint64_t max_transfer; > bool plugged; > > + bool supports_write_zeros; > + I suppose the spelling of "zeroes" is not as established as I thought it was. Actually, what's worse is that the NVME writers apparently couldn't decide what to name it themselves either: "Write Zeroes" has 23 hits. "Write Zeros" has just two, in Figure 114 for the Identify NS Data. Oh, in QEMU we're not much better: jhuston@probe (review) ~/s/qemu> git grep -i zeros | wc -l 265 jhuston@probe (review) ~/s/qemu> git grep -i zeroes | wc -l 747 I'm going to suggest that we use 'zeroes' as the spelling here, to match the existing 'pwrite_zeroes', and then otherwise to match the NVME spec's usual spelling. > CoMutex dma_map_lock; > CoQueue dma_flush_queue; > > @@ -421,6 +423,7 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp) > NvmeIdNs *idns; > NvmeLBAF *lbaf; > uint8_t *resp; > + uint16_t oncs; > int r; > uint64_t iova; > NvmeCmd cmd = { > @@ -458,6 +461,9 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp) > s->max_transfer = MIN_NON_ZERO(s->max_transfer, > s->page_size / sizeof(uint64_t) * s->page_size); > > + oncs = le16_to_cpu(idctrl->oncs); > + s->supports_write_zeros = (oncs & NVME_ONCS_WRITE_ZEROS) != 0; > + For other reviewers: oncs is "Optional NVM Command Support". I think it's better to say `!!(oncs & NVME_ONCS_WRITE_ZEROES)` to remove doubt over the width of the bitmask. > memset(resp, 0, 4096); > > cmd.cdw10 = 0; > @@ -470,6 +476,12 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp) > s->nsze = le64_to_cpu(idns->nsze); > lbaf = &idns->lbaf[NVME_ID_NS_FLBAS_INDEX(idns->flbas)]; > > + if (NVME_ID_NS_DLFEAT_WRITE_ZEROS(idns->dlfeat) && > + NVME_ID_NS_DLFEAT_READ_BEHAVIOR(idns->dlfeat) == > + NVME_ID_NS_DLFEAT_READ_BEHAVIOR_ZEROS) { > + bs->supported_write_flags |= BDRV_REQ_MAY_UNMAP; > + } > + > if (lbaf->ms) { > error_setg(errp, "Namespaces with metadata are not yet supported"); > goto out; > @@ -764,6 +776,8 @@ static int nvme_file_open(BlockDriverState *bs, QDict *options, int flags, > int ret; > BDRVNVMeState *s = bs->opaque; > > + bs->supported_write_flags = BDRV_REQ_FUA; > + Is this a related change? > opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); > qemu_opts_absorb_qdict(opts, options, &error_abort); > device = qemu_opt_get(opts, NVME_BLOCK_OPT_DEVICE); > @@ -792,7 +806,6 @@ static int nvme_file_open(BlockDriverState *bs, QDict *options, int flags, > goto fail; > } > } > - bs->supported_write_flags = BDRV_REQ_FUA; > return 0; > fail: > nvme_close(bs); > @@ -1086,6 +1099,60 @@ static coroutine_fn int nvme_co_flush(BlockDriverState *bs) > } > > > +static coroutine_fn int nvme_co_pwrite_zeroes(BlockDriverState *bs, > + int64_t offset, > + int bytes, > + BdrvRequestFlags flags) > +{ > + BDRVNVMeState *s = bs->opaque; > + NVMeQueuePair *ioq = s->queues[1]; I think it'd be slick to name the queues, but that's not related to this patch. > + NVMeRequest *req; > + > + uint32_t cdw12 = ((bytes >> s->blkshift) - 1) & 0xFFFF; > + > + if (!s->supports_write_zeros) { > + return -ENOTSUP; > + } > + > + NvmeCmd cmd = { > + .opcode = NVME_CMD_WRITE_ZEROS, > + .nsid = cpu_to_le32(s->nsid), > + .cdw10 = cpu_to_le32((offset >> s->blkshift) & 0xFFFFFFFF), > + .cdw11 = cpu_to_le32(((offset >> s->blkshift) >> 32) & 0xFFFFFFFF), > + }; > + > + NVMeCoData data = { > + .ctx = bdrv_get_aio_context(bs), > + .ret = -EINPROGRESS, > + }; > + > + if (flags & BDRV_REQ_MAY_UNMAP) { > + cdw12 |= (1 << 25); > + } > + > + if (flags & BDRV_REQ_FUA) { > + cdw12 |= (1 << 30); > + } > + > + cmd.cdw12 = cpu_to_le32(cdw12); > + > + trace_nvme_write_zeros(s, offset, bytes, flags); > + assert(s->nr_queues > 1); > + req = nvme_get_free_req(ioq); > + assert(req); > + > + nvme_submit_command(s, ioq, req, &cmd, nvme_rw_cb, &data); > + > + data.co = qemu_coroutine_self(); > + while (data.ret == -EINPROGRESS) { > + qemu_coroutine_yield(); > + } > + > + trace_nvme_rw_done(s, true, offset, bytes, data.ret); > + return data.ret; > +} > + > + > static int nvme_reopen_prepare(BDRVReopenState *reopen_state, > BlockReopenQueue *queue, Error **errp) > { > @@ -1190,6 +1257,9 @@ static BlockDriver bdrv_nvme = { > > .bdrv_co_preadv = nvme_co_preadv, > .bdrv_co_pwritev = nvme_co_pwritev, > + > + .bdrv_co_pwrite_zeroes = nvme_co_pwrite_zeroes, > + > .bdrv_co_flush_to_disk = nvme_co_flush, > .bdrv_reopen_prepare = nvme_reopen_prepare, > > diff --git a/block/trace-events b/block/trace-events > index 04209f058d..8209fbd0c7 100644 > --- a/block/trace-events > +++ b/block/trace-events > @@ -149,6 +149,7 @@ nvme_submit_command_raw(int c0, int c1, int c2, int c3, int c4, int c5, int c6, > nvme_handle_event(void *s) "s %p" > nvme_poll_cb(void *s) "s %p" > nvme_prw_aligned(void *s, int is_write, uint64_t offset, uint64_t bytes, int flags, int niov) "s %p is_write %d offset %"PRId64" bytes %"PRId64" flags %d niov %d" > +nvme_write_zeros(void *s, uint64_t offset, uint64_t bytes, int flags) "s %p offset %"PRId64" bytes %"PRId64" flags %d" I was told once that we wanted trace events to match the name of the function they occurred within whenever possible. Maybe we haven't really been very good about enforcing that. Oh well. > nvme_qiov_unaligned(const void *qiov, int n, void *base, size_t size, int align) "qiov %p n %d base %p size 0x%zx align 0x%x" > nvme_prw_buffered(void *s, uint64_t offset, uint64_t bytes, int niov, int is_write) "s %p offset %"PRId64" bytes %"PRId64" niov %d is_write %d" > nvme_rw_done(void *s, int is_write, uint64_t offset, uint64_t bytes, int ret) "s %p is_write %d offset %"PRId64" bytes %"PRId64" ret %d" > diff --git a/include/block/nvme.h b/include/block/nvme.h > index 3ec8efcc43..1f5b406344 100644 > --- a/include/block/nvme.h > +++ b/include/block/nvme.h > @@ -653,12 +653,29 @@ typedef struct NvmeIdNs { > uint8_t mc; > uint8_t dpc; > uint8_t dps; > - uint8_t res30[98]; > + > + uint8_t nmic; > + uint8_t rescap; > + uint8_t fpi; > + uint8_t dlfeat; > + > + uint8_t res30[94]; I wouldn't call this "res30" anymore, because now it starts at the 34th byte instead of the 30th. > NvmeLBAF lbaf[16]; > uint8_t res192[192]; > uint8_t vs[3712]; > } NvmeIdNs; > Pre-existing, but what are any of these names supposed to mean? (I imagine they match the spec, but where?...) Leaving me some breadcrumbs would greatly reduce the time it takes someone who doesn't already know NVME to review this, and I suspect you've looked them up recently, so leaving little notes in the cover letter at least for relevant sections is very nice for hardware spec patches like this. > + > +/*Deallocate Logical Block Features*/ ah. "dlfeat" --> "Deallocate Logical (Block) Features". >From here: NVME Express 1.3, Figure 114: Identify - Identify Namespace Data Structure, NVM Command Set Specific > +#define NVME_ID_NS_DLFEAT_GUARD_CRC(dlfeat) ((dlfeat) & 0x10) Not used in this patch? > +#define NVME_ID_NS_DLFEAT_WRITE_ZEROS(dlfeat) ((dlfeat) & 0x08) > + > +#define NVME_ID_NS_DLFEAT_READ_BEHAVIOR(dlfeat) ((dlfeat) & 0x7) > +#define NVME_ID_NS_DLFEAT_READ_BEHAVIOR_UNDEFINED 0 > +#define NVME_ID_NS_DLFEAT_READ_BEHAVIOR_ZEROS 1 > +#define NVME_ID_NS_DLFEAT_READ_BEHAVIOR_ONES 2 > + > + > #define NVME_ID_NS_NSFEAT_THIN(nsfeat) ((nsfeat & 0x1)) > #define NVME_ID_NS_FLBAS_EXTENDED(flbas) ((flbas >> 4) & 0x1) > #define NVME_ID_NS_FLBAS_INDEX(flbas) ((flbas & 0xf)) > Seems good otherwise, but I didn't trace the actual execution of the new command too far -- I'll assume it works. :) --js