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: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 4/4] xen: events: allocate GSIs and dynamic IRQs from separate IRQ ranges.
From: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
Date: Tue, 11 Jan 2011 19:39:19 +0000
Cc: Fitzhardinge <jeremy@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Jeremy, Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxxxxx>
Delivery-date: Tue, 11 Jan 2011 11:40:03 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20110111191457.GC29378@xxxxxxxxxxxx>
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>
Organization: Citrix Systems, Inc.
References: <1294766389.3831.5902.camel@xxxxxxxxxxxxxxxxxxxxxx> <1294766416-11407-4-git-send-email-ian.campbell@xxxxxxxxxx> <20110111191457.GC29378@xxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
On Tue, 2011-01-11 at 19:14 +0000, Konrad Rzeszutek Wilk wrote: 
> On Tue, Jan 11, 2011 at 05:20:16PM +0000, Ian Campbell wrote:

> >  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?

It's not obvious due to the way diff has chosen to represent this change
but this is checking the return value of irq_alloc_desc_from and not
irq_alloc_desc_at. The former cannot return EEXIST.

> >     }
> >  
> > -   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?

Actually because the core registers the first NR_IRQS_LEGACY interrupts
the PV interrupts actually start at 16 so 5 is a bad example but the
gist is otherwise correct, yes.

Ian.



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