From mboxrd@z Thu Jan 1 00:00:00 1970 From: Will Deacon Subject: Re: [RFC PATCH 4/4] ARM: gic: use a private mapping for CPU target interfaces Date: Tue, 6 Nov 2012 22:16:51 +0000 Message-ID: <20121106221651.GE21764@mudshark.cambridge.arm.com> References: <1350393709-23546-1-git-send-email-lorenzo.pieralisi@arm.com> <1350393709-23546-5-git-send-email-lorenzo.pieralisi@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1350393709-23546-5-git-send-email-lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Sender: "devicetree-discuss" To: Lorenzo Pieralisi Cc: Nicolas Pitre , Kukjin Kim , Russell King , Pawel Moll , Catalin Marinas , "devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org" , Amit Kucheria , "rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org" , David Brown , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" List-Id: devicetree@vger.kernel.org On Tue, Oct 16, 2012 at 02:21:48PM +0100, Lorenzo Pieralisi wrote: > From: Nicolas Pitre > > The GIC interface numbering does not necessarily follow the logical > CPU numbering, especially for complex topologies such as multi-cluster > systems. > > Fortunately we can easily probe the GIC to create a mapping as the > Interrupt Processor Targets Registers for the first 32 interrupts are > read-only, and each field returns a value that always corresponds to > the processor reading the register. > > Initially all mappings target all CPUs in case an IPI is required to > boot secondary CPUs. It is refined as those CPUs discover what their > actual mapping is. > > Signed-off-by: Nicolas Pitre This is a really neat idea! > --- > arch/arm/common/gic.c | 42 ++++++++++++++++++++++++++++++++++-------- > 1 file changed, 34 insertions(+), 8 deletions(-) > > diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c > index aa52699..1338a55 100644 > --- a/arch/arm/common/gic.c > +++ b/arch/arm/common/gic.c > @@ -70,6 +70,13 @@ struct gic_chip_data { > static DEFINE_RAW_SPINLOCK(irq_controller_lock); > > /* > + * The GIC mapping of CPU interfaces does not necessarily match > + * the logical CPU numbering. Let's use a mapping as returned > + * by the GIC itself. > + */ > +static u8 gic_cpu_map[8] __read_mostly; Can we have a #define for the number CPUs supported by the GIC? It gets used a fair amount in this patch for loop bounds etc. > +/* > * Supported arch specific GIC irq extension. > * Default make them NULL. > */ > @@ -242,7 +249,7 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val, > return -EINVAL; > > mask = 0xff << shift; > - bit = 1 << (cpu_logical_map(cpu) + shift); > + bit = gic_cpu_map[cpu] << shift; > > raw_spin_lock(&irq_controller_lock); > val = readl_relaxed(reg) & ~mask; > @@ -349,11 +356,6 @@ static void __init gic_dist_init(struct gic_chip_data *gic) > u32 cpumask; > unsigned int gic_irqs = gic->gic_irqs; > void __iomem *base = gic_data_dist_base(gic); > - u32 cpu = cpu_logical_map(smp_processor_id()); > - > - cpumask = 1 << cpu; > - cpumask |= cpumask << 8; > - cpumask |= cpumask << 16; > > writel_relaxed(0, base + GIC_DIST_CTRL); > > @@ -366,6 +368,7 @@ static void __init gic_dist_init(struct gic_chip_data *gic) > /* > * Set all global interrupts to this CPU only. > */ > + cpumask = readl_relaxed(base + GIC_DIST_TARGET + 0); > for (i = 32; i < gic_irqs; i += 4) > writel_relaxed(cpumask, base + GIC_DIST_TARGET + i * 4 / 4); > > @@ -389,9 +392,25 @@ static void __cpuinit gic_cpu_init(struct gic_chip_data *gic) > { > void __iomem *dist_base = gic_data_dist_base(gic); > void __iomem *base = gic_data_cpu_base(gic); > + unsigned int cpu_mask, cpu = smp_processor_id(); > int i; > > /* > + * Get what the GIC says our CPU mask is. > + */ > + BUG_ON(cpu >= 8); > + cpu_mask = readl_relaxed(dist_base + GIC_DIST_TARGET + 0); Making the mask a u8 and using readb_relaxed here makes this bit of code clearer to me (and the GIC apparently allows such an access to this register). > + gic_cpu_map[cpu] = cpu_mask; > + > + /* > + * Clear our mask from the other map entries in case they're > + * still undefined. > + */ > + for (i = 0; i < 8; i++) > + if (i != cpu) > + gic_cpu_map[i] &= ~cpu_mask; > + > + /* > * Deal with the banked PPI and SGI interrupts - disable all > * PPI interrupts, ensure all SGI interrupts are enabled. > */ > @@ -646,7 +665,7 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, > { > irq_hw_number_t hwirq_base; > struct gic_chip_data *gic; > - int gic_irqs, irq_base; > + int gic_irqs, irq_base, i; > > BUG_ON(gic_nr >= MAX_GIC_NR); > > @@ -683,6 +702,13 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, > } > > /* > + * Initialize the CPU interface map to all CPUs. > + * It will be refined as each CPU probes its ID. > + */ > + for (i = 0; i < 8; i++) > + gic_cpu_map[i] = 0xff; > + > + /* > * For primary GICs, skip over SGIs. > * For secondary GICs, skip over PPIs, too. > */ > @@ -737,7 +763,7 @@ void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) > > /* Convert our logical CPU mask into a physical one. */ > for_each_cpu(cpu, mask) > - map |= 1 << cpu_logical_map(cpu); > + map |= gic_cpu_map[cpu]; > > /* > * Ensure that stores to Normal memory are visible to the With those changes: Acked-by: Will Deacon Will