[PATCH v2 1/1] block/crypto: better error message when creating too large files

2020-04-20 Thread Maxim Levitsky
, detect -EFBIG and in this case present a better error message, which is what this patch does The new error message is: qemu-img: error creating test.luks: The requested file size is too large Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1534898 Signed-off-by: Maxim Levitsky Reviewed

[PATCH v2 0/1] LUKS: Fix error message when underlying fs don't support large enough files

2020-04-20 Thread Maxim Levitsky
This is a repost of the same trivial patch I already, which fell through the cracks. Could someone queue it up so I close the bugzilla I have for this? V2: fixed usage of local_err Best regards, Maxim Levitsky Maxim Levitsky (1): block/crypto: better error message when creating too

Re: [PATCH 1/1] block/crypto: better error message when creating too large files

2020-04-20 Thread Maxim Levitsky
On Mon, 2020-04-20 at 11:02 +0200, Kevin Wolf wrote: > Am 16.04.2020 um 11:50 hat Maxim Levitsky geschrieben: > > Currently if you attampt to create too large file with luks you > > get the following error message: > > > > Formatting 'test.luks', fmt=luks size=175

Re: [PATCH 0/1] LUKS: Fix error message when underlying fs don't support large enough files

2020-04-16 Thread Maxim Levitsky
On Thu, 2020-04-16 at 12:50 +0300, Maxim Levitsky wrote: > This is a repost of the same trivial patch I already, which fell through the > cracks. > Could someone queue it up so I close the bugzilla I have for this? I a word - need more coffee :-) I mean I already posted this patch few m

[PATCH 1/1] block/crypto: better error message when creating too large files

2020-04-16 Thread Maxim Levitsky
, detect -EFBIG and in this case present a better error message, which is what this patch does The new error message is: qemu-img: error creating test.luks: The requested file size is too large Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1534898 Signed-off-by: Maxim Levitsky Reviewed-by: Daniel

[PATCH 0/1] LUKS: Fix error message when underlying fs don't support large enough files

2020-04-16 Thread Maxim Levitsky
This is a repost of the same trivial patch I already, which fell through the cracks. Could someone queue it up so I close the bugzilla I have for this? Best regards, Maxim Levitsky Maxim Levitsky (1): block/crypto: better error message when creating too large files block/crypto.c

Re: [PATCH v6 12/42] nvme: add support for the get log page command

2020-03-31 Thread Maxim Levitsky
On Tue, 2020-03-31 at 14:49 +0200, Klaus Birkelund Jensen wrote: > On Mar 31 12:45, Maxim Levitsky wrote: > > On Tue, 2020-03-31 at 07:41 +0200, Klaus Birkelund Jensen wrote: > > > On Mar 25 12:40, Maxim Levitsky wrote: > > > > On Mon, 2020-03-16 at 07:28 -0700, Klau

Re: [PATCH v6 07/42] nvme: refactor nvme_addr_read

2020-03-31 Thread Maxim Levitsky
On Tue, 2020-03-31 at 14:48 +0200, Klaus Birkelund Jensen wrote: > On Mar 31 13:41, Maxim Levitsky wrote: > > On Tue, 2020-03-31 at 07:39 +0200, Klaus Birkelund Jensen wrote: > > > On Mar 25 12:38, Maxim Levitsky wrote: > > > > Note that this patch still contains a

Re: [PATCH v6 04/42] nvme: bump spec data structures to v1.3

2020-03-31 Thread Maxim Levitsky
On Tue, 2020-03-31 at 07:38 +0200, Klaus Birkelund Jensen wrote: > On Mar 25 12:37, Maxim Levitsky wrote: > > On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > > > From: Klaus Jensen > > > > > > Add missing fields in the Identify Controller and Iden

Re: [PATCH v6 07/42] nvme: refactor nvme_addr_read

2020-03-31 Thread Maxim Levitsky
On Tue, 2020-03-31 at 07:39 +0200, Klaus Birkelund Jensen wrote: > On Mar 25 12:38, Maxim Levitsky wrote: > > On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > > > From: Klaus Jensen > > > > > > Pull the controller memory buffer check to its own fu

Re: [PATCH v6 09/42] nvme: add max_ioqpairs device parameter

2020-03-31 Thread Maxim Levitsky
On Tue, 2020-03-31 at 07:40 +0200, Klaus Birkelund Jensen wrote: > On Mar 25 12:39, Maxim Levitsky wrote: > > On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > > > From: Klaus Jensen > > > > > > The num_queues device paramater has a slightly confusi

Re: [PATCH v6 11/42] nvme: add temperature threshold feature

2020-03-31 Thread Maxim Levitsky
On Tue, 2020-03-31 at 07:40 +0200, Klaus Birkelund Jensen wrote: > On Mar 25 12:40, Maxim Levitsky wrote: > > On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > > > From: Klaus Jensen > > > > > > It might seem wierd to implement

Re: [PATCH v6 12/42] nvme: add support for the get log page command

2020-03-31 Thread Maxim Levitsky
On Tue, 2020-03-31 at 07:41 +0200, Klaus Birkelund Jensen wrote: > On Mar 25 12:40, Maxim Levitsky wrote: > > On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > > > From: Klaus Jensen > > > > > > Add support for the Get Log Page command and basic im

Re: [PATCH v6 14/42] nvme: add missing mandatory features

2020-03-31 Thread Maxim Levitsky
On Tue, 2020-03-31 at 07:41 +0200, Klaus Birkelund Jensen wrote: > On Mar 25 12:41, Maxim Levitsky wrote: > > On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > > > From: Klaus Jensen > > > > > > Add support for returning a resonable response to Get/Set

Re: [PATCH v6 19/42] nvme: enforce valid queue creation sequence

2020-03-31 Thread Maxim Levitsky
On Tue, 2020-03-31 at 07:41 +0200, Klaus Birkelund Jensen wrote: > On Mar 25 12:43, Maxim Levitsky wrote: > > On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > > > From: Klaus Jensen > > > > > > Support returning Command Sequence Error if Set Features

Re: [PATCH v6 23/42] nvme: add mapping helpers

2020-03-31 Thread Maxim Levitsky
On Tue, 2020-03-31 at 07:44 +0200, Klaus Birkelund Jensen wrote: > On Mar 25 12:45, Maxim Levitsky wrote: > > On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > > > From: Klaus Jensen > > > > > > Add nvme_map_addr, nvme_map_addr_cmb and nvme_ad

Re: [PATCH v6 24/42] nvme: remove redundant has_sg member

2020-03-31 Thread Maxim Levitsky
On Tue, 2020-03-31 at 07:44 +0200, Klaus Birkelund Jensen wrote: > On Mar 25 12:45, Maxim Levitsky wrote: > > On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > > > From: Klaus Jensen > > > > > > Remove the has_sg member from NvmeRequest since i

Re: [PATCH v6 29/42] nvme: refactor request bounds checking

2020-03-31 Thread Maxim Levitsky
On Tue, 2020-03-31 at 07:44 +0200, Klaus Birkelund Jensen wrote: > On Mar 25 12:56, Maxim Levitsky wrote: > > On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > > > From: Klaus Jensen > > > > > > Signed-off-by: Klaus Jensen &

Re: [PATCH v6 31/42] nvme: add check for prinfo

2020-03-31 Thread Maxim Levitsky
On Tue, 2020-03-31 at 07:45 +0200, Klaus Birkelund Jensen wrote: > On Mar 25 12:57, Maxim Levitsky wrote: > > On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > > > From: Klaus Jensen > > > > > > Check the validity of the PRINFO field. >

Re: [PATCH v6 32/42] nvme: allow multiple aios per command

2020-03-31 Thread Maxim Levitsky
On Tue, 2020-03-31 at 07:47 +0200, Klaus Birkelund Jensen wrote: > On Mar 25 12:57, Maxim Levitsky wrote: > > On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > > > From: Klaus Jensen > > > > > > This refactors how the device issues async

