Message ID | 20240404120418.1611513-1-jhnberg@amazon.co.uk |
---|---|
State | New |
Headers | show |
Series | hw/nvme: Add support for setting the MQES for the NVMe emulation | expand |
On Apr 4 13:04, John Berg wrote: > From: John Berg <jhnberg@amazon.com> > > The MQES field in the CAP register describes the Maximum Queue Entries > Supported for the IO queues of an NVMe controller. Adding a +1 to the > value in this field results in the total queue size. A full queue is > when a queue of size N contains N - 1 entries, and the minimum queue > size is 2. Thus the lowest MQES value is 1. > > This patch adds the new mqes property to the NVMe emulation which allows > a user to specify the maximum queue size by setting this property. This > is useful as it enables testing of NVMe controller where the MQES is > relatively small. The smallest NVMe queue size supported in NVMe is 2 > submission and completion entries, which means that the smallest legal > mqes value is 1. > > The following example shows how the mqes can be set for a the NVMe > emulation: > > -drive id=nvme0,if=none,file=nvme.img,format=raw > -device nvme,drive=nvme0,serial=foo,mqes=1 > > If the mqes property is not provided then the default mqes will still be > 0x7ff (the queue size is 2048 entries). > > Signed-off-by: John Berg <jhnberg@amazon.co.uk> LGTM, Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
On Apr 4 13:04, John Berg wrote: > From: John Berg <jhnberg@amazon.com> > > The MQES field in the CAP register describes the Maximum Queue Entries > Supported for the IO queues of an NVMe controller. Adding a +1 to the > value in this field results in the total queue size. A full queue is > when a queue of size N contains N - 1 entries, and the minimum queue > size is 2. Thus the lowest MQES value is 1. > > This patch adds the new mqes property to the NVMe emulation which allows > a user to specify the maximum queue size by setting this property. This > is useful as it enables testing of NVMe controller where the MQES is > relatively small. The smallest NVMe queue size supported in NVMe is 2 > submission and completion entries, which means that the smallest legal > mqes value is 1. > > The following example shows how the mqes can be set for a the NVMe > emulation: > > -drive id=nvme0,if=none,file=nvme.img,format=raw > -device nvme,drive=nvme0,serial=foo,mqes=1 > > If the mqes property is not provided then the default mqes will still be > 0x7ff (the queue size is 2048 entries). > > Signed-off-by: John Berg <jhnberg@amazon.co.uk> > --- > hw/nvme/ctrl.c | 9 ++++++++- > hw/nvme/nvme.h | 1 + > 2 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c > index 127c3d2383..86cda9bc73 100644 > --- a/hw/nvme/ctrl.c > +++ b/hw/nvme/ctrl.c > @@ -7805,6 +7805,12 @@ static bool nvme_check_params(NvmeCtrl *n, Error **errp) > return false; > } > > + if (params->mqes < 1) > + { Please keep the `{` on the same line as the `if`. I think checkpatch.pl should catch this. No need to send a v2, I'll fix it up when I apply it to nvme-next :) Thanks!
On Thu, Apr 04, 2024 at 01:04:18PM +0100, John Berg wrote: > The MQES field in the CAP register describes the Maximum Queue Entries > Supported for the IO queues of an NVMe controller. Adding a +1 to the > value in this field results in the total queue size. A full queue is > when a queue of size N contains N - 1 entries, and the minimum queue > size is 2. Thus the lowest MQES value is 1. > > This patch adds the new mqes property to the NVMe emulation which allows > a user to specify the maximum queue size by setting this property. This > is useful as it enables testing of NVMe controller where the MQES is > relatively small. The smallest NVMe queue size supported in NVMe is 2 > submission and completion entries, which means that the smallest legal > mqes value is 1. > > The following example shows how the mqes can be set for a the NVMe > emulation: > > -drive id=nvme0,if=none,file=nvme.img,format=raw > -device nvme,drive=nvme0,serial=foo,mqes=1 > > If the mqes property is not provided then the default mqes will still be > 0x7ff (the queue size is 2048 entries). Looks good. I had to double check where nvme_create_sq() was getting its limit from when processing the host command, and sure enough it's directly from the register field. Reviewed-by: Keith Busch <kbusch@kernel.org>
On Thu, 2024-04-04 at 15:01 +0200, Klaus Jensen wrote: > On Apr 4 13:04, John Berg wrote: > > From: John Berg <jhnberg@amazon.com> > > > > The MQES field in the CAP register describes the Maximum Queue > > Entries > > Supported for the IO queues of an NVMe controller. Adding a +1 to > > the > > value in this field results in the total queue size. A full queue > > is > > when a queue of size N contains N - 1 entries, and the minimum > > queue > > size is 2. Thus the lowest MQES value is 1. > > > > This patch adds the new mqes property to the NVMe emulation which > > allows > > a user to specify the maximum queue size by setting this property. > > This > > is useful as it enables testing of NVMe controller where the MQES > > is > > relatively small. The smallest NVMe queue size supported in NVMe is > > 2 > > submission and completion entries, which means that the smallest > > legal > > mqes value is 1. > > > > The following example shows how the mqes can be set for a the NVMe > > emulation: > > > > -drive id=nvme0,if=none,file=nvme.img,format=raw > > -device nvme,drive=nvme0,serial=foo,mqes=1 > > > > If the mqes property is not provided then the default mqes will > > still be > > 0x7ff (the queue size is 2048 entries). > > > > Signed-off-by: John Berg <jhnberg@amazon.co.uk> > > --- > > hw/nvme/ctrl.c | 9 ++++++++- > > hw/nvme/nvme.h | 1 + > > 2 files changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c > > index 127c3d2383..86cda9bc73 100644 > > --- a/hw/nvme/ctrl.c > > +++ b/hw/nvme/ctrl.c > > @@ -7805,6 +7805,12 @@ static bool nvme_check_params(NvmeCtrl *n, > > Error **errp) > > return false; > > } > > > > + if (params->mqes < 1) > > + { > > Please keep the `{` on the same line as the `if`. I think > checkpatch.pl > should catch this. > > No need to send a v2, I'll fix it up when I apply it to nvme-next :) > > Thanks! Hello Sorry for chasing. I was just wondering when this patch will be applied. I can send a second revision if that helps. Kind regards, John
On May 1 12:27, Berg, John wrote: > On Thu, 2024-04-04 at 15:01 +0200, Klaus Jensen wrote: > > On Apr 4 13:04, John Berg wrote: > > > From: John Berg <jhnberg@amazon.com> > > > > > > The MQES field in the CAP register describes the Maximum Queue > > > Entries > > > Supported for the IO queues of an NVMe controller. Adding a +1 to > > > the > > > value in this field results in the total queue size. A full queue > > > is > > > when a queue of size N contains N - 1 entries, and the minimum > > > queue > > > size is 2. Thus the lowest MQES value is 1. > > > > > > This patch adds the new mqes property to the NVMe emulation which > > > allows > > > a user to specify the maximum queue size by setting this property. > > > This > > > is useful as it enables testing of NVMe controller where the MQES > > > is > > > relatively small. The smallest NVMe queue size supported in NVMe is > > > 2 > > > submission and completion entries, which means that the smallest > > > legal > > > mqes value is 1. > > > > > > The following example shows how the mqes can be set for a the NVMe > > > emulation: > > > > > > -drive id=nvme0,if=none,file=nvme.img,format=raw > > > -device nvme,drive=nvme0,serial=foo,mqes=1 > > > > > > If the mqes property is not provided then the default mqes will > > > still be > > > 0x7ff (the queue size is 2048 entries). > > > > > > Signed-off-by: John Berg <jhnberg@amazon.co.uk> > > > --- > > > hw/nvme/ctrl.c | 9 ++++++++- > > > hw/nvme/nvme.h | 1 + > > > 2 files changed, 9 insertions(+), 1 deletion(-) > > > > > > diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c > > > index 127c3d2383..86cda9bc73 100644 > > > --- a/hw/nvme/ctrl.c > > > +++ b/hw/nvme/ctrl.c > > > @@ -7805,6 +7805,12 @@ static bool nvme_check_params(NvmeCtrl *n, > > > Error **errp) > > > return false; > > > } > > > > > > + if (params->mqes < 1) > > > + { > > > > Please keep the `{` on the same line as the `if`. I think > > checkpatch.pl > > should catch this. > > > > No need to send a v2, I'll fix it up when I apply it to nvme-next :) > > > > Thanks! > > > Hello > > Sorry for chasing. I was just wondering when this patch will be > applied. I can send a second revision if that helps. > No need for the sorry. My apologies. It feel off my radar, so thanks for the bump. I've queued this on nvme-next, will send a pull for master ASAP.
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index 127c3d2383..86cda9bc73 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -7805,6 +7805,12 @@ static bool nvme_check_params(NvmeCtrl *n, Error **errp) return false; } + if (params->mqes < 1) + { + error_setg(errp, "mqes property cannot be less than 1"); + return false; + } + if (n->pmr.dev) { if (params->msix_exclusive_bar) { error_setg(errp, "not enough BARs available to enable PMR"); @@ -8281,7 +8287,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev) id->ctratt = cpu_to_le32(ctratt); - NVME_CAP_SET_MQES(cap, 0x7ff); + NVME_CAP_SET_MQES(cap, n->params.mqes); NVME_CAP_SET_CQR(cap, 1); NVME_CAP_SET_TO(cap, 0xf); NVME_CAP_SET_CSS(cap, NVME_CAP_CSS_NVM); @@ -8451,6 +8457,7 @@ static Property nvme_props[] = { params.sriov_max_vq_per_vf, 0), DEFINE_PROP_BOOL("msix-exclusive-bar", NvmeCtrl, params.msix_exclusive_bar, false), + DEFINE_PROP_UINT16("mqes", NvmeCtrl, params.mqes, 0x7ff), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h index bed8191bd5..2e7d31c0ae 100644 --- a/hw/nvme/nvme.h +++ b/hw/nvme/nvme.h @@ -521,6 +521,7 @@ typedef struct NvmeParams { uint32_t num_queues; /* deprecated since 5.1 */ uint32_t max_ioqpairs; uint16_t msix_qsize; + uint16_t mqes; uint32_t cmb_size_mb; uint8_t aerl; uint32_t aer_max_queued;