[haiku-commits] haiku: hrev53069 - src/add-ons/kernel/drivers/disk/nvme

  • From: waddlesplash <waddlesplash@xxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Sun, 14 Apr 2019 20:17:37 -0400 (EDT)

hrev53069 adds 7 changesets to branch 'master'
old head: 2338299bcbafc6aeb1f3fed7a6871196d4b4a71b
new head: 0c26c6f155cae53675203c712fce745db84ce544
overview: 
https://git.haiku-os.org/haiku/log/?qt=range&q=0c26c6f155ca+%5E2338299bcbaf

----------------------------------------------------------------------------

26657c395dd5: nvme: Fix the build with GCC2.

ace66546187b: nvme_disk: Get the correct PCI information struct.
  
  We want the one of the parent object, not the parent's parent.

067b0d08b2ee: kernel: Add NVMe to the device manager.

6ed6023d01e4: nvme_disk: Add more error checks in read().

c9ba44b70a7a: nvme_disk: Properly implement partial reads.
  
  The previous version did not account for the position not being a
  multiple of the block size, among other problems.
  
  Tested by hand with DiskProbe and then "dd skip=..." to read single
  bytes from partial blocks, and validated as correct.

a3dc2dfa7079: nvme_disk: Disable tracing.

0c26c6f155ca: build: Add the NVMe driver to the image.

                              [ Augustin Cavalier <waddlesplash@xxxxxxxxx> ]

----------------------------------------------------------------------------

4 files changed, 75 insertions(+), 53 deletions(-)
build/jam/packages/Haiku                         |   2 +
.../drivers/disk/nvme/libnvme/nvme_common.h      |   8 ++
.../kernel/drivers/disk/nvme/nvme_disk.cpp       | 115 ++++++++++---------
.../kernel/device_manager/device_manager.cpp     |   3 +

############################################################################

Commit:      26657c395dd532ba9876e2e933167a4fa2d58406
URL:         https://git.haiku-os.org/haiku/commit/?id=26657c395dd5
Author:      Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date:        Sun Apr 14 19:48:07 2019 UTC

nvme: Fix the build with GCC2.

----------------------------------------------------------------------------

diff --git a/src/add-ons/kernel/drivers/disk/nvme/libnvme/nvme_common.h 
b/src/add-ons/kernel/drivers/disk/nvme/libnvme/nvme_common.h
index 64c1ab7fa6..d039dfa821 100644
--- a/src/add-ons/kernel/drivers/disk/nvme/libnvme/nvme_common.h
+++ b/src/add-ons/kernel/drivers/disk/nvme/libnvme/nvme_common.h
@@ -68,14 +68,22 @@
  * Check if a branch is likely to be taken.
  */
 #ifndef likely
+#if __GNUC__ < 3
+#define likely(x) x
+#else
 #define likely(x)  __builtin_expect((x),1)
+#endif
 #endif /* likely */
 
 /*
  * Check if a branch is unlikely to be taken.
  */
 #ifndef unlikely
+#if __GNUC__ < 3
+#define unlikely(x) x
+#else
 #define unlikely(x)  __builtin_expect((x),0)
+#endif
 #endif /* unlikely */
 
 #ifndef typeof
diff --git a/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp 
b/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp
index a55535ed3a..2c72d113a0 100644
--- a/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp
+++ b/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp
@@ -162,19 +162,17 @@ nvme_disk_init_device(void* _info, void** _cookie)
 
        // construct the libnvme pci_device struct
        pci_device* device = new pci_device;
-       *device = {
-               .vendor_id = info->info.vendor_id,
-               .device_id = info->info.device_id,
-               .subvendor_id = 0,
-               .subdevice_id = 0,
-
-               .domain = 0,
-               .bus = info->info.bus,
-               .dev = info->info.device,
-               .func = info->info.function,
-
-               .pci_info = &info->info,
-       };
+       device->vendor_id = info->info.vendor_id;
+       device->device_id = info->info.device_id;
+       device->subvendor_id = 0;
+       device->subdevice_id = 0;
+
+       device->domain = 0;
+       device->bus = info->info.bus;
+       device->dev = info->info.device;
+       device->func = info->info.function;
+
+       device->pci_info = &info->info;
 
        // open the controller
        info->ctrlr = nvme_ctrlr_open(device, NULL);

############################################################################

Commit:      ace66546187b0073a411eeff09b36e8cb55699d0
URL:         https://git.haiku-os.org/haiku/commit/?id=ace66546187b
Author:      Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date:        Sun Apr 14 22:50:10 2019 UTC