Re: [PATCH v6 36/42] nvme: add support for scatter gather lists

2020-03-31 Thread Maxim Levitsky
On Tue, 2020-03-31 at 07:48 +0200, Klaus Birkelund Jensen wrote: > On Mar 25 12:58, Maxim Levitsky wrote: > > On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > > > From: Klaus Jensen > > > > > > For now, support the Data Block, Segment and

Re: [PATCH v6 38/42] nvme: support multiple namespaces

2020-03-31 Thread Maxim Levitsky
On Tue, 2020-03-31 at 07:48 +0200, Klaus Birkelund Jensen wrote: > On Mar 25 12:59, Maxim Levitsky wrote: > > On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > > > From: Klaus Jensen > > > > > > This adds support for multiple namespaces by introducin

Re: [PATCH 2/2] block: trickle down the fallback image creation function use to the block drivers

2020-03-26 Thread Maxim Levitsky
On Thu, 2020-03-26 at 08:20 -0500, Eric Blake wrote: > On 3/25/20 8:12 PM, Maxim Levitsky wrote: > > Instead of checking the .bdrv_co_create_opts to see if we need the failback, > > fallback 100% true. > > > just implement the .bdrv_co_create_opts in the drivers that ne

Re: [PATCH 1/2] block: pass BlockDriver reference to the .bdrv_co_create

2020-03-26 Thread Maxim Levitsky
On Thu, 2020-03-26 at 08:18 -0500, Eric Blake wrote: > On 3/25/20 8:12 PM, Maxim Levitsky wrote: > > This will allow to reuse a single generic .bdrv_co_create > > "allow to ${verb}" is not idiomatic, better is "allow ${subject} to > ${verb}" or &qu

[PATCH 2/2] block: trickle down the fallback image creation function use to the block drivers

2020-03-25 Thread Maxim Levitsky
it is better to leave it not supported as it was prior to generic image creation patches. Also drop iscsi_create_opts which was left accidently Signed-off-by: Maxim Levitsky --- block.c | 35 --- block/file-posix.c| 7 ++- block/iscsi.c

[PATCH 0/2] Fix the generic image creation code

2020-03-25 Thread Maxim Levitsky
if failback was used or not. Best regards, Maxim Levitsky Maxim Levitsky (2): block: pass BlockDriver reference to the .bdrv_co_create block: trickle down the fallback image creation function use to the block drivers block.c | 38

[PATCH 1/2] block: pass BlockDriver reference to the .bdrv_co_create

2020-03-25 Thread Maxim Levitsky
This will allow to reuse a single generic .bdrv_co_create implementation for several drivers. No functional changes. Signed-off-by: Maxim Levitsky --- block.c | 3 ++- block/crypto.c| 3 ++- block/file-posix.c| 4 +++- block/file-win32.c| 4

Re: [PATCH v6 38/42] nvme: support multiple namespaces

2020-03-25 Thread Maxim Levitsky
ize; > uint32_tnum_namespaces; > uint32_tmax_q_ents; > -uint64_tns_size; > uint8_t outstanding_aers; > uint8_t *cmbuf; > uint64_tirq_status; > @@ -219,7 +209,8 @@ typedef struct NvmeCtrl { > QTAILQ_HEAD(, NvmeAsyncEvent) aer_queue; > int aer_queued; > > -NvmeNamespace *namespaces; > +NvmeNamespace namespace; > +NvmeNamespace *namespaces[NVME_MAX_NAMESPACES]; > NvmeSQueue **sq; > NvmeCQueue **cq; > NvmeSQueue admin_sq; > @@ -228,9 +219,13 @@ typedef struct NvmeCtrl { > NvmeFeatureVal features; > } NvmeCtrl; > > -static inline uint64_t nvme_ns_nlbas(NvmeCtrl *n, NvmeNamespace *ns) > +static inline NvmeNamespace *nvme_ns(NvmeCtrl *n, uint32_t nsid) > { > -return n->ns_size >> nvme_ns_lbads(ns); > +if (!nsid || nsid > n->num_namespaces) { > +return NULL; > +} > + > +return n->namespaces[nsid - 1]; > } > > static inline uint16_t nvme_cid(NvmeRequest *req) > @@ -253,4 +248,6 @@ static inline NvmeCtrl *nvme_ctrl(NvmeRequest *req) > return req->sq->ctrl; > } > > +int nvme_register_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp); > + > #endif /* HW_NVME_H */ > diff --git a/hw/block/trace-events b/hw/block/trace-events > index 70702cc67d5a..3d907eaf0800 100644 > --- a/hw/block/trace-events > +++ b/hw/block/trace-events > @@ -29,6 +29,7 @@ hd_geometry_guess(void *blk, uint32_t cyls, uint32_t heads, > uint32_t secs, int t > > # nvme.c > # nvme traces for successful events > +nvme_dev_register_namespace(uint32_t nsid) "nsid %"PRIu32"" > nvme_dev_irq_msix(uint32_t vector) "raising MSI-X IRQ vector %u" > nvme_dev_irq_pin(void) "pulsing IRQ pin" > nvme_dev_irq_masked(void) "IRQ is masked" > @@ -38,7 +39,7 @@ nvme_dev_map_sgl(uint16_t cid, uint8_t typ, uint32_t nlb, > uint64_t len) "cid %"P > nvme_dev_req_register_aio(uint16_t cid, void *aio, const char *blkname, > uint64_t offset, uint64_t count, const char *opc, void *req) "cid %"PRIu16" > aio %p blk \"%s\" offset %"PRIu64" count > %"PRIu64" opc \"%s\" req %p" > nvme_dev_aio_cb(uint16_t cid, void *aio, const char *blkname, uint64_t > offset, const char *opc, void *req) "cid %"PRIu16" aio %p blk \"%s\" offset > %"PRIu64" opc \"%s\" req %p" > nvme_dev_io_cmd(uint16_t cid, uint32_t nsid, uint16_t sqid, uint8_t opcode) > "cid %"PRIu16" nsid %"PRIu32" sqid %"PRIu16" opc 0x%"PRIx8"" > -nvme_dev_rw(const char *verb, uint32_t blk_count, uint64_t byte_count, > uint64_t lba) "%s %"PRIu32" blocks (%"PRIu64" bytes) from LBA %"PRIu64"" > +nvme_dev_rw(uint16_t cid, const char *verb, uint32_t nsid, uint32_t nlb, > uint64_t count, uint64_t lba) "cid %"PRIu16" %s nsid %"PRIu32" nlb %"PRIu32" > count %"PRIu64" lba 0x%"PRIx64"" > nvme_dev_rw_cb(uint16_t cid, uint32_t nsid) "cid %"PRIu16" nsid %"PRIu32"" > nvme_dev_write_zeroes(uint16_t cid, uint32_t nsid, uint64_t slba, uint32_t > nlb) "cid %"PRIu16" nsid %"PRIu32" slba %"PRIu64" nlb %"PRIu32"" > nvme_dev_create_sq(uint64_t addr, uint16_t sqid, uint16_t cqid, uint16_t > qsize, uint16_t qflags) "create submission queue, addr=0x%"PRIx64", > sqid=%"PRIu16", cqid=%"PRIu16", qsize=%"PRIu16", > qflags=%"PRIu16"" > @@ -98,7 +99,6 @@ nvme_dev_err_invalid_prplist_ent(uint64_t prplist) "PRP > list entry is null or no > nvme_dev_err_invalid_prp2_align(uint64_t prp2) "PRP2 is not page aligned: > 0x%"PRIx64"" > nvme_dev_err_invalid_prp2_missing(void) "PRP2 is null and more data to be > transferred" > nvme_dev_err_invalid_prp(void) "invalid PRP" > -nvme_dev_err_invalid_ns(uint32_t ns, uint32_t limit) "invalid namespace %u > not within 1-%u" > nvme_dev_err_invalid_opc(uint8_t opc) "invalid opcode 0x%"PRIx8"" > nvme_dev_err_invalid_admin_opc(uint8_t opc) "invalid admin opcode 0x%"PRIx8"" > nvme_dev_err_invalid_lba_range(uint64_t start, uint64_t len, uint64_t limit) > "Invalid LBA start=%"PRIu64" len=%"PRIu64" limit=%"PRIu64"" Best regards, Maxim Levitsky

