Re: [PATCH 15/25] block/nvme: Correctly initialize Admin Queue Attributes

[Auger Eric]

On 10/27/20 2:55 PM, Philippe Mathieu-Daudé wrote:
> From the specification chapter 3.1.8 "AQA - Admin Queue Attributes"
> the Admin Submission Queue Size field is a 0’s based value:
> 
>   Admin Submission Queue Size (ASQS):
> 
>     Defines the size of the Admin Submission Queue in entries.
>     Enabling a controller while this field is cleared to 00h
>     produces undefined results. The minimum size of the Admin
>     Submission Queue is two entries. The maximum size of the
>     Admin Submission Queue is 4096 entries.
>     This is a 0’s based value.
> 
> This bug has never been hit because the device initialization
> uses a single command synchronously :)
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>


Eric

> ---
>  block/nvme.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index 2dfcf8c41d7..d5df30ec074 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -789,9 +789,9 @@ static int nvme_init(BlockDriverState *bs, const char 
> *device, int namespace,
>          goto out;
>      }
>      s->queue_count = 1;
> -    QEMU_BUILD_BUG_ON(NVME_QUEUE_SIZE & 0xF000);
> -    regs->aqa = cpu_to_le32((NVME_QUEUE_SIZE << AQA_ACQS_SHIFT) |
> -                            (NVME_QUEUE_SIZE << AQA_ASQS_SHIFT));
> +    QEMU_BUILD_BUG_ON((NVME_QUEUE_SIZE - 1) & 0xF000);
> +    regs->aqa = cpu_to_le32(((NVME_QUEUE_SIZE - 1) << AQA_ACQS_SHIFT) |
> +                            ((NVME_QUEUE_SIZE - 1) << AQA_ASQS_SHIFT));
>      regs->asq = cpu_to_le64(s->queues[INDEX_ADMIN]->sq.iova);
>      regs->acq = cpu_to_le64(s->queues[INDEX_ADMIN]->cq.iova);
>  
>