[1/2] pci-assign: Switch to pci_device_route_intx_to_irq interface

Commit Message

Jan Kiszka Aug. 3, 2012, 10:51 a.m. UTC

From: Jan Kiszka <[e-mail]>

Drop pci_map_irq/piix_get_irq in favor of upstream's new interface. This
should also properly model disabling of the line at PCI host controller
level.

Signed-off-by: Jan Kiszka <[e-mail]>
---
 hw/device-assignment.c |   29 +++++++++++++++++++----------
 hw/pc.h                |    4 ----
 hw/pci.c               |    4 ----
 hw/pci.h               |    2 --
 hw/piix_pci.c          |    7 -------
 5 files changed, 19 insertions(+), 27 deletions(-)

Comments

Alex Williamson Aug. 3, 2012, 3:26 p.m. UTC | #1

On Fri, 2012-08-03 at 12:51 +0200, Jan Kiszka wrote:
> From: Jan Kiszka <[e-mail]>
> 
> Drop pci_map_irq/piix_get_irq in favor of upstream's new interface. This
> should also properly model disabling of the line at PCI host controller
> level.
> 
> Signed-off-by: Jan Kiszka <[e-mail]>
> ---
>  hw/device-assignment.c |   29 +++++++++++++++++++----------
>  hw/pc.h                |    4 ----
>  hw/pci.c               |    4 ----
>  hw/pci.h               |    2 --
>  hw/piix_pci.c          |    7 -------
>  5 files changed, 19 insertions(+), 27 deletions(-)
> 
> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> index cc39958..d14c327 100644
> --- a/hw/device-assignment.c
> +++ b/hw/device-assignment.c
> @@ -115,7 +115,7 @@ typedef struct AssignedDevice {
>      AssignedDevRegion v_addrs[PCI_NUM_REGIONS - 1];
>      PCIDevRegions real_device;
>      int run;
> -    int girq;
> +    PCIINTxRoute intx_route;
>      uint16_t h_segnr;
>      uint8_t h_busnr;
>      uint8_t h_devfn;
> @@ -865,21 +865,24 @@ static int assign_device(AssignedDevice *dev)
>  static int assign_irq(AssignedDevice *dev)
>  {
>      struct kvm_assigned_irq assigned_irq_data;
> -    int irq, r = 0;
> +    PCIINTxRoute intx_route;
> +    int r = 0;
>  
>      /* Interrupt PIN 0 means don't use INTx */
>      if (assigned_dev_pci_read_byte(&dev->dev, PCI_INTERRUPT_PIN) == 0)
>          return 0;
>  
> -    irq = pci_map_irq(&dev->dev, dev->intpin);
> -    irq = piix_get_irq(irq);
> +    intx_route = pci_device_route_intx_to_irq(&dev->dev, 0);
> +    assert(intx_route.mode != PCI_INTX_INVERTED);
>  
> -    if (dev->girq == irq)
> +    if (dev->intx_route.mode == intx_route.mode &&
> +        dev->intx_route.irq == intx_route.irq) {
>          return r;
> +    }
>  
>      memset(&assigned_irq_data, 0, sizeof(assigned_irq_data));
>      assigned_irq_data.assigned_dev_id = calc_assigned_dev_id(dev);
> -    assigned_irq_data.guest_irq = irq;
> +    assigned_irq_data.guest_irq = intx_route.irq;
>      if (dev->irq_requested_type) {
>          assigned_irq_data.flags = dev->irq_requested_type;
>          r = kvm_deassign_irq(kvm_state, &assigned_irq_data);
> @@ -889,6 +892,11 @@ static int assign_irq(AssignedDevice *dev)
>          dev->irq_requested_type = 0;
>      }
>  
> +    if (intx_route.mode == PCI_INTX_DISABLED) {
> +        dev->intx_route = intx_route;
> +        return 0;
> +    }
> +
>  retry:
>      assigned_irq_data.flags = KVM_DEV_IRQ_GUEST_INTX;
>      if (dev->features & ASSIGNED_DEVICE_PREFER_MSI_MASK &&
> @@ -917,7 +925,7 @@ retry:
>          return r;
>      }
>  
> -    dev->girq = irq;
> +    dev->intx_route = intx_route;
>      dev->irq_requested_type = assigned_irq_data.flags;
>      return r;
>  }
> @@ -1029,7 +1037,7 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev)
>              perror("assigned_dev_enable_msi: assign irq");
>          }
>  
> -        assigned_dev->girq = -1;
> +        assigned_dev->intx_route.mode = PCI_INTX_DISABLED;
>          assigned_dev->irq_requested_type = assigned_irq_data.flags;
>      } else {
>          assign_irq(assigned_dev);
> @@ -1160,7 +1168,7 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev)
>                  return;
>              }
>          }
> -        assigned_dev->girq = -1;
> +        assigned_dev->intx_route.mode = PCI_INTX_DISABLED;

