| Message ID | 1372418757-19213-1-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Fri, 2013-06-28 at 12:25 +0100, Julien Grall wrote: > When GICD_ICENABLERn is read, all the 1s bit represent enabled IRQs. > Currently gic_shutdown_irq: > - read GICD_ICENABLER > - set the corresping bit to 1 > - write back the new value > That means, Xen will disable more IRQs than necessary. > > Signed-off-by: Julien Grall <julien.grall@linaro.org> Doh! Acked-by: Ian Campbell <ian.campbell@citrix.com> > --- > xen/arch/arm/gic.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index 177560e..cafb681 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -117,12 +117,10 @@ static unsigned int gic_irq_startup(struct irq_desc *desc) > > static void gic_irq_shutdown(struct irq_desc *desc) > { > - uint32_t enabler; > int irq = desc->irq; > > /* Disable routing */ > - enabler = GICD[GICD_ICENABLER + irq / 32]; > - GICD[GICD_ICENABLER + irq / 32] = enabler | (1u << (irq % 32)); > + GICD[GICD_ICENABLER + irq / 32] = (1u << (irq % 32)); > } > > static void gic_irq_enable(struct irq_desc *desc)
On Fri, 28 Jun 2013, Julien Grall wrote: > When GICD_ICENABLERn is read, all the 1s bit represent enabled IRQs. > Currently gic_shutdown_irq: > - read GICD_ICENABLER > - set the corresping bit to 1 > - write back the new value > That means, Xen will disable more IRQs than necessary. > > Signed-off-by: Julien Grall <julien.grall@linaro.org> It's impressive for how long we got this bug. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > xen/arch/arm/gic.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index 177560e..cafb681 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -117,12 +117,10 @@ static unsigned int gic_irq_startup(struct irq_desc *desc) > > static void gic_irq_shutdown(struct irq_desc *desc) > { > - uint32_t enabler; > int irq = desc->irq; > > /* Disable routing */ > - enabler = GICD[GICD_ICENABLER + irq / 32]; > - GICD[GICD_ICENABLER + irq / 32] = enabler | (1u << (irq % 32)); > + GICD[GICD_ICENABLER + irq / 32] = (1u << (irq % 32)); > } > > static void gic_irq_enable(struct irq_desc *desc) > -- > 1.7.10.4 >
On Fri, 2013-06-28 at 12:32 +0100, Stefano Stabellini wrote: > On Fri, 28 Jun 2013, Julien Grall wrote: > > When GICD_ICENABLERn is read, all the 1s bit represent enabled IRQs. > > Currently gic_shutdown_irq: > > - read GICD_ICENABLER > > - set the corresping bit to 1 > > - write back the new value > > That means, Xen will disable more IRQs than necessary. > > > > Signed-off-by: Julien Grall <julien.grall@linaro.org> > > It's impressive for how long we got this bug. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> I think you meant Acked-by, so applied with both our acks.
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c index 177560e..cafb681 100644 --- a/xen/arch/arm/gic.c +++ b/xen/arch/arm/gic.c @@ -117,12 +117,10 @@ static unsigned int gic_irq_startup(struct irq_desc *desc) static void gic_irq_shutdown(struct irq_desc *desc) { - uint32_t enabler; int irq = desc->irq; /* Disable routing */ - enabler = GICD[GICD_ICENABLER + irq / 32]; - GICD[GICD_ICENABLER + irq / 32] = enabler | (1u << (irq % 32)); + GICD[GICD_ICENABLER + irq / 32] = (1u << (irq % 32)); } static void gic_irq_enable(struct irq_desc *desc)
When GICD_ICENABLERn is read, all the 1s bit represent enabled IRQs. Currently gic_shutdown_irq: - read GICD_ICENABLER - set the corresping bit to 1 - write back the new value That means, Xen will disable more IRQs than necessary. Signed-off-by: Julien Grall <julien.grall@linaro.org> --- xen/arch/arm/gic.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)