[Qemu-devel] [PATCH] nvme: Fix get/set number of queues feature

[Qemu-devel] [PATCH] nvme: Fix get/set number of queues feature

[alex]

From: Alex Friedman <alex@e8storage.com>

According to the specification, the low 16 bits should contain the number of
I/O submission queues, and the high 16 bits should contain the number of
I/O completion queues.

Signed-off-by: Alex Friedman <alex@e8storage.com>
---
 hw/block/nvme.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index b6263dc..405cf4f 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -476,7 +476,8 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
 
     switch (dw10) {
     case NVME_NUMBER_OF_QUEUES:
-        req->cqe.result = cpu_to_le32(n->num_queues);
+        req->cqe.result =
+            cpu_to_le32((n->num_queues - 1) | ((n->num_queues - 1) << 16));
         break;
     default:
         return NVME_INVALID_FIELD | NVME_DNR;
@@ -490,7 +491,8 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
 
     switch (dw10) {
     case NVME_NUMBER_OF_QUEUES:
-        req->cqe.result = cpu_to_le32(n->num_queues);
+        req->cqe.result =
+            cpu_to_le32((n->num_queues - 1) | ((n->num_queues - 1) << 16));
         break;
     default:
         return NVME_INVALID_FIELD | NVME_DNR;
-- 
1.9.3

Re: [Qemu-devel] [PATCH] nvme: Fix get/set number of queues feature

[Keith Busch]

On Fri, 5 Dec 2014, alex@e8storage.com wrote:
> From: Alex Friedman <alex@e8storage.com>
>
> According to the specification, the low 16 bits should contain the number of
> I/O submission queues, and the high 16 bits should contain the number of
> I/O completion queues.
>
> Signed-off-by: Alex Friedman <alex@e8storage.com>

Good catch, thanks for the fix!

Acked-by: Keith Busch <keith.busch@intel.com>

> ---
> hw/block/nvme.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index b6263dc..405cf4f 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -476,7 +476,8 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
>
>     switch (dw10) {
>     case NVME_NUMBER_OF_QUEUES:
> -        req->cqe.result = cpu_to_le32(n->num_queues);
> +        req->cqe.result =
> +            cpu_to_le32((n->num_queues - 1) | ((n->num_queues - 1) << 16));
>         break;
>     default:
>         return NVME_INVALID_FIELD | NVME_DNR;
> @@ -490,7 +491,8 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
>
>     switch (dw10) {
>     case NVME_NUMBER_OF_QUEUES:
> -        req->cqe.result = cpu_to_le32(n->num_queues);
> +        req->cqe.result =
> +            cpu_to_le32((n->num_queues - 1) | ((n->num_queues - 1) << 16));
>         break;
>     default:
>         return NVME_INVALID_FIELD | NVME_DNR;
> -- 
> 1.9.3

Re: [Qemu-devel] [PATCH] nvme: Fix get/set number of queues feature

[Alex Friedman]

[-- Attachment #1: Type: text/plain, Size: 1718 bytes --]

*ping*

https://patchwork.ozlabs.org/patch/418107/

Alex Friedman
E8 Storage VP R&D
alex@e8storage.com
Mobile: +972-54-579-7646

On Fri, Dec 5, 2014 at 5:45 PM, Keith Busch <keith.busch@intel.com> wrote:

> On Fri, 5 Dec 2014, alex@e8storage.com wrote:
>
>> From: Alex Friedman <alex@e8storage.com>
>>
>> According to the specification, the low 16 bits should contain the number
>> of
>> I/O submission queues, and the high 16 bits should contain the number of
>> I/O completion queues.
>>
>> Signed-off-by: Alex Friedman <alex@e8storage.com>
>>
>
> Good catch, thanks for the fix!
>
> Acked-by: Keith Busch <keith.busch@intel.com>
>
>
>  ---
>> hw/block/nvme.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
>> index b6263dc..405cf4f 100644
>> --- a/hw/block/nvme.c
>> +++ b/hw/block/nvme.c
>> @@ -476,7 +476,8 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd
>> *cmd, NvmeRequest *req)
>>
>>     switch (dw10) {
>>     case NVME_NUMBER_OF_QUEUES:
>> -        req->cqe.result = cpu_to_le32(n->num_queues);
>> +        req->cqe.result =
>> +            cpu_to_le32((n->num_queues - 1) | ((n->num_queues - 1) <<
>> 16));
>>         break;
>>     default:
>>         return NVME_INVALID_FIELD | NVME_DNR;
>> @@ -490,7 +491,8 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd
>> *cmd, NvmeRequest *req)
>>
>>     switch (dw10) {
>>     case NVME_NUMBER_OF_QUEUES:
>> -        req->cqe.result = cpu_to_le32(n->num_queues);
>> +        req->cqe.result =
>> +            cpu_to_le32((n->num_queues - 1) | ((n->num_queues - 1) <<
>> 16));
>>         break;
>>     default:
>>         return NVME_INVALID_FIELD | NVME_DNR;
>> --
>> 1.9.3
>>
>

[-- Attachment #2: Type: text/html, Size: 3224 bytes --]

Re: [Qemu-devel] [PATCH] nvme: Fix get/set number of queues feature

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 744 bytes --]

On Fri, Dec 05, 2014 at 02:40:24PM +0200, alex@e8storage.com wrote:
> From: Alex Friedman <alex@e8storage.com>
> 
> According to the specification, the low 16 bits should contain the number of
> I/O submission queues, and the high 16 bits should contain the number of
> I/O completion queues.
> 
> Signed-off-by: Alex Friedman <alex@e8storage.com>
> ---
>  hw/block/nvme.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Sorry, I missed this patch.

Please CC kwolf@redhat.com and stefanha@redhat.com (block subsystem
maintainers) on NVME patches, unless Keith is going to send a separate
pull request to us himself.

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH] nvme: Fix get/set number of queues feature

[Keith Busch]

On Wed, 7 Jan 2015, Stefan Hajnoczi wrote:
> On Fri, Dec 05, 2014 at 02:40:24PM +0200, alex@e8storage.com wrote:
>> From: Alex Friedman <alex@e8storage.com>
>>
>> According to the specification, the low 16 bits should contain the number of
>> I/O submission queues, and the high 16 bits should contain the number of
>> I/O completion queues.
>>
>> Signed-off-by: Alex Friedman <alex@e8storage.com>
>> ---
>>  hw/block/nvme.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> Sorry, I missed this patch.
>
> Please CC kwolf@redhat.com and stefanha@redhat.com (block subsystem
> maintainers) on NVME patches, unless Keith is going to send a separate
> pull request to us himself.

Thanks, I'm more than okay to see this go through the block subsystem. The
fix is already in my qemu-nvme tree, but that repo turned into a bit of
a free-for-all and needs some maintenance to make it fit for pulls. I'll
fix it up once I'm inbetween projects and have more free time.

> Thanks, applied to my block tree:
> https://github.com/stefanha/qemu/commits/block
>
> Stefan
>