[Qemu-devel] Re: [PATCH 02/26] pci: add opaque argument to pci_map_irq_fn

[Michael S. Tsirkin]

On Wed, Mar 16, 2011 at 06:29:13PM +0900, Isaku Yamahata wrote:
> Pass opaque argument to pci_map_irq_fn like pci_set_irq_fn.
> ICH9 irq routing is not static, but configurable by chipset configuration
> registers, so the corresponding irq mapping function of pci_map_irq_fn
> needs to know the pointer to ich9.
> 
> Cc: Michael S. Tsirkin <address@hidden>
> Signed-off-by: Isaku Yamahata <address@hidden>

Not an objection to this patch - just thinking aloud: as mapping changes
rarely, something I wanted to do for a while now is replace
the walk up the bus with indirect function calls
at each step on each set_irq call with:
- walk down the bus on each mapping change, update all devices
- pci_set_irq which is not indirect, just calls directly to
  the interrupt controller

But we can do this optimization later.

> ---
>  hw/apb_pci.c       |    4 ++--
>  hw/bonito.c        |    2 +-
>  hw/dec_pci.c       |    2 +-
>  hw/grackle_pci.c   |    2 +-
>  hw/gt64xxx.c       |    2 +-
>  hw/pci.c           |    2 +-
>  hw/pci.h           |    2 +-
>  hw/piix_pci.c      |    2 +-
>  hw/ppc4xx_pci.c    |    2 +-
>  hw/ppce500_pci.c   |    2 +-
>  hw/prep_pci.c      |    2 +-
>  hw/sh_pci.c        |    2 +-
>  hw/unin_pci.c      |    2 +-
>  hw/versatile_pci.c |    2 +-
>  14 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/apb_pci.c b/hw/apb_pci.c
> index 84e9af7..c56ea9a 100644
> --- a/hw/apb_pci.c
> +++ b/hw/apb_pci.c
> @@ -264,12 +264,12 @@ static CPUReadMemoryFunc * const pci_apb_ioread[] = {
>  };
>  
>  /* The APB host has an IRQ line for each IRQ line of each slot.  */
> -static int pci_apb_map_irq(PCIDevice *pci_dev, int irq_num)
> +static int pci_apb_map_irq(void *opaque, PCIDevice *pci_dev, int irq_num)
>  {
>      return ((pci_dev->devfn & 0x18) >> 1) + irq_num;
>  }
>  
> -static int pci_pbm_map_irq(PCIDevice *pci_dev, int irq_num)
> +static int pci_pbm_map_irq(void *opaque, PCIDevice *pci_dev, int irq_num)
>  {
>      int bus_offset;
>      if (pci_dev->devfn & 1)
> diff --git a/hw/bonito.c b/hw/bonito.c
> index 65a4a63..94e69f5 100644
> --- a/hw/bonito.c
> +++ b/hw/bonito.c
> @@ -632,7 +632,7 @@ static void pci_bonito_set_irq(void *opaque, int irq_num, 
> int level)
>  }
>  
>  /* map the original irq (0~3) to bonito irq (16~47, but 16~31 are unused) */
> -static int pci_bonito_map_irq(PCIDevice * pci_dev, int irq_num)
> +static int pci_bonito_map_irq(void *opaque, PCIDevice * pci_dev, int irq_num)
>  {
>      int slot;
>  
> diff --git a/hw/dec_pci.c b/hw/dec_pci.c
> index bf88f2a..3cc4f04 100644
> --- a/hw/dec_pci.c
> +++ b/hw/dec_pci.c
> @@ -45,7 +45,7 @@ typedef struct DECState {
>      PCIHostState host_state;
>  } DECState;
>  
> -static int dec_map_irq(PCIDevice *pci_dev, int irq_num)
> +static int dec_map_irq(void *opaque, PCIDevice *pci_dev, int irq_num)
>  {
>      return irq_num;
>  }
> diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c
> index d35701f..4ed1ec6 100644
> --- a/hw/grackle_pci.c
> +++ b/hw/grackle_pci.c
> @@ -44,7 +44,7 @@ typedef struct GrackleState {
>  } GrackleState;
>  
>  /* Don't know if this matches real hardware, but it agrees with OHW.  */
> -static int pci_grackle_map_irq(PCIDevice *pci_dev, int irq_num)
> +static int pci_grackle_map_irq(void *opaque, PCIDevice *pci_dev, int irq_num)
>  {
>      return (irq_num + (pci_dev->devfn >> 3)) & 3;
>  }
> diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
> index c66188f..e6e2828 100644
> --- a/hw/gt64xxx.c
> +++ b/hw/gt64xxx.c
> @@ -862,7 +862,7 @@ static CPUReadMemoryFunc * const gt64120_read[] = {
>      &gt64120_readl,
>  };
>  
> -static int gt64120_pci_map_irq(PCIDevice *pci_dev, int irq_num)
> +static int gt64120_pci_map_irq(void *opaque, PCIDevice *pci_dev, int irq_num)
>  {
>      int slot;
>  
> diff --git a/hw/pci.c b/hw/pci.c
> index 8d7bfff..5349488 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -117,7 +117,7 @@ static void pci_change_irq_level(PCIDevice *pci_dev, int 
> irq_num, int change)
>      PCIBus *bus;
>      for (;;) {
>          bus = pci_dev->bus;
> -        irq_num = bus->map_irq(pci_dev, irq_num);
> +        irq_num = bus->map_irq(bus->irq_opaque, pci_dev, irq_num);
>          if (bus->set_irq)
>              break;
>          pci_dev = bus->parent_dev;
> diff --git a/hw/pci.h b/hw/pci.h
> index 7d56337..1a08139 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -219,7 +219,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f);
>  int pci_device_load(PCIDevice *s, QEMUFile *f);
>  
>  typedef void (*pci_set_irq_fn)(void *opaque, int irq_num, int level);
> -typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
> +typedef int (*pci_map_irq_fn)(void *opaque, PCIDevice *pci_dev, int irq_num);
>  
>  typedef enum {
>      PCI_HOTPLUG_DISABLED,
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 358da58..892c576 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -60,7 +60,7 @@ static void piix3_set_irq(void *opaque, int irq_num, int 
> level);
>  /* return the global irq number corresponding to a given device irq
>     pin. We could also use the bus number to have a more precise
>     mapping. */
> -static int pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
> +static int pci_slot_get_pirq(void *opaque, PCIDevice *pci_dev, int irq_num)
>  {
>      int slot_addend;
>      slot_addend = (pci_dev->devfn >> 3) - 1;
> diff --git a/hw/ppc4xx_pci.c b/hw/ppc4xx_pci.c
> index f62f1f9..11ca763 100644
> --- a/hw/ppc4xx_pci.c
> +++ b/hw/ppc4xx_pci.c
> @@ -267,7 +267,7 @@ static void ppc4xx_pci_reset(void *opaque)
>  
>  /* On Bamboo, all pins from each slot are tied to a single board IRQ. This
>   * may need further refactoring for other boards. */
> -static int ppc4xx_pci_map_irq(PCIDevice *pci_dev, int irq_num)
> +static int ppc4xx_pci_map_irq(void *opaque, PCIDevice *pci_dev, int irq_num)
>  {
>      int slot = pci_dev->devfn >> 3;
>  
> diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
> index 11edd03..029a3f9 100644
> --- a/hw/ppce500_pci.c
> +++ b/hw/ppce500_pci.c
> @@ -187,7 +187,7 @@ static CPUWriteMemoryFunc * const e500_pci_reg_write[] = {
>      &pci_reg_write4,
>  };
>  
> -static int mpc85xx_pci_map_irq(PCIDevice *pci_dev, int irq_num)
> +static int mpc85xx_pci_map_irq(void *opaque, PCIDevice *pci_dev, int irq_num)
>  {
>      int devno = pci_dev->devfn >> 3, ret = 0;
>  
> diff --git a/hw/prep_pci.c b/hw/prep_pci.c
> index f88b825..8287246 100644
> --- a/hw/prep_pci.c
> +++ b/hw/prep_pci.c
> @@ -98,7 +98,7 @@ static CPUReadMemoryFunc * const PPC_PCIIO_read[] = {
>      &PPC_PCIIO_readl,
>  };
>  
> -static int prep_map_irq(PCIDevice *pci_dev, int irq_num)
> +static int prep_map_irq(void *opaque, PCIDevice *pci_dev, int irq_num)
>  {
>      return (irq_num + (pci_dev->devfn >> 3)) & 1;
>  }
> diff --git a/hw/sh_pci.c b/hw/sh_pci.c
> index e99d8db..f108759 100644
> --- a/hw/sh_pci.c
> +++ b/hw/sh_pci.c
> @@ -93,7 +93,7 @@ static MemOp sh_pci_reg = {
>      { NULL, NULL, sh_pci_reg_write },
>  };
>  
> -static int sh_pci_map_irq(PCIDevice *d, int irq_num)
> +static int sh_pci_map_irq(void *opaque, PCIDevice *d, int irq_num)
>  {
>      return (d->devfn >> 3);
>  }
> diff --git a/hw/unin_pci.c b/hw/unin_pci.c
> index c57c0a1..1496625 100644
> --- a/hw/unin_pci.c
> +++ b/hw/unin_pci.c
> @@ -44,7 +44,7 @@ typedef struct UNINState {
>      ReadWriteHandler data_handler;
>  } UNINState;
>  
> -static int pci_unin_map_irq(PCIDevice *pci_dev, int irq_num)
> +static int pci_unin_map_irq(void *opaque, PCIDevice *pci_dev, int irq_num)
>  {
>      int retval;
>      int devfn = pci_dev->devfn & 0x00FFFFFF;
> diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c
> index 2fed8a0..c002fb2 100644
> --- a/hw/versatile_pci.c
> +++ b/hw/versatile_pci.c
> @@ -74,7 +74,7 @@ static CPUReadMemoryFunc * const pci_vpb_config_read[] = {
>      &pci_vpb_config_readl,
>  };
>  
> -static int pci_vpb_map_irq(PCIDevice *d, int irq_num)
> +static int pci_vpb_map_irq(void *opaque, PCIDevice *d, int irq_num)
>  {
>      return irq_num;
>  }
> -- 
> 1.7.1.1