Re: [PATCH v6 35/42] nvme: handle dma errors

2020-03-25 Thread Maxim Levitsky
_write(uint64_t addr) "addr 0x%"PRIx64"" > nvme_dev_err_invalid_dma(void) "PRP/SGL is too small for transfer size" > nvme_dev_err_invalid_prplist_ent(uint64_t prplist) "PRP list entry is null > or not page aligned: 0x%"PRIx64"" > nvme_dev_err_invalid_prp2_align(uint64_t prp2) "PRP2 is not page aligned: > 0x%"PRIx64"" > diff --git a/include/block/nvme.h b/include/block/nvme.h > index 293d68553538..d1ccde4cda4b 100644 > --- a/include/block/nvme.h > +++ b/include/block/nvme.h > @@ -458,7 +458,7 @@ enum NvmeStatusCodes { > NVME_INVALID_OPCODE = 0x0001, > NVME_INVALID_FIELD = 0x0002, > NVME_CID_CONFLICT = 0x0003, > -NVME_DATA_TRAS_ERROR= 0x0004, > +NVME_DATA_TRANSFER_ERROR= 0x0004, > NVME_POWER_LOSS_ABORT = 0x0005, > NVME_INTERNAL_DEV_ERROR = 0x0006, > NVME_CMD_ABORT_REQ = 0x0007, Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v6 29/42] nvme: refactor request bounds checking

2020-03-25 Thread Maxim Levitsky
return status; > } > > if (nvme_map(n, cmd, >qsg, >iov, data_size, req)) { Looks good as well, once we get support for discard, it will use this as well, but for now indeed only write zeros and read/write need bounds checking on the IO path. Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v6 32/42] nvme: allow multiple aios per command

2020-03-25 Thread Maxim Levitsky
nvme_status_is_error(uint16_t status, uint16_t err) > +{ > +/* strip DNR and MORE */ > +return (status & 0xfff) == err; > +} > + > +static inline NvmeCtrl *nvme_ctrl(NvmeRequest *req) > +{ > +return req->sq->ctrl; > +} > + > #endif /* HW_NVME_H */ > diff --git a/hw/block/trace-events b/hw/block/trace-events > index 2aceb0537e05..aa449e314818 100644 > --- a/hw/block/trace-events > +++ b/hw/block/trace-events > @@ -34,7 +34,12 @@ nvme_dev_irq_pin(void) "pulsing IRQ pin" > nvme_dev_irq_masked(void) "IRQ is masked" > nvme_dev_dma_read(uint64_t prp1, uint64_t prp2) "DMA read, prp1=0x%"PRIx64" > prp2=0x%"PRIx64"" > nvme_dev_map_prp(uint16_t cid, uint64_t trans_len, uint32_t len, uint64_t > prp1, uint64_t prp2, int num_prps) "cid %"PRIu16" trans_len %"PRIu64" len > %"PRIu32" prp1 0x%"PRIx64" prp2 0x%"PRIx64" > num_prps %d" > +nvme_dev_req_register_aio(uint16_t cid, void *aio, const char *blkname, > uint64_t offset, uint64_t count, const char *opc, void *req) "cid %"PRIu16" > aio %p blk \"%s\" offset %"PRIu64" count > %"PRIu64" opc \"%s\" req %p" > +nvme_dev_aio_cb(uint16_t cid, void *aio, const char *blkname, uint64_t > offset, const char *opc, void *req) "cid %"PRIu16" aio %p blk \"%s\" offset > %"PRIu64" opc \"%s\" req %p" > +nvme_dev_io_cmd(uint16_t cid, uint32_t nsid, uint16_t sqid, uint8_t opcode) > "cid %"PRIu16" nsid %"PRIu32" sqid %"PRIu16" opc 0x%"PRIx8"" > nvme_dev_rw(const char *verb, uint32_t blk_count, uint64_t byte_count, > uint64_t lba) "%s %"PRIu32" blocks (%"PRIu64" bytes) from LBA %"PRIu64"" > +nvme_dev_rw_cb(uint16_t cid, uint32_t nsid) "cid %"PRIu16" nsid %"PRIu32"" > +nvme_dev_write_zeroes(uint16_t cid, uint32_t nsid, uint64_t slba, uint32_t > nlb) "cid %"PRIu16" nsid %"PRIu32" slba %"PRIu64" nlb %"PRIu32"" > nvme_dev_create_sq(uint64_t addr, uint16_t sqid, uint16_t cqid, uint16_t > qsize, uint16_t qflags) "create submission queue, addr=0x%"PRIx64", > sqid=%"PRIu16", cqid=%"PRIu16", qsize=%"PRIu16", > qflags=%"PRIu16"" > nvme_dev_create_cq(uint64_t addr, uint16_t cqid, uint16_t vector, uint16_t > size, uint16_t qflags, int ien) "create completion queue, addr=0x%"PRIx64", > cqid=%"PRIu16", vector=%"PRIu16", > qsize=%"PRIu16", qflags=%"PRIu16", ien=%d" > nvme_dev_del_sq(uint16_t qid) "deleting submission queue sqid=%"PRIu16"" > @@ -81,6 +86,7 @@ nvme_dev_mmio_doorbell_sq(uint16_t sqid, uint16_t new_tail) > "cqid %"PRIu16" new_ > # nvme traces for error conditions > nvme_dev_err_mdts(uint16_t cid, size_t mdts, size_t len) "cid %"PRIu16" mdts > %"PRIu64" len %"PRIu64"" > nvme_dev_err_prinfo(uint16_t cid, uint16_t ctrl) "cid %"PRIu16" ctrl > %"PRIu16"" > +nvme_dev_err_aio(uint16_t cid, void *aio, const char *blkname, uint64_t > offset, const char *opc, void *req, uint16_t status) "cid %"PRIu16" aio %p > blk \"%s\" offset %"PRIu64" opc \"%s\" req %p > status 0x%"PRIx16"" > nvme_dev_err_invalid_dma(void) "PRP/SGL is too small for transfer size" > nvme_dev_err_invalid_prplist_ent(uint64_t prplist) "PRP list entry is null > or not page aligned: 0x%"PRIx64"" > nvme_dev_err_invalid_prp2_align(uint64_t prp2) "PRP2 is not page aligned: > 0x%"PRIx64"" The patch is still too large IMHO to review properly and few things can be split from it. I tried my best to review it but I might have missed something. Best regards, Maxim Levitsky

Re: [PATCH v6 42/42] nvme: make lba data size configurable

2020-03-25 Thread Maxim Levitsky
624,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error > **errp) > if (n->namespace.blk) { > ns = >namespace; > ns->params.nsid = 1; > +ns->params.lbads = BDRV_SECTOR_BITS; > > if (nvme_ns_setup(n, ns, errp)) { > return; Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v6 37/42] nvme: refactor identify active namespace id list

2020-03-25 Thread Maxim Levitsky
i = 1; i <= n->num_namespaces; i++) { > +if (i <= min_nsid) { > continue; > } > -list[j++] = cpu_to_le32(i + 1); > +list[j++] = cpu_to_le32(i); > if (j == data_len / sizeof(uint32_t)) { > break; > } Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v6 36/42] nvme: add support for scatter gather lists

