WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH 4/4] xen: events: allocate GSIs and dynamic IRQs

To: Ian Campbell <ian.campbell@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 4/4] xen: events: allocate GSIs and dynamic IRQs from separate IRQ ranges.
From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Date: Tue, 11 Jan 2011 14:14:57 -0500
Cc: Jeremy Fitzhardinge <jeremy@xxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxx, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Delivery-date: Tue, 11 Jan 2011 11:15:58 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1294766416-11407-4-git-send-email-ian.campbell@xxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <1294766389.3831.5902.camel@xxxxxxxxxxxxxxxxxxxxxx> <1294766416-11407-4-git-send-email-ian.campbell@xxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.20 (2009-06-14)
On Tue, Jan 11, 2011 at 05:20:16PM +0000, Ian Campbell wrote:
> There are three cases which we need to care about, PV guest, PV domain
> 0 and HVM guest.
> 
> The PV guest case is simple since it has no access to ACPI or real
> APICs and therefore has no GSIs therefore we simply dynamically
> allocate all IRQs. The potentially interesting case here is PIRQ type
> event channels associated with passed through PCI devices. However
> even in this case the guest has no direct interaction with the
> physical GSI since that happens in the PCI backend.
> 
> The PV domain 0 and HVM guest cases are actually the same. In domain 0
> case the kernel sees the host ACPI and GSIs (although it only sees the
> APIC indirectly via the hypervisor) and in the HVM guest case it sees
> the virtualised ACPI and emulated APICs. In these cases we start
> allocating dynamic IRQs at nr_irqs_gsi so that they cannot clash with
> any GSI.
> 
> Currently xen_allocate_irq_dynamic starts at nr_irqs and works
> backwards looking for a free IRQ in order to (try and) avoid clashing
> with GSIs used in domain 0 and in HVM guests. This change avoids that
> although we retain the behaviour of allowing dynamic IRQs to encroach
> on the GSI range if no suitable IRQs are available since a future IRQ
> clash is deemed preferable to failure right now.
> 
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> Cc: Jeremy Fitzhardinge <jeremy@xxxxxxxx>
> ---
>  drivers/xen/events.c |   84 +++++++++++++++----------------------------------
>  1 files changed, 26 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index 74fb216..a7b60f6 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -373,81 +373,49 @@ static void unmask_evtchn(int port)
>       put_cpu();
>  }
>  
> -static int get_nr_hw_irqs(void)
> +static int xen_allocate_irq_dynamic(void)
>  {
> -     int ret = 1;
> +     int first = 0;
> +     int irq;
>  
>  #ifdef CONFIG_X86_IO_APIC
> -     ret = get_nr_irqs_gsi();
> +     /*
> +      * For an HVM guest or domain 0 which see "real" (emulated or
> +      * actual repectively) GSIs we allocate dynamic IRQs
> +      * e.g. those corresponding to event channels or MSIs
> +      * etc. from the range above those "real" GSIs to avoid
> +      * collisions.
> +      */
> +     if (xen_initial_domain() || xen_hvm_domain())
> +             first = get_nr_irqs_gsi();
>  #endif
>  
> -     return ret;
> -}
> -
> -static int xen_allocate_irq_dynamic(void)
> -{
> -     struct irq_data *data;
> -     int irq, res;
> -     int bottom = get_nr_hw_irqs();
> -     int top = nr_irqs-1;
> -
> -     if (bottom == nr_irqs)
> -             goto no_irqs;
> -
>  retry:
> -     /* This loop starts from the top of IRQ space and goes down.
> -      * We need this b/c if we have a PCI device in a Xen PV guest
> -      * we do not have an IO-APIC (though the backend might have them)
> -      * mapped in. To not have a collision of physical IRQs with the Xen
> -      * event channels start at the top of the IRQ space for virtual IRQs.
> -      */
> -     for (irq = top; irq > bottom; irq--) {
> -             data = irq_get_irq_data(irq);
> -             /* only 15->0 have init'd desc; handle irq > 16 */
> -             if (!data)
> -                     break;
> -             if (data->chip == &no_irq_chip)
> -                     break;
> -             if (data->chip != &xen_dynamic_chip)
> -                     continue;
> -             if (irq_info[irq].type == IRQT_UNBOUND)
> -                     return irq;
> -     }
> +     irq = irq_alloc_desc_from(first, -1);
>  
> -     if (irq == bottom)
> -             goto no_irqs;
> -
> -     res = irq_alloc_desc_at(irq, -1);
> -     if (res == -EEXIST) {
> -             top--;
> -             if (bottom > top)
> -                     printk(KERN_ERR "Eating in GSI/MSI space (%d)!" \
> -                             " Your PCI device might not work!\n", top);
> -             if (top > NR_IRQS_LEGACY)
> -                     goto retry;
> +     if (irq == -ENOMEM && first > NR_IRQS_LEGACY) {
> +             printk(KERN_ERR "Out of dynamic IRQ space and eating into GSI 
> space. You should increase nr_irqs\n");
> +             first = max(NR_IRQS_LEGACY, first - NR_IRQS_LEGACY);
> +             goto retry;

You don't check for irq == -EEXIST. So if specific IRQ (first) is already
occupied you panic. Would it be better to check for that too in this got
and retry with that value?

>       }
>  
> -     if (WARN_ON(res != irq))
> -             return -1;
> +     if (irq < 0)
> +             panic("No available IRQ to bind to: increase nr_irqs!\n");
>  
>       return irq;
> -
> -no_irqs:
> -     panic("No available IRQ to bind to: increase nr_irqs!\n");
> -}
> -
> -static bool identity_mapped_irq(unsigned irq)
> -{
> -     /* identity map all the hardware irqs */
> -     return irq < get_nr_hw_irqs();
>  }
>  
>  static int xen_allocate_irq_gsi(unsigned gsi)
>  {
>       int irq;
>  
> -     if (!identity_mapped_irq(gsi) &&
> -         (xen_initial_domain() || !xen_pv_domain()))
> +     /*
> +      * A PV guest has no concept of a GSI (since it has no ACPI
> +      * nor access to/knowledge of the physical APICs). Therefore
> +      * all IRQs are dynamically allocated from the entire IRQ
> +      * space.
> +      */
> +     if (xen_pv_domain() && !xen_initial_domain())
>               return xen_allocate_irq_dynamic();

OK. So with a IDE disk at IRQ 14 it can end up with irq = 5 (say the first five
IRQs are used by xen-spinlock, xen-timer, xen-debug, xen-console, and something 
else).
The gsi that gets stuck via the mk_pirq_info[5] ends up being 14, and pirq = 14.
When you do EVTCHNOP_bind_pirq you end up passing in pirq=14 and bind that to 
the
Linux irq five, right?


>  
>       /* Legacy IRQ descriptors are already allocated by the arch. */
> -- 
> 1.5.6.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel