From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH 7/7] xen/arm: Use the hardware ID TMP boot correctly secondary cpus Date: Tue, 10 Sep 2013 17:24:48 +0100 Message-ID: <522F47D0.7020202@linaro.org> References: <1377869433-15385-1-git-send-email-julien.grall@linaro.org> <1377869433-15385-8-git-send-email-julien.grall@linaro.org> <1378734017.19967.138.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: <1378734017.19967.138.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:40 PM, Ian Campbell wrote: > On Fri, 2013-08-30 at 14:30 +0100, Julien Grall wrote: > > TMP in subject == "to"? Yes. I didn't pay attention when I sent this patch. I will fix it. > >> Secondary CPUs will spin in head.S until their MPIDR[23:0] correspond to >> the smp_up_cpu. Actually Xen will set the value with the logical CPU ID >> which is wrong. Use the cpu_logical_map to get the correct CPU ID. >> >> Acked-by: Julien Grall > > S-o-b? Same here :/ > >> --- >> xen/arch/arm/smpboot.c | 20 +++++++++++++++----- >> 1 file changed, 15 insertions(+), 5 deletions(-) >> >> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c >> index c0d25de..1952287 100644 >> --- a/xen/arch/arm/smpboot.c >> +++ b/xen/arch/arm/smpboot.c >> @@ -124,8 +124,7 @@ make_cpus_ready(unsigned int max_cpus, unsigned long boot_phys_offset) >> for ( i = 1; i < max_cpus; i++ ) >> { >> /* Tell the next CPU to get ready */ >> - /* TODO: handle boards where CPUIDs are not contiguous */ >> - *gate = i; >> + *gate = cpu_logical_map(i); >> flush_xen_dcache(*gate); >> isb(); >> sev(); >> @@ -139,11 +138,22 @@ make_cpus_ready(unsigned int max_cpus, unsigned long boot_phys_offset) >> /* Boot the current CPU */ >> void __cpuinit start_secondary(unsigned long boot_phys_offset, >> unsigned long fdt_paddr, >> - unsigned long cpuid) >> + unsigned long hwid) >> { >> + unsigned int cpuid; >> + >> memset(get_cpu_info(), 0, sizeof (struct cpu_info)); >> >> - /* TODO: handle boards where CPUIDs are not contiguous */ >> + /* Browse the logical map and find the associate logical cpu ID */ >> + for ( cpuid = 1; cpuid < nr_cpu_ids; cpuid++ ) >> + { >> + if ( cpu_logical_map(cpuid) == hwid ) >> + break; >> + } >> + >> + if ( cpuid == nr_cpu_ids ) >> + panic("Can't find logical CPU id for cpu MPIDR[23:0] = 0x%lx\n", hwid); > > We could stumble on without this cpu. No. The boot cpu will wait forever the secondary cpu because we don't have timeout in __cpu_up. > >> + >> set_processor_id(cpuid); >> >> current_cpu_data = boot_cpu_data; >> @@ -232,7 +242,7 @@ int __cpu_up(unsigned int cpu) >> >> /* Unblock the CPU. It should be waiting in the loop in head.S >> * for an event to arrive when smp_up_cpu matches its cpuid. */ >> - smp_up_cpu = cpu; >> + smp_up_cpu = cpu_logical_map(cpu); >> /* we need to make sure that the change to smp_up_cpu is visible to >> * secondary cpus with D-cache off */ >> flush_xen_dcache(smp_up_cpu); > > -- Julien Grall