2020-03-25 Thread Maxim Levitsky
, uint64_t trans_len, uint32_t len, uint64_t > prp1, uint64_t prp2, int num_prps) "cid %"PRIu16" trans_len %"PRIu64" len > %"PRIu32" prp1 0x%"PRIx64" prp2 0x%"PRIx64" > num_prps %d" > +nvme_dev_map_sgl(uint16_t cid, uint8_t typ, uint32_t nlb, uint64_t len) "cid > %"PRIu16" type 0x%"PRIx8" nlb %"PRIu32" len %"PRIu64"" > nvme_dev_req_register_aio(uint16_t cid, void *aio, const char *blkname, > uint64_t offset, uint64_t count, const char *opc, void *req) "cid %"PRIu16" > aio %p blk \"%s\" offset %"PRIu64" count > %"PRIu64" opc \"%s\" req %p" > nvme_dev_aio_cb(uint16_t cid, void *aio, const char *blkname, uint64_t > offset, const char *opc, void *req) "cid %"PRIu16" aio %p blk \"%s\" offset > %"PRIu64" opc \"%s\" req %p" > nvme_dev_io_cmd(uint16_t cid, uint32_t nsid, uint16_t sqid, uint8_t opcode) > "cid %"PRIu16" nsid %"PRIu32" sqid %"PRIu16" opc 0x%"PRIx8"" > @@ -89,6 +90,9 @@ nvme_dev_err_prinfo(uint16_t cid, uint16_t ctrl) "cid > %"PRIu16" ctrl %"PRIu16"" > nvme_dev_err_aio(uint16_t cid, void *aio, const char *blkname, uint64_t > offset, const char *opc, void *req, uint16_t status) "cid %"PRIu16" aio %p > blk \"%s\" offset %"PRIu64" opc \"%s\" req %p > status 0x%"PRIx16"" > nvme_dev_err_addr_read(uint64_t addr) "addr 0x%"PRIx64"" > nvme_dev_err_addr_write(uint64_t addr) "addr 0x%"PRIx64"" > +nvme_dev_err_invalid_sgld(uint16_t cid, uint8_t typ) "cid %"PRIu16" type > 0x%"PRIx8"" > +nvme_dev_err_invalid_num_sgld(uint16_t cid, uint8_t typ) "cid %"PRIu16" type > 0x%"PRIx8"" > +nvme_dev_err_invalid_sgl_excess_length(uint16_t cid) "cid %"PRIu16"" > nvme_dev_err_invalid_dma(void) "PRP/SGL is too small for transfer size" > nvme_dev_err_invalid_prplist_ent(uint64_t prplist) "PRP list entry is null > or not page aligned: 0x%"PRIx64"" > nvme_dev_err_invalid_prp2_align(uint64_t prp2) "PRP2 is not page aligned: > 0x%"PRIx64"" Best regards, Maxim Levitsky

Re: [PATCH v6 31/42] nvme: add check for prinfo

2020-03-25 Thread Maxim Levitsky
4" len %"PRIu64"" > +nvme_dev_err_prinfo(uint16_t cid, uint16_t ctrl) "cid %"PRIu16" ctrl > %"PRIu16"" > nvme_dev_err_invalid_dma(void) "PRP/SGL is too small for transfer size" > nvme_dev_err_invalid_prplist_ent(uint64_t prplist) "PRP list entry is null > or not page aligned: 0x%"PRIx64"" > nvme_dev_err_invalid_prp2_align(uint64_t prp2) "PRP2 is not page aligned: > 0x%"PRIx64"" > diff --git a/include/block/nvme.h b/include/block/nvme.h > index ecc02fbe8bb8..293d68553538 100644 > --- a/include/block/nvme.h > +++ b/include/block/nvme.h > @@ -394,6 +394,7 @@ enum { > NVME_RW_PRINFO_PRCHK_GUARD = 1 << 12, > NVME_RW_PRINFO_PRCHK_APP= 1 << 11, > NVME_RW_PRINFO_PRCHK_REF= 1 << 10, > +NVME_RW_PRINFO_PRCHK_MASK = 7 << 10, > }; > > typedef struct NvmeDsmCmd { Best regards, Maxim Levitsky

Re: [PATCH v6 24/42] nvme: remove redundant has_sg member

2020-03-25 Thread Maxim Levitsky
ame patch, but that should be mentioned in the commit message With this fixed, Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky > > Signed-off-by: Klaus Jensen > --- > hw/block/nvme.c | 18 -- > hw/block/nvme.h | 1 - > 2 files changed, 12

Re: [PATCH v6 33/42] nvme: use preallocated qsg/iov in nvme_dma_prp

2020-03-25 Thread Maxim Levitsky
-off-by: Klaus Jensen > Acked-by: Keith Busch > Reviewed-by: Maxim Levitsky > --- > hw/block/nvme.c | 18 ++ > 1 file changed, 6 insertions(+), 12 deletions(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index 817384e3b1a9..15ca2417af04 100644 &g

Re: [PATCH v6 30/42] nvme: add check for mdts

2020-03-25 Thread Maxim Levitsky
k/trace-events b/hw/block/trace-events > index e31e652fa04e..2df6aa38df1b 100644 > --- a/hw/block/trace-events > +++ b/hw/block/trace-events > @@ -79,6 +79,7 @@ nvme_dev_mmio_doorbell_cq(uint16_t cqid, uint16_t new_head) > "cqid %"PRIu16" new_ > nvme_dev_mmio_doorbell_sq(uint16_t sqid,

Re: [PATCH v6 28/42] nvme: verify validity of prp lists in the cmb

2020-03-25 Thread Maxim Levitsky
>bar.cmbsz, 1); > NVME_CMBSZ_SET_CQS(n->bar.cmbsz, 0); > - NVME_CMBSZ_SET_LISTS(n->bar.cmbsz, 0); > +NVME_CMBSZ_SET_LISTS(n->bar.cmbsz, 1); > NVME_CMBSZ_SET_RDS(n->bar.cmbsz, 1); > NVME_CMBSZ_SET_WDS(n->bar.cmbsz, 1); > NVME_CMBSZ_SET_SZU(n->bar.cmbsz, 2); Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v6 27/42] nvme: add request mapping helper

2020-03-25 Thread Maxim Levitsky
>qsg, >iov, prp1, prp2, data_size, req)) { > +if (nvme_map(n, cmd, >qsg, >iov, data_size, req)) { > block_acct_invalid(blk_get_stats(n->conf.blk), acct); > return NVME_INVALID_FIELD | NVME_DNR; > } Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v6 23/42] nvme: add mapping helpers

2020-03-25 Thread Maxim Levitsky
ev_irq_pin(void) "pulsing IRQ pin" > nvme_dev_irq_masked(void) "IRQ is masked" > nvme_dev_dma_read(uint64_t prp1, uint64_t prp2) "DMA read, prp1=0x%"PRIx64" > prp2=0x%"PRIx64"" > +nvme_dev_map_prp(uint16_t cid, uint8_t opc, uint64_t trans_len, uint32_t > len, uint64_t prp1, uint64_t prp2, int num_prps) "cid %"PRIu16" opc > 0x%"PRIx8" trans_len %"PRIu64" len %"PRIu32" prp1 > 0x%"PRIx64" prp2 0x%"PRIx64" num_prps %d" > nvme_dev_rw(const char *verb, uint32_t blk_count, uint64_t byte_count, > uint64_t lba) "%s %"PRIu32" blocks (%"PRIu64" bytes) from LBA %"PRIu64"" > nvme_dev_create_sq(uint64_t addr, uint16_t sqid, uint16_t cqid, uint16_t > qsize, uint16_t qflags) "create submission queue, addr=0x%"PRIx64", > sqid=%"PRIu16", cqid=%"PRIu16", qsize=%"PRIu16", > qflags=%"PRIu16"" > nvme_dev_create_cq(uint64_t addr, uint16_t cqid, uint16_t vector, uint16_t > size, uint16_t qflags, int ien) "create completion queue, addr=0x%"PRIx64", > cqid=%"PRIu16", vector=%"PRIu16", > qsize=%"PRIu16", qflags=%"PRIu16", ien=%d" Looks very good overall, Best regards, Maxim Levitsky

Re: [PATCH v6 26/42] nvme: pass request along for tracing

