linux-kernel - Re: [patch 10/23] x86/x2apic: Disable x2apic from nox2apic setup

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: <20150116190126.GL18880@pd.tnic>
Date:	Fri, 16 Jan 2015 20:01:26 +0100
From:	Borislav Petkov <bp@...en8.de>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Jiang Liu <jiang.liu@...ux.intel.com>,
	Joerg Roedel <joro@...tes.org>, x86@...nel.org,
	Tony Luck <tony.luck@...el.com>
Subject: Re: [patch 10/23] x86/x2apic: Disable x2apic from nox2apic setup

On Thu, Jan 15, 2015 at 09:22:24PM -0000, Thomas Gleixner wrote:
> There is no point in postponing the hardware disablement of x2apic. It
> can be disabled right away in the nox2apic setup function.
> 
> Disable it right away and set the state to DISABLED . This allows to
> remove all the nox2apic conditionals all over the place.
> 
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> ---

...

> @@ -1488,6 +1484,19 @@ enum {
>  };
>  static int x2apic_state;
>  
> +static inline void disable_x2apic(void)
> +{
> +	u64 msr;
> +
> +	rdmsrl(MSR_IA32_APICBASE, msr);
> +	if (!(msr & X2APIC_ENABLE))
> +		return;
> +	/* Disable xapic and x2apic first and then reenable xapic mode */
> +	wrmsrl(MSR_IA32_APICBASE, msr & ~(X2APIC_ENABLE | XAPIC_ENABLE));
> +	wrmsrl(MSR_IA32_APICBASE, msr & ~X2APIC_ENABLE);
> +	printk_once(KERN_INFO "x2apic disabled\n");
> +}
> +
>  static int __init setup_nox2apic(char *str)
>  {
>  	if (x2apic_enabled()) {
> @@ -1498,28 +1507,17 @@ static int __init setup_nox2apic(char *s
>  				   apicid);
>  			return 0;
>  		}
> -
> -		pr_warning("x2apic already enabled. will disable it\n");
> -	} else
> -		setup_clear_cpu_cap(X86_FEATURE_X2APIC);
> -
> -	nox2apic = true;
> +		pr_warning("x2apic already enabled.\n");
> +		disable_x2apic();
> +	}
> +	setup_clear_cpu_cap(X86_FEATURE_X2APIC);
>  	x2apic_state = X2APIC_DISABLED;
> +	x2apic_mode = 0;
>  	return 0;
>  }
>  early_param("nox2apic", setup_nox2apic);
>  
> -/*
> - * Need to disable xapic and x2apic at the same time and then enable xapic mode
> - */
> -static inline void __disable_x2apic(u64 msr)
> -{
> -	wrmsrl(MSR_IA32_APICBASE,
> -	       msr & ~(X2APIC_ENABLE | XAPIC_ENABLE));
> -	wrmsrl(MSR_IA32_APICBASE, msr & ~X2APIC_ENABLE);
> -}
> -
> -static __init void disable_x2apic(void)
> +static __init void x2apic_disable(void)

This is still misleading: we have x2apic_disable() and disable_x2apic().
What is what? Can we clarify them more and maybe prepend one of them
with "__" to show which is the lower level helper...

Or is the logic that all functions beginning with the "x2apic_" prefix
are called from outside and the "enable_/disable_x2apic" ones are the
internal helpers?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
--
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/