| Message ID | 20231127105830.2104954-3-42.hyeyoo@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | A Fixup for "QEMU: CXL mailbox rework and features (Part 1)" | expand |
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
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 = ®s->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);
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(-)