Hello, While I was reading gic handling part of xen arm, I noticed that, in gic_set_guest_irq function, if lr_pending is not empty, the arrival irq is also stored into pending list. I think although lr_pending is not empty, there maybe empty slots in lr, and if so, we can insert IRQ immediately. Since I''d like to keep the IRQ in priority order, how about checking the highest priority in lr_pending list rather than emptiness? Here goes the simple patch: Signed-off-by: Jaemin Kim <jmkim.kim@samsung.com> --- diff -urN xen/arch/arm/gic.c xen2/arch/arm/gic.c --- xen/arch/arm/gic.c 2013-07-30 10:16:45.076382483 +0900 +++ xen2/arch/arm/gic.c 2013-07-31 11:19:28.094799335 +0900 @@ -587,12 +587,17 @@ unsigned int state, unsigned int priority) { int i; - struct pending_irq *iter, *n; + struct pending_irq *iter, *n, *first_item; unsigned long flags; spin_lock_irqsave(&gic.lock, flags); + + first_item = list_entry( (&v->arch.vgic.lr_pending)->next, + typeof(*iter), lr_queue ); - if (( v == current ) && (list_empty(&v->arch.vgic.lr_pending) )) + if ( ( v == current ) && + ( list_empty(&v->arch.vgic.lr_pending) || + ( priority > first_item->priority ) ) ) { i = find_first_zero_bit(&this_cpu(lr_mask), nr_lrs); if (i < nr_lrs) { --- Thanks. Best Regards. Jaemin Kim
On Mon, 2013-08-19 at 02:29 +0000, 김재민 wrote:> Hello, > While I was reading gic handling part of xen arm, I noticed that, in gic_set_guest_irq function, > if lr_pending is not empty, the arrival irq is also stored into pending list. > I think although lr_pending is not empty, there maybe empty slots in lr, and if so, we can insert > IRQ immediately. Since I'd like to keep the IRQ in priority order, how about checking the > highest priority in lr_pending list rather than emptiness? > Here goes the simple patch:Julien & Stefano know about this stuff -- guys? Julien was planning to rework a bunch of the interrupt handling stuff, I don't know if that was orthogonal to this or not.> > Signed-off-by: Jaemin Kim <jmkim.kim@samsung.com> > --- > diff -urN xen/arch/arm/gic.c xen2/arch/arm/gic.c > --- xen/arch/arm/gic.c 2013-07-30 10:16:45.076382483 +0900 > +++ xen2/arch/arm/gic.c 2013-07-31 11:19:28.094799335 +0900 > @@ -587,12 +587,17 @@ > unsigned int state, unsigned int priority) > { > int i; > - struct pending_irq *iter, *n; > + struct pending_irq *iter, *n, *first_item; > unsigned long flags; > spin_lock_irqsave(&gic.lock, flags); > + > + first_item = list_entry( (&v->arch.vgic.lr_pending)->next, > + typeof(*iter), lr_queue ); > - if (( v == current ) && (list_empty(&v->arch.vgic.lr_pending) )) > + if ( ( v == current ) && > + ( list_empty(&v->arch.vgic.lr_pending) || > + ( priority > first_item->priority ) ) ) > { > i = find_first_zero_bit(&this_cpu(lr_mask), nr_lrs); > if (i < nr_lrs) { > --- > Thanks. > Best Regards. > Jaemin Kim > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
On 19 August 2013 15:53, Ian Campbell <Ian.Campbell@citrix.com> wrote:> On Mon, 2013-08-19 at 02:29 +0000, 김재민 wrote: >> Hello, >> While I was reading gic handling part of xen arm, I noticed that, in gic_set_guest_irq function, >> if lr_pending is not empty, the arrival irq is also stored into pending list. >> I think although lr_pending is not empty, there maybe empty slots in lr, and if so, we can insert >> IRQ immediately. Since I'd like to keep the IRQ in priority order, how about checking the >> highest priority in lr_pending list rather than emptiness? >> Here goes the simple patch:> Julien & Stefano know about this stuff -- guys? > > Julien was planning to rework a bunch of the interrupt handling stuff, I > don't know if that was orthogonal to this or not.Not yet.. I though there is no Operating System with IRQ priority support. I will add to my TODO list for interrupt. -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
On 19 August 2013 03:29, 김재민 <jmkim.kim@samsung.com> wrote:> Hello,Hi,> While I was reading gic handling part of xen arm, I noticed that, in gic_set_guest_irq function, > if lr_pending is not empty, the arrival irq is also stored into pending list. > I think although lr_pending is not empty, there maybe empty slots in lr, and if so, we can insert > IRQ immediately. Since I'd like to keep the IRQ in priority order, how about checking the > highest priority in lr_pending list rather than emptiness?Thanks for this patch. Do you plan to have a full support for priority? Cheers,> Here goes the simple patch: > > Signed-off-by: Jaemin Kim <jmkim.kim@samsung.com> > --- > diff -urN xen/arch/arm/gic.c xen2/arch/arm/gic.c > --- xen/arch/arm/gic.c 2013-07-30 10:16:45.076382483 +0900 > +++ xen2/arch/arm/gic.c 2013-07-31 11:19:28.094799335 +0900 > @@ -587,12 +587,17 @@ > unsigned int state, unsigned int priority) > { > int i; > - struct pending_irq *iter, *n; > + struct pending_irq *iter, *n, *first_item; > unsigned long flags; > spin_lock_irqsave(&gic.lock, flags); > + > + first_item = list_entry( (&v->arch.vgic.lr_pending)->next, > + typeof(*iter), lr_queue ); > - if (( v == current ) && (list_empty(&v->arch.vgic.lr_pending) )) > + if ( ( v == current ) && > + ( list_empty(&v->arch.vgic.lr_pending) || > + ( priority > first_item->priority ) ) ) > { > i = find_first_zero_bit(&this_cpu(lr_mask), nr_lrs); > if (i < nr_lrs) {-- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
> From: xen-devel-bounces@lists.xen.org [mailto:xen-devel- > bounces@lists.xen.org] On Behalf Of Julien Grall > Sent: Tuesday, August 20, 2013 10:16 PM > To: jmkim.kim@samsung.com > Cc: Stefano Stabellini; Ian Campbell; xen-devel@lists.xen.org > Subject: Re: [Xen-devel] A suggestion about processing irq > > On 19 August 2013 03:29, 김재민 wrote: > > Hello, > > HiHi,> > > While I was reading gic handling part of xen arm, I noticed that, in > > gic_set_guest_irq function, if lr_pending is not empty, the arrival irq > is also stored into pending list. > > I think although lr_pending is not empty, there maybe empty slots in > > lr, and if so, we can insert IRQ immediately. Since I'd like to keep > > the IRQ in priority order, how about checking the highest priority in > lr_pending list rather than emptiness? > > Thanks for this patch. Do you plan to have a full support for priority? > > Cheers,Does it fix the emulated GICD part for "guest domain" or GICD part for xen ? As I know, the part of involved in handling irq priority is implemented already in xen code. The meaning of "full support" is ambiguous to me. Could you tell me the scope of that ?> > > Here goes the simple patch: > > > > Signed-off-by: Jaemin Kim > > --- > > diff -urN xen/arch/arm/gic.c xen2/arch/arm/gic.c > > --- xen/arch/arm/gic.c 2013-07-30 10:16:45.076382483 +0900 > > +++ xen2/arch/arm/gic.c 2013-07-31 11:19:28.094799335 +0900 > > @@ -587,12 +587,17 @@ > > unsigned int state, unsigned int priority) { > > int i; > > - struct pending_irq *iter, *n; > > + struct pending_irq *iter, *n, *first_item; > > unsigned long flags; > > spin_lock_irqsave(&gic.lock, flags); > > + > > + first_item = list_entry( (&v->arch.vgic.lr_pending)->next, > > + typeof(*iter), lr_queue ); > > - if (( v == current ) && (list_empty(&v->arch.vgic.lr_pending) )) > > + if ( ( v == current ) && > > + ( list_empty(&v->arch.vgic.lr_pending) || > > + ( priority > first_item->priority ) ) ) > > { > > i = find_first_zero_bit(&this_cpu(lr_mask), nr_lrs); > > if (i < nr_lrs) { > > -- > Julien Grall > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Jaemin, thank you for the patch and sorry for my late reply. On Mon, 19 Aug 2013, 김재민 wrote:> Hello, > While I was reading gic handling part of xen arm, I noticed that, in gic_set_guest_irq function, > if lr_pending is not empty, the arrival irq is also stored into pending list. > I think although lr_pending is not empty, there maybe empty slots in lr, and if so, we can insert > IRQ immediately.Yes, the idea is that if not list_empty(&v->arch.vgic.lr_pending), somebody else is already queuing for a free LR register, so there is no point in trying to find one. If you look at maintenance_interrupt, you''ll see that as soon as an LR is freed, the first irq in the lr_pending queue is going to be removed from the queue and added to the register. So if at least one irq is prensent in lr_pending, all lr registers must be already in use.> Since I''d like to keep the IRQ in priority order, how about checking the > highest priority in lr_pending list rather than emptiness?Injecting irqs in priority order is a good goal to have, but it requires more than this change. From gic_set_guest_irq you would need to find an LR register with an IRQ of lower priority that is PENDING and not ACTIVE and remove it from the register, replacing it with the new irq. You would need to add the removed irq to lr_pending again. Also keep in mind that lower priority numbers mean higher priority.> Here goes the simple patch: > > Signed-off-by: Jaemin Kim <jmkim.kim@samsung.com> > --- > diff -urN xen/arch/arm/gic.c xen2/arch/arm/gic.c > --- xen/arch/arm/gic.c 2013-07-30 10:16:45.076382483 +0900 > +++ xen2/arch/arm/gic.c 2013-07-31 11:19:28.094799335 +0900 > @@ -587,12 +587,17 @@ > unsigned int state, unsigned int priority) > { > int i; > - struct pending_irq *iter, *n; > + struct pending_irq *iter, *n, *first_item; > unsigned long flags; > spin_lock_irqsave(&gic.lock, flags); > + > + first_item = list_entry( (&v->arch.vgic.lr_pending)->next, > + typeof(*iter), lr_queue ); > - if (( v == current ) && (list_empty(&v->arch.vgic.lr_pending) )) > + if ( ( v == current ) && > + ( list_empty(&v->arch.vgic.lr_pending) || > + ( priority > first_item->priority ) ) ) > { > i = find_first_zero_bit(&this_cpu(lr_mask), nr_lrs); > if (i < nr_lrs) {_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel