From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [PATCH] kvm-ia64 irq assignment 2/2 userspace
Date: Wed, 11 Jun 2008 13:54:41 -0300
Message-ID: <20080611165441.GA7926@dmt.cnet>
References: <51CFAB8CB6883745AE7B93B3E084EBE201CC8760@pdsmsx412.ccr.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Avi Kivity <avi@qumranet.com>, Jes Sorensen <jes@sgi.com>,
	kvm@vger.kernel.org, kvm-ia64@vger.kernel.org
To: "Xu, Anthony" <anthony.xu@intel.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([66.187.233.31]:36312 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753315AbYFKQz1 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 11 Jun 2008 12:55:27 -0400
Content-Disposition: inline
In-Reply-To: <51CFAB8CB6883745AE7B93B3E084EBE201CC8760@pdsmsx412.ccr.corp.intel.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Fri, Jun 06, 2008 at 11:59:02PM +0800, Xu, Anthony wrote:
> In kvm-ia64, PCI devices use 48-pin virtual IOAPIC to deliver interrup.
> 
> 
> Signed-off-by: Anthony Xu < anthony.xu@intel.com >
> 
> diff --git a/qemu/hw/piix_pci.c b/qemu/hw/piix_pci.c
> index 90cb3a6..797ece7 100644
> --- a/qemu/hw/piix_pci.c
> +++ b/qemu/hw/piix_pci.c
> @@ -28,6 +28,7 @@
> 
>  typedef uint32_t pci_addr_t;
>  #include "pci_host.h"
> +#include "qemu-kvm.h"
> 
>  typedef PCIHostState I440FXState;
> 
> @@ -51,6 +52,11 @@ static void piix3_set_irq(qemu_irq *pic, int irq_num,
> int level);
>  static int pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
>  {
>      int slot_addend;
> +#if defined(KVM_CAP_IRQCHIP) && defined(TARGET_IA64)
> +    int dev;
> +    dev = pci_dev->devfn >> 3;
> +    return (((((dev) << 2) + ((dev) >> 3) + (irq_num)) & 31) + 16);
> +#endif
>      slot_addend = (pci_dev->devfn >> 3) - 1;
>      return (irq_num + slot_addend) & 3;
>  }

You don't support -no-kvm-irqchip on IA64? 

Also, cosmetically would be nicer to create a separate get_pirq function
for IA64.

> @@ -171,12 +177,18 @@ static int i440fx_load(QEMUFile* f, void *opaque,
> int version_id)
> 
>  PCIBus *i440fx_init(PCIDevice **pi440fx_state, qemu_irq *pic)
>  {
> +    int nirq;
>      PCIBus *b;
>      PCIDevice *d;
>      I440FXState *s;
> 
> +#if defined(KVM_CAP_IRQCHIP) && defined(TARGET_IA64)
> +    nirq = 48;
> +#else
> +    nirq = 4;
> +#endif
>      s = qemu_mallocz(sizeof(I440FXState));
> -    b = pci_register_bus(piix3_set_irq, pci_slot_get_pirq, pic, 0, 4);
> +    b = pci_register_bus(piix3_set_irq, pci_slot_get_pirq, pic, 0,
> nirq);
>      s->bus = b;

This should be dynamic if possible. Don't want another ifdef for when x86 
routes through IOAPIC.

>      register_ioport_write(0xcf8, 4, 4, i440fx_addr_writel, s);
> @@ -220,6 +232,11 @@ static void piix3_set_irq(qemu_irq *pic, int
> irq_num, int level)
>  {
>      int i, pic_irq, pic_level;
> 
> +#if defined(KVM_CAP_IRQCHIP) && defined(TARGET_IA64)
> +    if(kvm_enabled())
> +        kvm_set_irq(irq_num, level);
> +    return;
> +#endif
>      pci_irq_levels[irq_num] = level;

Why the current hook into i8259_set_irq does not work for IA64? 

I think this breaks interrupt sharing, because the logic to detect if
all devices sharing the line are low or high is in i8259_set_irq, and
you simply skip that.