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

[Konrad Rzeszutek Wilk]

On Mon, Sep 24, 2012 at 04:27:19PM -0700, H. Peter Anvin wrote:
> From: "H. Peter Anvin" <hpa@xxxxxxxxxxxxxxx>
> 
> %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.
> 

>From a glance at this it looks ok -  as we do not ever end up in this
function at all. Our entry point in the kernel is from startup_xen.

But just to make sure that nothing is amiss let me take this patch
on a spin.

And thank you for CC-ing me.

> Signed-off-by: H. Peter Anvin <hpa@xxxxxxxxxxxxxxx>
> ---
>  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
> -- 
> 1.7.6.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/