Should both of these set assigned_dev->intx_route.irq = -1 as well?  I
don't think it hurts anything this way but we test both mode & irq in
assign_irq() and we'll end up calling kvm_deassign_irq() again.

>          assigned_dev->irq_requested_type = assigned_irq_data.flags;
>      } else {
>          assign_irq(assigned_dev);
> @@ -1784,7 +1792,8 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
>      e_intx = dev->dev.config[0x3d] - 1;
>      dev->intpin = e_intx;
>      dev->run = 0;
> -    dev->girq = -1;
> +    dev->intx_route.mode = PCI_INTX_DISABLED;
> +    dev->intx_route.irq = -1;

And here we set both, so it's a little inconsistent.

>      dev->h_segnr = dev->host.domain;
>      dev->h_busnr = dev->host.bus;
>      dev->h_devfn = PCI_DEVFN(dev->host.slot, dev->host.function);
> diff --git a/hw/pc.h b/hw/pc.h
> index a662090..5b36eb5 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -171,10 +171,6 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
>  extern PCIDevice *piix4_dev;
>  int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn);
>  
> -int piix_get_irq(int pin);
> -
> -int ipf_map_irq(PCIDevice *pci_dev, int irq_num);
> -
>  /* vga.c */
>  enum vga_retrace_method {
>      VGA_RETRACE_DUMB,
> diff --git a/hw/pci.c b/hw/pci.c
> index 5ef3453..0b22913 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1089,10 +1089,6 @@ static void pci_set_irq(void *opaque, int irq_num, int level)
>      pci_change_irq_level(pci_dev, irq_num, change);
>  }
>  
> -int pci_map_irq(PCIDevice *pci_dev, int pin)
> -{
> -    return pci_dev->bus->map_irq(pci_dev, pin);
> -}
>  /* Special hooks used by device assignment */
>  void pci_bus_set_route_irq_fn(PCIBus *bus, pci_route_irq_fn route_intx_to_irq)
>  {
> diff --git a/hw/pci.h b/hw/pci.h
> index c69af01..4b6ab3d 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -274,8 +274,6 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
>                        uint8_t attr, MemoryRegion *memory);
>  pcibus_t pci_get_bar_addr(PCIDevice *pci_dev, int region_num);
>  
> -int pci_map_irq(PCIDevice *pci_dev, int pin);
> -
>  int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
>                         uint8_t offset, uint8_t size);
>  
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index a5090cb..355814f 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -435,13 +435,6 @@ static void piix3_write_config(PCIDevice *dev,
>      }
>  }
>  
> -int piix_get_irq(int pin)
> -{
> -    if (piix3_dev)
> -        return piix3_dev->dev.config[0x60+pin];
> -    return 0;
> -}
> -
>  static void piix3_write_config_xen(PCIDevice *dev,
>                                 uint32_t address, uint32_t val, int len)
>  {



--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index cc39958..d14c327 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -115,7 +115,7 @@  typedef struct AssignedDevice {
     AssignedDevRegion v_addrs[PCI_NUM_REGIONS - 1];
     PCIDevRegions real_device;
     int run;
-    int girq;
+    PCIINTxRoute intx_route;
     uint16_t h_segnr;
     uint8_t h_busnr;
     uint8_t h_devfn;
@@ -865,21 +865,24 @@  static int assign_device(AssignedDevice *dev)
 static int assign_irq(AssignedDevice *dev)
 {
     struct kvm_assigned_irq assigned_irq_data;
-    int irq, r = 0;
+    PCIINTxRoute intx_route;
+    int r = 0;
 
     /* Interrupt PIN 0 means don't use INTx */
     if (assigned_dev_pci_read_byte(&dev->dev, PCI_INTERRUPT_PIN) == 0)
         return 0;
 
-    irq = pci_map_irq(&dev->dev, dev->intpin);
-    irq = piix_get_irq(irq);
+    intx_route = pci_device_route_intx_to_irq(&dev->dev, 0);
+    assert(intx_route.mode != PCI_INTX_INVERTED);
 
-    if (dev->girq == irq)
+    if (dev->intx_route.mode == intx_route.mode &&
+        dev->intx_route.irq == intx_route.irq) {
         return r;
+    }
 
     memset(&assigned_irq_data, 0, sizeof(assigned_irq_data));
     assigned_irq_data.assigned_dev_id = calc_assigned_dev_id(dev);
-    assigned_irq_data.guest_irq = irq;
+    assigned_irq_data.guest_irq = intx_route.irq;
     if (dev->irq_requested_type) {
         assigned_irq_data.flags = dev->irq_requested_type;
         r = kvm_deassign_irq(kvm_state, &assigned_irq_data);
@@ -889,6 +892,11 @@  static int assign_irq(AssignedDevice *dev)
         dev->irq_requested_type = 0;
     }
 
+    if (intx_route.mode == PCI_INTX_DISABLED) {
+        dev->intx_route = intx_route;
+        return 0;
+    }
+
 retry:
     assigned_irq_data.flags = KVM_DEV_IRQ_GUEST_INTX;
     if (dev->features & ASSIGNED_DEVICE_PREFER_MSI_MASK &&
@@ -917,7 +925,7 @@  retry:
         return r;
     }
 
-    dev->girq = irq;
+    dev->intx_route = intx_route;
     dev->irq_requested_type = assigned_irq_data.flags;
     return r;
 }
@@ -1029,7 +1037,7 @@  static void assigned_dev_update_msi(PCIDevice *pci_dev)
             perror("assigned_dev_enable_msi: assign irq");
         }
 
-        assigned_dev->girq = -1;
+        assigned_dev->intx_route.mode = PCI_INTX_DISABLED;
         assigned_dev->irq_requested_type = assigned_irq_data.flags;
     } else {
         assign_irq(assigned_dev);
@@ -1160,7 +1168,7 @@  static void assigned_dev_update_msix(PCIDevice *pci_dev)
                 return;
             }
         }
