public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: "Brown, Len" <len.brown@intel.com>,
	"Zhang, Rui" <rui.zhang@intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: "Gross, Jurgen" <jgross@suse.com>,
	"mikelley@microsoft.com" <mikelley@microsoft.com>,
	"arjan@linux.intel.com" <arjan@linux.intel.com>,
	"x86@kernel.org" <x86@kernel.org>,
	"thomas.lendacky@amd.com" <thomas.lendacky@amd.com>,
	"ray.huang@amd.com" <ray.huang@amd.com>,
	"andrew.cooper3@citrix.com" <andrew.cooper3@citrix.com>,
	"Sivanich, Dimitri" <dimitri.sivanich@hpe.com>,
	"wei.liu@kernel.org" <wei.liu@kernel.org>
Subject: RE: [patch V3 27/40] x86/cpu: Provide a sane leaf 0xb/0x1f parser
Date: Mon, 21 Aug 2023 12:27:28 +0200	[thread overview]
Message-ID: <874jksher3.ffs@tglx> (raw)
In-Reply-To: <MN0PR11MB6010D09FF7C20779B70A3B83E01BA@MN0PR11MB6010.namprd11.prod.outlook.com>

Len!

On Fri, Aug 18 2023 at 05:01, Len Brown wrote:
> Discussing further w/ Rui, we realized that is it not possible to
> future-proof the CPUID.1F code against the insertion of a new Domain
> at an unknown level in the hierarchy.  Further, CPUID.1F explicitly
> documents that we can not count on any numeric relationship between
> the Domain Type IDs to tell us between what levels of the hierarchy a
> future Domain may reside.

I realized that long before and explained it to Rui already quite some
time ago, no?

> For this reason, I recommend that we proceed with implementing
> internal kernel parsing of Domain Type ID 6 (DieGrp).  Even if DieGrp
> is a temporary name that changes to something else, we can assert that
> Domain Type ID 6 contains Die.  Thus, on a system with both DieGrp and
> Die, we know that multiple DieGrp produce multiple Die, rather than
> the reverse.
>
> If I understand your description correctly, you propose that if a
> level is not enumerated, that we assume its id is always 0.  I agree
> this is the right thing to do with Package, and we should continue
> doing it, because we always have a package.
>
> However, I don't think this is the right direction for the
> intermediate levels -- for their existence should be able to be used
> to enumerate the existence of level-specific features, and 0 suggests
> that those features exist when they may not.
>
> Take for example, module-scope MSRs and counters that do not exist on
> systems that do not have modules.
>
> We should only have a valid module_id if CPUID.1F has a non-zero shift
> width for Domain Type ID 3 (module).  Otherwise, we should probably be
> setting module_id to -1, so that code looking for those module
> features knows that they don't exist.  If we don't do this, that code
> will have to check model # (as it historically has) to determine if
> module-only features exist -- and we've thrown away the value of
> architectural level enumeration.
>
> The situation for die_id is similar, except that there are no die
> features that don't already exist as package features when there is
> just 1 die/package.  Ie. The die-aware code can simply revert to the
> package code when die_id= -1.
>
> This, of course, raises the question of the sysfs interface for
> die_id, and whether it should return -1 when there is no Die
> enumerated, or make that attribute simply not exist when there is no
> die_id.  Either would probably be an improvement over conjuring up a
> phony die_id=0 when no Die is actually enumerated.

You are still conflating things at different levels and the above is a
mixture of implementation details, historical anecdotes, user space
interface issues and whatever.

But you are still failing to look at it from a ground up conceptual
level.

1.) Parsing and storing enumeration

    The way how 0x1F is designed and how the APIC ID space is
    partitioned, makes it entirely clear that all domain levels must be
    evaluated no matter what and it's more than obvious that
    non-enumerated domain levels occupy _zero_ bits in the APIC ID
    partitioning and therefore end up being size _one_.

2.) IDs

    As a logical consequence of #1 each level - whether enumerated or
    not - has an ID. Obviously non-enumerated levels share the ID with
    the levels up the hierarchy.

    Overloading ID with some "it's not enumerated" value is conceptually
    completely wrong.

3.) Evaluation

    Runtime evaluation of the stored information can and in most cases
    has to take the (non-)enumeration into account. But that's a
    conceptually separate issue from #1 and #2.

    #1 is providing a trivial mechanism for that as it's obvious that a
    domain level is only enumerated when it occupies more than 0 bits.

4.) User space exposure

    That's an orthogonal problem, which obviously has to be addressed,
    but #1 - #3 provide _all_ required mechanisms to preserve the
    existing (bogus) semantics and to provide a new set of sensible
    interfaces which are more than just a new name and paint for the
    already existing bogosities.

