On May 5 07:48, Klaus Jensen wrote: > From: Klaus Jensen <k.jen...@samsung.com> > > The num_queues device paramater has a slightly confusing meaning because > it accounts for the admin queue pair which is not really optional. > Secondly, it is really a maximum value of queues allowed. > > Add a new max_ioqpairs parameter that only accounts for I/O queue pairs, > but keep num_queues for compatibility. > > Signed-off-by: Klaus Jensen <k.jen...@samsung.com> > Reviewed-by: Maxim Levitsky <mlevi...@redhat.com> > Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com> > Reviewed-by: Keith Busch <kbu...@kernel.org> > --- > hw/block/nvme.c | 51 ++++++++++++++++++++++++++++++------------------- > hw/block/nvme.h | 3 ++- > 2 files changed, 33 insertions(+), 21 deletions(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index 623a88be93dc..3875a5f3dcbf 100644 > --- a/hw/block/nvme.c > +++ b/hw/block/nvme.c > @@ -1571,7 +1581,8 @@ static Property nvme_props[] = { > HostMemoryBackend *), > DEFINE_PROP_STRING("serial", NvmeCtrl, params.serial), > DEFINE_PROP_UINT32("cmb_size_mb", NvmeCtrl, params.cmb_size_mb, 0), > - DEFINE_PROP_UINT32("num_queues", NvmeCtrl, params.num_queues, 64), > + DEFINE_PROP_UINT32("num_queues", NvmeCtrl, params.num_queues, 0), > + DEFINE_PROP_UINT32("max_ioqpairs", NvmeCtrl, params.max_ioqpairs, 64), > DEFINE_PROP_END_OF_LIST(), > }; >
I noticed that this default of 64 makes the default configuration unsafe by allowing the cq->cqid < 64 assert in nvme_irq_{,de}assert() to trigger if the pin-based interrupt logic is used (under SPDK for instance). The assert protects against undefined behavior caused by shifting by more than 63 into the 64 bit irq_status variable. As far as I can tell, the assert, the shift and the size of the irq_status variable is bogus, so I posted a patch for this in "hw/block/nvme: fixes for interrupt behavior". Preferably that should go in before this series.