Re: [PATCH] device-assignment: move irqs update to piix emulation

[Alex Williamson <alex.williamson@redhat.com>]

On Sun, 2011-03-27 at 23:22 +0400, Konstantin Khlebnikov wrote:
> Move assigned devices irq reroute hook from generic pci code to piix emulation.
> 
> Actually without this patch this hook had never worked, because pci.c not
> include config.h and CONFIG_KVM_DEVICE_ASSIGNMENT there always undefined.

Actually it's worked fine until very recently.  I've got a patch on the
list to move pci.c back to a target object so the right config gets
included.

> Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
> ---
>  hw/pc.h       |    4 ----
>  hw/pci.c      |    8 --------
>  hw/piix_pci.c |   14 ++++++++++++++
>  3 files changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/pc.h b/hw/pc.h
> index 1291e2d..34f32ee 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -172,10 +172,6 @@ extern int no_hpet;
>  void pcspk_init(ISADevice *pit);
>  int pcspk_audio_init(qemu_irq *pic);
>  
> -/* piix_pci.c */
> -/* config space register for IRQ routing */
> -#define PIIX_CONFIG_IRQ_ROUTE 0x60
> -
>  struct PCII440FXState;
>  typedef struct PCII440FXState PCII440FXState;
>  
> diff --git a/hw/pci.c b/hw/pci.c
> index 730df5f..27af9fe 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -31,7 +31,6 @@
>  #include "loader.h"
>  #include "hw/pc.h"
>  #include "kvm.h"
> -#include "device-assignment.h"
>  #include "qemu-objects.h"
>  #include "range.h"
>  
> @@ -1165,13 +1164,6 @@ void pci_default_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int l)
>          d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear */
>      }
>  
> -#ifdef CONFIG_KVM_DEVICE_ASSIGNMENT
> -    if (kvm_enabled() && kvm_irqchip_in_kernel() &&
> -        addr >= PIIX_CONFIG_IRQ_ROUTE &&
> -	addr < PIIX_CONFIG_IRQ_ROUTE + 4)
> -        assigned_dev_update_irqs();
> -#endif /* CONFIG_KVM_DEVICE_ASSIGNMENT */
> -
>      if (ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) ||
>          ranges_overlap(addr, l, PCI_ROM_ADDRESS, 4) ||
>          ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4) ||
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 24bfe8e..d1fcdcd 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -30,6 +30,7 @@
>  #include "sysbus.h"
>  #include "range.h"
>  #include "kvm.h"
> +#include "device-assignment.h"
>  
>  /*
>   * I440FX chipset data sheet.
> @@ -353,6 +354,18 @@ static int piix3_initfn(PCIDevice *dev)
>      return 0;
>  }
>  
> +static void piix3_write_config(PCIDevice *dev,
> +                               uint32_t addr, uint32_t val, int len)
> +{
> +    pci_default_write_config(dev, addr, val, len);
> +
> +#ifdef CONFIG_KVM_DEVICE_ASSIGNMENT
> +    if (kvm_enabled() && kvm_irqchip_in_kernel() &&
> +            ranges_overlap(addr, len, 0x60, 4))

I think this should still have a #define somewhere, or else 0x60 becomes
an unsearchable magic number.  Otherwise, this looks fine to me.  I've
actually sent out RFC patches making a similar change to try to make
this device assignment hook more generic.

Alex

> +        assigned_dev_update_irqs();
> +#endif /* CONFIG_KVM_DEVICE_ASSIGNMENT */
> +}
> +
>  static PCIDeviceInfo i440fx_info[] = {
>      {
>          .qdev.name    = "i440FX",
> @@ -371,6 +384,7 @@ static PCIDeviceInfo i440fx_info[] = {
>          .qdev.no_user = 1,
>          .no_hotplug   = 1,
>          .init         = piix3_initfn,
> +        .config_write = piix3_write_config,
>      },{
>          /* end of list */
>      }
>