2012-10-10 19:59:31

by Andres Salomon

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

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 <[email protected]>


On Mon, 24 Sep 2012 16:27:19 -0700 "H. Peter
Anvin" <[email protected]> wrote:

> From: "H. Peter Anvin" <[email protected]>
>
> %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 <[email protected]>
> ---
> 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