From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@qumranet.com>
Subject: Re: [RFC] kvm irq assignment
Date: Fri, 13 Jun 2008 17:22:49 +0300
Message-ID: <485282B9.7040709@qumranet.com>
References: <51CFAB8CB6883745AE7B93B3E084EBE201CC9077@pdsmsx412.ccr.corp.intel.com> <48517616.1050607@qumranet.com> <51CFAB8CB6883745AE7B93B3E084EBE201CC9210@pdsmsx412.ccr.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: Marcelo Tosatti <mtosatti@redhat.com>,
	Alexander Graf <agraf@suse.de>, 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 il.qumranet.com ([212.179.150.194]:18367 "EHLO il.qumranet.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1754936AbYFMOWy (ORCPT <rfc822;kvm@vger.kernel.org>);
	Fri, 13 Jun 2008 10:22:54 -0400
In-Reply-To: <51CFAB8CB6883745AE7B93B3E084EBE201CC9210@pdsmsx412.ccr.corp.intel.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Xu, Anthony wrote:
> Hi Avi and all
>
> This is the revised one,
>
> All PCI devices send interrupt to both PIC and IOAPIC,  
> a). When PIC is enabled and IOAPIC is disabled,  all redirect entries in
> IOAPIC are masked.
> B) When PIC is disabled and IPAPIC is enabled, link entry bit7 is set,
> means this link entry is disable.
> Guest OS need to guarantee PIC and IOAPIC are not enabled in the same
> time. Otherwise cause many suspicious interrupt to guest.
>
> Test by running guest linux in kvm/ia32 and kvm/ia64.
>
>
>   

Looks good.

> diff --git a/qemu/hw/apic.c b/qemu/hw/apic.c
> index a14cab2..c3014fa 100644
> --- a/qemu/hw/apic.c
> +++ b/qemu/hw/apic.c
> @@ -1053,9 +1053,25 @@ static void ioapic_service(IOAPICState *s)
>      }
>  }
>  
> +int ioapic_map_irq(int devfn, int irq_num)
> +{
> +    int irq;
> +    irq = ((devfn >> 3) & 7) + 16;
> +    return irq;
> +}
> +#ifdef KVM_CAP_IRQCHIP
> +static int ioapic_irq_count[IOAPIC_NUM_PINS];
> +#endif
> +
>  void ioapic_set_irq(void *opaque, int vector, int level)
>  {
>      IOAPICState *s = opaque;
> +#ifdef KVM_CAP_IRQCHIP
> +    ioapic_irq_count[vector] += level;
> +    if (kvm_enabled())
> +	if (kvm_set_irq(vector, ioapic_irq_count[vector] == 0))
> +	    return;
> +#endif
>   

I think this can be done more cleanly using the qemu_irq infrastructure. 
qem_irq_invert can do the inversion, and to get the "irq fork", you can 
have a qemu_irq instance that forwards its argument to two other 
qemu_irq instances.

There shouldn't be any #ifdef KVM_CAP_IRQCHIPs in there - it should work 
for plain qemu as well (well, if we fix qemu ioapic polarity code).

> diff --git a/qemu/hw/piix_pci.c b/qemu/hw/piix_pci.c
> index 90cb3a6..96316ca 100644
> --- a/qemu/hw/piix_pci.c
> +++ b/qemu/hw/piix_pci.c
> @@ -225,6 +226,9 @@ static void piix3_set_irq(qemu_irq *pic, int
> irq_num, int level)
>      /* now we change the pic irq level according to the piix irq
> mappings */
>      /* XXX: optimize */
>      pic_irq = piix3_dev->config[0x60 + irq_num];
> +    /* if bit7 set 1, this link is disabled */
> +    if (pic_irq & 0x80)  
> +        return;
>   

Already caught by the test below... hacky.

>      if (pic_irq < 16) {
>          /* The pic level is the logical OR of all the PCI irqs mapped
>             to it */
>
>
>   

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.