2020-03-25 Thread Maxim Levitsky
> len, uint64_t prp1, uint64_t prp2, int num_prps) "cid %"PRIu16" opc > 0x%"PRIx8" trans_len %"PRIu64" len %"PRIu32" prp1 > 0x%"PRIx64" prp2 0x%"PRIx64" num_prps %d" > +nvme_dev_map_prp(uint16_t cid, uint64_t trans_len, uint32_t len, uint64_t > prp1, uint64_t prp2, int num_prps) "cid %"PRIu16" trans_len %"PRIu64" len > %"PRIu32" prp1 0x%"PRIx64" prp2 0x%"PRIx64" > num_prps %d" > nvme_dev_rw(const char *verb, uint32_t blk_count, uint64_t byte_count, > uint64_t lba) "%s %"PRIu32" blocks (%"PRIu64" bytes) from LBA %"PRIu64"" > nvme_dev_create_sq(uint64_t addr, uint16_t sqid, uint16_t cqid, uint16_t > qsize, uint16_t qflags) "create submission queue, addr=0x%"PRIx64", > sqid=%"PRIu16", cqid=%"PRIu16", qsize=%"PRIu16", > qflags=%"PRIu16"" > nvme_dev_create_cq(uint64_t addr, uint16_t cqid, uint16_t vector, uint16_t > size, uint16_t qflags, int ien) "create completion queue, addr=0x%"PRIx64", > cqid=%"PRIu16", vector=%"PRIu16", > qsize=%"PRIu16", qflags=%"PRIu16", ien=%d" Another very easy to review patch Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v6 21/42] nvme: bump supported version to v1.3

2020-03-25 Thread Maxim Levitsky
> /* > @@ -1942,7 +1944,7 @@ static void nvme_init_ctrl(NvmeCtrl *n) > NVME_CAP_SET_CSS(n->bar.cap, 1); > NVME_CAP_SET_MPSMAX(n->bar.cap, 4); > > - n->bar.vs = 0x00010200; > + n->bar.vs = NVME_SPEC_VER; > n->bar.intmc = n->bar.intms = 0; > } > Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v6 20/42] nvme: provide the mandatory subnqn field

2020-03-25 Thread Maxim Levitsky
id->subnqn, sizeof(id->subnqn), > "nqn.2019-08.org.qemu:"); > +pstrcat((char *) id->subnqn, sizeof(id->subnqn), n->params.serial); > + > id->psd[0].mp = cpu_to_le16(0x9c4); > id->psd[0].enlat = cpu_to_le32(0x10); > id->psd[0].exlat = cpu_to_le32(0x4); Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v6 19/42] nvme: enforce valid queue creation sequence

2020-03-25 Thread Maxim Levitsky
0..b4d1738a3d0a 100644 > --- a/hw/block/nvme.h > +++ b/hw/block/nvme.h > @@ -99,6 +99,7 @@ typedef struct NvmeCtrl { > BlockConfconf; > NvmeParams params; > > +boolqs_created; > uint32_tpage_size; > uint16_tpage_bits; > uint16_tmax_prp_ents; Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v6 22/42] nvme: memset preallocated requests structures

2020-03-25 Thread Maxim Levitsky
init_sq(NvmeSQueue *sq, NvmeCtrl *n, > uint64_t dma_addr, > sq->size = size; > sq->cqid = cqid; > sq->head = sq->tail = 0; > -sq->io_req = g_new(NvmeRequest, sq->size); > +sq->io_req = g_new0(NvmeRequest, sq->size); > >

Re: [PATCH v6 16/42] nvme: make sure ncqr and nsqr is valid

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > 0x is not an allowed value for NCQR and NSQR in Set Features on > Number of Queues. > > Signed-off-by: Klaus Jensen > Acked-by: Keith Busch > Reviewed-by: Maxim Levitsky >

Re: [PATCH v6 25/42] nvme: refactor dma read/write

2020-03-25 Thread Maxim Levitsky
_DEVICE); > } > > static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req) > @@ -1214,8 +1211,8 @@ static uint16_t nvme_set_feature_timestamp(NvmeCtrl *n, > NvmeCmd *cmd) > uint64_t prp1 = le64_to_cpu(cmd->dptr.prp1); > uint64_t prp2 = le64_to_cpu(cmd->dptr.prp2); > > -ret = nvme_dma_write_prp(n, (uint8_t *), > -sizeof(timestamp), prp1, prp2); > +ret = nvme_dma_prp(n, (uint8_t *), sizeof(timestamp), prp1, > + prp2, DMA_DIRECTION_TO_DEVICE); > if (ret != NVME_SUCCESS) { > return ret; > } Looks OK to me. It was a bit difficult to read the diff, so I also read the code after it was applied. I hope I didn't miss anything. Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v6 18/42] nvme: support identify namespace descriptor list

2020-03-25 Thread Maxim Levitsky
ist(uint32_t ns) "nsid %"PRIu32"" > nvme_dev_getfeat(uint16_t cid, uint32_t fid) "cid %"PRIu16" fid 0x%"PRIx32"" > nvme_dev_setfeat(uint16_t cid, uint32_t fid, uint32_t val) "cid %"PRIu16" > fid 0x%"PRIx32" val 0x%"PRIx32"" > nvme_dev_getfeat_vwcache(const char* result) "get feature volatile write > cache, result=%s" I think that we should add namespace uuid as a device parameter, but its OK to do this in follow up patch. Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v6 11/42] nvme: add temperature threshold feature

2020-03-25 Thread Maxim Levitsky
a/include/block/nvme.h > +++ b/include/block/nvme.h > @@ -688,7 +688,13 @@ enum NvmeIdCtrlOncs { > typedef struct NvmeFeatureVal { > uint32_tarbitration; > uint32_tpower_mgmt; > -uint32_ttemp_thresh; > +union { > +struct { > +uint16_t temp_thresh_hi; > +uint16_t temp_thresh_low; > +}; > +uint32_t temp_thresh; > +}; > uint32_terr_rec; > uint32_tvolatile_wc; > uint32_tnum_queues; With 'temperature' field removed from the header: Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v6 15/42] nvme: additional tracing

2020-03-25 Thread Maxim Levitsky
) "wrote MMIO, > interrupt mask set, data=0x%"PRIx64", new_mask=0x%"PRIx64"" > nvme_dev_mmio_intm_clr(uint64_t data, uint64_t new_mask) "wrote MMIO, > interrupt mask clr, data=0x%"PRIx64", new_mask=0x%"PRIx64"" > nvme_dev_mmio_cfg(uint64_t data) "wrote MMIO, config controller > config=0x%"PRIx64"" > @@ -70,6 +73,8 @@ nvme_dev_mmio_start_success(void) "setting controller > enable bit succeeded" > nvme_dev_mmio_stopped(void) "cleared controller enable bit" > nvme_dev_mmio_shutdown_set(void) "shutdown bit set" > nvme_dev_mmio_shutdown_cleared(void) "shutdown bit cleared" > +nvme_dev_mmio_doorbell_cq(uint16_t cqid, uint16_t new_head) "cqid %"PRIu16" > new_head %"PRIu16"" > +nvme_dev_mmio_doorbell_sq(uint16_t sqid, uint16_t new_tail) "cqid %"PRIu16" > new_tail %"PRIu16"" > > # nvme traces for error conditions > nvme_dev_err_invalid_dma(void) "PRP/SGL is too small for transfer size" Looks good. Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v6 10/42] nvme: refactor device realization

2020-03-25 Thread Maxim Levitsky
465560eea 100644 > --- a/hw/block/nvme.h > +++ b/hw/block/nvme.h > @@ -67,6 +67,22 @@ typedef struct NvmeNamespace { > NvmeIdNsid_ns; > } NvmeNamespace; > > +static inline NvmeLBAF *nvme_ns_lbaf(NvmeNamespace *ns) > +{ > +NvmeIdNs *id_ns = >id_ns; > +return _ns->lbaf[NVME_ID_NS_FLBAS_INDEX(id_ns->flbas)]; > +} > + > +static inline uint8_t nvme_ns_lbads(NvmeNamespace *ns) > +{ > +return nvme_ns_lbaf(ns)->ds; > +} > + > +static inline size_t nvme_ns_lbads_bytes(NvmeNamespace *ns) > +{ > +return 1 << nvme_ns_lbads(ns); > +} > + > #define TYPE_NVME "nvme" > #define NVME(obj) \ > OBJECT_CHECK(NvmeCtrl, (obj), TYPE_NVME) > @@ -88,8 +104,6 @@ typedef struct NvmeCtrl { > uint32_tnum_namespaces; > uint32_tmax_q_ents; > uint64_tns_size; > -uint32_tcmbsz; > -uint32_tcmbloc; > uint8_t *cmbuf; > uint64_tirq_status; > uint64_thost_timestamp; /* Timestamp sent by the > host */ > @@ -103,4 +117,9 @@ typedef struct NvmeCtrl { > NvmeIdCtrl id_ctrl; > } NvmeCtrl; > > +static inline uint64_t nvme_ns_nlbas(NvmeCtrl *n, NvmeNamespace *ns) > +{ > +return n->ns_size >> nvme_ns_lbads(ns); > +} > + > #endif /* HW_NVME_H */ Small nitpick: To be honest this not only refactoring in the device realization since you also (rightfully) removed the duplicated cmbsz/cmbloc so I would add a mention for this in the commit message. But that doesn't matter that much, so Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v6 13/42] nvme: add support for the asynchronous event request command

