From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 4/7] xen/arm: set the SMP bit in the ACTLR register Date: Thu, 25 Oct 2012 13:04:30 +0100 Message-ID: <1351166670.18035.177.camel@zakaz.uk.xensource.com> References: <1351091027-20740-4-git-send-email-stefano.stabellini@eu.citrix.com> <1351158737.18035.142.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: "xen-devel@lists.xensource.com" , "Tim (Xen.org)" List-Id: xen-devel@lists.xenproject.org On Thu, 2012-10-25 at 12:57 +0100, Stefano Stabellini wrote: > On Thu, 25 Oct 2012, Ian Campbell wrote: > > On Wed, 2012-10-24 at 16:03 +0100, Stefano Stabellini wrote: > > > From the Cortex A15 manual: > > > > > > "Enables the processor to receive instruction cache, BTB, and TLB maintenance > > > operations from other processors" > > > > > > ... > > > > > > "You must set this bit before enabling the caches and MMU, or > > > performing any cache and TLB maintenance operations. The only time > > > you must clear this bit is during a processor power-down sequence" > > > > Is it considered a bug that the firmware doesn't do this? > > Why would it be? You can run fairly complicated pieces of software > without caches or MMU. True, I guess I just considered that not setting the SMP bit on the second processor when the firmware starts it seemed a bit odd. If you aren't caches/MMU/etc them then setting the bit is pretty much a NOP (or maybe it isn't?). > > > > > Signed-off-by: Stefano Stabellini > > > --- > > > xen/arch/arm/head.S | 6 ++++++ > > > xen/include/asm-arm/processor.h | 30 ++++++++++++++++++++++++++++++ > > > 2 files changed, 36 insertions(+), 0 deletions(-) > > > > > > diff --git a/xen/arch/arm/head.S b/xen/arch/arm/head.S > > > index 3d01be0..c784f4d 100644 > > > --- a/xen/arch/arm/head.S > > > +++ b/xen/arch/arm/head.S > > > @@ -148,6 +148,12 @@ skip_bss: > > > > > > PRINT("- Setting up control registers -\r\n") > > > > > > + /* XXX: Cortex A15 specific */ > > > > We probably need some sort of early proc info keyed of the processor id > > (like Linux) has so we can push this processor specific stuff into the > > right places. > > Indeed. > I wrote something simple that should be OK until we get more than > ~10 different processors. > > > > > + /* Set up the SMP bit in ACTLR */ > > > + mrc CP32(r0, ACTLR) > > > + orr r0, r0, #(ACTLR_SMP) /* enable SMP bit*/ > > > + mcr CP32(r0, ACTLR) > > > > Linux does this IFF it isn't already set for some reason: > > #ifdef CONFIG_SMP > > ALT_SMP(mrc p15, 0, r0, c1, c0, 1) > > ALT_UP(mov r0, #(1 << 6)) @ fake it for UP > > tst r0, #(1 << 6) @ SMP/nAMP mode enabled? > > orreq r0, r0, #(1 << 6) @ Enable SMP/nAMP mode > > orreq r0, r0, r10 @ Enable CPU-specific SMP bits > > mcreq p15, 0, r0, c1, c0, 1 > > #endif > > > > Might just relate to the "fake it up for UP" thing I suppose. > > Considering that we don't have a UP config option for Xen (I am aware > of), there is no need for us to do that, I think. > > > > > + > > > /* Set up memory attribute type tables */ > > > ldr r0, =MAIR0VAL > > > ldr r1, =MAIR1VAL > > > diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h > > > index 3849b23..91a5836 100644 > > > --- a/xen/include/asm-arm/processor.h > > > +++ b/xen/include/asm-arm/processor.h > > > @@ -34,6 +34,36 @@ > > > #define SCTLR_A (1<<1) > > > #define SCTLR_M (1<<0) > > > > > > +/* ACTLR Auxiliary Control Register */ > > > +#define ACTLR_SNOOP_DELAYED (1<<31) > > > > If these are CortexA15 specific then I think they ought to be > > ACTLR_CA15_FOO or something. And possibly in a new processor-ca15.h > > header instead. > > OK