Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33582
Change subject: libpayload: nvme driver ......................................................................
libpayload: nvme driver
Change-Id: Ie75b1dc743dac3426c230c57ee23b771ba3a6e0c Signed-off-by: Thomas Heijligen thomas.heijligen@secunet.com --- M payloads/libpayload/drivers/Makefile.inc M payloads/libpayload/drivers/storage/Kconfig A payloads/libpayload/drivers/storage/nvme.c M payloads/libpayload/drivers/storage/storage.c A payloads/libpayload/include/storage/nvme.h M payloads/libpayload/include/storage/storage.h M payloads/libpayload/include/x86/arch/io.h A payloads/libpayload/sample/nvme_test.c 8 files changed, 429 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/33582/1
diff --git a/payloads/libpayload/drivers/Makefile.inc b/payloads/libpayload/drivers/Makefile.inc index 40e587c..676dbd3 100644 --- a/payloads/libpayload/drivers/Makefile.inc +++ b/payloads/libpayload/drivers/Makefile.inc @@ -77,6 +77,7 @@ libc-$(CONFIG_LP_STORAGE) += storage/storage.c libc-$(CONFIG_LP_STORAGE_AHCI) += storage/ahci.c libc-$(CONFIG_LP_STORAGE_AHCI) += storage/ahci_common.c +libc-$(CONFIG_LP_STORAGE_NVME) += storage/nvme.c ifeq ($(CONFIG_LP_STORAGE_ATA),y) libc-$(CONFIG_LP_STORAGE_ATA) += storage/ata.c libc-$(CONFIG_LP_STORAGE_ATA) += storage/ahci_ata.c diff --git a/payloads/libpayload/drivers/storage/Kconfig b/payloads/libpayload/drivers/storage/Kconfig index 04e9a29..3eabf6a 100644 --- a/payloads/libpayload/drivers/storage/Kconfig +++ b/payloads/libpayload/drivers/storage/Kconfig @@ -57,3 +57,10 @@ help If this option is selected only AHCI controllers which are known to work will be used. + +config STORAGE_NVME + bool "Support for NVMe devices" + depends on STORAGE && PCI + default y + help + Select this option if you want support for NVMe devices diff --git a/payloads/libpayload/drivers/storage/nvme.c b/payloads/libpayload/drivers/storage/nvme.c new file mode 100644 index 0000000..871759d --- /dev/null +++ b/payloads/libpayload/drivers/storage/nvme.c @@ -0,0 +1,331 @@ +#include <libpayload.h> +#include <stdlib.h> +#include <stdint.h> +#include <stdio.h> +#include <pci.h> +#include <pci/pci.h> +#include <storage/nvme.h> +#include <storage/storage.h> + +#define PCI_CLASS_CODE_NVME 0x0108 + +// NVME Controller Configuration +#define NVME_CC_EN (1 << 0) +#define NVME_CC_CSS (0 << 4) +#define NVME_CC_MPS (0 << 7) +#define NVME_CC_AMS (0 << 11) +#define NVME_CC_SHN (0 << 14) +#define NVME_CC_IOSQES (6 << 16) +#define NVME_CC_IOCQES (4 << 20) + +#define NVME_QUEUE_SIZE 2 +#define NVME_SQ_ENTRY_SIZE 64 +#define NVME_CQ_ENTRY_SIZE 16 + +struct nvme_s_queue_entry { + uint32_t dw[16]; +}; + +struct nvme_c_queue_entry { + uint32_t dw[4]; +}; + +enum nvme_queue { + NVME_ADMIN_QUEUE = 0, + NVME_IO_QUEUE = 2, + ads = 0, + adc = 1, + ios = 2, + ioc = 3, +}; + +static storage_poll_t nvme_poll(struct storage_dev *dev); +static void nvme_detach_device(struct storage_dev *dev); +static ssize_t nvme_read_blocks512(struct storage_dev *dev, lba_t start, size_t count, unsigned char *buf); + +static int create_admin_queues(struct nvme_dev *nvme); +static int create_io_submission_queue(struct nvme_dev *nvme); +static int create_io_completion_queue(struct nvme_dev *nvme); +static void delete_admin_queues(struct nvme_dev *nvme); +static void delete_io_submission_queue(struct nvme_dev *nvme); +static void delete_io_completion_queue(struct nvme_dev *nvme); +static int nvme_cmd(struct nvme_dev *nvme, enum nvme_queue q, const struct nvme_s_queue_entry *cmd); +static int read(struct nvme_dev *nvme, void *buffer, uint64_t base, uint16_t count); + + +static storage_poll_t nvme_poll(struct storage_dev *dev) +{ + return POLL_MEDIUM_PRESENT; +} + +static void nvme_detach_device(struct storage_dev *dev) +{ + //nvme_free(dev->driver_struct); + // FIXME remove from list +} + +static ssize_t nvme_read_blocks512(struct storage_dev *dev, lba_t start, size_t count, unsigned char *buf) +{ + void *buffer = memalign(0x1000, count * 512); + if (!buffer) + return 0; + + for (int i = 0; i < count; i++) { + if (read((struct nvme_dev*)dev, buffer+(i*512), start + i, 1)) { + free(buffer); + return 0; + } + } + + memcpy(buf, buffer, count * 512); + free(buffer); + return count; +} + +static int read(struct nvme_dev *nvme, void *buffer, uint64_t base, uint16_t count) +{ + if (count == 0) + return -1; + + struct nvme_s_queue_entry e = { + .dw[0] = 0x02, + .dw[1] = 0x1, + .dw[6] = virt_to_phys(buffer), + .dw[10] = base, + .dw[11] = base >> 32, + .dw[12] = count - 1, + }; + return nvme_cmd(nvme, ios, &e); +} + +static void delete_io_submission_queue(struct nvme_dev *nvme) +{ + // TODO +} + +static int create_io_submission_queue(struct nvme_dev *nvme) +{ + void *sq_buffer = memalign(0x1000, NVME_SQ_ENTRY_SIZE * NVME_QUEUE_SIZE); + if (!sq_buffer) { + printf("NVMe ERROR: Faild to allocate memory for io submission queue.\n"); + return -1; + } + memset(sq_buffer, 0, NVME_SQ_ENTRY_SIZE * NVME_QUEUE_SIZE); + + struct nvme_s_queue_entry e = { + .dw[0] = 0x01, + .dw[6] = virt_to_phys(sq_buffer), + .dw[10] = (NVME_QUEUE_SIZE << 16) | ios >> 1, + .dw[11] = (1 << 16) | 1, + }; + + int res = nvme_cmd(nvme, NVME_ADMIN_QUEUE, &e); + if (res) { + printf("NVMe ERROR: nvme_cmd returned with %i.\n", res); + free(sq_buffer); + return res; + } + + uint8_t cap_dstrd = (read64(nvme->config) >> 32) & 0xf; + nvme->queue[ios].base = sq_buffer; + nvme->queue[ios].bell = nvme->config + 0x1000 + (ios * (4 << cap_dstrd)); + nvme->queue[ios].idx = 0; + return 0; +} + +static void delete_io_completion_queue(struct nvme_dev *nvme) +{ + // TODO +} + +static int create_io_completion_queue(struct nvme_dev *nvme) +{ + void *const cq_buffer = memalign(0x1000, NVME_CQ_ENTRY_SIZE * NVME_QUEUE_SIZE); + if (!cq_buffer) { + printf("NVMe ERROR: Faild to allocate memory for io competion queue.\n"); + return -1; + } + memset(cq_buffer, 0, NVME_CQ_ENTRY_SIZE * NVME_QUEUE_SIZE); + + const struct nvme_s_queue_entry e = { + .dw[0] = 0x05, + .dw[6] = virt_to_phys(cq_buffer), + .dw[10] = (NVME_QUEUE_SIZE << 16) | ioc >> 1, + .dw[11] = 1, + }; + + int res = nvme_cmd(nvme, NVME_ADMIN_QUEUE, &e); + if (res) { + printf("NVMe ERROR: nvme_cmd returned with %i.\n", res); + free(cq_buffer); + return res; + } + + uint8_t cap_dstrd = (read64(nvme->config) >> 32) & 0xf; + nvme->queue[ioc].base = cq_buffer; + nvme->queue[ioc].bell = nvme->config + 0x1000 + (ioc * (4 << cap_dstrd)); + nvme->queue[ioc].idx = 0; + nvme->queue[ioc].round = 0; + + return 0; +} + +static int nvme_cmd(struct nvme_dev *nvme, enum nvme_queue q, const struct nvme_s_queue_entry *cmd) +{ + int sq = q, cq = q+1; + + void *s_entry = nvme->queue[sq].base + (nvme->queue[sq].idx * NVME_SQ_ENTRY_SIZE); + memcpy(s_entry, cmd, NVME_SQ_ENTRY_SIZE); + write32(nvme->queue[sq].bell, nvme->queue[sq].idx + 1); + nvme->queue[sq].idx = (nvme->queue[sq].idx + 1) & 1; + + struct nvme_c_queue_entry *c_entry = nvme->queue[cq].base + (nvme->queue[cq].idx * NVME_CQ_ENTRY_SIZE); + while (((c_entry->dw[3] >> 16 ) & 0x1) == nvme->queue[cq].round) + ; // FIXME timeout + write32(nvme->queue[cq].bell, nvme->queue[cq].idx + 1); + nvme->queue[cq].idx = (nvme->queue[cq].idx + 1) & 1; + if (nvme->queue[cq].idx == 0) + nvme->queue[cq].round = (nvme->queue[cq].round + 1) & 1; + return c_entry->dw[3] >> 17; +} + +static void delete_admin_queues(struct nvme_dev *nvme) +{ + free(nvme->queue[ads].base); + free(nvme->queue[adc].base); + // TODO clean nvme admin queue struct ??? +} + +static int create_admin_queues(struct nvme_dev *nvme) +{ + uint8_t cap_dstrd = (read64(nvme->config) >> 32) & 0xf; + write32(nvme->config + 0x24, NVME_QUEUE_SIZE << 16 | NVME_QUEUE_SIZE); + + void *sq_buffer = memalign(0x1000, NVME_SQ_ENTRY_SIZE * NVME_QUEUE_SIZE); + if (!sq_buffer) { + printf("NVMe ERROR: faild to allocated memory for admin submission queue\n"); + return -1; + } + memset(sq_buffer, 0, NVME_SQ_ENTRY_SIZE * NVME_QUEUE_SIZE); + write64(nvme->config + 0x28, virt_to_phys(sq_buffer)); + + nvme->queue[ads].base = sq_buffer; + nvme->queue[ads].bell = nvme->config + 0x1000 + (ads * (4 << cap_dstrd)); + nvme->queue[ads].idx = 0; + + void *cq_buffer = memalign(0x1000, NVME_CQ_ENTRY_SIZE * NVME_QUEUE_SIZE); + if (!cq_buffer) { + printf("NVMe ERROR: Faild to allocate memory for admin completion queue\n"); + free(cq_buffer); + return -1; + } + memset(cq_buffer, 0, NVME_CQ_ENTRY_SIZE * NVME_QUEUE_SIZE); + write64(nvme->config + 0x30, virt_to_phys(cq_buffer)); + + nvme->queue[adc].base = cq_buffer; + nvme->queue[adc].bell = nvme->config + 0x1000 + (adc * (4 << cap_dstrd)); + nvme->queue[adc].idx = 0; + nvme->queue[adc].round = 0; + + return 0; +} + +static void nvme_init(pcidev_t dev) +{ + printf("NVMe init (Device %02x:%02x.%02x)\n", PCI_BUS(dev), PCI_SLOT(dev), PCI_FUNC(dev)); + + void *pci_bar0 = phys_to_virt(pci_read_config32(dev, 0x10) & ~0x3ff); + + if ( !(((read64(pci_bar0) >> 37 ) & 0xff) == 0x01)) { + printf("NVMe ERROR: PCIe device does not support the NVMe command set\n"); + return; + } + + struct nvme_dev *nvme = malloc(sizeof(*nvme)); + if (!nvme) { + printf("NVMe ERROR: Faild to allocate buffer for nvme driver struct\n"); + return; + } + nvme->storage_dev.port_type = PORT_TYPE_NVME; + nvme->storage_dev.poll = nvme_poll; + nvme->storage_dev.read_blocks512 = nvme_read_blocks512; + nvme->storage_dev.write_blocks512 = NULL; // not implemented + nvme->storage_dev.detach_device = nvme_detach_device; + nvme->config = pci_bar0; + + uint32_t cc = 0; + write32(nvme->config + 0x1c, 0); + + int status, timeout = (read64(nvme->config) >> 24 & 0xff) * 500; + do { + status = read32(nvme->config + 0x1c) & 0x3; + if (status == 0x2) { + printf("NVMe ERROR: Faild to disable controller. FATAL ERROR\n"); + goto abort; + } + if (timeout < 0) { + printf("NVMe ERROR: Faild to disable controller. Timeout.\n"); + goto abort; + } + timeout -= 10; + mdelay(10); + } while (status != 0x0); + + if (create_admin_queues(nvme)) + goto abort; + + cc = NVME_CC_EN | NVME_CC_CSS | NVME_CC_MPS | NVME_CC_AMS |NVME_CC_SHN + | NVME_CC_IOSQES | NVME_CC_IOCQES; + write32(nvme->config + 0x14, cc); + + timeout = (read64(nvme->config) >> 24 & 0xff) * 500; + do { + status = read32(nvme->config + 0x1c) & 0x3; + if (status == 0x2) { + printf("NVMe ERROR: Faild to disable controller. FATAL ERROR\n"); + goto abort; + } + if (timeout < 0) { + printf("NVMe ERROR: Faild to disable controller. Timeout.\n"); + goto abort; + } + timeout -= 10; + mdelay(10); + } while (status != 0x1); + + uint16_t command = pci_read_config16(dev, PCI_COMMAND); + pci_write_config16(dev, PCI_COMMAND, command | PCI_COMMAND_MASTER); + + if (create_io_completion_queue(nvme)) + goto abort; + if (create_io_submission_queue(nvme)) + goto abort; + + storage_attach_device((storage_dev_t*)nvme); + printf("NVMe init done.\n"); + return; + +abort: + delete_io_submission_queue(nvme); + delete_io_completion_queue(nvme); + delete_admin_queues(nvme); + free(nvme); + printf("failed\n"); + return; +} + +void nvme_initialize(void) +{ + int bus, dev, func; + uint16_t class; + + for (bus = 0; bus < 256; ++bus) { + for (dev = 0; dev < 32; ++dev) { + for (func = 0; func < 8; ++func) { + class = pci_read_config16(PCI_DEV(bus, dev, func), 0xa); + if (class == PCI_CLASS_CODE_NVME) + nvme_init(PCI_DEV(bus, dev, func)); + } + } + } +} diff --git a/payloads/libpayload/drivers/storage/storage.c b/payloads/libpayload/drivers/storage/storage.c index a7141ee..55cb60d 100644 --- a/payloads/libpayload/drivers/storage/storage.c +++ b/payloads/libpayload/drivers/storage/storage.c @@ -31,6 +31,9 @@ #if CONFIG(LP_STORAGE_AHCI) # include <storage/ahci.h> #endif +#if CONFIG(LP_STORAGE_NVME) +#include <storage/nvme.h> +#endif #include <storage/storage.h>
@@ -113,4 +116,7 @@ #if CONFIG(LP_STORAGE_AHCI) ahci_initialize(); #endif +#if CONFIG(LP_STORAGE_NVME) + nvme_initialize(); +#endif } diff --git a/payloads/libpayload/include/storage/nvme.h b/payloads/libpayload/include/storage/nvme.h new file mode 100644 index 0000000..090c6d0 --- /dev/null +++ b/payloads/libpayload/include/storage/nvme.h @@ -0,0 +1,22 @@ +#ifndef _STORAGE_NVME_H +#define _STORAGE_NVME_H + +#include <stdint.h> +#include "storage.h" + +struct nvme_dev { + storage_dev_t storage_dev; + + void *config; + void *admin_s_queue; + struct { + void *base; + uint32_t *bell; + uint16_t idx; // bool pos 0 or 1 + uint16_t round; // bool round 0 or 1+0xd + } queue[4]; +}; + +void nvme_initialize(void); + +#endif /* _STORAGE_NVME_H */ diff --git a/payloads/libpayload/include/storage/storage.h b/payloads/libpayload/include/storage/storage.h index 2dc70b0..d1f998e 100644 --- a/payloads/libpayload/include/storage/storage.h +++ b/payloads/libpayload/include/storage/storage.h @@ -45,6 +45,7 @@ PORT_TYPE_IDE = (1 << 0), PORT_TYPE_SATA = (1 << 1), PORT_TYPE_USB = (1 << 2), + PORT_TYPE_NVME = (1 << 3), } storage_port_t;
typedef enum { diff --git a/payloads/libpayload/include/x86/arch/io.h b/payloads/libpayload/include/x86/arch/io.h index c417ce0..46836d9 100644 --- a/payloads/libpayload/include/x86/arch/io.h +++ b/payloads/libpayload/include/x86/arch/io.h @@ -64,6 +64,11 @@ return *((volatile uint32_t *)(addr)); }
+static inline __attribute__((always_inline)) uint64_t read64(const volatile void *addr) +{ + return *((volatile uint64_t *)(addr)); +} + static inline __attribute__((always_inline)) void write8(volatile void *addr, uint8_t value) { *((volatile uint8_t *)(addr)) = value; @@ -79,6 +84,11 @@ *((volatile uint32_t *)(addr)) = value; }
+static inline __attribute__((always_inline)) void write64(volatile void *addr, uint64_t value) +{ + *((volatile uint64_t *)(addr)) = value; +} + static inline unsigned int inl(int port) { unsigned long val; diff --git a/payloads/libpayload/sample/nvme_test.c b/payloads/libpayload/sample/nvme_test.c new file mode 100644 index 0000000..d2bdf10 --- /dev/null +++ b/payloads/libpayload/sample/nvme_test.c @@ -0,0 +1,51 @@ +/* + * This file is part of the libpayload project. + * + * Copyright (C) 2008 Advanced Micro Devices, Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. The name of the author may not be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +/* Example file for libpayload. */ + +#include <libpayload-config.h> +#include <libpayload.h> +#include <storage/storage.h> + +#define STORAGE_ID 0 + +int main(void) +{ + printf("---------- TEST PROGRAM BEGIN ----------\n"); + storage_initialize(); + + void *buffer = memalign(0x1000, 0x2000); + storage_read_blocks512(STORAGE_ID, 0, 15, buffer); + printf("\nbuffer content:\n"); + //hexdump(buffer,0x2000); + + printf("----------- TEST PROGRAM END -----------\n"); + halt(); + return 0; +}
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload: nvme driver ......................................................................
Patch Set 1:
(49 comments)
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... File payloads/libpayload/drivers/storage/nvme.c:
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 13: #define NVME_CC_EN (1 << 0) please, no space before tabs
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 44: static ssize_t nvme_read_blocks512(struct storage_dev *dev, lba_t start, size_t count, unsigned char *buf); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 52: static int nvme_cmd(struct nvme_dev *nvme, enum nvme_queue q, const struct nvme_s_queue_entry *cmd); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 53: static int read(struct nvme_dev *nvme, void *buffer, uint64_t base, uint16_t count); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 67: static ssize_t nvme_read_blocks512(struct storage_dev *dev, lba_t start, size_t count, unsigned char *buf) line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 74: if (read((struct nvme_dev*)dev, buffer+(i*512), start + i, 1)) { "(foo*)" should be "(foo *)"
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 85: static int read(struct nvme_dev *nvme, void *buffer, uint64_t base, uint16_t count) line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 108: void *sq_buffer = memalign(0x1000, NVME_SQ_ENTRY_SIZE * NVME_QUEUE_SIZE); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 110: printf("NVMe ERROR: Faild to allocate memory for io submission queue.\n"); 'Faild' may be misspelled - perhaps 'Failed'?
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 110: printf("NVMe ERROR: Faild to allocate memory for io submission queue.\n"); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 131: nvme->queue[ios].bell = nvme->config + 0x1000 + (ios * (4 << cap_dstrd)); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 143: void *const cq_buffer = memalign(0x1000, NVME_CQ_ENTRY_SIZE * NVME_QUEUE_SIZE); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 145: printf("NVMe ERROR: Faild to allocate memory for io competion queue.\n"); 'Faild' may be misspelled - perhaps 'Failed'?
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 145: printf("NVMe ERROR: Faild to allocate memory for io competion queue.\n"); 'competion' may be misspelled - perhaps 'completion'?
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 145: printf("NVMe ERROR: Faild to allocate memory for io competion queue.\n"); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 166: nvme->queue[ioc].bell = nvme->config + 0x1000 + (ioc * (4 << cap_dstrd)); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 173: static int nvme_cmd(struct nvme_dev *nvme, enum nvme_queue q, const struct nvme_s_queue_entry *cmd) line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 177: void *s_entry = nvme->queue[sq].base + (nvme->queue[sq].idx * NVME_SQ_ENTRY_SIZE); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 182: struct nvme_c_queue_entry *c_entry = nvme->queue[cq].base + (nvme->queue[cq].idx * NVME_CQ_ENTRY_SIZE); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 183: while (((c_entry->dw[3] >> 16 ) & 0x1) == nvme->queue[cq].round) space prohibited before that close parenthesis ')'
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 204: void *sq_buffer = memalign(0x1000, NVME_SQ_ENTRY_SIZE * NVME_QUEUE_SIZE); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 206: printf("NVMe ERROR: faild to allocated memory for admin submission queue\n"); 'faild' may be misspelled - perhaps 'failed'?
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 206: printf("NVMe ERROR: faild to allocated memory for admin submission queue\n"); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 213: nvme->queue[ads].bell = nvme->config + 0x1000 + (ads * (4 << cap_dstrd)); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 215: trailing whitespace
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 216: void *cq_buffer = memalign(0x1000, NVME_CQ_ENTRY_SIZE * NVME_QUEUE_SIZE); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 218: printf("NVMe ERROR: Faild to allocate memory for admin completion queue\n"); 'Faild' may be misspelled - perhaps 'Failed'?
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 218: printf("NVMe ERROR: Faild to allocate memory for admin completion queue\n"); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 226: nvme->queue[adc].bell = nvme->config + 0x1000 + (adc * (4 << cap_dstrd)); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 235: printf("NVMe init (Device %02x:%02x.%02x)\n", PCI_BUS(dev), PCI_SLOT(dev), PCI_FUNC(dev)); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 239: if ( !(((read64(pci_bar0) >> 37 ) & 0xff) == 0x01)) { space prohibited after that open parenthesis '('
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 239: if ( !(((read64(pci_bar0) >> 37 ) & 0xff) == 0x01)) { space prohibited before that close parenthesis ')'
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 240: printf("NVMe ERROR: PCIe device does not support the NVMe command set\n"); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 246: printf("NVMe ERROR: Faild to allocate buffer for nvme driver struct\n"); 'Faild' may be misspelled - perhaps 'Failed'?
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 246: printf("NVMe ERROR: Faild to allocate buffer for nvme driver struct\n"); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 263: printf("NVMe ERROR: Faild to disable controller. FATAL ERROR\n"); 'Faild' may be misspelled - perhaps 'Failed'?
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 263: printf("NVMe ERROR: Faild to disable controller. FATAL ERROR\n"); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 267: printf("NVMe ERROR: Faild to disable controller. Timeout.\n"); 'Faild' may be misspelled - perhaps 'Failed'?
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 267: printf("NVMe ERROR: Faild to disable controller. Timeout.\n"); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 277: cc = NVME_CC_EN | NVME_CC_CSS | NVME_CC_MPS | NVME_CC_AMS |NVME_CC_SHN need consistent spacing around '|' (ctx:WxV)
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 285: printf("NVMe ERROR: Faild to disable controller. FATAL ERROR\n"); 'Faild' may be misspelled - perhaps 'Failed'?
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 285: printf("NVMe ERROR: Faild to disable controller. FATAL ERROR\n"); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 289: printf("NVMe ERROR: Faild to disable controller. Timeout.\n"); 'Faild' may be misspelled - perhaps 'Failed'?
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 289: printf("NVMe ERROR: Faild to disable controller. Timeout.\n"); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 304: storage_attach_device((storage_dev_t*)nvme); "(foo*)" should be "(foo *)"
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 315: } void function return statements are not generally useful
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 325: class = pci_read_config16(PCI_DEV(bus, dev, func), 0xa); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/include/x86/arch... File payloads/libpayload/include/x86/arch/io.h:
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/include/x86/arch... PS1, Line 67: static inline __attribute__((always_inline)) uint64_t read64(const volatile void *addr) line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/include/x86/arch... PS1, Line 87: static inline __attribute__((always_inline)) void write64(volatile void *addr, uint64_t value) line over 80 characters
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload: nvme driver ......................................................................
Patch Set 2:
Patch Set 2:
(1 comment)
Can this be tested with QEMU?
For QEMU i440fx: qemu-system-x86_64 \ -drive file=/path/to/disk/file,if=none,id=drive0 \ -device nvme,drive=drive0,serial=512,num_queues=6 \ -bios /path/to/coreboot.rom
The sample/nvme_test.c is currently a leftover from developing and will be removed before final review. This driver can be used to boot from NVMe with FILO.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload: nvme driver ......................................................................
Patch Set 5:
This change is ready for review.
Hello build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33582
to look at the new patch set (#6).
Change subject: libpayload: add nvme driver ......................................................................
libpayload: add nvme driver
tested on qemu with custom payload and filo
Change-Id: Ie75b1dc743dac3426c230c57ee23b771ba3a6e0c Signed-off-by: Thomas Heijligen thomas.heijligen@secunet.com --- M payloads/libpayload/drivers/Makefile.inc M payloads/libpayload/drivers/storage/Kconfig A payloads/libpayload/drivers/storage/nvme.c M payloads/libpayload/drivers/storage/storage.c A payloads/libpayload/include/storage/nvme.h M payloads/libpayload/include/storage/storage.h M payloads/libpayload/include/x86/arch/io.h 7 files changed, 441 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/33582/6
Hello build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33582
to look at the new patch set (#7).
Change subject: libpayload: add nvme driver ......................................................................
libpayload: add nvme driver
tested on qemu with custom payload and filo
Change-Id: Ie75b1dc743dac3426c230c57ee23b771ba3a6e0c Signed-off-by: Thomas Heijligen thomas.heijligen@secunet.com --- M payloads/libpayload/drivers/Makefile.inc M payloads/libpayload/drivers/storage/Kconfig A payloads/libpayload/drivers/storage/nvme.c M payloads/libpayload/drivers/storage/storage.c A payloads/libpayload/include/storage/nvme.h M payloads/libpayload/include/storage/storage.h M payloads/libpayload/include/x86/arch/io.h 7 files changed, 499 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/33582/7
Hello build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33582
to look at the new patch set (#8).
Change subject: libpayload: add nvme driver ......................................................................
libpayload: add nvme driver
Tested with qemu virtual NVMe and Intel hardware. Works with FILO
Change-Id: Ie75b1dc743dac3426c230c57ee23b771ba3a6e0c Signed-off-by: Thomas Heijligen thomas.heijligen@secunet.com --- M payloads/libpayload/drivers/Makefile.inc M payloads/libpayload/drivers/storage/Kconfig A payloads/libpayload/drivers/storage/nvme.c M payloads/libpayload/drivers/storage/storage.c A payloads/libpayload/include/storage/nvme.h M payloads/libpayload/include/storage/storage.h 6 files changed, 489 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/33582/8
Nico Huber has uploaded a new patch set (#9) to the change originally created by Thomas Heijligen. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload: add nvme driver ......................................................................
libpayload: add nvme driver
Tested with qemu virtual NVMe and Intel hardware. Works with FILO
Change-Id: Ie75b1dc743dac3426c230c57ee23b771ba3a6e0c Signed-off-by: Thomas Heijligen thomas.heijligen@secunet.com Signed-off-by: Nico Huber nico.huber@secunet.com --- M payloads/libpayload/drivers/Makefile.inc M payloads/libpayload/drivers/storage/Kconfig A payloads/libpayload/drivers/storage/nvme.c M payloads/libpayload/drivers/storage/storage.c A payloads/libpayload/include/storage/nvme.h M payloads/libpayload/include/storage/storage.h 6 files changed, 513 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/33582/9
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload: add nvme driver ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33582/9/payloads/libpayload/drivers... File payloads/libpayload/drivers/storage/nvme.c:
https://review.coreboot.org/c/coreboot/+/33582/9/payloads/libpayload/drivers... PS9, Line 371: goto abort;; Statements terminations use 1 semicolon
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload: add nvme driver ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33582/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/33582/9//COMMIT_MSG@9 PS9, Line 9: Tested with qemu virtual NVMe and Intel hardware. Works with FILO Please add a dot/period at the end.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload/storage: Add NVMe driver ......................................................................
Patch Set 14:
(7 comments)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/33582/11/payloads/libpayload/driver... File payloads/libpayload/drivers/storage/nvme.c:
https://review.coreboot.org/c/coreboot/+/33582/11/payloads/libpayload/driver... PS11, Line 2: * This file is part of the libpayload project. : * : * Copyright (C) 2019 secunet Security Networks AG : * : * Redistribution and use in source and binary forms, with or without : * modification, are permitted provided that the following conditions : * are met: : * 1. Redistributions of source code must retain the above copyright : * notice, this list of conditions and the following disclaimer. : * 2. Redistributions in binary form must reproduce the above copyright : * notice, this list of conditions and the following disclaimer in the : * documentation and/or other materials provided with the distribution. : * 3. The name of the author may not be used to endorse or promote products : * derived from this software without specific prior written permission. : * : * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND : * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE : * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE : * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE : * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL : * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS : * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) : * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT : * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY : * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF : * SUCH DAMAGE.
We can use SPDX.
Done
https://review.coreboot.org/c/coreboot/+/33582/11/payloads/libpayload/driver... PS11, Line 289: // if io queues NULL
seems redundant
Done
https://review.coreboot.org/c/coreboot/+/33582/11/payloads/libpayload/driver... PS11, Line 304: // reset controller
huh? seems spurious
Done
https://review.coreboot.org/c/coreboot/+/33582/11/payloads/libpayload/driver... PS11, Line 372: uint32_t cc = 0;
Why declare it already? we could also just have […]
Done
https://review.coreboot.org/c/coreboot/+/33582/11/payloads/libpayload/driver... PS11, Line 426: printf("failed\n");
There are already more elaborate error messages on every path leading here.
Done
https://review.coreboot.org/c/coreboot/+/33582/11/payloads/libpayload/includ... File payloads/libpayload/include/storage/nvme.h:
https://review.coreboot.org/c/coreboot/+/33582/11/payloads/libpayload/includ... PS11, Line 1: /* : * This file is part of the libpayload project. : * : * Copyright (C) 2019 secunet Security Networks AG : * : * Redistribution and use in source and binary forms, with or without : * modification, are permitted provided that the following conditions : * are met: : * 1. Redistributions of source code must retain the above copyright : * notice, this list of conditions and the following disclaimer. : * 2. Redistributions in binary form must reproduce the above copyright : * notice, this list of conditions and the following disclaimer in the : * documentation and/or other materials provided with the distribution. : * 3. The name of the author may not be used to endorse or promote products : * derived from this software without specific prior written permission. : * : * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND : * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE : * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE : * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE : * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL : * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS : * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) : * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT : * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY : * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF : * SUCH DAMAGE. : */
We can use SPDX.
Done
https://review.coreboot.org/c/coreboot/+/33582/11/payloads/libpayload/includ... PS11, Line 36: struct nvme_dev { : storage_dev_t storage_dev; : : pcidev_t pci_dev; : void *config; : struct { : void *base; : uint32_t *bell; : uint16_t idx; // bool pos 0 or 1 : uint16_t round; // bool round 0 or 1+0xd : } queue[4]; : : uint64_t *prp_list; : };
Doesn't need to be in the API header file, does it?
Done
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload/storage: Add NVMe driver ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33582/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/33582/9//COMMIT_MSG@9 PS9, Line 9: Tested with qemu virtual NVMe and Intel hardware. Works with FILO
Please add a dot/period at the end.
Done
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload/storage: Add NVMe driver ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33582/2/payloads/libpayload/drivers... File payloads/libpayload/drivers/storage/Kconfig:
https://review.coreboot.org/c/coreboot/+/33582/2/payloads/libpayload/drivers... PS2, Line 66: Select this option if you want support for NVMe devices
Please add a dot at the end.
Done
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload/storage: Add NVMe driver ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33582/13/payloads/libpayload/includ... File payloads/libpayload/include/storage/nvme.h:
https://review.coreboot.org/c/coreboot/+/33582/13/payloads/libpayload/includ... PS13, Line 33: #include <stdint.h> can be removed?
Hello Felix Singer, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33582
to look at the new patch set (#15).
Change subject: libpayload/storage: Add NVMe driver ......................................................................
libpayload/storage: Add NVMe driver
Tested with qemu virtual NVMe and Intel hardware. Works with FILO.
Change-Id: Ie75b1dc743dac3426c230c57ee23b771ba3a6e0c Signed-off-by: Thomas Heijligen thomas.heijligen@secunet.com Signed-off-by: Nico Huber nico.huber@secunet.com --- M payloads/libpayload/drivers/Makefile.inc M payloads/libpayload/drivers/storage/Kconfig A payloads/libpayload/drivers/storage/nvme.c M payloads/libpayload/drivers/storage/storage.c M payloads/libpayload/include/pci/pci.h A payloads/libpayload/include/storage/nvme.h M payloads/libpayload/include/storage/storage.h 7 files changed, 454 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/33582/15
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload/storage: Add NVMe driver ......................................................................
Patch Set 15:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33582/13/payloads/libpayload/includ... File payloads/libpayload/include/storage/nvme.h:
https://review.coreboot.org/c/coreboot/+/33582/13/payloads/libpayload/includ... PS13, Line 33: #include <stdint.h>
can be removed?
Done
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload/storage: Add NVMe driver ......................................................................
Patch Set 15: Code-Review+1
(5 comments)
https://review.coreboot.org/c/coreboot/+/33582/15/payloads/libpayload/driver... File payloads/libpayload/drivers/storage/nvme.c:
https://review.coreboot.org/c/coreboot/+/33582/15/payloads/libpayload/driver... PS15, Line 60: : static storage_poll_t nvme_poll(struct storage_dev *dev); : static void nvme_detach_device(struct storage_dev *dev); : static ssize_t nvme_read_blocks512( : struct storage_dev *dev, lba_t start, size_t count, unsigned char *buf); : : static int create_admin_queues(struct nvme_dev *nvme); : static int create_io_submission_queue(struct nvme_dev *nvme); : static int create_io_completion_queue(struct nvme_dev *nvme); : static int delete_admin_queues(struct nvme_dev *nvme); : static int delete_io_submission_queue(struct nvme_dev *nvme); : static int delete_io_completion_queue(struct nvme_dev *nvme); : static int nvme_cmd( : struct nvme_dev *nvme, enum nvme_queue q, const struct nvme_s_queue_entry *cmd); : static int read(struct nvme_dev *nvme, unsigned char *buffer, uint64_t base, uint16_t count); : It would contribute to the readability of the code to reorder the functions so that this is not needed.
https://review.coreboot.org/c/coreboot/+/33582/15/payloads/libpayload/driver... PS15, Line 130: static int read even though this is static, but please don't use posix/bsd/sysv reserved function names (unless you are implementing standard functions)
https://review.coreboot.org/c/coreboot/+/33582/15/payloads/libpayload/driver... PS15, Line 390: FATAL ERROR How are tgese two functions more fatal than the other ones goto-ing to abort:?
https://review.coreboot.org/c/coreboot/+/33582/15/payloads/libpayload/driver... PS15, Line 394: NVMe ERROR: Failed to disable controller. Timeout.\n It would be beneficial to make the error message unique, so it is possible to determine where in the code we die, when something goes wrong. (Although this is mostly a copy of the above code, apart from the abort condition)
https://review.coreboot.org/c/coreboot/+/33582/15/payloads/libpayload/driver... File payloads/libpayload/drivers/storage/storage.c:
https://review.coreboot.org/c/coreboot/+/33582/15/payloads/libpayload/driver... PS15, Line 31: #if CONFIG(LP_STORAGE_AHCI) : # include <storage/ahci.h> : #endif : #if CONFIG(LP_STORAGE_NVME) : #include <storage/nvme.h> : #endif With these only containing one prototype each, are we sure that wrapping the includes in CONFIG() is warranted?
Hello Felix Singer, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth, Duncan Laurie, Stefan Reinauer,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33582
to look at the new patch set (#16).
Change subject: libpayload/storage: Add NVMe driver ......................................................................
libpayload/storage: Add NVMe driver
Tested with qemu virtual NVMe and Intel hardware. Works with FILO.
Change-Id: Ie75b1dc743dac3426c230c57ee23b771ba3a6e0c Signed-off-by: Thomas Heijligen thomas.heijligen@secunet.com Signed-off-by: Nico Huber nico.huber@secunet.com --- M payloads/libpayload/drivers/Makefile.inc M payloads/libpayload/drivers/storage/Kconfig A payloads/libpayload/drivers/storage/nvme.c M payloads/libpayload/drivers/storage/storage.c M payloads/libpayload/include/pci/pci.h A payloads/libpayload/include/storage/nvme.h M payloads/libpayload/include/storage/storage.h 7 files changed, 433 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/33582/16
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload/storage: Add NVMe driver ......................................................................
Patch Set 16:
(4 comments)
https://review.coreboot.org/c/coreboot/+/33582/15/payloads/libpayload/driver... File payloads/libpayload/drivers/storage/nvme.c:
https://review.coreboot.org/c/coreboot/+/33582/15/payloads/libpayload/driver... PS15, Line 60: : static storage_poll_t nvme_poll(struct storage_dev *dev); : static void nvme_detach_device(struct storage_dev *dev); : static ssize_t nvme_read_blocks512( : struct storage_dev *dev, lba_t start, size_t count, unsigned char *buf); : : static int create_admin_queues(struct nvme_dev *nvme); : static int create_io_submission_queue(struct nvme_dev *nvme); : static int create_io_completion_queue(struct nvme_dev *nvme); : static int delete_admin_queues(struct nvme_dev *nvme); : static int delete_io_submission_queue(struct nvme_dev *nvme); : static int delete_io_completion_queue(struct nvme_dev *nvme); : static int nvme_cmd( : struct nvme_dev *nvme, enum nvme_queue q, const struct nvme_s_queue_entry *cmd); : static int read(struct nvme_dev *nvme, unsigned char *buffer, uint64_t base, uint16_t count); :
It would contribute to the readability of the code to reorder the functions so that this is not need […]
Done
https://review.coreboot.org/c/coreboot/+/33582/15/payloads/libpayload/driver... PS15, Line 130: static int read
even though this is static, but please don't use posix/bsd/sysv reserved function names (unless you […]
Done
https://review.coreboot.org/c/coreboot/+/33582/15/payloads/libpayload/driver... PS15, Line 390: FATAL ERROR
How are tgese two functions more fatal than the other ones goto-ing to abort:?
I unified the error message in the abort section. This could be enough
https://review.coreboot.org/c/coreboot/+/33582/15/payloads/libpayload/driver... File payloads/libpayload/drivers/storage/storage.c:
https://review.coreboot.org/c/coreboot/+/33582/15/payloads/libpayload/driver... PS15, Line 31: #if CONFIG(LP_STORAGE_AHCI) : # include <storage/ahci.h> : #endif : #if CONFIG(LP_STORAGE_NVME) : #include <storage/nvme.h> : #endif
With these only containing one prototype each, are we sure that wrapping the includes in CONFIG() is […]
Done
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload/storage: Add NVMe driver ......................................................................
Patch Set 16:
(2 comments)
https://review.coreboot.org/c/coreboot/+/33582/16/payloads/libpayload/driver... File payloads/libpayload/drivers/storage/nvme.c:
https://review.coreboot.org/c/coreboot/+/33582/16/payloads/libpayload/driver... PS16, Line 173: static int nvme_read(struct nvme_dev *nvme, unsigned char *buffer, uint64_t base, uint16_t count) line over 96 characters
https://review.coreboot.org/c/coreboot/+/33582/16/payloads/libpayload/driver... PS16, Line 392: printf("NVMe faild to init.\n"); 'faild' may be misspelled - perhaps 'failed'?
Hello Felix Singer, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth, Duncan Laurie, Stefan Reinauer,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33582
to look at the new patch set (#17).
Change subject: libpayload/storage: Add NVMe driver ......................................................................
libpayload/storage: Add NVMe driver
Tested with qemu virtual NVMe and Intel hardware. Works with FILO.
Change-Id: Ie75b1dc743dac3426c230c57ee23b771ba3a6e0c Signed-off-by: Thomas Heijligen thomas.heijligen@secunet.com Signed-off-by: Nico Huber nico.huber@secunet.com --- M payloads/libpayload/drivers/Makefile.inc M payloads/libpayload/drivers/storage/Kconfig A payloads/libpayload/drivers/storage/nvme.c M payloads/libpayload/drivers/storage/storage.c M payloads/libpayload/include/pci/pci.h A payloads/libpayload/include/storage/nvme.h M payloads/libpayload/include/storage/storage.h 7 files changed, 433 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/33582/17
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload/storage: Add NVMe driver ......................................................................
Patch Set 17:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33582/17/payloads/libpayload/driver... File payloads/libpayload/drivers/storage/nvme.c:
https://review.coreboot.org/c/coreboot/+/33582/17/payloads/libpayload/driver... PS17, Line 173: static int nvme_read(struct nvme_dev *nvme, unsigned char *buffer, uint64_t base, uint16_t count) line over 96 characters
Attention is currently required from: Stefan Reinauer, Thomas Heijligen. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload/storage: Add NVMe driver ......................................................................
Patch Set 17:
(1 comment)
File payloads/libpayload/drivers/storage/nvme.c:
https://review.coreboot.org/c/coreboot/+/33582/comment/4993c69f_d2f8a49b PS15, Line 394: NVMe ERROR: Failed to disable controller. Timeout.\n
It would be beneficial to make the error message unique, so it is possible to determine where in the […]
Done, I guess.
Attention is currently required from: Stefan Reinauer, Thomas Heijligen. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload/storage: Add NVMe driver ......................................................................
Patch Set 17: Code-Review+2
(1 comment)
Patchset:
PS17: Needs another +2 because I co-authored.
Attention is currently required from: Thomas Heijligen. Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload/storage: Add NVMe driver ......................................................................
Patch Set 17: Code-Review+2
Attention is currently required from: Thomas Heijligen. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload/storage: Add NVMe driver ......................................................................
Patch Set 17: Code-Review+2
Attention is currently required from: Felix Singer. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload/storage: Add NVMe driver ......................................................................
Patch Set 18:
(1 comment)
File payloads/libpayload/drivers/storage/nvme.c:
https://review.coreboot.org/c/coreboot/+/33582/comment/c74d47b6_d558bf92 PS18, Line 173: static int nvme_read(struct nvme_dev *nvme, unsigned char *buffer, uint64_t base, uint16_t count) line over 96 characters
Attention is currently required from: Thomas Heijligen. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload/storage: Add NVMe driver ......................................................................
Patch Set 18: Code-Review+1
(1 comment)
Patchset:
PS17:
Needs another +2 because I co-authored.
Marking this as resolved since Stefan and Angel gave +2 :)
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload/storage: Add NVMe driver ......................................................................
libpayload/storage: Add NVMe driver
Tested with qemu virtual NVMe and Intel hardware. Works with FILO.
Change-Id: Ie75b1dc743dac3426c230c57ee23b771ba3a6e0c Signed-off-by: Thomas Heijligen thomas.heijligen@secunet.com Signed-off-by: Nico Huber nico.huber@secunet.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/33582 Reviewed-by: Felix Singer felixsinger@posteo.net Reviewed-by: Nico Huber nico.h@gmx.de Reviewed-by: Stefan Reinauer stefan.reinauer@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M payloads/libpayload/drivers/Makefile.inc M payloads/libpayload/drivers/storage/Kconfig A payloads/libpayload/drivers/storage/nvme.c M payloads/libpayload/drivers/storage/storage.c M payloads/libpayload/include/pci/pci.h A payloads/libpayload/include/storage/nvme.h M payloads/libpayload/include/storage/storage.h 7 files changed, 433 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved Nico Huber: Looks good to me, approved Felix Singer: Looks good to me, but someone else must approve Angel Pons: Looks good to me, approved
diff --git a/payloads/libpayload/drivers/Makefile.inc b/payloads/libpayload/drivers/Makefile.inc index c4f7bf6..41fda5b 100644 --- a/payloads/libpayload/drivers/Makefile.inc +++ b/payloads/libpayload/drivers/Makefile.inc @@ -77,6 +77,7 @@ libc-$(CONFIG_LP_STORAGE) += storage/storage.c libc-$(CONFIG_LP_STORAGE_AHCI) += storage/ahci.c libc-$(CONFIG_LP_STORAGE_AHCI) += storage/ahci_common.c +libc-$(CONFIG_LP_STORAGE_NVME) += storage/nvme.c ifeq ($(CONFIG_LP_STORAGE_ATA),y) libc-$(CONFIG_LP_STORAGE_ATA) += storage/ata.c libc-$(CONFIG_LP_STORAGE_ATA) += storage/ahci_ata.c diff --git a/payloads/libpayload/drivers/storage/Kconfig b/payloads/libpayload/drivers/storage/Kconfig index 0c2cc8a..7fbfddc 100644 --- a/payloads/libpayload/drivers/storage/Kconfig +++ b/payloads/libpayload/drivers/storage/Kconfig @@ -49,3 +49,10 @@ help If this option is selected, only AHCI controllers which are known to work will be used. + +config STORAGE_NVME + bool "Support for NVMe devices" + depends on STORAGE && PCI + default y + help + Select this option if you want support for NVMe devices. diff --git a/payloads/libpayload/drivers/storage/nvme.c b/payloads/libpayload/drivers/storage/nvme.c new file mode 100644 index 0000000..8d6f409 --- /dev/null +++ b/payloads/libpayload/drivers/storage/nvme.c @@ -0,0 +1,403 @@ +// SPDX-License-Identifier: BSD-3-Clause +/* + * Libpayload NVMe device driver + * Copyright (C) 2019 secunet Security Networks AG + */ + +#include <stdlib.h> +#include <stdint.h> +#include <stdio.h> +#include <pci.h> +#include <pci/pci.h> +#include <libpayload.h> +#include <storage/storage.h> +#include <storage/nvme.h> + +#define NVME_CC_EN (1 << 0) +#define NVME_CC_CSS (0 << 4) +#define NVME_CC_MPS (0 << 7) +#define NVME_CC_AMS (0 << 11) +#define NVME_CC_SHN (0 << 14) +#define NVME_CC_IOSQES (6 << 16) +#define NVME_CC_IOCQES (4 << 20) + +#define NVME_QUEUE_SIZE 2 +#define NVME_SQ_ENTRY_SIZE 64 +#define NVME_CQ_ENTRY_SIZE 16 + +struct nvme_dev { + storage_dev_t storage_dev; + + pcidev_t pci_dev; + void *config; + struct { + void *base; + uint32_t *bell; + uint16_t idx; // bool pos 0 or 1 + uint16_t round; // bool round 0 or 1+0xd + } queue[4]; + + uint64_t *prp_list; +}; + + +struct nvme_s_queue_entry { + uint32_t dw[16]; +}; + +struct nvme_c_queue_entry { + uint32_t dw[4]; +}; + +enum nvme_queue { + NVME_ADMIN_QUEUE = 0, + ads = 0, + adc = 1, + NVME_IO_QUEUE = 2, + ios = 2, + ioc = 3, +}; + +static storage_poll_t nvme_poll(struct storage_dev *dev) +{ + return POLL_MEDIUM_PRESENT; +} + +static int nvme_cmd( + struct nvme_dev *nvme, enum nvme_queue q, const struct nvme_s_queue_entry *cmd) +{ + int sq = q, cq = q+1; + + void *s_entry = nvme->queue[sq].base + (nvme->queue[sq].idx * NVME_SQ_ENTRY_SIZE); + memcpy(s_entry, cmd, NVME_SQ_ENTRY_SIZE); + nvme->queue[sq].idx = (nvme->queue[sq].idx + 1) & (NVME_QUEUE_SIZE - 1); + write32(nvme->queue[sq].bell, nvme->queue[sq].idx); + + struct nvme_c_queue_entry *c_entry = nvme->queue[cq].base + + (nvme->queue[cq].idx * NVME_CQ_ENTRY_SIZE); + while (((read32(&c_entry->dw[3]) >> 16) & 0x1) == nvme->queue[cq].round) + ; + nvme->queue[cq].idx = (nvme->queue[cq].idx + 1) & (NVME_QUEUE_SIZE - 1); + write32(nvme->queue[cq].bell, nvme->queue[cq].idx); + if (nvme->queue[cq].idx == 0) + nvme->queue[cq].round = (nvme->queue[cq].round + 1) & 1; + return c_entry->dw[3] >> 17; +} + +static int delete_io_submission_queue(struct nvme_dev *nvme) +{ + const struct nvme_s_queue_entry e = { + .dw[0] = 0, + .dw[10] = ios, + }; + + int res = nvme_cmd(nvme, NVME_ADMIN_QUEUE, &e); + + free(nvme->queue[ios].base); + nvme->queue[ios].base = NULL; + nvme->queue[ios].bell = NULL; + nvme->queue[ios].idx = 0; + return res; +} + +static int delete_io_completion_queue(struct nvme_dev *nvme) +{ + const struct nvme_s_queue_entry e = { + .dw[0] = 1, + .dw[10] = ioc, + }; + + int res = nvme_cmd(nvme, NVME_ADMIN_QUEUE, &e); + free(nvme->queue[ioc].base); + + nvme->queue[ioc].base = NULL; + nvme->queue[ioc].bell = NULL; + nvme->queue[ioc].idx = 0; + nvme->queue[ioc].round = 0; + return res; +} + +static int delete_admin_queues(struct nvme_dev *nvme) +{ + if (nvme->queue[ios].base || nvme->queue[ioc].base) + printf("NVMe ERROR: IO queues still active.\n"); + + free(nvme->queue[ads].base); + nvme->queue[ads].base = NULL; + nvme->queue[ads].bell = NULL; + nvme->queue[ads].idx = 0; + + free(nvme->queue[adc].base); + nvme->queue[adc].base = NULL; + nvme->queue[adc].bell = NULL; + nvme->queue[adc].idx = 0; + nvme->queue[adc].round = 0; + + return 0; +} + +static void nvme_detach_device(struct storage_dev *dev) +{ + struct nvme_dev *nvme = (struct nvme_dev *)dev; + + if (delete_io_submission_queue(nvme)) + printf("NVMe ERROR: Failed to delete io submission queue\n"); + if (delete_io_completion_queue(nvme)) + printf("NVME ERROR: Failed to delete io completion queue\n"); + if (delete_admin_queues(nvme)) + printf("NVME ERROR: Failed to delete admin queues\n"); + + write32(nvme->config + 0x1c, 0); + + int status, timeout = (read64(nvme->config) >> 24 & 0xff) * 500; + do { + status = read32(nvme->config + 0x1c) & 0x3; + if (status == 0x2) { + printf("NVMe ERROR: Failed to disable controller. FATAL ERROR\n"); + break; + } + if (timeout < 0) { + printf("NVMe ERROR: Failed to disable controller. Timeout.\n"); + break; + } + timeout -= 10; + mdelay(10); + } while (status != 0x0); + + uint16_t command = pci_read_config16(nvme->pci_dev, PCI_COMMAND); + pci_write_config16(nvme->pci_dev, PCI_COMMAND, command & ~PCI_COMMAND_MASTER); + + free(nvme->prp_list); +} + +static int nvme_read(struct nvme_dev *nvme, unsigned char *buffer, uint64_t base, uint16_t count) +{ + if (count == 0 || count > 512) + return -1; + + struct nvme_s_queue_entry e = { + .dw[0] = 0x02, + .dw[1] = 0x1, + .dw[6] = virt_to_phys(buffer), + .dw[10] = base, + .dw[11] = base >> 32, + .dw[12] = count - 1, + }; + + const unsigned int start_page = (uintptr_t)buffer >> 12; + const unsigned int end_page = ((uintptr_t)buffer + count * 512 - 1) >> 12; + if (end_page == start_page) { + /* No page crossing, PRP2 is reserved */ + } else if (end_page == start_page + 1) { + /* Crossing exactly one page boundary, PRP2 is second page */ + e.dw[8] = virt_to_phys(buffer + 0x1000) & ~0xfff; + } else { + /* Use a single page as PRP list, PRP2 points to the list */ + unsigned int i; + for (i = 0; i < end_page - start_page; ++i) { + buffer += 0x1000; + nvme->prp_list[i] = virt_to_phys(buffer) & ~0xfff; + } + e.dw[8] = virt_to_phys(nvme->prp_list); + } + + return nvme_cmd(nvme, ios, &e); +} + +static ssize_t nvme_read_blocks512( + struct storage_dev *const dev, + const lba_t start, const size_t count, unsigned char *const buf) +{ + unsigned int off = 0; + while (off < count) { + const unsigned int blocks = MIN(count - off, 512); + if (nvme_read((struct nvme_dev *)dev, buf + (off * 512), start + off, blocks)) + return off; + off += blocks; + } + return count; +} + +static int create_io_submission_queue(struct nvme_dev *nvme) +{ + void *sq_buffer = memalign(0x1000, NVME_SQ_ENTRY_SIZE * NVME_QUEUE_SIZE); + if (!sq_buffer) { + printf("NVMe ERROR: Failed to allocate memory for io submission queue.\n"); + return -1; + } + memset(sq_buffer, 0, NVME_SQ_ENTRY_SIZE * NVME_QUEUE_SIZE); + + struct nvme_s_queue_entry e = { + .dw[0] = 0x01, + .dw[6] = virt_to_phys(sq_buffer), + .dw[10] = ((NVME_QUEUE_SIZE - 1) << 16) | ios >> 1, + .dw[11] = (1 << 16) | 1, + }; + + int res = nvme_cmd(nvme, NVME_ADMIN_QUEUE, &e); + if (res) { + printf("NVMe ERROR: nvme_cmd returned with %i.\n", res); + free(sq_buffer); + return res; + } + + uint8_t cap_dstrd = (read64(nvme->config) >> 32) & 0xf; + nvme->queue[ios].base = sq_buffer; + nvme->queue[ios].bell = nvme->config + 0x1000 + (ios * (4 << cap_dstrd)); + nvme->queue[ios].idx = 0; + return 0; +} + +static int create_io_completion_queue(struct nvme_dev *nvme) +{ + void *const cq_buffer = memalign(0x1000, NVME_CQ_ENTRY_SIZE * NVME_QUEUE_SIZE); + if (!cq_buffer) { + printf("NVMe ERROR: Failed to allocate memory for io completion queue.\n"); + return -1; + } + memset(cq_buffer, 0, NVME_CQ_ENTRY_SIZE * NVME_QUEUE_SIZE); + + const struct nvme_s_queue_entry e = { + .dw[0] = 0x05, + .dw[6] = virt_to_phys(cq_buffer), + .dw[10] = ((NVME_QUEUE_SIZE - 1) << 16) | ioc >> 1, + .dw[11] = 1, + }; + + int res = nvme_cmd(nvme, NVME_ADMIN_QUEUE, &e); + if (res) { + printf("NVMe ERROR: nvme_cmd returned with %i.\n", res); + free(cq_buffer); + return res; + } + + uint8_t cap_dstrd = (read64(nvme->config) >> 32) & 0xf; + nvme->queue[ioc].base = cq_buffer; + nvme->queue[ioc].bell = nvme->config + 0x1000 + (ioc * (4 << cap_dstrd)); + nvme->queue[ioc].idx = 0; + nvme->queue[ioc].round = 0; + + return 0; +} + +static int create_admin_queues(struct nvme_dev *nvme) +{ + uint8_t cap_dstrd = (read64(nvme->config) >> 32) & 0xf; + write32(nvme->config + 0x24, (NVME_QUEUE_SIZE - 1) << 16 | (NVME_QUEUE_SIZE - 1)); + + void *sq_buffer = memalign(0x1000, NVME_SQ_ENTRY_SIZE * NVME_QUEUE_SIZE); + if (!sq_buffer) { + printf("NVMe ERROR: Failed to allocated memory for admin submission queue\n"); + return -1; + } + memset(sq_buffer, 0, NVME_SQ_ENTRY_SIZE * NVME_QUEUE_SIZE); + write64(nvme->config + 0x28, virt_to_phys(sq_buffer)); + + nvme->queue[ads].base = sq_buffer; + nvme->queue[ads].bell = nvme->config + 0x1000 + (ads * (4 << cap_dstrd)); + nvme->queue[ads].idx = 0; + + void *cq_buffer = memalign(0x1000, NVME_CQ_ENTRY_SIZE * NVME_QUEUE_SIZE); + if (!cq_buffer) { + printf("NVMe ERROR: Failed to allocate memory for admin completion queue\n"); + free(cq_buffer); + return -1; + } + memset(cq_buffer, 0, NVME_CQ_ENTRY_SIZE * NVME_QUEUE_SIZE); + write64(nvme->config + 0x30, virt_to_phys(cq_buffer)); + + nvme->queue[adc].base = cq_buffer; + nvme->queue[adc].bell = nvme->config + 0x1000 + (adc * (4 << cap_dstrd)); + nvme->queue[adc].idx = 0; + nvme->queue[adc].round = 0; + + return 0; +} + +static void nvme_init(pcidev_t dev) +{ + printf("NVMe init (Device %02x:%02x.%02x)\n", + PCI_BUS(dev), PCI_SLOT(dev), PCI_FUNC(dev)); + + void *pci_bar0 = phys_to_virt(pci_read_config32(dev, 0x10) & ~0x3ff); + + if (!(((read64(pci_bar0) >> 37) & 0xff) == 0x01)) { + printf("NVMe ERROR: PCIe device does not support the NVMe command set\n"); + return; + } + struct nvme_dev *nvme = malloc(sizeof(*nvme)); + if (!nvme) { + printf("NVMe ERROR: Failed to allocate buffer for nvme driver struct\n"); + return; + } + nvme->storage_dev.port_type = PORT_TYPE_NVME; + nvme->storage_dev.poll = nvme_poll; + nvme->storage_dev.read_blocks512 = nvme_read_blocks512; + nvme->storage_dev.write_blocks512 = NULL; + nvme->storage_dev.detach_device = nvme_detach_device; + nvme->pci_dev = dev; + nvme->config = pci_bar0; + nvme->prp_list = memalign(0x1000, 0x1000); + + if (!nvme->prp_list) { + printf("NVMe ERROR: Failed to allocate buffer for PRP list\n"); + goto abort; + } + + const uint32_t cc = NVME_CC_EN | NVME_CC_CSS | NVME_CC_MPS | NVME_CC_AMS | NVME_CC_SHN + | NVME_CC_IOSQES | NVME_CC_IOCQES; + + write32(nvme->config + 0x1c, 0); + + int status, timeout = (read64(nvme->config) >> 24 & 0xff) * 500; + do { + status = read32(nvme->config + 0x1c) & 0x3; + if (status == 0x2) { + printf("NVMe ERROR: Failed to disable controller. FATAL ERROR\n"); + goto abort; + } + if (timeout < 0) { + printf("NVMe ERROR: Failed to disable controller. Timeout.\n"); + goto abort; + } + timeout -= 10; + mdelay(10); + } while (status != 0x0); + if (create_admin_queues(nvme)) + goto abort; + write32(nvme->config + 0x14, cc); + + timeout = (read64(nvme->config) >> 24 & 0xff) * 500; + do { + status = read32(nvme->config + 0x1c) & 0x3; + if (status == 0x2) + goto abort; + if (timeout < 0) + goto abort; + timeout -= 10; + mdelay(10); + } while (status != 0x1); + + uint16_t command = pci_read_config16(dev, PCI_COMMAND); + pci_write_config16(dev, PCI_COMMAND, command | PCI_COMMAND_MASTER); + if (create_io_completion_queue(nvme)) + goto abort; + if (create_io_submission_queue(nvme)) + goto abort; + storage_attach_device((storage_dev_t *)nvme); + printf("NVMe init done.\n"); + return; + +abort: + printf("NVMe init failed.\n"); + delete_io_submission_queue(nvme); + delete_io_completion_queue(nvme); + delete_admin_queues(nvme); + free(nvme->prp_list); + free(nvme); +} + +void nvme_initialize(struct pci_dev *dev) +{ + nvme_init(PCI_DEV(dev->bus, dev->dev, dev->func)); +} diff --git a/payloads/libpayload/drivers/storage/storage.c b/payloads/libpayload/drivers/storage/storage.c index 4b585ba..a1c1b45 100644 --- a/payloads/libpayload/drivers/storage/storage.c +++ b/payloads/libpayload/drivers/storage/storage.c @@ -29,6 +29,7 @@ #include <libpayload.h> #include <pci/pci.h> #include <storage/ahci.h> +#include <storage/nvme.h> #include <storage/storage.h>
static storage_dev_t **devices = NULL; @@ -116,6 +117,11 @@ ahci_initialize(dev); break; #endif +#if CONFIG(LP_STORAGE_NVME) + case PCI_CLASS_STORAGE_NVME: + nvme_initialize(dev); + break; +#endif default: break; } diff --git a/payloads/libpayload/include/pci/pci.h b/payloads/libpayload/include/pci/pci.h index 5e21060..d9ce755 100644 --- a/payloads/libpayload/include/pci/pci.h +++ b/payloads/libpayload/include/pci/pci.h @@ -67,6 +67,7 @@ #define PCI_ROM_ADDRESS_MASK ~0x7ff
#define PCI_CLASS_STORAGE_AHCI 0x0106 +#define PCI_CLASS_STORAGE_NVME 0x0108 #define PCI_CLASS_MEMORY_OTHER 0x0580
#define PCI_VENDOR_ID_INTEL 0x8086 diff --git a/payloads/libpayload/include/storage/nvme.h b/payloads/libpayload/include/storage/nvme.h new file mode 100644 index 0000000..61abf74 --- /dev/null +++ b/payloads/libpayload/include/storage/nvme.h @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: BSD-3-Clause +/* + * Libpayload NVMe device driver + * Copyright (C) 2019 secunet Security Networks AG + */ + +#ifndef _STORAGE_NVME_H +#define _STORAGE_NVME_H + +#include "storage.h" + +void nvme_initialize(struct pci_dev *dev); + +#endif /* _STORAGE_NVME_H */ diff --git a/payloads/libpayload/include/storage/storage.h b/payloads/libpayload/include/storage/storage.h index 32933fd..78cfd69 100644 --- a/payloads/libpayload/include/storage/storage.h +++ b/payloads/libpayload/include/storage/storage.h @@ -42,6 +42,7 @@ PORT_TYPE_IDE = (1 << 0), PORT_TYPE_SATA = (1 << 1), PORT_TYPE_USB = (1 << 2), + PORT_TYPE_NVME = (1 << 3), } storage_port_t;
typedef enum {
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
Felix Singer (Code Review)
-
Nico Huber (Code Review)
-
Paul Menzel (Code Review)
-
Stefan Reinauer (Code Review)
-
Thomas Heijligen (Code Review)