Re: [PATCH v2 2/3] hw/nvme: use KVM irqfd when available
On Aug 26 09:34, Keith Busch wrote: > On Fri, Aug 26, 2022 at 11:12:04PM +0800, Jinhao Fan wrote: > > Use KVM's irqfd to send interrupts when possible. This approach is > > thread safe. Moreover, it does not have the inter-thread communication > > overhead of plain event notifiers since handler callback are called > > in the same system call as irqfd write. > > > > Signed-off-by: Jinhao Fan > > Signed-off-by: Klaus Jensen > > No idea what's going on here... This one is causing the following assert > failure with --enable-kvm: > > qemu-system-x86_64: ../accel/kvm/kvm-all.c:1781: kvm_irqchip_commit_routes: > Assertion `ret == 0' failed. > > I find it calls KVM_SET_GSI_ROUTING ioctl with gsi set to KVM_IRQ_ROUTING_MSI, > and linux kernel returns EINVAL in that case. It's never set that way without > this patch. Am I the only one seeing this? Argh, sorry, I threw that patch together a bit too quickly. I was just so pumped because I believed I had solved the issue hehe. Are you missing the ioeventfd=on and irq-eventfd=on parameters by any chance? Without those I'm also getting an assertion, but a different one qemu-system-x86_64: ../hw/pci/msix.c:119: msix_fire_vector_notifier: Assertion `ret >= 0' failed. signature.asc Description: PGP signature
Re: [PATCH v2 2/3] hw/nvme: use KVM irqfd when available
On Aug 26 09:54, Keith Busch wrote: > On Fri, Aug 26, 2022 at 05:45:21PM +0200, Klaus Jensen wrote: > > On Aug 26 09:34, Keith Busch wrote: > > > On Fri, Aug 26, 2022 at 11:12:04PM +0800, Jinhao Fan wrote: > > > > Use KVM's irqfd to send interrupts when possible. This approach is > > > > thread safe. Moreover, it does not have the inter-thread communication > > > > overhead of plain event notifiers since handler callback are called > > > > in the same system call as irqfd write. > > > > > > > > Signed-off-by: Jinhao Fan > > > > Signed-off-by: Klaus Jensen > > > > > > No idea what's going on here... This one is causing the following assert > > > failure with --enable-kvm: > > > > > > qemu-system-x86_64: ../accel/kvm/kvm-all.c:1781: > > > kvm_irqchip_commit_routes: Assertion `ret == 0' failed. > > > > > > I find it calls KVM_SET_GSI_ROUTING ioctl with gsi set to > > > KVM_IRQ_ROUTING_MSI, > > > and linux kernel returns EINVAL in that case. It's never set that way > > > without > > > this patch. Am I the only one seeing this? > > > > Argh, sorry, I threw that patch together a bit too quickly. I was just > > so pumped because I believed I had solved the issue hehe. > > > > Are you missing the ioeventfd=on and irq-eventfd=on parameters by any > > chance? Without those I'm also getting an assertion, but a different one > > I had not enabled those yet. This was purely a regrsession test with my > previously working paramaters for a sanity check. > Yeah, sorry, I just threw it out there with tunnel vision on the kvm irq part, not doing my due diligence ;) I'll fix it up! > If I enable those new nvme parameters, then it is successful. Great :) signature.asc Description: PGP signature
Re: [PATCH v2 2/3] hw/nvme: use KVM irqfd when available
On Fri, Aug 26, 2022 at 11:12:04PM +0800, Jinhao Fan wrote: > Use KVM's irqfd to send interrupts when possible. This approach is > thread safe. Moreover, it does not have the inter-thread communication > overhead of plain event notifiers since handler callback are called > in the same system call as irqfd write. > > Signed-off-by: Jinhao Fan > Signed-off-by: Klaus Jensen No idea what's going on here... This one is causing the following assert failure with --enable-kvm: qemu-system-x86_64: ../accel/kvm/kvm-all.c:1781: kvm_irqchip_commit_routes: Assertion `ret == 0' failed. I find it calls KVM_SET_GSI_ROUTING ioctl with gsi set to KVM_IRQ_ROUTING_MSI, and linux kernel returns EINVAL in that case. It's never set that way without this patch. Am I the only one seeing this?
Re: [PATCH v2 2/3] hw/nvme: use KVM irqfd when available
at 11:34 PM, Keith Busch wrote: > On Fri, Aug 26, 2022 at 11:12:04PM +0800, Jinhao Fan wrote: >> Use KVM's irqfd to send interrupts when possible. This approach is >> thread safe. Moreover, it does not have the inter-thread communication >> overhead of plain event notifiers since handler callback are called >> in the same system call as irqfd write. >> >> Signed-off-by: Jinhao Fan >> Signed-off-by: Klaus Jensen > > No idea what's going on here... This one is causing the following assert > failure with --enable-kvm: > > qemu-system-x86_64: ../accel/kvm/kvm-all.c:1781: kvm_irqchip_commit_routes: > Assertion `ret == 0' failed. My intuition is that if irq-eventfd is off we shouldn’t call kvm_irqchip_commit_routes(). Probably we missed some check here. > I find it calls KVM_SET_GSI_ROUTING ioctl with gsi set to KVM_IRQ_ROUTING_MSI, > and linux kernel returns EINVAL in that case. It's never set that way without > this patch. Am I the only one seeing this?
Re: [PATCH v2 2/3] hw/nvme: use KVM irqfd when available
On Fri, Aug 26, 2022 at 05:45:21PM +0200, Klaus Jensen wrote: > On Aug 26 09:34, Keith Busch wrote: > > On Fri, Aug 26, 2022 at 11:12:04PM +0800, Jinhao Fan wrote: > > > Use KVM's irqfd to send interrupts when possible. This approach is > > > thread safe. Moreover, it does not have the inter-thread communication > > > overhead of plain event notifiers since handler callback are called > > > in the same system call as irqfd write. > > > > > > Signed-off-by: Jinhao Fan > > > Signed-off-by: Klaus Jensen > > > > No idea what's going on here... This one is causing the following assert > > failure with --enable-kvm: > > > > qemu-system-x86_64: ../accel/kvm/kvm-all.c:1781: > > kvm_irqchip_commit_routes: Assertion `ret == 0' failed. > > > > I find it calls KVM_SET_GSI_ROUTING ioctl with gsi set to > > KVM_IRQ_ROUTING_MSI, > > and linux kernel returns EINVAL in that case. It's never set that way > > without > > this patch. Am I the only one seeing this? > > Argh, sorry, I threw that patch together a bit too quickly. I was just > so pumped because I believed I had solved the issue hehe. > > Are you missing the ioeventfd=on and irq-eventfd=on parameters by any > chance? Without those I'm also getting an assertion, but a different one I had not enabled those yet. This was purely a regrsession test with my previously working paramaters for a sanity check. If I enable those new nvme parameters, then it is successful.
Re: [PATCH v2 2/3] hw/nvme: use KVM irqfd when available
at 11:34 PM, Keith Busch wrote: > On Fri, Aug 26, 2022 at 11:12:04PM +0800, Jinhao Fan wrote: >> Use KVM's irqfd to send interrupts when possible. This approach is >> thread safe. Moreover, it does not have the inter-thread communication >> overhead of plain event notifiers since handler callback are called >> in the same system call as irqfd write. >> >> Signed-off-by: Jinhao Fan >> Signed-off-by: Klaus Jensen > > No idea what's going on here... This one is causing the following assert > failure with --enable-kvm: > > qemu-system-x86_64: ../accel/kvm/kvm-all.c:1781: kvm_irqchip_commit_routes: > Assertion `ret == 0' failed. > > I find it calls KVM_SET_GSI_ROUTING ioctl with gsi set to KVM_IRQ_ROUTING_MSI, > and linux kernel returns EINVAL in that case. It's never set that way without > this patch. Am I the only one seeing this? nvme_start_ctrl() registers MSI-X masking handlers without checking irq-eventfd. This causes nvme_kvm_vector_unmask() to be called when it is not supposed to.
