linux-kernel - Re: [PATCH] x86: simplify mtrr_bp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAE9FiQUkt7s_zfbn+iZcKXaRpvOHm9CWjxvqpNsLsnSywdHoug@mail.gmail.com>
Date:	Fri, 6 Jul 2012 15:02:28 -0700
From:	Yinghai Lu <yinghai@...nel.org>
To:	Jan Beulich <JBeulich@...e.com>
Cc:	mingo@...e.hu, tglx@...utronix.de, hpa@...or.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86: simplify mtrr_bp_init()

On Fri, Jul 6, 2012 at 7:18 AM, Jan Beulich <JBeulich@...e.com> wrote:
> Now that the x86_phys_bits cpuinfo field is uniformly available on
> 32- and 64-bit, the function no longer needs to determine this anew.
>
> Additionally, both size_or_mask and size_and_mask can be set once at
> the end of the function instead of individually in each special case,
> as their value solely depends on the physical address size determined
> to be used here.
>
> Signed-off-by: Jan Beulich <jbeulich@...e.com>
>
> ---
>  arch/x86/kernel/cpu/mtrr/main.c |   47 ++++++++++------------------------------
>  1 file changed, 12 insertions(+), 35 deletions(-)
>
> --- 3.5-rc5/arch/x86/kernel/cpu/mtrr/main.c
> +++ 3.5-rc5-x86-mtrr-init-simplify/arch/x86/kernel/cpu/mtrr/main.c
> @@ -592,67 +592,41 @@ int __initdata changed_by_mtrr_cleanup;
>   */
>  void __init mtrr_bp_init(void)
>  {
> -       u32 phys_addr;
> +       unsigned int phys_addr = boot_cpu_data.x86_phys_bits;
>
>         init_ifs();
>
> -       phys_addr = 32;
> -
>         if (cpu_has_mtrr) {
>                 mtrr_if = &generic_mtrr_ops;
> -               size_or_mask = 0xff000000;                      /* 36 bits */
> -               size_and_mask = 0x00f00000;
> -               phys_addr = 36;
> -
> -               /*
> -                * This is an AMD specific MSR, but we assume(hope?) that
> -                * Intel will implement it to when they extend the address
> -                * bus of the Xeon.
> -                */
> -               if (cpuid_eax(0x80000000) >= 0x80000008) {
> -                       phys_addr = cpuid_eax(0x80000008) & 0xff;
> -                       /* CPUID workaround for Intel 0F33/0F34 CPU */
> -                       if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
> -                           boot_cpu_data.x86 == 0xF &&
> -                           boot_cpu_data.x86_model == 0x3 &&
> -                           (boot_cpu_data.x86_mask == 0x3 ||
> -                            boot_cpu_data.x86_mask == 0x4))
> -                               phys_addr = 36;
> -
> -                       size_or_mask = ~((1ULL << (phys_addr - PAGE_SHIFT)) - 1);
> -                       size_and_mask = ~size_or_mask & 0xfffff00000ULL;
> -               } else if (boot_cpu_data.x86_vendor == X86_VENDOR_CENTAUR &&
> -                          boot_cpu_data.x86 == 6) {
> +
> +               if (boot_cpu_data.x86_vendor == X86_VENDOR_CENTAUR &&
> +                   boot_cpu_data.x86 == 6) {
>                         /*
>                          * VIA C* family have Intel style MTRRs,
>                          * but don't support PAE
>                          */
> -                       size_or_mask = 0xfff00000;              /* 32 bits */
> -                       size_and_mask = 0;
>                         phys_addr = 32;
> -               }
> +               } else if (phys_addr < 36)
> +                       phys_addr = 36;
>         } else {
>                 switch (boot_cpu_data.x86_vendor) {
>                 case X86_VENDOR_AMD:
>                         if (cpu_has_k6_mtrr) {
>                                 /* Pre-Athlon (K6) AMD CPU MTRRs */
>                                 mtrr_if = mtrr_ops[X86_VENDOR_AMD];
> -                               size_or_mask = 0xfff00000;      /* 32 bits */
> -                               size_and_mask = 0;
> +                               phys_addr = 32;
>                         }
>                         break;
>                 case X86_VENDOR_CENTAUR:
>                         if (cpu_has_centaur_mcr) {
>                                 mtrr_if = mtrr_ops[X86_VENDOR_CENTAUR];
> -                               size_or_mask = 0xfff00000;      /* 32 bits */
> -                               size_and_mask = 0;
> +                               phys_addr = 32;
>                         }
>                         break;
>                 case X86_VENDOR_CYRIX:
>                         if (cpu_has_cyrix_arr) {
>                                 mtrr_if = mtrr_ops[X86_VENDOR_CYRIX];
> -                               size_or_mask = 0xfff00000;      /* 32 bits */
> -                               size_and_mask = 0;
> +                               phys_addr = 32;

should drop all phys_addr assignment in this function.

x86_phys_bits should have all correct value?

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/