The whole problem of the existing mechanims and the resulting nonsense
at the usage sites of topology information is that there is no concept
behind any of this. It's a mess of ad-hoc mechanisms which have been
glued on over time, which completely lacks consistency.

As a consequence the kernel has grown warts at all ends to deal with
this and the two series I'm working on are laying the groundwork to
clean this up on top of a consistent and understandable topology
mechanism, which also allows to grow into a central storage of such
information instead of having home brewn solutions in every other
subsystem.

I'm happy to listen to your ideas and thoughts as long as we are
debating things at the proper conceptual design levels. But I'm not
interested at all in random recommendations which just address the
symptoms and therefore are proliferating the lack of consistent design.

Thanks,

        tglx



  reply	other threads:[~2023-08-21 10:27 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-02 10:20 [patch V3 00/40] x86/cpu: Rework the topology evaluation Thomas Gleixner
2023-08-02 10:20 ` [patch V3 01/40] cpu/SMT: Make SMT control more robust against enumeration failures Thomas Gleixner
2023-08-04 17:50   ` Borislav Petkov
2023-08-04 20:01     ` Thomas Gleixner
2023-08-02 10:21 ` [patch V3 02/40] x86/apic: Fake primary thread mask for XEN/PV Thomas Gleixner
2023-08-04 18:12   ` Borislav Petkov
2023-08-04 20:02     ` Thomas Gleixner
2023-08-02 10:21 ` [patch V3 03/40] x86/cpu: Encapsulate topology information in cpuinfo_x86 Thomas Gleixner
2023-08-02 10:21 ` [patch V3 04/40] x86/cpu: Move phys_proc_id into topology info Thomas Gleixner
2023-08-02 10:21 ` [patch V3 05/40] x86/cpu: Move cpu_die_id " Thomas Gleixner
2023-08-09 14:32   ` Zhang, Rui
2023-08-09 15:14     ` Thomas Gleixner
2023-08-02 10:21 ` [patch V3 06/40] scsi: lpfc: Use topology_core_id() Thomas Gleixner
2023-08-02 10:21 ` [patch V3 07/40] hwmon: (fam15h_power) " Thomas Gleixner
2023-08-02 10:21 ` [patch V3 08/40] x86/cpu: Move cpu_core_id into topology info Thomas Gleixner
2023-08-02 10:21 ` [patch V3 09/40] x86/cpu: Move cu_id " Thomas Gleixner
2023-08-02 10:21 ` [patch V3 10/40] x86/cpu: Remove pointless evaluation of x86_coreid_bits Thomas Gleixner
2023-08-02 10:21 ` [patch V3 11/40] x86/cpu: Move logical package and die IDs into topology info Thomas Gleixner
2023-08-02 10:21 ` [patch V3 12/40] x86/cpu: Move cpu_l[l2]c_id " Thomas Gleixner
2023-08-02 10:21 ` [patch V3 13/40] x86/apic: Use BAD_APICID consistently Thomas Gleixner
2023-08-02 10:21 ` [patch V3 14/40] x86/apic: Use u32 for APIC IDs in global data Thomas Gleixner
2023-08-02 10:21 ` [patch V3 15/40] x86/apic: Use u32 for check_apicid_used() Thomas Gleixner
2023-08-02 10:21 ` [patch V3 16/40] x86/apic: Use u32 for cpu_present_to_apicid() Thomas Gleixner
2023-08-02 10:21 ` [patch V3 17/40] x86/apic: Use u32 for phys_pkg_id() Thomas Gleixner
2023-08-02 10:21 ` [patch V3 18/40] x86/apic: Use u32 for [gs]et_apic_id() Thomas Gleixner
2023-08-02 10:21 ` [patch V3 19/40] x86/apic: Use u32 for wakeup_secondary_cpu[_64]() Thomas Gleixner
2023-08-10  7:58   ` Qiuxu Zhuo
2023-08-02 10:21 ` [patch V3 20/40] x86/cpu/topology: Cure the abuse of cpuinfo for persisting logical ids Thomas Gleixner
2023-08-02 10:21 ` [patch V3 21/40] x86/cpu: Provide debug interface Thomas Gleixner
2023-08-02 10:21 ` [patch V3 22/40] x86/cpu: Provide cpuid_read() et al Thomas Gleixner
2023-08-02 10:21 ` [patch V3 23/40] x86/cpu: Provide cpu_init/parse_topology() Thomas Gleixner
2023-08-04  8:14   ` K Prateek Nayak
2023-08-04  8:28     ` Thomas Gleixner
2023-08-04  8:34       ` K Prateek Nayak
2023-08-12  6:41   ` Zhang, Rui
2023-08-12  8:00     ` Zhang, Rui
2023-08-14  6:26       ` Thomas Gleixner
2023-08-14  7:11         ` Zhang, Rui
2023-08-13 13:30     ` Zhang, Rui
2023-08-13 14:36       ` Thomas Gleixner
2023-08-14  6:20         ` Thomas Gleixner
2023-08-14  6:42           ` Zhang, Rui
2023-08-02 10:21 ` [patch V3 24/40] x86/cpu: Add legacy topology parser Thomas Gleixner
2023-08-02 10:21 ` [patch V3 25/40] x86/cpu: Use common topology code for Centaur and Zhaoxin Thomas Gleixner
2023-08-02 10:21 ` [patch V3 26/40] x86/cpu: Move __max_die_per_package to common.c Thomas Gleixner
2023-08-02 10:21 ` [patch V3 27/40] x86/cpu: Provide a sane leaf 0xb/0x1f parser Thomas Gleixner
2023-08-12  8:21   ` Zhang, Rui
2023-08-12 20:04     ` Thomas Gleixner
2023-08-13 15:04       ` Thomas Gleixner
2023-08-14  8:25         ` Zhang, Rui
2023-08-14 12:26           ` Thomas Gleixner
2023-08-14 14:48             ` Brown, Len
2023-08-14 16:13               ` Thomas Gleixner
2023-08-15 19:30                 ` Brown, Len
2023-08-17  9:09                   ` Thomas Gleixner
2023-08-18  5:01                     ` Brown, Len
2023-08-21 10:27                       ` Thomas Gleixner [this message]
2023-08-30  2:46                         ` Brown, Len
2023-08-30 12:39                           ` Thomas Gleixner
2023-09-01  3:09                             ` Brown, Len
2023-09-01  7:45                               ` Thomas Gleixner
2023-08-14 15:28             ` Zhang, Rui
2023-08-14 17:12               ` Thomas Gleixner
2023-08-02 10:21 ` [patch V3 28/40] x86/cpu: Use common topology code for Intel Thomas Gleixner
2023-08-02 10:21 ` [patch V3 29/40] x86/cpu/amd: Provide a separate accessor for Node ID Thomas Gleixner
2023-08-02 10:21 ` [patch V3 30/40] x86/cpu: Provide an AMD/HYGON specific topology parser Thomas Gleixner
2023-08-02 19:28   ` Michael Kelley (LINUX)
2023-08-02 19:46     ` Thomas Gleixner
2023-08-02 19:51   ` [patch V3a " Thomas Gleixner
2023-08-11 12:58     ` Pu Wen
2023-08-11 17:11       ` Thomas Gleixner
2023-08-12  3:58         ` Pu Wen
2023-10-13  9:38           ` [tip: x86/core] x86/cpu/hygon: Fix the CPU topology evaluation for real tip-bot2 for Pu Wen
2023-08-02 10:21 ` [patch V3 31/40] x86/smpboot: Teach it about topo.amd_node_id Thomas Gleixner
2023-08-02 10:21 ` [patch V3 32/40] x86/cpu: Use common topology code for AMD Thomas Gleixner
2023-08-02 10:21 ` [patch V3 33/40] x86/cpu: Use common topology code for HYGON Thomas Gleixner
2023-08-11 13:00   ` Pu Wen
2023-08-11 17:11     ` Thomas Gleixner
2023-08-02 10:21 ` [patch V3 34/40] x86/mm/numa: Use core domain size on AMD Thomas Gleixner
2023-08-02 10:21 ` [patch V3 35/40] x86/cpu: Make topology_amd_node_id() use the actual node info Thomas Gleixner
2023-08-02 10:21 ` [patch V3 36/40] x86/cpu: Remove topology.c Thomas Gleixner
2023-08-02 10:21 ` [patch V3 37/40] x86/cpu: Remove x86_coreid_bits Thomas Gleixner
2023-08-02 10:21 ` [patch V3 38/40] x86/apic: Remove unused phys_pkg_id() callback Thomas Gleixner
2023-08-02 10:21 ` [patch V3 39/40] x86/xen/smp_pv: Remove cpudata fiddling Thomas Gleixner
2023-08-02 10:22 ` [patch V3 40/40] x86/apic/uv: Remove the private leaf 0xb parser Thomas Gleixner
2023-08-02 11:56 ` [patch V3 00/40] x86/cpu: Rework the topology evaluation Juergen Gross
2023-08-03  0:07 ` Sohil Mehta
2023-08-03  3:44 ` Michael Kelley (LINUX)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=874jksher3.ffs@tglx \
    --to=tglx@linutronix.de \
    --cc=andrew.cooper3@citrix.com \
    --cc=arjan@linux.intel.com \
    --cc=dimitri.sivanich@hpe.com \
    --cc=jgross@suse.com \
    --cc=len.brown@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mikelley@microsoft.com \
    --cc=ray.huang@amd.com \
    --cc=rui.zhang@intel.com \
    --cc=thomas.lendacky@amd.com \
    --cc=wei.liu@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox