diff mbox series

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

Message ID 20201027135547.374946-16-philmd@redhat.com (mailing list archive)
State New, archived
Headers show
Series block/nvme: Fix Aarch64 host | expand

Commit Message

Philippe Mathieu-Daudé Oct. 27, 2020, 1:55 p.m. UTC
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>
---
 block/nvme.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Eric Auger Oct. 28, 2020, 2:21 p.m. UTC | #1
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);
>  
>
Stefan Hajnoczi Oct. 28, 2020, 3:17 p.m. UTC | #2
On Tue, Oct 27, 2020 at 02:55:37PM +0100, 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>
> ---
>  block/nvme.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff mbox series

Patch

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);