nvme_disk: Get the correct PCI information struct.

We want the one of the parent object, not the parent's parent.

----------------------------------------------------------------------------

diff --git a/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp 
b/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp
index 2c72d113a0..1295547702 100644
--- a/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp
+++ b/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp
@@ -67,8 +67,6 @@ static device_manager_info* sDeviceManager;
 
 typedef struct {
        device_node*                    node;
-       pci_device_module_info* pci;
-       pci_device*                             device;
        pci_info                                info;
 
        struct nvme_ctrlr*              ctrlr;
@@ -151,15 +149,14 @@ nvme_disk_init_device(void* _info, void** _cookie)
        CALLED();
        nvme_disk_driver_info* info = (nvme_disk_driver_info*)_info;
 
+       pci_device_module_info* pci;
+       pci_device* pcidev;
        device_node* parent = sDeviceManager->get_parent_node(info->node);
-       device_node* pciParent = sDeviceManager->get_parent_node(parent);
-       sDeviceManager->get_driver(pciParent, (driver_module_info**)&info->pci,
-               (void**)&info->device);
-       sDeviceManager->put_node(pciParent);
+       sDeviceManager->get_driver(parent, (driver_module_info**)&pci,
+               (void**)&pcidev);
+       pci->get_pci_info(pcidev, &info->info);
        sDeviceManager->put_node(parent);
 
-       info->pci->get_pci_info(info->device, &info->info);
-
        // construct the libnvme pci_device struct
        pci_device* device = new pci_device;
        device->vendor_id = info->info.vendor_id;
@@ -425,16 +422,15 @@ nvme_disk_supports_device(device_node *parent)
 
 
 static status_t
-nvme_disk_register_device(device_node *node)
+nvme_disk_register_device(device_node* parent)
 {
        CALLED();
 
-       // ready to register
        device_attr attrs[] = {
                { NULL }
        };
 
-       return sDeviceManager->register_node(node, NVME_DISK_DRIVER_MODULE_NAME,
+       return sDeviceManager->register_node(parent, 
NVME_DISK_DRIVER_MODULE_NAME,
                attrs, NULL, NULL);
 }
 

############################################################################

Commit:      067b0d08b2ee4a9f893d68e73754de43f8346918
URL:         https://git.haiku-os.org/haiku/commit/?id=067b0d08b2ee
Author:      Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date:        Sun Apr 14 22:59:23 2019 UTC

kernel: Add NVMe to the device manager.

----------------------------------------------------------------------------

diff --git a/src/system/kernel/device_manager/device_manager.cpp 
b/src/system/kernel/device_manager/device_manager.cpp
index f42a03094d..e73fa8be84 100644
--- a/src/system/kernel/device_manager/device_manager.cpp
+++ b/src/system/kernel/device_manager/device_manager.cpp
@@ -1560,6 +1560,9 @@ device_node::_GetNextDriverPath(void*& cookie, KPath& 
_path)
                                                _AddPath(*stack, "busses", 
"ata");
                                                _AddPath(*stack, "busses", 
"ide");
                                                break;
+                                       case PCI_nvm:
+                                               _AddPath(*stack, "drivers", 
"disk");
+                                               break;
                                        default:
                                                _AddPath(*stack, "busses");
                                                break;

############################################################################

Commit:      6ed6023d01e4acf3bd2450f32e23546712f2b1f6
URL:         https://git.haiku-os.org/haiku/commit/?id=6ed6023d01e4
Author:      Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date:        Sun Apr 14 23:02:30 2019 UTC

nvme_disk: Add more error checks in read().

----------------------------------------------------------------------------

diff --git a/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp 
b/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp
index 1295547702..149c2a3035 100644
--- a/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp
+++ b/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp
@@ -182,7 +182,7 @@ nvme_disk_init_device(void* _info, void** _cookie)
        int err = nvme_ctrlr_stat(info->ctrlr, &cstat);
        if (err != 0) {
                TRACE_ERROR("failed to get controller information!\n");
-               return -err;
+               return err;
        }
 
        // TODO: export more than just the first namespace!
@@ -196,7 +196,7 @@ nvme_disk_init_device(void* _info, void** _cookie)
        err = nvme_ns_stat(info->ns, &nsstat);
        if (err != 0) {
                TRACE_ERROR("failed to get namespace information!\n");
-               return -err;
+               return err;
        }
 
        // store capacity information
@@ -268,9 +268,9 @@ nvme_disk_free(void* cookie)
 
 
 static void
-disk_read_callback(bool* done, const struct nvme_cpl*)
+disk_read_callback(status_t* status, const struct nvme_cpl* cpl)
 {
-       *done = true;
+       *status = nvme_cpl_is_error(cpl) ? B_IO_ERROR : B_OK;
 }
 
 
@@ -280,28 +280,39 @@ nvme_disk_read(void* cookie, off_t pos, void* buffer, 
size_t* _length)
        CALLED();
        nvme_disk_handle* handle = (nvme_disk_handle*)cookie;
 
-       bool done = false;
-       size_t rounded_len = ROUNDUP(*_length, handle->info->block_size);
+       status_t status = EINPROGRESS;
 
+       // libnvme does transfers in units of device sectors, so we must
+       // round up the length to the sector size, and then allocate a
+       // bounce buffer.
+       const size_t block_size = handle->info->block_size;
+       size_t rounded_len = ROUNDUP(*_length, block_size);
        void* real_buffer;
        if (rounded_len == *_length && !IS_USER_ADDRESS(buffer))
                real_buffer = buffer;
        else
                real_buffer = malloc(rounded_len);
 
+       if (real_buffer == NULL)
+               return B_NO_MEMORY;
+
        MutexLocker _(handle->info->qpair_mtx);
        int ret = nvme_ns_read(handle->info->ns, handle->info->qpair,
-               real_buffer, pos / handle->info->block_size, rounded_len / 
handle->info->block_size,
-               (nvme_cmd_cb)disk_read_callback, &done, 0);
+               real_buffer, pos / block_size, rounded_len / block_size,
+               (nvme_cmd_cb)disk_read_callback, &status, 0);
        if (ret != 0)
                return ret;
 
-       while (!done) {
+       while (status == EINPROGRESS) {
                nvme_ioqp_poll(handle->info->qpair, 1);
-               snooze(1);
+               snooze(5);
+       }
+
+       if (status != B_OK) {
+               *_length = 0;
+               return status;
        }
 
-       status_t status = B_OK;
        if (real_buffer != buffer) {
                if (IS_USER_ADDRESS(buffer))
                        status = user_memcpy(buffer, real_buffer, *_length);

############################################################################

Commit:      c9ba44b70a7a5bd1b6896d9054fb4f4b36f33d06
URL:         https://git.haiku-os.org/haiku/commit/?id=c9ba44b70a7a
Author:      Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date:        Mon Apr 15 00:14:53 2019 UTC

nvme_disk: Properly implement partial reads.

The previous version did not account for the position not being a
multiple of the block size, among other problems.

Tested by hand with DiskProbe and then "dd skip=..." to read single
bytes from partial blocks, and validated as correct.

----------------------------------------------------------------------------

diff --git a/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp 
b/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp
index 149c2a3035..43b71f6dbd 100644
--- a/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp
+++ b/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp
@@ -267,6 +267,9 @@ nvme_disk_free(void* cookie)
 }
 
 
+// #pragma mark - I/O functions
+
+
 static void
 disk_read_callback(status_t* status, const struct nvme_cpl* cpl)
 {
@@ -275,30 +278,44 @@ disk_read_callback(status_t* status, const struct 
nvme_cpl* cpl)
 
 
 static status_t
-nvme_disk_read(void* cookie, off_t pos, void* buffer, size_t* _length)
+nvme_disk_read(void* cookie, off_t pos, void* buffer, size_t* length)
 {
        CALLED();
        nvme_disk_handle* handle = (nvme_disk_handle*)cookie;
+       const size_t block_size = handle->info->block_size;
 
        status_t status = EINPROGRESS;
 
-       // libnvme does transfers in units of device sectors, so we must
-       // round up the length to the sector size, and then allocate a
-       // bounce buffer.
-       const size_t block_size = handle->info->block_size;
-       size_t rounded_len = ROUNDUP(*_length, block_size);
-       void* real_buffer;
-       if (rounded_len == *_length && !IS_USER_ADDRESS(buffer))
-               real_buffer = buffer;
-       else
-               real_buffer = malloc(rounded_len);
-
-       if (real_buffer == NULL)
-               return B_NO_MEMORY;
+       // libnvme does transfers in units of device sectors, so if we have to
+       // round either the position or the length, we will need a bounce 
buffer.
+       off_t rounded_pos = ROUNDDOWN(pos, block_size);
+       size_t rounded_len = ROUNDUP(*length, block_size);
+       if (rounded_pos != pos || rounded_len != *length
+                       || IS_USER_ADDRESS(buffer)) {
+               void* bounceBuffer = malloc(rounded_len);
+               if (bounceBuffer == NULL)
+                       return B_NO_MEMORY;
+
+               status = nvme_disk_read(cookie, rounded_pos, bounceBuffer,
+                       &rounded_len);
+               if (status != B_OK) {
+                       *length = 0;
+                       return status;
+               }
 
+               void* offsetBuffer = ((int8*)bounceBuffer) + (pos - 
rounded_pos);
+               if (IS_USER_ADDRESS(buffer))
+                       status = user_memcpy(buffer, offsetBuffer, *length);
+               else
+                       memcpy(buffer, offsetBuffer, *length);
+               free(bounceBuffer);
+               return status;
+       }
+
+       // Actually perform the read.
        MutexLocker _(handle->info->qpair_mtx);
        int ret = nvme_ns_read(handle->info->ns, handle->info->qpair,
-               real_buffer, pos / block_size, rounded_len / block_size,
+               buffer, rounded_pos / block_size, rounded_len / block_size,
                (nvme_cmd_cb)disk_read_callback, &status, 0);
        if (ret != 0)
                return ret;
@@ -308,19 +325,8 @@ nvme_disk_read(void* cookie, off_t pos, void* buffer, 
size_t* _length)
                snooze(5);
        }
 
-       if (status != B_OK) {
-               *_length = 0;
-               return status;
-       }
-
-       if (real_buffer != buffer) {
-               if (IS_USER_ADDRESS(buffer))
-                       status = user_memcpy(buffer, real_buffer, *_length);
-               else
-                       memcpy(buffer, real_buffer, *_length);
-               free(real_buffer);
-       }
-
+       if (status != B_OK)
+               *length = 0;
        return status;
 }
 
@@ -419,8 +425,6 @@ nvme_disk_supports_device(device_node *parent)
                || sDeviceManager->get_attr_uint16(parent, B_DEVICE_SUB_TYPE, 
&subClass, false) != B_OK)
                return -1.0f;
 
-       TRACE("strcmp: %d, baseclass: %d\n", strcmp(bus, "pci") != 0, baseClass 
!= PCI_mass_storage);
-
        if (strcmp(bus, "pci") != 0 || baseClass != PCI_mass_storage)
                return 0.0f;
 

############################################################################

Commit:      a3dc2dfa70799aaa0906f3d7ecf5763a8705eebd
URL:         https://git.haiku-os.org/haiku/commit/?id=a3dc2dfa7079
Author:      Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date:        Mon Apr 15 00:16:14 2019 UTC

nvme_disk: Disable tracing.

----------------------------------------------------------------------------

diff --git a/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp 
b/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp
index 43b71f6dbd..402c894b28 100644
--- a/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp
+++ b/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp
@@ -21,7 +21,7 @@ extern "C" {
 }
 
 
-#define TRACE_NVME_DISK
+//#define TRACE_NVME_DISK
 #ifdef TRACE_NVME_DISK
 #      define TRACE(x...) dprintf("nvme_disk: " x)
 #else

############################################################################

Revision:    hrev53069
Commit:      0c26c6f155cae53675203c712fce745db84ce544
URL:         https://git.haiku-os.org/haiku/commit/?id=0c26c6f155ca
Author:      Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date:        Mon Apr 15 00:16:36 2019 UTC

build: Add the NVMe driver to the image.

----------------------------------------------------------------------------

diff --git a/build/jam/packages/Haiku b/build/jam/packages/Haiku
index 67d8904b66..10eeaba7dc 100644
--- a/build/jam/packages/Haiku
+++ b/build/jam/packages/Haiku
@@ -58,6 +58,7 @@ if $(TARGET_ARCH) = x86 || $(TARGET_ARCH) = x86_64 {
 }
 
 # drivers
+AddNewDriversToPackage disk                    : nvme_disk ;
 AddNewDriversToPackage disk scsi       : scsi_cd scsi_disk ;
 AddNewDriversToPackage disk virtual : virtio_block ram_disk ;
 AddNewDriversToPackage power           : $(SYSTEM_ADD_ONS_DRIVERS_POWER) ;
@@ -191,6 +192,7 @@ AddBootModuleSymlinksToPackage
        generic_ide_pci
        ide_isa@x86 isa@x86,x86_64 intel it8211
        legacy_sata locked_pool
+       nvme_disk
        openpic@ppc
        packagefs pci
        scsi scsi_cd scsi_disk scsi_periph silicon_image_3112 highpoint_ide_pci


Other related posts:

  • » [haiku-commits] haiku: hrev53069 - src/add-ons/kernel/drivers/disk/nvme - waddlesplash