Re: [RFC PATCH] x86-32: Start out eflags and cr4 clean

[Andres Salomon <dilinger@queued.net>]

Bad news on this patch; I've been told that it breaks booting on an
XO-1.5.  Does anyone from OLPC know why yet?


On Wed, 10 Oct 2012 12:59:27 -0700
Andres Salomon <dilinger@queued.net> wrote:

> Thanks for Cc'ing me.  I just tested this on an XO-1, and it doesn't
> appear to break anything.  So, looks fine to me.
> 
> Acked-by: Andres Salomon <dilinger@queued.net>
> 
> 
> On Mon, 24 Sep 2012 16:27:19 -0700 "H. Peter
> Anvin" <hpa@linux.intel.com> wrote:
> 
> > From: "H. Peter Anvin" <hpa@linux.intel.com>
> > 
> > %cr4 is supposed to reflect a set of features into which the
> > operating system is opting in.  If the BIOS or bootloader leaks
> > bits here, this is not desirable.  Consider a bootloader passing in
> > %cr4.pae set to a legacy paging kernel, for example -- it will not
> > have any immediate effect, but the kernel would crash when turning
> > paging on.
> > 
> > A similar argument applies to %eflags, and since we have to look for
> > %eflags.id being settable we can use a sequence which clears %eflags
> > as a side effect.
> > 
> > Note that we already do this for x86-64.
> > 
> > I would like opinions on this especially from the PV crowd and
> > nonstandard platforms (e.g. OLPC) to make sure we don't screw them
> > up.
> > 
> > Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
> > ---
> >  arch/x86/kernel/head_32.S |   31 ++++++++++++++++---------------
> >  1 files changed, 16 insertions(+), 15 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
> > index d42ab17..957a47a 100644
> > --- a/arch/x86/kernel/head_32.S
> > +++ b/arch/x86/kernel/head_32.S
> > @@ -287,27 +287,28 @@ ENTRY(startup_32_smp)
> >  	leal -__PAGE_OFFSET(%ecx),%esp
> >  
> >  default_entry:
> > -
> >  /*
> >   *	New page tables may be in 4Mbyte page mode and may
> >   *	be using the global pages. 
> >   *
> >   *	NOTE! If we are on a 486 we may have no cr4 at all!
> > - *	So we do not try to touch it unless we really have
> > - *	some bits in it to set.  This won't work if the BSP
> > - *	implements cr4 but this AP does not -- very unlikely
> > - *	but be warned!  The same applies to the pse feature
> > - *	if not equally supported. --macro
> > - *
> > - *	NOTE! We have to correct for the fact that we're
> > - *	not yet offset PAGE_OFFSET..
> > + *	Specifically, cr4 exists if and only if CPUID exists,
> > + *	which in turn exists if and only if EFLAGS.ID exists.
> >   */
> > -#define cr4_bits pa(mmu_cr4_features)
> > -	movl cr4_bits,%edx
> > -	andl %edx,%edx
> > -	jz 6f
> > -	movl %cr4,%eax		# Turn on paging options
> > (PSE,PAE,..)
> > -	orl %edx,%eax
> > +	movl $X86_EFLAGS_ID,%ecx
> > +	pushl %ecx
> > +	popfl
> > +	pushfl
> > +	popl %eax
> > +	pushl $0
> > +	popfl
> > +	pushfl
> > +	popl %edx
> > +	xorl %edx,%eax
> > +	testl %ecx,%eax
> > +	jz 6f			# No ID flag = no CPUID = no
> > CR4 +
> > +	movl pa(mmu_cr4_features),%eax
> >  	movl %eax,%cr4
> >  
> >  	testb $X86_CR4_PAE, %al		# check if PAE is
> > enabled