2020-03-25 Thread Maxim Levitsky
ith a max number > of queued events (controllable with the aer_max_queued device > parameter). The spec states that the controller *should* retain > events, so we do best effort here. > > Signed-off-by: Klaus Jensen > Acked-by: Keith Busch > Reviewed-by: Max

Re: [PATCH v6 14/42] nvme: add missing mandatory features

2020-03-25 Thread Maxim Levitsky
ALID_QUEUE_DEL = 0x010c, > NVME_FID_NOT_SAVEABLE = 0x010d, > -NVME_FID_NOT_NSID_SPEC = 0x010f, > +NVME_FEAT_NOT_CHANGABLE = 0x010e, > +NVME_FEAT_NOT_NS_SPEC = 0x010f, > NVME_FW_REQ_SUSYSTEM_RESET = 0x0110, > NVME_CONFLICTING_ATTRS = 0x0180, > NVME_INVALID_PROT_INFO = 0x0181, > @@ -706,6 +707,7 @@ typedef struct NvmeFeatureVal { > } NvmeFeatureVal; > > #define NVME_ARB_AB(arb)(arb & 0x7) > +#define NVME_ARB_AB_NOLIMIT 0x7 > #define NVME_ARB_LPW(arb) ((arb >> 8) & 0xff) > #define NVME_ARB_MPW(arb) ((arb >> 16) & 0xff) > #define NVME_ARB_HPW(arb) ((arb >> 24) & 0xff) > @@ -713,6 +715,8 @@ typedef struct NvmeFeatureVal { > #define NVME_INTC_THR(intc) (intc & 0xff) > #define NVME_INTC_TIME(intc)((intc >> 8) & 0xff) > > +#define NVME_INTVC_NOCOALESCING (0x1 << 16) > + > #define NVME_TEMP_THSEL(temp) ((temp >> 20) & 0x3) > #define NVME_TEMP_TMPSEL(temp) ((temp >> 16) & 0xf) > #define NVME_TEMP_TMPTH(temp) ((temp >> 0) & 0x) Best regards, Maxim Levitsky

Re: [PATCH v6 17/42] nvme: add log specific field to trace events

2020-03-25 Thread Maxim Levitsky
6" lid 0x%"PRIx8" rae 0x%"PRIx8" len %"PRIu32" off > %"PRIu64"" > +nvme_dev_get_log(uint16_t cid, uint8_t lid, uint8_t lsp, uint8_t rae, > uint32_t len, uint64_t off) "cid %"PRIu16" lid 0x%"PRIx8" lsp 0x%"PRIx8" rae > 0x%"PRIx8" len %"PRIu32" off %"PRIu64"" > nvme_dev_process_aers(int queued) "queued %d" > nvme_dev_aer(uint16_t cid) "cid %"PRIu16"" > nvme_dev_aer_aerl_exceeded(void) "aerl exceeded" Perfect! Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v6 12/42] nvme: add support for the get log page command

2020-03-25 Thread Maxim Levitsky
t lid, uint8_t rae, uint32_t len, > uint64_t off) "cid %"PRIu16" lid 0x%"PRIx8" rae 0x%"PRIx8" len %"PRIu32" off > %"PRIu64"" > nvme_dev_mmio_intm_set(uint64_t data, uint64_t new_mask) "wrote MMIO, > interrupt mask set, data=0x%"PRIx64", new_mask=0x%"PRIx64"" > nvme_dev_mmio_intm_clr(uint64_t data, uint64_t new_mask) "wrote MMIO, > interrupt mask clr, data=0x%"PRIx64", new_mask=0x%"PRIx64"" > nvme_dev_mmio_cfg(uint64_t data) "wrote MMIO, config controller > config=0x%"PRIx64"" > @@ -85,6 +86,7 @@ nvme_dev_err_invalid_create_cq_qflags(uint16_t qflags) > "failed creating completi > nvme_dev_err_invalid_identify_cns(uint16_t cns) "identify, invalid > cns=0x%"PRIx16"" > nvme_dev_err_invalid_getfeat(int dw10) "invalid get features, > dw10=0x%"PRIx32"" > nvme_dev_err_invalid_setfeat(uint32_t dw10) "invalid set features, > dw10=0x%"PRIx32"" > +nvme_dev_err_invalid_log_page(uint16_t cid, uint16_t lid) "cid %"PRIu16" lid > 0x%"PRIx16"" > nvme_dev_err_startfail_cq(void) "nvme_start_ctrl failed because there are > non-admin completion queues" > nvme_dev_err_startfail_sq(void) "nvme_start_ctrl failed because there are > non-admin submission queues" > nvme_dev_err_startfail_nbarasq(void) "nvme_start_ctrl failed because the > admin submission queue address is null" Best regards, Maxim Levitsky

Re: [PATCH v6 06/42] nvme: add identify cns values in header

2020-03-25 Thread Maxim Levitsky
trace_nvme_dev_err_invalid_identify_cns(le32_to_cpu(c->cns)); This is a very good candidate to be squished with the patch 5 IMHO, but you can leave this as is as well. I don't mind. Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v6 08/42] nvme: add support for the abort command

2020-03-25 Thread Maxim Levitsky
ommand Limit > (four > + * concurrently outstanding Abort commands), so lets use that though it > is > + * inconsequential. > + */ > +id->acl = 3; > id->frmw = 7 << 1; > id->lpa = 1 << 0; > id->sqes = (0x6 << 4) | 0x6; You forgot to move my reviewed-by from the previous version I see that you also fixed the white space problem, thanks! So, Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v6 09/42] nvme: add max_ioqpairs device parameter

2020-03-25 Thread Maxim Levitsky
DDRESS_MEM_TYPE_64, > >iomem); > -msix_init_exclusive_bar(pci_dev, n->params.num_queues, 4, NULL); > +msix_init_exclusive_bar(pci_dev, n->params.max_ioqpairs + 1, 4, NULL); > > id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID)); > id->ssvid = cpu_to_le16(pci_get_word(pci_conf + > PCI_SUBSYSTEM_VENDOR_ID)); > diff --git a/hw/block/nvme.h b/hw/block/nvme.h > index 9957c4a200e2..98f5b9479244 100644 > --- a/hw/block/nvme.h > +++ b/hw/block/nvme.h > @@ -6,11 +6,13 @@ > #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) > +DEFINE_PROP_UINT32("num_queues", _state, _props.num_queues, 0), \ > +DEFINE_PROP_UINT32("max_ioqpairs", _state, _props.max_ioqpairs, 64) > > typedef struct NvmeParams { > char *serial; > uint32_t num_queues; > +uint32_t max_ioqpairs; > uint32_t cmb_size_mb; > } NvmeParams; > Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v6 07/42] nvme: refactor nvme_addr_read

