linux-kernel - Re: [PATCH v2 -tip] x86/percpu: Use C for arch_raw_cpu

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: <3F9D776E-AD7E-4814-9E3C-508550AD9287@vmware.com>
Date:   Wed, 18 Oct 2023 14:46:23 +0000
From:   Nadav Amit <namit@...are.com>
To:     Uros Bizjak <ubizjak@...il.com>
CC:     Linus Torvalds <torvalds@...ux-foundation.org>,
        the arch/x86 maintainers <x86@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Andy Lutomirski <luto@...nel.org>,
        Brian Gerst <brgerst@...il.com>,
        Denys Vlasenko <dvlasenk@...hat.com>,
        "H . Peter Anvin" <hpa@...or.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Nick Desaulniers <ndesaulniers@...gle.com>
Subject: Re: [PATCH v2 -tip] x86/percpu: Use C for arch_raw_cpu_ptr()


> On Oct 18, 2023, at 4:15 PM, Uros Bizjak <ubizjak@...il.com> wrote:
> 
>> 
>> Looks like another case of underspecified functionality where both
>> compilers differ. Luckily, both DTRT when aliases are hidden in
>> another TU.
> 
> Attached is the prototype patch that works for me (together with
> Linus' FPU switching patch).

In general looks good. See some minor issues below.

> --- a/arch/x86/include/asm/current.h
> +++ b/arch/x86/include/asm/current.h
> @@ -36,10 +36,23 @@ static_assert(sizeof(struct pcpu_hot) == 64);
>  
>  DECLARE_PER_CPU_ALIGNED(struct pcpu_hot, pcpu_hot);
>  
> +/*
> + *
> + */

Obviously some further comments to clarify why struct pcpu_hot is
defined in percpu-hot.c (the GCC manual says:  "It is an error if
the alias target is not defined in the same translation unit as the
alias” which can be used as part of the explanation.)

> +DECLARE_PER_CPU_ALIGNED(const struct pcpu_hot __percpu_seg_override,
> +			const_pcpu_hot);
> +
> +#ifdef CONFIG_USE_X86_SEG_SUPPORT
> +static __always_inline struct task_struct *get_current(void)
> +{
> +	return const_pcpu_hot.current_task;
> +}
> +#else
>  static __always_inline struct task_struct *get_current(void)
>  {
>  	return this_cpu_read_stable(pcpu_hot.current_task);
>  }
> +#endif


Please consider using IS_ENABLED() to avoid the ifdef’ry.

So this would turn to be:

static __always_inline struct task_struct *get_current(void)
{
	if (IS_ENABLED(CONFIG_USE_X86_SEG_SUPPORT))
		return const_pcpu_hot.current_task;

	return this_cpu_read_stable(pcpu_hot.current_task);
}