On Sun, Mar 20, 2016 at 03:36:45AM +1100, Jonathan Gray wrote:
> On Wed, Mar 02, 2016 at 03:31:57PM +0100, Patrick Wildt wrote:
> > Hi,
> > 
> > currently we have a rather big list of bits that we remove and set in
> > the System Control Register.  Those differ per processor, so not every
> > armv7/armv8 processor has the same bits, although they tend to be
> > rather similar.
> > 
> > I think we should only reset and set a few specific ones that we really
> > really care about.  The SYST bit for instance is reserved on the A7
> > and enables something completely else (SETEND instruction) on the A53.
> > 
> > This is basically the same set as FreeBSD had before they removed the
> > armv7_setup code, as they now do that stuff in their locore.S.
> > 
> > Additionally this diff removes unused code and adds a TODO comment.
> > What we will need to do is set up the Auxiliary Control Register
> > to allow us to use e.g. instructions for atomic operations on
> > ARMv7.  Then we can replace those interrupt disable/enable hacks
> > with proper atomics.
> > 
> > Patrick
> 
> This diff is effectively
> 
> -       cpuctrl = | CPU_CONTROL_SYST_ENABLE
> 
> -       cpuctrlmask = | CPU_CONTROL_SYST_ENABLE
> -           | CPU_CONTROL_ROM_ENABLE 
> -           | CPU_CONTROL_BEND_ENABLE 
> -           | CPU_CONTROL_ROUNDROBIN | CPU_CONTROL_CPCLK
> -           | CPU_CONTROL_FI | CPU_CONTROL_VE
> -           | CPU_CONTROL_TRE | CPU_CONTROL_AFE;
> 
> Almost all of these do indeed appear to be reserved for v7.
> 
> Shouldn't the TRE/AFE v7 bits continue to be masked though?
> 
> CPU_CONTROL_TRE (TEX remap enable)
> CPU_CONTROL_AFE (Access Flag Enable)

That's a very good question.  In my opinion I would say, don't mask
them.

These bits are tightly coupled with pmap.  In our pmap the assumption
is that we do not use TEX remap and do not use the Access Flag.  I think
we should make the pmap control those bits.  Then it can decide itself
if it wants to make use of those bits or prefers to leave them disabled.
Also it would enable us to run two pmap more easily.  Like, the current
pmap and a rewrite.

Though, maybe this is out of scope for this diff and we should leave
those bits in the mask for now.  For the future I would prefer to have
the pmap take control over those.

> 
> > 
> > diff --git sys/arch/arm/arm/cpufunc.c sys/arch/arm/arm/cpufunc.c
> > index aab8bc6..2ca5329 100644
> > --- sys/arch/arm/arm/cpufunc.c
> > +++ sys/arch/arm/arm/cpufunc.c
> > @@ -1342,16 +1342,18 @@ armv7_setup()
> >  {
> >     int cpuctrl, cpuctrlmask;
> >  
> > -   cpuctrl = CPU_CONTROL_MMU_ENABLE | CPU_CONTROL_SYST_ENABLE
> > -       | CPU_CONTROL_IC_ENABLE | CPU_CONTROL_DC_ENABLE
> > -       | CPU_CONTROL_BPRD_ENABLE | CPU_CONTROL_AFLT_ENABLE;
> > -   cpuctrlmask = CPU_CONTROL_MMU_ENABLE | CPU_CONTROL_SYST_ENABLE
> > -       | CPU_CONTROL_IC_ENABLE | CPU_CONTROL_DC_ENABLE
> > -       | CPU_CONTROL_ROM_ENABLE | CPU_CONTROL_BPRD_ENABLE
> > -       | CPU_CONTROL_BEND_ENABLE | CPU_CONTROL_AFLT_ENABLE
> > -       | CPU_CONTROL_ROUNDROBIN | CPU_CONTROL_CPCLK
> > -       | CPU_CONTROL_VECRELOC | CPU_CONTROL_FI | CPU_CONTROL_VE
> > -       | CPU_CONTROL_TRE | CPU_CONTROL_AFE;
> > +   cpuctrlmask = CPU_CONTROL_MMU_ENABLE
> > +       | CPU_CONTROL_AFLT_ENABLE
> > +       | CPU_CONTROL_DC_ENABLE
> > +       | CPU_CONTROL_BPRD_ENABLE
> > +       | CPU_CONTROL_IC_ENABLE
> > +       | CPU_CONTROL_VECRELOC;
> > +
> > +   cpuctrl = CPU_CONTROL_MMU_ENABLE
> > +       | CPU_CONTROL_AFLT_ENABLE
> > +       | CPU_CONTROL_DC_ENABLE
> > +       | CPU_CONTROL_BPRD_ENABLE
> > +       | CPU_CONTROL_IC_ENABLE;
> >  
> >     if (vector_page == ARM_VECTORS_HIGH)
> >             cpuctrl |= CPU_CONTROL_VECRELOC;
> > @@ -1359,16 +1361,12 @@ armv7_setup()
> >     /* Clear out the cache */
> >     cpu_idcache_wbinv_all();
> >  
> > -   /* Now really make sure they are clean.  */
> > -   /* XXX */
> > -   /*
> > -   asm volatile ("mcr\tp15, 0, r0, c7, c7, 0" : : );
> > -   */
> > -
> >     /* Set the control register */
> >     curcpu()->ci_ctrl = cpuctrl;
> >     cpu_control(cpuctrlmask, cpuctrl);
> >  
> > +   /* TODO: Set ACTLR to e.g. allow LDREX/STREX. */
> > +
> >     /* And again. */
> >     cpu_idcache_wbinv_all();
> >  }
> > 
> 

Reply via email to