2020-03-25 Thread Maxim Levitsky
} > > static int nvme_check_sqid(NvmeCtrl *n, uint16_t sqid) Note that this patch still contains a bug that it removes the check against the accessed size, which you fix in later patch. I prefer to not add a bug in first place However if you have a reason for this, I won't mind. Best regards, Maxim Levitsky

Re: [PATCH v6 05/42] nvme: use constant for identify data size

2020-03-25 Thread Maxim Levitsky
don't mind leaving this as is. Fine grained patches never cause any harm. Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v6 03/42] nvme: move device parameters to separate struct

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Move device configuration parameters to separate struct to make it > explicit what is configurable and what is set internally. > > Signed-off-by: Klaus Jensen > Acked-by: Keith Busch > Rev

Re: [PATCH v6 01/42] nvme: rename trace events to nvme_dev

2020-03-25 Thread Maxim Levitsky
"PRIu16"" > +nvme_dev_err_invalid_identify_cns(uint16_t cns) "identify, invalid > cns=0x%"PRIx16"" > +nvme_dev_err_invalid_getfeat(int dw10) "invalid get features, > dw10=0x%"PRIx32"" > +nvme_dev_

Re: [PATCH v6 04/42] nvme: bump spec data structures to v1.3

2020-03-25 Thread Maxim Levitsky
values as well > +#define NVME_TEMP_TMPSEL(temp) ((temp >> 16) & 0xf) > +#define NVME_TEMP_TMPTH(temp) ((temp >> 0) & 0x) > + > enum NvmeFeatureIds { > NVME_ARBITRATION= 0x1, > NVME_POWER_MANAGEMENT = 0x2, > @@ -653,18 +753,37 @@ typedef struct NvmeIdNs { > uint8_t mc; > uint8_t dpc; > uint8_t dps; > - > uint8_t nmic; > uint8_t rescap; > uint8_t fpi; > uint8_t dlfeat; > - > -uint8_t res34[94]; > +uint16_tnawun; > +uint16_tnawupf; > +uint16_tnacwu; > +uint16_tnabsn; > +uint16_tnabo; > +uint16_tnabspf; > +uint16_tnoiob; > +uint8_t nvmcap[16]; > +uint8_t rsvd64[40]; > +uint8_t nguid[16]; > +uint64_teui64; > NvmeLBAFlbaf[16]; > -uint8_t res192[192]; > +uint8_t rsvd192[192]; > uint8_t vs[3712]; > } NvmeIdNs; Also checked this against V5, looks OK now > > +typedef struct NvmeIdNsDescr { > +uint8_t nidt; > +uint8_t nidl; > +uint8_t rsvd2[2]; > +} NvmeIdNsDescr; OK > + > +#define NVME_NIDT_UUID_LEN 16 > + > +enum { > +NVME_NIDT_UUID = 0x3, Very minor nitpick: I'll would add others as well just for the sake of better understanding what this is > +}; > > /*Deallocate Logical Block Features*/ > #define NVME_ID_NS_DLFEAT_GUARD_CRC(dlfeat) ((dlfeat) & 0x10) Looks very good. Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v6 00/42] nvme: support NVMe v1.3d, SGLs and multiple namespaces

2020-03-25 Thread Maxim Levitsky
of sense. I don't think this is bad. You might actually found the ultimate question of life the universe and everything. ;-) Best regards, Maxim Levitsky > > v6 primarily splits up the big nasty patches into more digestible parts. > Specifically the 'nvme: refactor prp mapping' and

Re: [PATCH v5 10/26] nvme: add support for the get log page command

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:45 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 11:35, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote: > > > Add support for the Get Log Page command and basic implementations of > > > the mandatory Error

Re: [PATCH v5 14/26] nvme: make sure ncqr and nsqr is valid

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:48 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 12:30, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote: > > > 0x is not an allowed value for NCQR and NSQR in Set Features on > > > Number of Queues. > &

Re: [PATCH v5 22/26] nvme: support multiple namespaces

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:55 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 14:34, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:52 +0100, Klaus Jensen wrote: > > > This adds support for multiple namespaces by introducing a new 'nvme-ns' > > > device model. Th

