From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH 4/7] xen/arm: gic: Use the correct CPU ID Date: Tue, 10 Sep 2013 16:42:27 +0100 Message-ID: <522F3DE3.7050504@linaro.org> References: <1377869433-15385-1-git-send-email-julien.grall@linaro.org> <1377869433-15385-5-git-send-email-julien.grall@linaro.org> <1378733314.19967.130.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1378733314.19967.130.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: stefano.stabellini@eu.citrix.com, patches@linaro.org, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 09/09/2013 02:28 PM, Ian Campbell wrote: > On Fri, 2013-08-30 at 14:30 +0100, Julien Grall wrote: >> The GIC mapping of CPU interfaces does not necessarily match the logical >> CPU numbering. >> >> When Xen wants to send an SGI to specific CPU, it needs to use the GIC CPU ID. >> It can be retrieved from ITARGETSR0, in fact when this field is read, the GIC >> will return a value that corresponds only to the processor reading the register. >> So Xen can use the PPI 0 to initialize the mapping. >> >> Signed-off-by: Julien Grall >> --- >> xen/arch/arm/gic.c | 35 ++++++++++++++++++++++++++++------- >> 1 file changed, 28 insertions(+), 7 deletions(-) >> >> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c >> index cadc258..4f3a8a5 100644 >> --- a/xen/arch/arm/gic.c >> +++ b/xen/arch/arm/gic.c >> @@ -57,6 +57,27 @@ static DEFINE_PER_CPU(uint64_t, lr_mask); >> >> static unsigned nr_lrs; >> >> +/* The GIC mapping of CPU interfaces does not necessarily match the >> + * logical CPU numbering. Let's use mapping as returned by the GIC >> + * itself >> + */ >> +#define NR_GIC_CPU_IF 8 >> +static u8 gic_cpu_map[NR_GIC_CPU_IF] __read_mostly = {0xff}; > > Isn't this mapping from logical cpus ids to gic cpus ids? IOW the size > of this array is the wrong way around? This array maps a logical cpu id (the index) to a gic cpus id. The size if correct, because Xen will allocate logical cpu id from 0 to 7. > Is what you want here is a per-cpu variable? Does per-cpu variable allow any cpu to retrieve data of another cpu? I didn't find a such function. >> + >> +static unsigned int gic_cpu_mask(const cpumask_t *cpumask) >> +{ >> + unsigned int cpu; >> + unsigned int mask = 0; >> + >> + for_each_cpu(cpu, cpumask) >> + { >> + ASSERT(cpu < NR_GIC_CPU_IF); >> + mask |= gic_cpu_map[cpu]; > > Further to above, can't cpu here be e.g. 0x100 for an a7 on a big.ITTLE > system? 0x100 is the hardware CPU id. Xen will allocate a logical cpu id from 0. >> + } >> + >> + return mask; >> +} >> + >> unsigned int gic_number_lines(void) >> { >> return gic.lines; >> @@ -206,9 +227,7 @@ static void gic_set_irq_properties(unsigned int irq, bool_t level, >> { >> volatile unsigned char *bytereg; >> uint32_t cfg, edgebit; >> - unsigned int mask = cpumask_bits(cpu_mask)[0]; >> - >> - ASSERT(!(mask & ~0xff)); /* Target bitmap only support 8 CPUS */ >> + unsigned int mask = gic_cpu_mask(cpu_mask); >> >> /* Set edge / level */ >> cfg = GICD[GICD_ICFGR + irq / 16]; >> @@ -317,6 +336,8 @@ static void __cpuinit gic_cpu_init(void) >> { >> int i; >> >> + gic_cpu_map[smp_processor_id()] = GICD[GICD_ITARGETSR] & 0xff; >> + >> /* The first 32 interrupts (PPI and SGI) are banked per-cpu, so >> * even though they are controlled with GICD registers, they must >> * be set up here with the other per-cpu state. */ >> @@ -443,13 +464,13 @@ void __init gic_init(void) >> >> void send_SGI_mask(const cpumask_t *cpumask, enum gic_sgi sgi) >> { >> - unsigned long mask = cpumask_bits(cpumask)[0]; >> + cpumask_t online_mask; >> + unsigned int mask = 0; >> >> ASSERT(sgi < 16); /* There are only 16 SGIs */ >> >> - mask &= cpumask_bits(&cpu_online_map)[0]; >> - >> - ASSERT(mask < 0x100); /* The target bitmap only supports 8 CPUs */ >> + cpumask_and(&online_mask, cpumask, &cpu_online_map); >> + mask = gic_cpu_mask(&online_mask); >> >> dsb(); >> > > -- Julien Grall