From: Klaus Jensen <k.jensen@samsung.com>

numcntl is one byte and so is max_vfs. Using cpu_to_le16 on big endian
hosts results in numcntl being set to 0.

Fix by dropping the endian conversion.

Fixes: 746d42b13368 ("hw/nvme: Initialize capability structures for primary/secondary controllers")
Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/nvme/ctrl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index f026245d1e9e..76fe0397045b 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -7924,7 +7924,7 @@ static void nvme_init_state(NvmeCtrl *n)
     n->aer_reqs = g_new0(NvmeRequest *, n->params.aerl + 1);
     QTAILQ_INIT(&n->aer_queue);
 
-    list->numcntl = cpu_to_le16(max_vfs);
+    list->numcntl = max_vfs;
     for (i = 0; i < max_vfs; i++) {
         sctrl = &list->sec[i];
         sctrl->pcid = cpu_to_le16(n->cntlid);

---
base-commit: 760b4dcdddba4a40b9fa0eb78fdfc7eda7cb83d0
change-id: 20240222-fix-sriov-numcntl-5ebe46a42176

Best regards,
-- 
Klaus Jensen <k.jensen@samsung.com>

On 22/2/24 10:29, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> numcntl is one byte and so is max_vfs. Using cpu_to_le16 on big endian
> hosts results in numcntl being set to 0.
> 
> Fix by dropping the endian conversion.
> 
> Fixes: 746d42b13368 ("hw/nvme: Initialize capability structures for primary/secondary controllers")
> Reported-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
>   hw/nvme/ctrl.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Hi Klaus, I'm not seeing other NVMe patches on the list,
so I'll queue this on my hw-misc tree, but feel free to
object and I'll unqueue :)

Thanks,

Phil.

On Feb 26 09:18, Philippe Mathieu-Daudé wrote:
> On 22/2/24 10:29, Klaus Jensen wrote:
> > From: Klaus Jensen <k.jensen@samsung.com>
> > 
> > numcntl is one byte and so is max_vfs. Using cpu_to_le16 on big endian
> > hosts results in numcntl being set to 0.
> > 
> > Fix by dropping the endian conversion.
> > 
> > Fixes: 746d42b13368 ("hw/nvme: Initialize capability structures for primary/secondary controllers")
> > Reported-by: Kevin Wolf <kwolf@redhat.com>
> > Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> > ---
> >   hw/nvme/ctrl.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Hi Klaus, I'm not seeing other NVMe patches on the list,
> so I'll queue this on my hw-misc tree, but feel free to
> object and I'll unqueue :)
> 
> Thanks,
> 

No, thats perfect! Thanks! :)

On 24-02-22 10:29:06, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> numcntl is one byte and so is max_vfs. Using cpu_to_le16 on big endian
> hosts results in numcntl being set to 0.
> 
> Fix by dropping the endian conversion.
> 
> Fixes: 746d42b13368 ("hw/nvme: Initialize capability structures for primary/secondary controllers")
> Reported-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>

Reviewed-by: Minwoo Im <minwoo.im@samsung.com>

Thanks,

Hi Klaus,

On 22/2/24 10:29, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> numcntl is one byte and so is max_vfs. Using cpu_to_le16 on big endian
> hosts results in numcntl being set to 0.
> 
> Fix by dropping the endian conversion.
> 
> Fixes: 746d42b13368 ("hw/nvme: Initialize capability structures for primary/secondary controllers")

Isn't it commit 99f48ae7ae ("Add support for Secondary Controller
List") instead?

> Reported-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
>   hw/nvme/ctrl.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index f026245d1e9e..76fe0397045b 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -7924,7 +7924,7 @@ static void nvme_init_state(NvmeCtrl *n)
>       n->aer_reqs = g_new0(NvmeRequest *, n->params.aerl + 1);
>       QTAILQ_INIT(&n->aer_queue);
>   
> -    list->numcntl = cpu_to_le16(max_vfs);
> +    list->numcntl = max_vfs;
>       for (i = 0; i < max_vfs; i++) {
>           sctrl = &list->sec[i];
>           sctrl->pcid = cpu_to_le16(n->cntlid);
> 
> ---
> base-commit: 760b4dcdddba4a40b9fa0eb78fdfc7eda7cb83d0
> change-id: 20240222-fix-sriov-numcntl-5ebe46a42176
> 
> Best regards,

On Feb 22 11:08, Philippe Mathieu-Daudé wrote:
> Hi Klaus,
> 
> On 22/2/24 10:29, Klaus Jensen wrote:
> > From: Klaus Jensen <k.jensen@samsung.com>
> > 
> > numcntl is one byte and so is max_vfs. Using cpu_to_le16 on big endian
> > hosts results in numcntl being set to 0.
> > 
> > Fix by dropping the endian conversion.
> > 
> > Fixes: 746d42b13368 ("hw/nvme: Initialize capability structures for primary/secondary controllers")
> 
> Isn't it commit 99f48ae7ae ("Add support for Secondary Controller
> List") instead?
> 

Thanks Philippe,

Yes, that is the commit that had the bug originally. I'll fix it up.