* A suggestion about processing irq
@ 2013-08-19 2:29 김재민
2013-08-19 14:53 ` Ian Campbell
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: 김재민 @ 2013-08-19 2:29 UTC (permalink / raw)
To: xen-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: A suggestion about processing irq
2013-08-19 2:29 A suggestion about processing irq 김재민
@ 2013-08-19 14:53 ` Ian Campbell
2013-08-20 12:41 ` Julien Grall
2013-08-20 13:15 ` Julien Grall
2013-08-27 12:04 ` Stefano Stabellini
2 siblings, 1 reply; 6+ messages in thread
From: Ian Campbell @ 2013-08-19 14:53 UTC (permalink / raw)
To: jmkim.kim; +Cc: Julien Grall, xen-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: A suggestion about processing irq
2013-08-19 14:53 ` Ian Campbell
@ 2013-08-20 12:41 ` Julien Grall
0 siblings, 0 replies; 6+ messages in thread
From: Julien Grall @ 2013-08-20 12:41 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel@lists.xen.org, jmkim.kim
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: A suggestion about processing irq
2013-08-19 2:29 A suggestion about processing irq 김재민
2013-08-19 14:53 ` Ian Campbell
@ 2013-08-20 13:15 ` Julien Grall
2013-08-27 12:04 ` Stefano Stabellini
2 siblings, 0 replies; 6+ messages in thread
From: Julien Grall @ 2013-08-20 13:15 UTC (permalink / raw)
To: jmkim.kim; +Cc: Stefano Stabellini, Ian Campbell, xen-devel@lists.xen.org
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: A suggestion about processing irq
2013-08-19 2:29 A suggestion about processing irq 김재민
2013-08-19 14:53 ` Ian Campbell
2013-08-20 13:15 ` Julien Grall
@ 2013-08-27 12:04 ` Stefano Stabellini
2 siblings, 0 replies; 6+ messages in thread
From: Stefano Stabellini @ 2013-08-27 12:04 UTC (permalink / raw)
To: 김재민; +Cc: xen-devel
[-- Attachment #1: Type: text/plain, Size: 2429 bytes --]
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) {
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: A suggestion about processing irq
@ 2013-08-23 9:45 jaemin kim
0 siblings, 0 replies; 6+ messages in thread
From: jaemin kim @ 2013-08-23 9:45 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel@lists.xen.org, Ian Campbell, Stefano Stabellini
> 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,
>
> Hi
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,
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-08-27 12:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-19 2:29 A suggestion about processing irq 김재민
2013-08-19 14:53 ` Ian Campbell
2013-08-20 12:41 ` Julien Grall
2013-08-20 13:15 ` Julien Grall
2013-08-27 12:04 ` Stefano Stabellini
-- strict thread matches above, loose matches on Subject: below --
2013-08-23 9:45 jaemin kim
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.