[v3,00/20] nvme: support NVMe v1.3d, SGLs and multiple namespaces
mbox series

Message ID 20191111122545.252478-1-its@irrelevant.dk
Headers show
Series
  • nvme: support NVMe v1.3d, SGLs and multiple namespaces
Related show

Message

Klaus Birkelund Jensen Nov. 11, 2019, 12:25 p.m. UTC
Hi all,

This v3 fixes a number of issues found doing v2[1]. Below is a list of
commits that changed for v3.

* In "nvme: add missing fields in the identify controller data
  structure", the size of the RTD3R field was incorrectly two instead of
  four bytes wide.

* Fix status code for an invalid NSID for the SMART/Health log page in
  "nvme: add support for the get log page command".

* The naming of reserved fields was changed in "nvme: bump supported
  specification version to 1.3" to align with existing convention.

* "nvme: support multiple namespaces" got a bunch of fixes. The nvme_ns
  function did not error out when the given nsid was above the number of
  valid namespace ids. As reported by Ross, the device did not correctly
  handle inactive namespaces. The controller should return a zeroed
  identify page in response to the Identify Namespace command for an
  inactive namespace.

  Previously, each namespace would contain all of the "common block
  parameters" such as "logical_block_size", "write-cache", etc. For the
  NVMe controller, the write cache is controller wide, so fix handling
  of this feature by removing all those parameters for the nvme-ns
  device and only keep the "drive" parameter. Setting the write-cache
  parameter on the nvme device will trickle down to the nvme-ns devices
  instead. Thus, sending a Set Feature command for the Volatile Write
  Cache feature will also enable/disable the write cache for all
  namespaces (as it should according to the specification).

* Fix a bunch of -Werror=int-to-pointer-cast errors in the "nvme: handle
  dma errors" patch.

After conversations with Michael S. Tsirkin, my patch for dma_memory_rw
("pci: pass along the return value of dma_memory_rw") is now included in
this series (with Reviewed-By by Philippe and Michael). The patch is
required for patch "nvme: handle dma errors" to actually do fix
anything.


  [1]: https://patchwork.kernel.org/cover/11190045/


Klaus Jensen (21):
  nvme: remove superfluous breaks
  nvme: move device parameters to separate struct
  nvme: add missing fields in the identify controller data structure
  nvme: populate the mandatory subnqn and ver fields
  nvme: allow completion queues in the cmb
  nvme: add support for the abort command
  nvme: refactor device realization
  nvme: add support for the get log page command
  nvme: add support for the asynchronous event request command
  nvme: add logging to error information log page
  nvme: add missing mandatory features
  nvme: bump supported specification version to 1.3
  nvme: refactor prp mapping
  nvme: allow multiple aios per command
  nvme: add support for scatter gather lists
  nvme: support multiple namespaces
  nvme: bump controller pci device id
  nvme: remove redundant NvmeCmd pointer parameter
  nvme: make lba data size configurable
  pci: pass along the return value of dma_memory_rw
  nvme: handle dma errors

 block/nvme.c           |   18 +-
 hw/block/Makefile.objs |    2 +-
 hw/block/nvme-ns.c     |  158 ++++
 hw/block/nvme-ns.h     |   62 ++
 hw/block/nvme.c        | 1867 +++++++++++++++++++++++++++++++++-------
 hw/block/nvme.h        |  230 ++++-
 hw/block/trace-events  |   38 +-
 include/block/nvme.h   |  130 ++-
 include/hw/pci/pci.h   |    3 +-
 9 files changed, 2126 insertions(+), 382 deletions(-)
 create mode 100644 hw/block/nvme-ns.c
 create mode 100644 hw/block/nvme-ns.h

Comments

