RE: [PATCH v2 1/2] x86: Enable fast strings on Intel if BIOS hasn'talready

[Yu, Fenghua]

> -----Original Message-----
> From: Andy Lutomirski [mailto:luto@xxxxxxx]
> Sent: Saturday, August 06, 2011 4:43 AM
> To: x86@xxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Cc: Yu, Fenghua; Matthew Garrett; Len Brown; linux-
> acpi@xxxxxxxxxxxxxxx; Ingo Molnar; Andy Lutomirski
> Subject: [PATCH v2 1/2] x86: Enable fast strings on Intel if BIOS
> hasn't already
> 
> Intel SDM volume 3A, 8.4.2 says:
> 
>   Software can disable fast-string operation by clearing the
>   fast-string-enable bit (bit 0) of IA32_MISC_ENABLE MSR.
>   However, Intel recomments that system software always enable
>   fast-string operation.
> 
> The Intel DQ67SW board (with latest BIOS) disables fast string
> operations if TXT is enabled.  A Lenovo X220 disables it regardless
> of TXT setting.  I doubt I'm the only person with a dumb BIOS like
> this.
> 
> Signed-off-by: Andy Lutomirski <luto@xxxxxxx>
> ---
>  arch/x86/kernel/cpu/intel.c |   25 ++++++++++++++++++++++---
>  1 files changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index ed6086e..c80ab41 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -30,6 +30,7 @@
>  static void __cpuinit early_init_intel(struct cpuinfo_x86 *c)
>  {
>  	u64 misc_enable;
> +	bool allow_fast_string = true;
> 
>  	/* Unmask CPUID levels if masked: */
>  	if (c->x86 > 6 || (c->x86 == 6 && c->x86_model >= 0xd)) {
> @@ -118,8 +119,9 @@ static void __cpuinit early_init_intel(struct
> cpuinfo_x86 *c)
>  	 * (model 2) with the same problem.
>  	 */
>  	if (c->x86 == 15) {
> -		rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
> +		allow_fast_string = false;
> 
> +		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");
> 
> @@ -130,11 +132,28 @@ static void __cpuinit early_init_intel(struct
> cpuinfo_x86 *c)
>  #endif
> 
>  	/*
> -	 * If fast string is not enabled in IA32_MISC_ENABLE for any
> reason,
> -	 * clear the fast string and enhanced fast string CPU
> capabilities.
> +	 * If BIOS didn't enable fast string operation, try to enable
> +	 * it ourselves.  If that fails, then clear the fast string
> +	 * and enhanced fast string CPU capabilities.
>  	 */
>  	if (c->x86 > 6 || (c->x86 == 6 && c->x86_model >= 0xd)) {
>  		rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
> +
> +		if (allow_fast_string &&
> +		    !(misc_enable & MSR_IA32_MISC_ENABLE_FAST_STRING)) {
> +			misc_enable |= MSR_IA32_MISC_ENABLE_FAST_STRING;
> +			wrmsr_safe(MSR_IA32_MISC_ENABLE, (u32)misc_enable,
> +				   (u32)(misc_enable >> 32));
> +
> +			/* Re-read to make sure it stuck. */
> +			rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
> +
> +			if (misc_enable & MSR_IA32_MISC_ENABLE_FAST_STRING)
> +				printk(KERN_WARNING FW_WARN "CPU #%d: "
> +				       "IA32_MISC_ENABLE.FAST_STRING_ENABLE "
> +				       "was not set", c->cpu_index);
This printk is redundant because the same info is dumped in the below printk. Plus it's not firmware's issue if we can not set fast string bit 0 in MISC_ENABLE register. So I don't think FW_WARN is right.

I would suggest to remove this printk (and the if () statement) and change KERN_INFO to KERN_WARNING in the following printk.

> +		}
> +
>  		if (!(misc_enable & MSR_IA32_MISC_ENABLE_FAST_STRING)) {
>  			printk(KERN_INFO "Disabled fast string
> operations\n");
>  			setup_clear_cpu_cap(X86_FEATURE_REP_GOOD);
> --
> 1.7.6

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