linux-kernel - Re: [PATCH v5 2/7] x86/mm: Use a single cache hot per-CPU variable to record pgdir

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: <20250520180101.GPaCzDXW2MlArU71xe@fat_crate.local>
Date: Tue, 20 May 2025 20:01:01 +0200
From: Borislav Petkov <bp@...en8.de>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: "Kirill A. Shutemov" <kirill@...temov.name>,
	Ard Biesheuvel <ardb+git@...gle.com>, linux-kernel@...r.kernel.org,
	x86@...nel.org, Ingo Molnar <mingo@...nel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Brian Gerst <brgerst@...il.com>
Subject: Re: [PATCH v5 2/7] x86/mm: Use a single cache hot per-CPU variable
 to record pgdir_shift

On Tue, May 20, 2025 at 07:46:33PM +0200, Ard Biesheuvel wrote:
> Look at pgtable_l5_enabled() please, that is the important one.

OMG. :-)

# 32 "./arch/x86/include/asm/pgtable_64_types.h" 1
        movb %gs:__pgdir_shift(%rip), %al       #, pfo_val__
# 0 "" 2
# ./arch/x86/include/asm/pgtable.h:1178:        if (!pgtable_l5_enabled())
#NO_APP
        testb   $1, %al #, pfo_val__

I hadn't seen his fun yet:

!(pgdir_shift() & 1)

> The variable access is identical in terms of instructions, the only
> difference is the %gs offset being applied, and the fact that using
> cache hot data is guaranteed not to increase the number of cachelines
> covering the working set of any existing workload (the region is
> bounded to a fixed number of cachelines)

Yes, but look at all the callers. I hardly see any serious hot paths to care
about the %gs offset being applied.

And I'll be really surprised if that *shows* in any sensible benchmark...

Also, it doesn't make any sense for a *global* system property - what paging
level the machine uses - to be in a *per-CPU* variable. That's a global value
which is the same everywhere. So why waste space?

> Happy to keep this as a simple __ro_after_init variable if there is
> consensus between the tip maintainers that we don't need this perf

Yeah, IMO, no need. But let's see what the others say.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette