Mailing List Archive: [PATCH 03 of 10] arm: Move some GIC distributor init out of the per-CPU init function

[PATCH 03 of 10] arm: Move some GIC distributor init out of the per-CPU init function

Feb 23, 2012, 9:40 AM

Post #1 of 4 (203 views)

# HG changeset patch
# User Tim Deegan <tim@xen.org>
# Date 1330018799 0
# Node ID 437ad1207a175c9ad376871f3f4c075dbcd5b6e6
# Parent ec051056db2b6d37344629e2f01d17240099d5ec
arm: Move some GIC distributor init out of the per-CPU init function

Signed-off-by: Tim Deegan <tim@xen.org>

diff -r ec051056db2b -r 437ad1207a17 xen/arch/arm/gic.c
--- a/xen/arch/arm/gic.c Thu Feb 23 17:39:59 2012 +0000
+++ b/xen/arch/arm/gic.c Thu Feb 23 17:39:59 2012 +0000
@@ -216,14 +216,6 @@ static void __init gic_dist_init(void)
for ( i = 32; i < gic.lines; i += 32 )
GICD[GICD_ICENABLER + i / 32] = ~0ul;

- /* Turn on the distributor */
- GICD[GICD_CTLR] = GICD_CTL_ENABLE;
-}
-
-static void __cpuinit gic_cpu_init(void)
-{
- int i;
-
/* Disable all PPI and enable all SGI */
GICD[GICD_ICENABLER] = 0xffff0000; /* Disable all PPI */
GICD[GICD_ISENABLER] = 0x0000ffff; /* Enable all SGI */
@@ -231,6 +223,12 @@ static void __cpuinit gic_cpu_init(void)
for (i = 0; i < 32; i += 4)
GICD[GICD_IPRIORITYR + i / 4] = 0xa0a0a0a0;

+ /* Turn on the distributor */
+ GICD[GICD_CTLR] = GICD_CTL_ENABLE;
+}
+
+static void __cpuinit gic_cpu_init(void)
+{
/* Local settings: interface controller */
GICC[GICC_PMR] = 0xff; /* Don't mask by priority */
GICC[GICC_BPR] = 0; /* Finest granularity of priority */

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 03 of 10] arm: Move some GIC distributor init out of the per-CPU init function [ In reply to ]

Ian.Campbell at citrix

Feb 27, 2012, 9:32 AM

Post #2 of 4 (204 views)

Permalink

On Thu, 2012-02-23 at 17:40 +0000, Tim Deegan wrote:
> # HG changeset patch
> # User Tim Deegan <tim@xen.org>
> # Date 1330018799 0
> # Node ID 437ad1207a175c9ad376871f3f4c075dbcd5b6e6
> # Parent ec051056db2b6d37344629e2f01d17240099d5ec
> arm: Move some GIC distributor init out of the per-CPU init function
>
> Signed-off-by: Tim Deegan <tim@xen.org>
>
> diff -r ec051056db2b -r 437ad1207a17 xen/arch/arm/gic.c
> --- a/xen/arch/arm/gic.c Thu Feb 23 17:39:59 2012 +0000
> +++ b/xen/arch/arm/gic.c Thu Feb 23 17:39:59 2012 +0000
> @@ -216,14 +216,6 @@ static void __init gic_dist_init(void)
> for ( i = 32; i < gic.lines; i += 32 )
> GICD[GICD_ICENABLER + i / 32] = ~0ul;
>
> - /* Turn on the distributor */
> - GICD[GICD_CTLR] = GICD_CTL_ENABLE;
> -}
> -
> -static void __cpuinit gic_cpu_init(void)
> -{
> - int i;
> -
> /* Disable all PPI and enable all SGI */
> GICD[GICD_ICENABLER] = 0xffff0000; /* Disable all PPI */
> GICD[GICD_ISENABLER] = 0x0000ffff; /* Enable all SGI */

PPIs and SGIs are per physical-CPU and therefore, I think, the GICD
registers of various sorts which refer to the first 32 interrupts are
per physical-CPU as well. IOW moving these from gic_cpu_init to
gic_dist_init is wrong?

> @@ -231,6 +223,12 @@ static void __cpuinit gic_cpu_init(void)
> for (i = 0; i < 32; i += 4)
> GICD[GICD_IPRIORITYR + i / 4] = 0xa0a0a0a0;
>
> + /* Turn on the distributor */
> + GICD[GICD_CTLR] = GICD_CTL_ENABLE;
> +}
> +
> +static void __cpuinit gic_cpu_init(void)
> +{
> /* Local settings: interface controller */
> GICC[GICC_PMR] = 0xff; /* Don't mask by priority */
> GICC[GICC_BPR] = 0; /* Finest granularity of priority */

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 03 of 10] arm: Move some GIC distributor init out of the per-CPU init function [ In reply to ]

tim at xen

Feb 27, 2012, 11:30 AM

Post #3 of 4 (201 views)

Permalink

At 17:32 +0000 on 27 Feb (1330363933), Ian Campbell wrote:
> On Thu, 2012-02-23 at 17:40 +0000, Tim Deegan wrote:
> > # HG changeset patch
> > # User Tim Deegan <tim@xen.org>
> > # Date 1330018799 0
> > # Node ID 437ad1207a175c9ad376871f3f4c075dbcd5b6e6
> > # Parent ec051056db2b6d37344629e2f01d17240099d5ec
> > arm: Move some GIC distributor init out of the per-CPU init function
> >
> > Signed-off-by: Tim Deegan <tim@xen.org>
> >
> > diff -r ec051056db2b -r 437ad1207a17 xen/arch/arm/gic.c
> > --- a/xen/arch/arm/gic.c Thu Feb 23 17:39:59 2012 +0000
> > +++ b/xen/arch/arm/gic.c Thu Feb 23 17:39:59 2012 +0000
> > @@ -216,14 +216,6 @@ static void __init gic_dist_init(void)
> > for ( i = 32; i < gic.lines; i += 32 )
> > GICD[GICD_ICENABLER + i / 32] = ~0ul;
> >
> > - /* Turn on the distributor */
> > - GICD[GICD_CTLR] = GICD_CTL_ENABLE;
> > -}
> > -
> > -static void __cpuinit gic_cpu_init(void)
> > -{
> > - int i;
> > -
> > /* Disable all PPI and enable all SGI */
> > GICD[GICD_ICENABLER] = 0xffff0000; /* Disable all PPI */
> > GICD[GICD_ISENABLER] = 0x0000ffff; /* Enable all SGI */
>
> PPIs and SGIs are per physical-CPU and therefore, I think, the GICD
> registers of various sorts which refer to the first 32 interrupts are
> per physical-CPU as well. IOW moving these from gic_cpu_init to
> gic_dist_init is wrong?

Yep, you're right ISENABLER0 and ICENABLER0 are banked. I'll replace
this with a comment explaining that.

Cheers,

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 03 of 10] arm: Move some GIC distributor init out of the per-CPU init function [ In reply to ]

Ian.Campbell at citrix

Feb 28, 2012, 2:24 AM

Post #4 of 4 (200 views)

Permalink

On Mon, 2012-02-27 at 19:30 +0000, Tim Deegan wrote:
> At 17:32 +0000 on 27 Feb (1330363933), Ian Campbell wrote:
> > On Thu, 2012-02-23 at 17:40 +0000, Tim Deegan wrote:
> > > # HG changeset patch
> > > # User Tim Deegan <tim@xen.org>
> > > # Date 1330018799 0
> > > # Node ID 437ad1207a175c9ad376871f3f4c075dbcd5b6e6
> > > # Parent ec051056db2b6d37344629e2f01d17240099d5ec
> > > arm: Move some GIC distributor init out of the per-CPU init function
> > >
> > > Signed-off-by: Tim Deegan <tim@xen.org>
> > >
> > > diff -r ec051056db2b -r 437ad1207a17 xen/arch/arm/gic.c
> > > --- a/xen/arch/arm/gic.c Thu Feb 23 17:39:59 2012 +0000
> > > +++ b/xen/arch/arm/gic.c Thu Feb 23 17:39:59 2012 +0000
> > > @@ -216,14 +216,6 @@ static void __init gic_dist_init(void)
> > > for ( i = 32; i < gic.lines; i += 32 )
> > > GICD[GICD_ICENABLER + i / 32] = ~0ul;
> > >
> > > - /* Turn on the distributor */
> > > - GICD[GICD_CTLR] = GICD_CTL_ENABLE;
> > > -}
> > > -
> > > -static void __cpuinit gic_cpu_init(void)
> > > -{
> > > - int i;
> > > -
> > > /* Disable all PPI and enable all SGI */
> > > GICD[GICD_ICENABLER] = 0xffff0000; /* Disable all PPI */
> > > GICD[GICD_ISENABLER] = 0x0000ffff; /* Enable all SGI */
> >
> > PPIs and SGIs are per physical-CPU and therefore, I think, the GICD
> > registers of various sorts which refer to the first 32 interrupts are
> > per physical-CPU as well. IOW moving these from gic_cpu_init to
> > gic_dist_init is wrong?
>
> Yep, you're right ISENABLER0 and ICENABLER0 are banked.

BTW so are GICD_IPRIORITYR0..4 which are initialised just outside the
context here. More generally all the per-IRQ registers are banked for
the first few which cover the PPIs and SGIs.

> I'll replace this with a comment explaining that.

Thanks.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel