From: 유재용 <jaeyong.yoo@samsung.com>
To: Julien Grall <julien.grall@linaro.org>
Cc: "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>,
Ian Campbell <Ian.Campbell@citrix.com>,
Stefano Stabellini <Stefano.Stabellini@eu.citrix.com>
Subject: Re: Bug report and patch about IRQ freezing after gic_restore_state
Date: Tue, 21 May 2013 11:13:49 +0000 (GMT) [thread overview]
Message-ID: <20628360.525981369134828242.JavaMail.weblogic@epv6ml03> (raw)
> > Signed-off-by: Jaeyong Yoo
> > ---
> > xen/arch/arm/domain.c | 4 ++--
> > xen/arch/arm/gic.c | 4 ++--
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> > index f71b582..2c3b132 100644
> > --- a/xen/arch/arm/domain.c
> > +++ b/xen/arch/arm/domain.c
> > @@ -141,6 +141,8 @@ static void ctxt_switch_to(struct vcpu *n)
> > /* VGIC */
> > gic_restore_state(n);
> > + local_irq_enable();
> > +
>
> Could you move the local_irq_enable right after ctxt_switch_to?
Of course.
Just one small concern:
Would it be more efficient (i.e., less scheduling latency) if we enable irq right after gic_restore state similar to lock-breaking effect?
>
> > /* XXX VFP */
> > /* XXX MPU */
> > @@ -215,8 +217,6 @@ static void schedule_tail(struct vcpu *prev)
> > {
> > ctxt_switch_from(prev);
> > - local_irq_enable();
> > -
> > /* TODO
> > update_runstate_area(current);
> > */
> > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> > index d4f0a43..8186ad8 100644
> > --- a/xen/arch/arm/gic.c
> > +++ b/xen/arch/arm/gic.c
> > @@ -81,11 +81,11 @@ void gic_restore_state(struct vcpu *v)
> > if ( is_idle_vcpu(v) )
> > return;
> > - spin_lock_irq(&gic.lock);
> > + spin_lock(&gic.lock);
> > this_cpu(lr_mask) = v->arch.lr_mask;
> > for ( i=0; i> GICH[GICH_LR + i] = v->arch.gic_lr[i];
> > - spin_unlock_irq(&gic.lock);
> > + spin_unlock(&gic.lock);
>
> As the IRQ is disabled and the GICH registers can only be modified by
> the current physical CPU, I think you can remove the spin_{,un}lock and
> replace it by a dsb.
dsb sounds ok.
>
> > GICH[GICH_APR] = v->arch.gic_apr;
> > GICH[GICH_HCR] = GICH_HCR_EN;
> > isb();
>
Best,
Jaeyong
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
next reply other threads:[~2013-05-21 11:13 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-21 11:13 유재용 [this message]
2013-05-21 13:00 ` Bug report and patch about IRQ freezing after gic_restore_state Ian Campbell
-- strict thread matches above, loose matches on Subject: below --
2013-05-23 23:57 Jaeyong Yoo
2013-05-22 2:34 Jaeyong Yoo
2013-05-22 16:55 ` Stefano Stabellini
2013-05-23 13:24 ` Ian Campbell
2013-05-20 0:41 Jaeyong Yoo
2013-05-20 13:11 ` Julien Grall
2013-05-21 12:00 ` Stefano Stabellini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20628360.525981369134828242.JavaMail.weblogic@epv6ml03 \
--to=jaeyong.yoo@samsung.com \
--cc=Ian.Campbell@citrix.com \
--cc=Stefano.Stabellini@eu.citrix.com \
--cc=julien.grall@linaro.org \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.