-        assigned_dev->girq = -1;
+        assigned_dev->intx_route.mode = PCI_INTX_DISABLED;
         assigned_dev->irq_requested_type = assigned_irq_data.flags;
     } else {
         assign_irq(assigned_dev);
@@ -1784,7 +1792,8 @@  static int assigned_initfn(struct PCIDevice *pci_dev)
     e_intx = dev->dev.config[0x3d] - 1;
     dev->intpin = e_intx;
     dev->run = 0;
-    dev->girq = -1;
+    dev->intx_route.mode = PCI_INTX_DISABLED;
+    dev->intx_route.irq = -1;
     dev->h_segnr = dev->host.domain;
     dev->h_busnr = dev->host.bus;
     dev->h_devfn = PCI_DEVFN(dev->host.slot, dev->host.function);
diff --git a/hw/pc.h b/hw/pc.h
index a662090..5b36eb5 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -171,10 +171,6 @@  PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
 extern PCIDevice *piix4_dev;
 int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn);
 
-int piix_get_irq(int pin);
-
-int ipf_map_irq(PCIDevice *pci_dev, int irq_num);
-
 /* vga.c */
 enum vga_retrace_method {
     VGA_RETRACE_DUMB,
diff --git a/hw/pci.c b/hw/pci.c
index 5ef3453..0b22913 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1089,10 +1089,6 @@  static void pci_set_irq(void *opaque, int irq_num, int level)
     pci_change_irq_level(pci_dev, irq_num, change);
 }
 
-int pci_map_irq(PCIDevice *pci_dev, int pin)
-{
-    return pci_dev->bus->map_irq(pci_dev, pin);
-}
 /* Special hooks used by device assignment */
 void pci_bus_set_route_irq_fn(PCIBus *bus, pci_route_irq_fn route_intx_to_irq)
 {
diff --git a/hw/pci.h b/hw/pci.h
index c69af01..4b6ab3d 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -274,8 +274,6 @@  void pci_register_bar(PCIDevice *pci_dev, int region_num,
                       uint8_t attr, MemoryRegion *memory);
 pcibus_t pci_get_bar_addr(PCIDevice *pci_dev, int region_num);
 
-int pci_map_irq(PCIDevice *pci_dev, int pin);
-
 int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
                        uint8_t offset, uint8_t size);
 
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index a5090cb..355814f 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -435,13 +435,6 @@  static void piix3_write_config(PCIDevice *dev,
     }
 }
 
-int piix_get_irq(int pin)
-{
-    if (piix3_dev)
-        return piix3_dev->dev.config[0x60+pin];
-    return 0;
-}
-
 static void piix3_write_config_xen(PCIDevice *dev,
                                uint32_t address, uint32_t val, int len)
 {