| Message ID | 1380717694-13091-6-git-send-email-marcel.a@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, 2013-10-02 at 15:41 +0300, Marcel Apfelbaum wrote: > pci_set_irq and the other pci irq wrappers use > PCI_INTERRUPT_PIN config register to compute device > INTx pin to assert/deassert. > > Save INTx pin into the config register before calling > pci_set_irq > > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> > --- > hw/misc/vfio.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) Seems ok, but why not take advantage of the pci_irq_raise/lower() wrappers? BTW, with PCI being active low, should those be assert/deassert to avoid confusion confusion with the actual signal level? Thanks, Alex > > diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c > index a1c08fb..3d7297c 100644 > --- a/hw/misc/vfio.c > +++ b/hw/misc/vfio.c > @@ -297,7 +297,7 @@ static void vfio_intx_interrupt(void *opaque) > 'A' + vdev->intx.pin); > > vdev->intx.pending = true; > - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 1); > + pci_set_irq(&vdev->pdev, 1); > vfio_mmap_set_enabled(vdev, false); > if (vdev->intx.mmap_timeout) { > timer_mod(vdev->intx.mmap_timer, > @@ -315,7 +315,7 @@ static void vfio_eoi(VFIODevice *vdev) > vdev->host.bus, vdev->host.slot, vdev->host.function); > > vdev->intx.pending = false; > - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0); > + pci_set_irq(&vdev->pdev, 0); > vfio_unmask_intx(vdev); > } > > @@ -341,7 +341,7 @@ static void vfio_enable_intx_kvm(VFIODevice *vdev) > qemu_set_fd_handler(irqfd.fd, NULL, NULL, vdev); > vfio_mask_intx(vdev); > vdev->intx.pending = false; > - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0); > + pci_set_irq(&vdev->pdev, 0); > > /* Get an eventfd for resample/unmask */ > if (event_notifier_init(&vdev->intx.unmask, 0)) { > @@ -417,7 +417,7 @@ static void vfio_disable_intx_kvm(VFIODevice *vdev) > */ > vfio_mask_intx(vdev); > vdev->intx.pending = false; > - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0); > + pci_set_irq(&vdev->pdev, 0); > > /* Tell KVM to stop listening for an INTx irqfd */ > if (kvm_vm_ioctl(kvm_state, KVM_IRQFD, &irqfd)) { > @@ -488,6 +488,7 @@ static int vfio_enable_intx(VFIODevice *vdev) > vfio_disable_interrupts(vdev); > > vdev->intx.pin = pin - 1; /* Pin A (1) -> irq[0] */ > + pci_config_set_interrupt_pin(vdev->pdev.config, pin); > > #ifdef CONFIG_KVM > /* > @@ -547,7 +548,7 @@ static void vfio_disable_intx(VFIODevice *vdev) > vfio_disable_intx_kvm(vdev); > vfio_disable_irqindex(vdev, VFIO_PCI_INTX_IRQ_INDEX); > vdev->intx.pending = false; > - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0); > + pci_set_irq(&vdev->pdev, 0); > vfio_mmap_set_enabled(vdev, true); > > fd = event_notifier_get_fd(&vdev->intx.interrupt);
On Wed, 2013-10-02 at 09:58 -0600, Alex Williamson wrote: > On Wed, 2013-10-02 at 15:41 +0300, Marcel Apfelbaum wrote: > > pci_set_irq and the other pci irq wrappers use > > PCI_INTERRUPT_PIN config register to compute device > > INTx pin to assert/deassert. > > > > Save INTx pin into the config register before calling > > pci_set_irq > > > > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> > > --- > > hw/misc/vfio.c | 11 ++++++----- > > 1 file changed, 6 insertions(+), 5 deletions(-) > > Seems ok, but why not take advantage of the pci_irq_raise/lower() > wrappers? BTW, with PCI being active low, should those be > assert/deassert to avoid confusion confusion with the actual signal > level? Thanks, Thanks for the review! I can use pci_irq_raise/lower(), but I wanted to preserve the current form, re-factoring: qemu_set_irq -> pci_set_irq, qemu_irq_lower -> pci_irq_lower ... If you think is worth it, I'll change it. (in all the places) About assert/deassert instead of lower/raise, I am afraid it will confuse users having two different set of naming for interrupts usage. Is easier to understand that pci_irq_lower behaves the same as qemu_pci_lower, then pci_irq_deassert. What do you think? Thanks, Marcel > > Alex > > > > > diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c > > index a1c08fb..3d7297c 100644 > > --- a/hw/misc/vfio.c > > +++ b/hw/misc/vfio.c > > @@ -297,7 +297,7 @@ static void vfio_intx_interrupt(void *opaque) > > 'A' + vdev->intx.pin); > > > > vdev->intx.pending = true; > > - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 1); > > + pci_set_irq(&vdev->pdev, 1); > > vfio_mmap_set_enabled(vdev, false); > > if (vdev->intx.mmap_timeout) { > > timer_mod(vdev->intx.mmap_timer, > > @@ -315,7 +315,7 @@ static void vfio_eoi(VFIODevice *vdev) > > vdev->host.bus, vdev->host.slot, vdev->host.function); > > > > vdev->intx.pending = false; > > - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0); > > + pci_set_irq(&vdev->pdev, 0); > > vfio_unmask_intx(vdev); > > } > > > > @@ -341,7 +341,7 @@ static void vfio_enable_intx_kvm(VFIODevice *vdev) > > qemu_set_fd_handler(irqfd.fd, NULL, NULL, vdev); > > vfio_mask_intx(vdev); > > vdev->intx.pending = false; > > - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0); > > + pci_set_irq(&vdev->pdev, 0); > > > > /* Get an eventfd for resample/unmask */ > > if (event_notifier_init(&vdev->intx.unmask, 0)) { > > @@ -417,7 +417,7 @@ static void vfio_disable_intx_kvm(VFIODevice *vdev) > > */ > > vfio_mask_intx(vdev); > > vdev->intx.pending = false; > > - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0); > > + pci_set_irq(&vdev->pdev, 0); > > > > /* Tell KVM to stop listening for an INTx irqfd */ > > if (kvm_vm_ioctl(kvm_state, KVM_IRQFD, &irqfd)) { > > @@ -488,6 +488,7 @@ static int vfio_enable_intx(VFIODevice *vdev) > > vfio_disable_interrupts(vdev); > > > > vdev->intx.pin = pin - 1; /* Pin A (1) -> irq[0] */ > > + pci_config_set_interrupt_pin(vdev->pdev.config, pin); > > > > #ifdef CONFIG_KVM > > /* > > @@ -547,7 +548,7 @@ static void vfio_disable_intx(VFIODevice *vdev) > > vfio_disable_intx_kvm(vdev); > > vfio_disable_irqindex(vdev, VFIO_PCI_INTX_IRQ_INDEX); > > vdev->intx.pending = false; > > - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0); > > + pci_set_irq(&vdev->pdev, 0); > > vfio_mmap_set_enabled(vdev, true); > > > > fd = event_notifier_get_fd(&vdev->intx.interrupt); > > >
diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c index a1c08fb..3d7297c 100644 --- a/hw/misc/vfio.c +++ b/hw/misc/vfio.c @@ -297,7 +297,7 @@ static void vfio_intx_interrupt(void *opaque) 'A' + vdev->intx.pin); vdev->intx.pending = true; - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 1); + pci_set_irq(&vdev->pdev, 1); vfio_mmap_set_enabled(vdev, false); if (vdev->intx.mmap_timeout) { timer_mod(vdev->intx.mmap_timer, @@ -315,7 +315,7 @@ static void vfio_eoi(VFIODevice *vdev) vdev->host.bus, vdev->host.slot, vdev->host.function); vdev->intx.pending = false; - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0); + pci_set_irq(&vdev->pdev, 0); vfio_unmask_intx(vdev); } @@ -341,7 +341,7 @@ static void vfio_enable_intx_kvm(VFIODevice *vdev) qemu_set_fd_handler(irqfd.fd, NULL, NULL, vdev); vfio_mask_intx(vdev); vdev->intx.pending = false; - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0); + pci_set_irq(&vdev->pdev, 0); /* Get an eventfd for resample/unmask */ if (event_notifier_init(&vdev->intx.unmask, 0)) { @@ -417,7 +417,7 @@ static void vfio_disable_intx_kvm(VFIODevice *vdev) */ vfio_mask_intx(vdev); vdev->intx.pending = false; - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0); + pci_set_irq(&vdev->pdev, 0); /* Tell KVM to stop listening for an INTx irqfd */ if (kvm_vm_ioctl(kvm_state, KVM_IRQFD, &irqfd)) { @@ -488,6 +488,7 @@ static int vfio_enable_intx(VFIODevice *vdev) vfio_disable_interrupts(vdev); vdev->intx.pin = pin - 1; /* Pin A (1) -> irq[0] */ + pci_config_set_interrupt_pin(vdev->pdev.config, pin); #ifdef CONFIG_KVM /* @@ -547,7 +548,7 @@ static void vfio_disable_intx(VFIODevice *vdev) vfio_disable_intx_kvm(vdev); vfio_disable_irqindex(vdev, VFIO_PCI_INTX_IRQ_INDEX); vdev->intx.pending = false; - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0); + pci_set_irq(&vdev->pdev, 0); vfio_mmap_set_enabled(vdev, true); fd = event_notifier_get_fd(&vdev->intx.interrupt);
pci_set_irq and the other pci irq wrappers use PCI_INTERRUPT_PIN config register to compute device INTx pin to assert/deassert. Save INTx pin into the config register before calling pci_set_irq Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> --- hw/misc/vfio.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)