linux-kernel - Re: [PATCH v4 1/6] x86/cpu: Use a new feature flag for 5 level paging

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: <20250521181141.GDaC4XXW8BmtvJFy6a@fat_crate.local>
Date: Wed, 21 May 2025 20:11:41 +0200
From: Borislav Petkov <bp@...en8.de>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Ingo Molnar <mingo@...nel.org>,
	"Ahmed S. Darwish" <darwi@...utronix.de>,
	Ard Biesheuvel <ardb+git@...gle.com>, linux-kernel@...r.kernel.org,
	x86@...nel.org, Ard Biesheuvel <ardb@...nel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Brian Gerst <brgerst@...il.com>,
	"Kirill A. Shutemov" <kirill@...temov.name>
Subject: Re: [PATCH v4 1/6] x86/cpu: Use a new feature flag for 5 level paging

On Wed, May 21, 2025 at 05:23:37PM +0200, Thomas Gleixner wrote:
> Now what about software defined (artificial) feature bits including BUG
> bits?
> 
> We still need them and there is no reason why we would replace them with
> something else. But, what we want to do here, is basically the same as
> we do for the real CPUID information:
> 
>    Create and document real artifical leafs (there is enough reserved
>    number space in the CPUID numbering scheme) and put those into the
>    CPUID database as well.

I presume here, when the kernel patch is being sent, the accompanying CPUID db
patch needs to go out too?

>    That allows to handle this stuff in the same way as any other CPUID
>    data and the autogeneration of bit offsets and text information for
>    cpuinfo just works the same way.
> 
> Coming back to the original question with the example of LA57 and the
> actual enablement. There is no reason why we can't have the actual CPUID
> bit and a software defined bit.
> 
> The way how this should work is:
> 
>     1) The CPUID info is in data.leaf_07.la57
> 
>     2) The enablement bit is in data.leaf_linux_N.la57 or such
> 
> The CPUID database contains the entries for those in leaf_07:
> 
>       <bit16 len="1"  id="la57"                    desc="57-bit linear addresses (five-level paging)">
>         <vendors>
>           <vendor>Intel</vendor>
>           <vendor>AMD</vendor>
>         </vendors>
>         <linux        feature="true"               proc="false" />
>       </bit16>
> 
> and leaf_linux_N:
> 
>       <bit3 len="1"  id="la57"                     desc="57-bit linear addresses (five-level paging)">
>         <vendors>
>           <vendor>Linux</vendor>
>         </vendors>
>         <linux        feature="true"               proc="true" />
>       </bit3>
> 
> As the "proc" property of leaf_07.la57 is false, the bit won't be
> exposed in cpuinfo, but the software defined bit will.
> 
> This also means, that we switch to a model where the software defined
> bits are not longer subject to random introduction and removal. We
> simply keep them around, mark them as not longer used and introduce new
> ones with proper documentation. That requires due process, which
> prevents the adhoc messing around with feature bits, which has us bitten
> more than once in the past.

Right, so in this particular example with la57, the CPUID bit which denotes
that the hw is capable of doing 5-level paging is needed only during kernel
init so that we can know whether we should even try to setup 5-level paging.

After that, the rest of the kernel will need to look only at "our" bit which
means, 5-level is *enabled*.

Because that's what the code cares for - whether it is running on 5-level or
not.

And 5-level *enabled* implies 5-level possible. So that first bit is kinda
redundant and perhaps even confusing. That's why I think merging the two bits
simplifies things.

You're already basically doing that with proc="false" but it should be even
less visible. No one besides us cares if the hw is capable - users care if the
feature is enabled or not.

I'd say...

Thx.

-- 
Regards/Gruss,
    Boris.

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