no-reply@patchew.org Nov. 11, 2019, 4:32 p.m. UTC | #1
Patchew URL: https://patchew.org/QEMU/20191111122545.252478-1-its@irrelevant.dk/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH v3 00/20] nvme: support NVMe v1.3d, SGLs and multiple namespaces
Type: series
Message-id: 20191111122545.252478-1-its@irrelevant.dk

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
d447043 nvme: handle dma errors
363b412 pci: pass along the return value of dma_memory_rw
9931480 nvme: make lba data size configurable
653dfec nvme: remove redundant NvmeCmd pointer parameter
f5d2d55 nvme: bump controller pci device id
899efe1 nvme: support multiple namespaces
4fa538c nvme: add support for scatter gather lists
77a96f7 nvme: allow multiple aios per command
c2e14a0 nvme: refactor prp mapping
8d7ce85 nvme: bump supported specification version to 1.3
3c4c466 nvme: add missing mandatory features
c2ae92c nvme: add logging to error information log page
15f570f nvme: add support for the asynchronous event request command
89736d5 nvme: add support for the get log page command
3e527d8 nvme: refactor device realization
3f13647 nvme: add support for the abort command
29c1323 nvme: allow completion queues in the cmb
c8e3900 nvme: populate the mandatory subnqn and ver fields
e4d9684 nvme: add missing fields in the identify controller data structure
3da94d4 nvme: move device parameters to separate struct
48600f3 nvme: remove superfluous breaks

=== OUTPUT BEGIN ===
1/21 Checking commit 48600f300908 (nvme: remove superfluous breaks)
2/21 Checking commit 3da94d4fe855 (nvme: move device parameters to separate struct)
ERROR: Macros with complex values should be enclosed in parenthesis
#177: FILE: hw/block/nvme.h:6:
+#define DEFINE_NVME_PROPERTIES(_state, _props) \
+    DEFINE_PROP_STRING("serial", _state, _props.serial), \
+    DEFINE_PROP_UINT32("cmb_size_mb", _state, _props.cmb_size_mb, 0), \
+    DEFINE_PROP_UINT32("num_queues", _state, _props.num_queues, 64)

total: 1 errors, 0 warnings, 181 lines checked

Patch 2/21 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/21 Checking commit e4d968402d62 (nvme: add missing fields in the identify controller data structure)
4/21 Checking commit c8e3900b8f9b (nvme: populate the mandatory subnqn and ver fields)
5/21 Checking commit 29c13234b204 (nvme: allow completion queues in the cmb)
6/21 Checking commit 3f1364798e4c (nvme: add support for the abort command)
7/21 Checking commit 3e527d850c0c (nvme: refactor device realization)
8/21 Checking commit 89736d5d2575 (nvme: add support for the get log page command)
9/21 Checking commit 15f570fc5e87 (nvme: add support for the asynchronous event request command)
10/21 Checking commit c2ae92ca1dce (nvme: add logging to error information log page)
11/21 Checking commit 3c4c466a54f3 (nvme: add missing mandatory features)
12/21 Checking commit 8d7ce85d02de (nvme: bump supported specification version to 1.3)
13/21 Checking commit c2e14a009c64 (nvme: refactor prp mapping)
14/21 Checking commit 77a96f7b0da7 (nvme: allow multiple aios per command)
15/21 Checking commit 4fa538c80d5f (nvme: add support for scatter gather lists)
16/21 Checking commit 899efe1f1b16 (nvme: support multiple namespaces)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#42: 
new file mode 100644

ERROR: Macros with complex values should be enclosed in parenthesis
#218: FILE: hw/block/nvme-ns.h:8:
+#define DEFINE_NVME_NS_PROPERTIES(_state, _props) \
+    DEFINE_PROP_DRIVE("drive", _state, blk), \
+    DEFINE_PROP_UINT32("nsid", _state, _props.nsid, 0)

total: 1 errors, 1 warnings, 832 lines checked

Patch 16/21 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

17/21 Checking commit f5d2d55972a1 (nvme: bump controller pci device id)
18/21 Checking commit 653dfecb75e9 (nvme: remove redundant NvmeCmd pointer parameter)
19/21 Checking commit 9931480ce8e3 (nvme: make lba data size configurable)
20/21 Checking commit 363b412864ea (pci: pass along the return value of dma_memory_rw)
21/21 Checking commit d447043cfb9b (nvme: handle dma errors)
WARNING: line over 80 characters
#77: FILE: hw/block/nvme.c:257:
+                    if (nvme_addr_read(n, prp_ent, (void *) prp_list, prp_trans)) {

WARNING: line over 80 characters
#103: FILE: hw/block/nvme.c:428:
+        if (nvme_addr_read(n, addr, segment, nsgld * sizeof(NvmeSglDescriptor))) {

total: 0 errors, 2 warnings, 148 lines checked

Patch 21/21 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20191111122545.252478-1-its@irrelevant.dk/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com