linux-kernel - Re: [PATCH v4 07/34] x86/cpuid: Introduce a centralized CPUID data model

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aKOE-FzHy0JatC8I@lx-t490>
Date: Mon, 18 Aug 2025 21:54:32 +0200
From: "Ahmed S. Darwish" <darwi@...utronix.de>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Borislav Petkov <bp@...en8.de>, Ingo Molnar <mingo@...hat.com>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Andrew Cooper <andrew.cooper3@...rix.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	David Woodhouse <dwmw2@...radead.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Sohil Mehta <sohil.mehta@...el.com>,
	John Ogness <john.ogness@...utronix.de>, x86@...nel.org,
	x86-cpuid@...ts.linux.dev, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 07/34] x86/cpuid: Introduce a centralized CPUID data
 model

Hi,

On Mon, 18 Aug 2025, Sean Christopherson wrote:
>
> Why not?  Like C structs, there can only be one variable sized array, i.e. there
> can't be multiple "n" subleafs.  If the concern is calling __cpuid_subleaf_n()
> with i < start, then I don't see how embedding start in the structure name helps
> in any way, since 'i' isn't a compile-time constant and so needs to be checked at
> runtime no matter what.
>

Hmmm...

I can indeed find the offset of the dynamic 'leaf_0x.._n' subleaf storage
by CPP tokenization, since the CPUID table will be in the form:

    struct leaf_0x0_0           leaf_0x0_0[1];
    struct leaf_0x1_0           leaf_0x1_0[1];
    struct leaf_0x2_0           leaf_0x2_0[1];
    struct leaf_0x4_0           leaf_0x4_0[8];
    struct leaf_0xd_0		leaf_0xd_0[1];
    struct leaf_0xd_1		leaf_0xd_1[1];
    struct leaf_0xd_n		leaf_0xd_n[62];
    struct leaf_0x16_0          leaf_0x16_0[1];
    struct leaf_0x80000000_0    leaf_0x80000000_0[1];
    struct leaf_0x80000005_0    leaf_0x80000005_0[1];
    struct leaf_0x80000006_0    leaf_0x80000006_0[1];
    struct leaf_0x8000001d_0    leaf_0x8000001d_0[8];

I was also kinda worried about the different subleaf semantics:

    struct leaf_0x4_n		=> starts from subleaf 0
    struct leaf_0xd_n		=> starts from subleaf 2

But, hopefully it would be clear when looking at the generated header in
total.

Still: for the API you're proposing, I'll need to generate an error for
things like:

    cpuid_subleaf_n(c, 0xd, 0);
    cpuid_subleaf_n(c, 0xd, 1);

which could not have happened in the API I proposed earlier.  But... I
can let the XML files generate some symbols in the form:

    LEAF_0x4_n_MIN_SUBLEAF	0
    LEAF_0xd_n_MIN_SUBLEAF	2

and generate an error (once) if the passed subleaf is less than the
minimum.  I can also generate that error (once) at compile-time if the
given subleaf was a compile-time constant.

Maybe there's a cleaner way for detecting this subleaf lower-bound error,
I will see; what's important now is the API implementation feasibility...

=> Then, we can have:

API Summary
~~~~~~~~~~~

For CPUID leaves with static subleaves:

    cpuid_leaf(_cpuinfo, _leaf)
    cpuid_subleaf(_cpuinfo, _leaf, _subleaf)

For CPUID leaves with dynamic subleaves:

    cpuid_subleaf_n(_cpuinfo, _leaf, _idx)

New SGX snippet
~~~~~~~~~~~~~~~

At arch/x86/kernel/cpu/sgx/main.c:

    const struct leaf_0x12_n *l;

    for (int i = 0; i < ARRAY_SIZE(sgx_epc_sections); i++) {

        l = cpuid_subleaf_n(0x12, i + SGX_CPUID_EPC);

	// Use l->subleaf_type, etc. as appropriate
    }

New KVM snippet
~~~~~~~~~~~~~~~

At arch/x86/kvm/cpuid.c:

    const struct leaf_0xd_n *l;

    for (int i = XFEATURE_YMM; i < ARRAY_SIZE(xstate_sizes); i++) {

	l = cpuid_subleaf_n(c, 0xd, i);

        // Use l->xsave_sz, l->xsave_offset, etc. as appropriate
    }

This looks much better indeed; for both KVM and SGX :-)

Thanks a lot!

--
Ahmed S. Darwish
Linutronix GmbH