| Message ID | 20131206204842.16731.29998.stgit@bling.home |
|---|---|
| State | New |
| Headers | show |
Il 06/12/2013 21:48, Alex Williamson ha scritto: > /* Extra debugging, trap acceleration paths for more logging */ > #define VFIO_ALLOW_MMAP 1 > #define VFIO_ALLOW_KVM_INTX 1 > +#define VFIO_ALLOW_KVM_MSI 1 > +#define VFIO_ALLOW_KVM_MSIX 1 Why not make these device properties instead? Paolo
On Fri, 2013-12-06 at 23:06 +0100, Paolo Bonzini wrote: > Il 06/12/2013 21:48, Alex Williamson ha scritto: > > /* Extra debugging, trap acceleration paths for more logging */ > > #define VFIO_ALLOW_MMAP 1 > > #define VFIO_ALLOW_KVM_INTX 1 > > +#define VFIO_ALLOW_KVM_MSI 1 > > +#define VFIO_ALLOW_KVM_MSIX 1 > > Why not make these device properties instead? Honestly, I don't think they're that useful to the average or even to the advanced user. Each of these disables an acceleration/bypass path and gives more logging output when debug is enabled. Otherwise we don't get debugging to see when mmap'd BARs are accessed and we can't tell when interrupts happen relative to other accesses. For instance it was pretty obvious to find the Nvidia MSI ACK when seeing a config space write immediately following an MSI injection. Without debugging enabled, these just slow things down and I don't know of any cases where a device behaves better with these features disabled. Thanks, Alex
Il 06/12/2013 23:36, Alex Williamson ha scritto: >>> > > #define VFIO_ALLOW_MMAP 1 >>> > > #define VFIO_ALLOW_KVM_INTX 1 >>> > > +#define VFIO_ALLOW_KVM_MSI 1 >>> > > +#define VFIO_ALLOW_KVM_MSIX 1 >> > >> > Why not make these device properties instead? > Honestly, I don't think they're that useful to the average or even to > the advanced user. Each of these disables an acceleration/bypass path > and gives more logging output when debug is enabled. Otherwise we don't > get debugging to see when mmap'd BARs are accessed and we can't tell > when interrupts happen relative to other accesses. For instance it was > pretty obvious to find the Nvidia MSI ACK when seeing a config space > write immediately following an MSI injection. Without debugging > enabled, these just slow things down and I don't know of any cases where > a device behaves better with these features disabled. Yeah, behaving better is unlikely. I was thinking of them as a debugging features. As to debug messages, have you considered replacing them with tracepoints? Paolo
diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c index 09bdddf..f367537 100644 --- a/hw/misc/vfio.c +++ b/hw/misc/vfio.c @@ -52,6 +52,8 @@ /* Extra debugging, trap acceleration paths for more logging */ #define VFIO_ALLOW_MMAP 1 #define VFIO_ALLOW_KVM_INTX 1 +#define VFIO_ALLOW_KVM_MSI 1 +#define VFIO_ALLOW_KVM_MSIX 1 struct VFIODevice; @@ -590,9 +592,21 @@ static void vfio_msi_interrupt(void *opaque) return; } - DPRINTF("%s(%04x:%02x:%02x.%x) vector %d\n", __func__, +#ifdef VFIO_DEBUG + MSIMessage msg; + + if (vdev->interrupt == VFIO_INT_MSIX) { + msg = msi_get_message(&vdev->pdev, nr); + } else if (vdev->interrupt == VFIO_INT_MSI) { + msg = msix_get_message(&vdev->pdev, nr); + } else { + abort(); + } + + DPRINTF("%s(%04x:%02x:%02x.%x) vector %d 0x%"PRIx64"/0x%x\n", __func__, vdev->host.domain, vdev->host.bus, vdev->host.slot, - vdev->host.function, nr); + vdev->host.function, nr, msg.address, msg.data); +#endif if (vdev->interrupt == VFIO_INT_MSIX) { msix_notify(&vdev->pdev, nr); @@ -660,7 +674,8 @@ static int vfio_msix_vector_do_use(PCIDevice *pdev, unsigned int nr, * Attempt to enable route through KVM irqchip, * default to userspace handling if unavailable. */ - vector->virq = msg ? kvm_irqchip_add_msi_route(kvm_state, *msg) : -1; + vector->virq = msg && VFIO_ALLOW_KVM_MSIX ? + kvm_irqchip_add_msi_route(kvm_state, *msg) : -1; if (vector->virq < 0 || kvm_irqchip_add_irqfd_notifier(kvm_state, &vector->interrupt, NULL, vector->virq) < 0) { @@ -827,7 +842,8 @@ retry: * Attempt to enable route through KVM irqchip, * default to userspace handling if unavailable. */ - vector->virq = kvm_irqchip_add_msi_route(kvm_state, vector->msg); + vector->virq = VFIO_ALLOW_KVM_MSI ? + kvm_irqchip_add_msi_route(kvm_state, vector->msg) : -1; if (vector->virq < 0 || kvm_irqchip_add_irqfd_notifier(kvm_state, &vector->interrupt, NULL, vector->virq) < 0) {
It's sometimes useful to be able to verify interrupts are passing through correctly. Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- hw/misc/vfio.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-)