Re: [PATCH] xen: fix interrupt routing

[Isaku Yamahata <yamahata@valinux.co.jp>]

On Wed, May 18, 2011 at 06:53:40PM +0100, stefano.stabellini@eu.citrix.com wrote:
> From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> Match the routing informations built by seabios:
> 
> - remove i440fx_write_config_xen
> we don't need to intercept pci config writes to i440FX;
> 
> - introduce piix3_write_config_xen
> we do need to intercept pci config write to the PCI-ISA bridge to update
> the PCI link routing;

So "i440FX-xen" isn't needed anymore, and something like PIIX3-xen
is necessary instead of overwriting piix3->dev.config_write.

thanks,

> 
> - remove xen_pci_slot_get_pirq
> we are now using the same link routing as seabios and qemu so we don't
> need a diffirent get_pirq function;
> 
> - fix xen_piix3_set_irq
> we always inject one of the 4 pci intx, so we can use
> xc_hvm_set_isa_irq_level to inject the interrupt. Use pci_irqs as
> initialized by seabios to map a pirq into an ISA irq. This has the
> benefit of removing all the calls to xc_hvm_set_pci_intx_level that
> doesn't work correctly anymore because from the same device number and
> intx Xen calculates a different PCI link compared to Qemu and Seabios.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  hw/piix_pci.c |   21 ++++++++++++---------
>  xen-all.c     |   16 ++++++++--------
>  2 files changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 7f1c4cc..aae6a48 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -78,6 +78,8 @@ struct PCII440FXState {
>  #define I440FX_SMRAM    0x72
>  
>  static void piix3_set_irq(void *opaque, int pirq, int level);
> +static void piix3_write_config_xen(PCIDevice *dev,
> +                               uint32_t address, uint32_t val, int len);
>  
>  /* return the global irq number corresponding to a given device irq
>     pin. We could also use the bus number to have a more precise
> @@ -173,13 +175,6 @@ static void i440fx_write_config(PCIDevice *dev,
>      }
>  }
>  
> -static void i440fx_write_config_xen(PCIDevice *dev,
> -                                    uint32_t address, uint32_t val, int len)
> -{
> -    xen_piix_pci_write_config_client(address, val, len);
> -    i440fx_write_config(dev, address, val, len);
> -}
> -
>  static int i440fx_load_old(QEMUFile* f, void *opaque, int version_id)
>  {
>      PCII440FXState *d = opaque;
> @@ -301,7 +296,8 @@ PCIBus *i440fx_xen_init(PCII440FXState **pi440fx_state, int *piix3_devfn,
>      PCIBus *b;
>  
>      b = i440fx_common_init("i440FX-xen", pi440fx_state, piix3_devfn, pic, ram_size);
> -    pci_bus_irqs(b, xen_piix3_set_irq, xen_pci_slot_get_pirq,
> +    (*pi440fx_state)->piix3->dev.config_write = piix3_write_config_xen;
> +    pci_bus_irqs(b, xen_piix3_set_irq, pci_slot_get_pirq,
>                   (*pi440fx_state)->piix3, PIIX_NUM_PIRQS);
>  
>      return b;
> @@ -365,6 +361,13 @@ static void piix3_write_config(PCIDevice *dev,
>      }
>  }
>  
> +static void piix3_write_config_xen(PCIDevice *dev,
> +                               uint32_t address, uint32_t val, int len)
> +{
> +    xen_piix_pci_write_config_client(address, val, len);
> +    piix3_write_config(dev, address, val, len);
> +}
> +
>  static void piix3_reset(void *opaque)
>  {
>      PIIX3State *d = opaque;
> @@ -471,7 +474,7 @@ static PCIDeviceInfo i440fx_info[] = {
>          .qdev.vmsd    = &vmstate_i440fx,
>          .qdev.no_user = 1,
>          .init         = i440fx_initfn,
> -        .config_write = i440fx_write_config_xen,
> +        .config_write = i440fx_write_config,
>      },{
>          .qdev.name    = "PIIX3",
>          .qdev.desc    = "ISA bridge",
> diff --git a/xen-all.c b/xen-all.c
> index 0eac202..7d7863f 100644
> --- a/xen-all.c
> +++ b/xen-all.c
> @@ -70,17 +70,16 @@ typedef struct XenIOState {
>      Notifier exit;
>  } XenIOState;
>  
> -/* Xen specific function for piix pci */
> +/* pci_irqs as initialized by seabios */
> +uint8_t pci_irqs[4] = {
> +    10, 10, 11, 11
> +};
>  
> -int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
> -{
> -    return irq_num + ((pci_dev->devfn >> 3) << 2);
> -}
> +/* Xen specific function for piix pci */
>  
> -void xen_piix3_set_irq(void *opaque, int irq_num, int level)
> +void xen_piix3_set_irq(void *opaque, int pirq, int level)
>  {
> -    xc_hvm_set_pci_intx_level(xen_xc, xen_domid, 0, 0, irq_num >> 2,
> -                              irq_num & 3, level);
> +    xc_hvm_set_isa_irq_level(xen_xc, xen_domid, pci_irqs[pirq], level);
>  }
>  
>  void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len)
> @@ -95,6 +94,7 @@ void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len)
>          }
>          v &= 0xf;
>          if (((address + i) >= 0x60) && ((address + i) <= 0x63)) {
> +            pci_irqs[address + i - 0x60] = v;
>              xc_hvm_set_pci_link_route(xen_xc, xen_domid, address + i - 0x60, v);
>          }
>      }
> -- 
> 1.7.2.3
> 
> 

-- 
yamahata