linux-kernel - Re: [PATCH -tip] x86: cpu/intel.c cleanup

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]

Date:	Sat, 14 Mar 2009 15:00:25 +0100
From:	Vegard Nossum <vegard.nossum@...il.com>
To:	Jaswinder Singh Rajput <jaswinder@...nel.org>
Cc:	Ingo Molnar <mingo@...e.hu>, Sam Ravnborg <sam@...nborg.org>,
	x86 maintainers <x86@...nel.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH -tip] x86: cpu/intel.c cleanup

2009/3/14 Jaswinder Singh Rajput <jaswinder@...nel.org>:
> From: Jaswinder Singh Rajput <jaswinderrajput@...il.com>
> Date: Sat, 14 Mar 2009 17:47:38 +0530
> Subject: [PATCH] x86: cpu/intel.c cleanup
>
> - fix various style problems
>  - fix header files issues
>

[...]

>  static void __cpuinit early_init_intel(struct cpuinfo_x86 *c)
>  {
> +       u64 misc_enable;
> +
>        /* Unmask CPUID levels if masked: */
>        if (c->x86 > 6 || (c->x86 == 6 && c->x86_model >= 0xd)) {
> -               u64 misc_enable;
>
>                rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
>
> @@ -44,16 +45,16 @@ static void __cpuinit early_init_intel(struct cpuinfo_x86 *c)
>        }
>
>        if ((c->x86 == 0xf && c->x86_model >= 0x03) ||
> -               (c->x86 == 0x6 && c->x86_model >= 0x0e))
> +           (c->x86 == 0x6 && c->x86_model >= 0x0e))
>                set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
>
>  #ifdef CONFIG_X86_64
>        set_cpu_cap(c, X86_FEATURE_SYSENTER32);
> -#else
> +#else /* CONFIG_X86_64 */
>        /* Netburst reports 64 bytes clflush size, but does IO in 128 bytes */
>        if (c->x86 == 15 && c->x86_cache_alignment == 64)
>                c->x86_cache_alignment = 128;
> -#endif
> +#endif /* CONFIG_X86_64 */
>
>        /* CPUID workaround for 0F33/0F34 CPU */
>        if (c->x86 == 0xF && c->x86_model == 0x3
> @@ -96,19 +97,18 @@ static void __cpuinit early_init_intel(struct cpuinfo_x86 *c)
>         * Ingo Molnar reported a Pentium D (model 6) and a Xeon
>         * (model 2) with the same problem.
>         */
> -       if (c->x86 == 15) {
> -               u64 misc_enable;
> +       if (c->x86 != 15)
> +               return;
>
> -               rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
> +       rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
>
> -               if (misc_enable & MSR_IA32_MISC_ENABLE_FAST_STRING) {
> -                       printk(KERN_INFO "kmemcheck: Disabling fast string operations\n");
> +       if (misc_enable & MSR_IA32_MISC_ENABLE_FAST_STRING) {
> +               pr_info("kmemcheck: Disabling fast string operations\n");
>
> -                       misc_enable &= ~MSR_IA32_MISC_ENABLE_FAST_STRING;
> -                       wrmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
> -               }
> +               misc_enable &= ~MSR_IA32_MISC_ENABLE_FAST_STRING;
> +               wrmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
>        }
> -#endif
> +#endif /* CONFIG_KMEMCHECK */
>  }

I don't really like this change (last hunk). Doesn't it seem a bit
pointless? It breaks the symmetry with the masked CPUID levels at the
beginning of the function. If somebody wants to add something else to
this function, it might have to be reindented again. Or is there a
problem with too long lines here?

But it's just a question of taste -- if this is the preferred style,
then it's fine.


Vegard
--
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/