Re: [PATCH v5 20/26] nvme: handle dma errors

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:53 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 13:52, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:52 +0100, Klaus Jensen wrote: > > > Handling DMA errors gracefully is required for the device to pass the > > > block/011 test ("di

Re: [PATCH v5 12/26] nvme: add missing mandatory features

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:47 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 12:27, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote: > > > Add support for returning a resonable response to Get/Set Features of > > > mandatory featur

Re: [PATCH v5 21/26] nvme: add support for scatter gather lists

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:54 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 14:07, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:52 +0100, Klaus Jensen wrote: > > > For now, support the Data Block, Segment and Last Segment descriptor > > > types. > > >

Re: [PATCH v5 15/26] nvme: bump supported specification to 1.3

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:50 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 12:35, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote: > > > Add new fields to the Identify Controller and Identify Namespace data > > > structures a

Re: [PATCH v5 10/26] nvme: add support for the get log page command

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:45 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 11:35, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote: > > > Add support for the Get Log Page command and basic implementations of > > > the mandatory Error

Re: [PATCH v5 17/26] nvme: allow multiple aios per command

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:53 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 13:48, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote: > > > This refactors how the device issues asynchronous block backend > > > requests. The NvmeRequest no

Re: [PATCH v5 09/26] nvme: add temperature threshold feature

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:44 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 11:31, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote: > > > It might seem wierd to implement this feature for an emulated device, > > > but it is mandatory

Re: [PATCH v5 16/26] nvme: refactor prp mapping

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:51 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 13:44, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote: > > > Refactor nvme_map_prp and allow PRPs to be located in the CMB. The logic > > > ensures that if so

Re: [PATCH v5 08/26] nvme: refactor device realization

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:43 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 11:27, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote: > > > This patch splits up nvme_realize into multiple individual functions, > > > each initializing a dif

Re: [PATCH v2 00/14] LUKS: encryption slot management using amend interface

2020-03-12 Thread Maxim Levitsky
On Thu, 2020-03-12 at 06:56 -0500, Eric Blake wrote: > On 3/8/20 10:18 AM, Maxim Levitsky wrote: > > Hi! > > Here is the updated series of my patches, incorporating all the feedback I > > received. > > > > Patches are strictly divided by topic to 3 groups, and

Re: [PATCH v2 02/14] qcrypto/luks: implement encryption key management

2020-03-11 Thread Maxim Levitsky
On Tue, 2020-03-10 at 14:02 +0200, Maxim Levitsky wrote: > On Tue, 2020-03-10 at 12:59 +0100, Kevin Wolf wrote: > > Am 10.03.2020 um 12:05 hat Maxim Levitsky geschrieben: > > > On Tue, 2020-03-10 at 11:58 +0100, Max Reitz wrote: > > > > On 08.03.20 16:18, Maxim Levi

Re: [PATCH v2 02/14] qcrypto/luks: implement encryption key management

2020-03-10 Thread Maxim Levitsky
On Tue, 2020-03-10 at 12:59 +0100, Kevin Wolf wrote: > Am 10.03.2020 um 12:05 hat Maxim Levitsky geschrieben: > > On Tue, 2020-03-10 at 11:58 +0100, Max Reitz wrote: > > > On 08.03.20 16:18, Maxim Levitsky wrote: > > > > Next few patches will expose that fun

Re: [PATCH v2 02/14] qcrypto/luks: implement encryption key management

2020-03-10 Thread Maxim Levitsky
On Tue, 2020-03-10 at 11:58 +0100, Max Reitz wrote: > On 08.03.20 16:18, Maxim Levitsky wrote: > > Next few patches will expose that functionality > > to the user. > > > > Signed-off-by: Maxim Levitsky > > --- > > crypto/block-luks.c | 398 +++

Re: [PATCH v5 00/11] HMP monitor handlers refactoring

2020-03-09 Thread Maxim Levitsky
On Mon, 2020-03-09 at 18:30 +, Dr. David Alan Gilbert wrote: > * Maxim Levitsky (mlevi...@redhat.com) wrote: > > This patch series is bunch of cleanups to the hmp monitor code. > > It mostly moves the blockdev related hmp handlers to its own file, > > and does s

Re: [PATCH v5 05/11] monitor/hmp: move hmp_drive_mirror and hmp_drive_backup to block-hmp-cmds.c Moved code was added after 2012-01-13, thus under GPLv2+

2020-03-09 Thread Maxim Levitsky
On Mon, 2020-03-09 at 16:31 +, Dr. David Alan Gilbert wrote: > * Maxim Levitsky (mlevi...@redhat.com) wrote: > > > > I see that I have the same issue of long subject line here. > > Its because I forgot the space after first line, when adding this. > > If I nee

[PATCH v2 05/14] block/amend: refactor qcow2 amend options

2020-03-08 Thread Maxim Levitsky
Some qcow2 create options can't be used for amend. Remove them from the qcow2 create options and add generic logic to detect such options in qemu-img Signed-off-by: Maxim Levitsky --- block/qcow2.c | 108 ++--- qemu-img.c | 18 +++- tests/qemu

[PATCH v2 08/14] block/qcow2: extend qemu-img amend interface with crypto options

2020-03-08 Thread Maxim Levitsky
Now that we have all the infrastructure in place, wire it in the qcow2 driver and expose this to the user. Signed-off-by: Maxim Levitsky --- block/qcow2.c | 80 -- tests/qemu-iotests/082.out | 45 + 2 files changed, 114

[PATCH v2 13/14] block/qcow2: implement blockdev-amend

2020-03-08 Thread Maxim Levitsky
Currently the implementation only supports amending the encryption options, unlike the qemu-img version Signed-off-by: Maxim Levitsky Reviewed-by: Daniel P. Berrangé --- block/qcow2.c| 39 +++ qapi/block-core.json | 16 +++- 2 files

[PATCH v2 14/14] iotests: add tests for blockdev-amend

2020-03-08 Thread Maxim Levitsky
This commit adds two tests that cover the new blockdev-amend functionality of luks and qcow2 driver Signed-off-by: Maxim Levitsky --- tests/qemu-iotests/302 | 278 + tests/qemu-iotests/302.out | 40 ++ tests/qemu-iotests/303 | 233

[PATCH v2 11/14] block/core: add generic infrastructure for x-blockdev-amend qmp command

2020-03-08 Thread Maxim Levitsky
blockdev-amend will be used similiar to blockdev-create to allow on the fly changes of the structure of the format based block devices. Current plan is to first support encryption keyslot management for luks based formats (raw and embedded in qcow2) Signed-off-by: Maxim Levitsky --- block

[PATCH v2 10/14] iotests: qemu-img tests for luks key management

2020-03-08 Thread Maxim Levitsky
This commit adds two tests, which test the new amend interface of both luks raw images and qcow2 luks encrypted images. Signed-off-by: Maxim Levitsky --- tests/qemu-iotests/300 | 207 + tests/qemu-iotests/300.out | 99 ++ tests/qemu

[PATCH v2 09/14] iotests: filter few more luks specific create options

2020-03-08 Thread Maxim Levitsky
This allows more tests to be able to have same output on both qcow2 luks encrypted images and raw luks images Signed-off-by: Maxim Levitsky --- tests/qemu-iotests/087.out | 6 +++--- tests/qemu-iotests/134.out | 2 +- tests/qemu-iotests/158.out | 4 ++-- tests/qemu-iotests

[PATCH v2 03/14] block/amend: add 'force' option

2020-03-08 Thread Maxim Levitsky
'force' option will be used for some unsafe amend operations. This includes things like erasing last keyslot in luks based formats (which destroys the data, unless the master key is backed up by external means), but that _might_ be desired result. Signed-off-by: Maxim Levitsky Reviewed

[PATCH v2 12/14] block/crypto: implement blockdev-amend

2020-03-08 Thread Maxim Levitsky
Signed-off-by: Maxim Levitsky Reviewed-by: Daniel P. Berrangé --- block/crypto.c | 72 qapi/block-core.json | 14 - 2 files changed, 66 insertions(+), 20 deletions(-) diff --git a/block/crypto.c b/block/crypto.c index 727a3fde58

[PATCH v2 04/14] block/amend: separate amend and create options for qemu-img

2020-03-08 Thread Maxim Levitsky
it in each action option list. In future it might be useful to remove some options which are not supported anyway from amend list, which currently cause an error message if amended. Signed-off-by: Maxim Levitsky --- block/qcow2.c | 160 +- include

[PATCH v2 07/14] block/crypto: implement the encryption key management

2020-03-08 Thread Maxim Levitsky
of unsharing read, rather that write permission which allows to avoid cases when the other user had opened the image read-only. Signed-off-by: Maxim Levitsky --- block/crypto.c | 127 +++-- block/crypto.h | 44 +++-- 2 files changed, 163 insertions

[PATCH v2 06/14] block/crypto: rename two functions

2020-03-08 Thread Maxim Levitsky
rename the write_func to create_write_func, and init_func to create_init_func. This is preparation for other write_func that will be used to update the encryption keys. No functional changes Signed-off-by: Maxim Levitsky Reviewed-by: Daniel P. Berrangé --- block/crypto.c | 25

[PATCH v2 02/14] qcrypto/luks: implement encryption key management

2020-03-08 Thread Maxim Levitsky
Next few patches will expose that functionality to the user. Signed-off-by: Maxim Levitsky --- crypto/block-luks.c | 398 +++- qapi/crypto.json| 61 ++- 2 files changed, 455 insertions(+), 4 deletions(-) diff --git a/crypto/block-luks.c b/crypto

[PATCH v2 01/14] qcrypto/core: add generic infrastructure for crypto options amendment

2020-03-08 Thread Maxim Levitsky
This will be used first to implement luks keyslot management. block_crypto_amend_opts_init will be used to convert qemu-img cmdline to QCryptoBlockAmendOptions Signed-off-by: Maxim Levitsky Reviewed-by: Daniel P. Berrangé --- block/crypto.c | 17 + block/crypto.h

[PATCH v2 00/14] LUKS: encryption slot management using amend interface

2020-03-08 Thread Maxim Levitsky
slot management also using the code from patches 1,2, and also add a bunch of iotests to cover this. Tested with -raw,-qcow2 and -luks iotests and 'make check' Best regards, Maxim Levitsky clone of "luks-keymgmnt-v2" Maxim Levitsky (14): qcrypto/core: add generic infr

Re: [PATCH v5 05/11] monitor/hmp: move hmp_drive_mirror and hmp_drive_backup to block-hmp-cmds.c Moved code was added after 2012-01-13, thus under GPLv2+

2020-03-08 Thread Maxim Levitsky
for noise. Best regards, Maxim Levitsky On Sun, 2020-03-08 at 11:24 +0200, Maxim Levitsky wrote: > Signed-off-by: Maxim Levitsky > Reviewed-by: Dr. David Alan Gilbert > --- > block/monitor/block-hmp-cmds.c | 60 ++ > include/block/block-

[PATCH v5 11/11] monitor/hmp: Move hmp_drive_add_node to block-hmp-cmds.c

2020-03-08 Thread Maxim Levitsky
Signed-off-by: Maxim Levitsky Reviewed-by: Dr. David Alan Gilbert --- block/monitor/block-hmp-cmds.c | 30 blockdev.c | 43 +++--- include/block/block_int.h | 5 ++-- 3 files changed, 41 insertions(+), 37 deletions

<    1   2   3   4   5   6   7   8   9   10   >