Re: [PATCH] hw/nvme: Add support for setting the MQES for the NVMe emulation

[Berg, John]

On Wed, 2024-05-01 at 14:35 +0200, Klaus Jensen wrote:
> 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.

Hello, just checking in again. Will this be merged into master soon?