commit 43efb0bfad2b ("hw/cxl/mbox: Wire up interrupts for background
completion") enables notifying background command completion via MSI-X
interrupt (vector number 9).

However, the commit uses vector number 9 but the maximum number of
entries is less thus resulting in error below. Fix it by passing
nentries = 10 when calling msix_init_exclusive_bar().

 # echo 1 > sanitize
 Background command 4400h finished: success
 qemu-system-x86_64: ../hw/pci/msix.c:529: msix_notify: Assertion `vector < dev->msix_entries_nr' failed.

Fixes: 43efb0bfad2b ("hw/cxl/mbox: Wire up interrupts for background completion")
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
---
 hw/mem/cxl_type3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index 52647b4ac7..72d9371347 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -685,7 +685,7 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp)
     ComponentRegisters *regs = &cxl_cstate->crb;
     MemoryRegion *mr = &regs->component_registers;
     uint8_t *pci_conf = pci_dev->config;
-    unsigned short msix_num = 6;
+    unsigned short msix_num = 10;
     int i, rc;
 
     QTAILQ_INIT(&ct3d->error_list);
-- 
2.39.1

On Mon, 27 Nov 2023, Hyeonggon Yoo wrote:

>commit 43efb0bfad2b ("hw/cxl/mbox: Wire up interrupts for background
>completion") enables notifying background command completion via MSI-X
>interrupt (vector number 9).
>
>However, the commit uses vector number 9 but the maximum number of
>entries is less thus resulting in error below. Fix it by passing
>nentries = 10 when calling msix_init_exclusive_bar().

Hmm yeah this was already set to 10 in Jonathan's tree, thanks for reporting.

On Tue, Nov 28, 2023 at 2:53 AM Davidlohr Bueso <dave@stgolabs.net> wrote:
>
> On Mon, 27 Nov 2023, Hyeonggon Yoo wrote:
>
> >commit 43efb0bfad2b ("hw/cxl/mbox: Wire up interrupts for background
> >completion") enables notifying background command completion via MSI-X
> >interrupt (vector number 9).
> >
> >However, the commit uses vector number 9 but the maximum number of
> >entries is less thus resulting in error below. Fix it by passing
> >nentries = 10 when calling msix_init_exclusive_bar().
>
> Hmm yeah this was already set to 10 in Jonathan's tree, thanks for reporting.

Oh, yeah, it's based on the mainline tree. I should have checked Jonathan's.

hmm it's already 10 there but vector number 9 is already being used by PCIe DOE.
So I think it should change msix_num = 11 and use vector number 10 for
background command completion interrupt instead?

https://gitlab.com/jic23/qemu/-/commit/2823f19188664a6d48a965ea8170c9efa23cddab

Thanks!

--
Hyeonggon

On Tue, 28 Nov 2023 09:27:28 +0900
Hyeonggon Yoo <42.hyeyoo@gmail.com> wrote:

> On Tue, Nov 28, 2023 at 2:53 AM Davidlohr Bueso <dave@stgolabs.net> wrote:
> >
> > On Mon, 27 Nov 2023, Hyeonggon Yoo wrote:
> >  
> > >commit 43efb0bfad2b ("hw/cxl/mbox: Wire up interrupts for background
> > >completion") enables notifying background command completion via MSI-X
> > >interrupt (vector number 9).
> > >
> > >However, the commit uses vector number 9 but the maximum number of
> > >entries is less thus resulting in error below. Fix it by passing
> > >nentries = 10 when calling msix_init_exclusive_bar().  
> >
> > Hmm yeah this was already set to 10 in Jonathan's tree, thanks for reporting.  
> 
> Oh, yeah, it's based on the mainline tree. I should have checked Jonathan's.
> 
> hmm it's already 10 there but vector number 9 is already being used by PCIe DOE.
> So I think it should change msix_num = 11 and use vector number 10 for
> background command completion interrupt instead?
> 
> https://gitlab.com/jic23/qemu/-/commit/2823f19188664a6d48a965ea8170c9efa23cddab

Whilst I clearly messed up a rebase as this wasn't intended, it should be fine
to have multiple things sharing a vector.

On my todo list is making the case of too few vectors being available work for
all the cases in which case everything may end up on one vector.
So we do need to expand the vectors to cover what we are asking for, but
moving this to 11 is a nice to have rather than required.

Jonathan

> 
> Thanks!
> 
> --
> Hyeonggon

On Tue, Nov 28, 2023 at 9:46 PM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Tue, 28 Nov 2023 09:27:28 +0900
> Hyeonggon Yoo <42.hyeyoo@gmail.com> wrote:
>
> > On Tue, Nov 28, 2023 at 2:53 AM Davidlohr Bueso <dave@stgolabs.net> wrote:
> > >
> > > On Mon, 27 Nov 2023, Hyeonggon Yoo wrote:
> > >
> > > >commit 43efb0bfad2b ("hw/cxl/mbox: Wire up interrupts for background
> > > >completion") enables notifying background command completion via MSI-X
> > > >interrupt (vector number 9).
> > > >
> > > >However, the commit uses vector number 9 but the maximum number of
> > > >entries is less thus resulting in error below. Fix it by passing
> > > >nentries = 10 when calling msix_init_exclusive_bar().
> > >
> > > Hmm yeah this was already set to 10 in Jonathan's tree, thanks for reporting.
> >
> > Oh, yeah, it's based on the mainline tree. I should have checked Jonathan's.
> >
> > hmm it's already 10 there but vector number 9 is already being used by PCIe DOE.
> > So I think it should change msix_num = 11 and use vector number 10 for
> > background command completion interrupt instead?
> >
> > https://gitlab.com/jic23/qemu/-/commit/2823f19188664a6d48a965ea8170c9efa23cddab
>
> Whilst I clearly messed up a rebase as this wasn't intended, it should be fine
> to have multiple things sharing a vector.

I wasn't 100% sure of that, thanks for confirming.
I'll drop this patch in v2 (assuming it's going to be fixed in the
next PR of your tree)

For my own learning, I would like to give a few questions
(sorry to bother, no pressures):

> On my todo list is making the case of too few vectors being available work for
> all the cases in which case everything may end up on one vector.

You mean the device needs to support sharing
a vector for different features?

> So we do need to expand the vectors to cover what we are asking for, but
> moving this to 11 is a nice to have rather than required.

May I ask what do you mean by "expand the vectors" to cover what we
are asking for?

Thanks!
--
Hyeonggon