Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <1ea129d9-19cc-5f33-fc10-3e832679c5a2@intel.com>
Date:   Thu, 13 Apr 2023 10:57:08 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH v2 2/2] x86/Documentation: Add documentation about cluster
Content-Language: en-US
To:     K Prateek Nayak <kprateek.nayak@amd.com>,
        linux-kernel@vger.kernel.org
Cc:     tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
        dave.hansen@linux.intel.com, hpa@zytor.com, corbet@lwn.net,
        jgross@suse.com, andrew.cooper3@citrix.com, peterz@infradead.org,
        Jason@zx2c4.com, thomas.lendacky@amd.com, puwen@hygon.cn,
        x86@kernel.org, linux-doc@vger.kernel.org,
        oleksandr@natalenko.name, bagasdotme@gmail.com
References: <20230413172918.1500-1-kprateek.nayak@amd.com>
 <20230413172918.1500-3-kprateek.nayak@amd.com>
From:   Dave Hansen <dave.hansen@intel.com>
In-Reply-To: <20230413172918.1500-3-kprateek.nayak@amd.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 4/13/23 10:29, K Prateek Nayak wrote:
> +  - cluster_id:
> +
> +    A per-CPU variable containing:
> +
> +      - On Intel, the common upper bits of APIC ID of the list of CPUs sharing
> +        the L2 Cache with lower bits set to 0.
> +
> +      - On AMD and Hygon, with Topology Extension, the common upper bits of the
> +        Extended APIC ID of the list of CPUs sharing the L2 Cache, left shifted
> +        to remove trailing 0s.

I think this is too much detail for Documentation.  We have the code if
anyone cares _this_ much.

Also, I'm perplexed by the "left shifted" comment.  I don't see a lot of
left shifting in the patch.  Am I just missing it?

Further, this makes it sound like all Intel CPUs have the cluster_id
populated.  I'm also not sure that folks reading this will have any
worldly idea what "Topology Extension" is.

Why don't we just say that some CPUs don't have this info?  That way we
don't need to spell out AMD vs. Intel or expect our users to go figuring
out of their CPU has "Topology Extension" or leaf 3 or wherever this
info is on Intel.

How about:

A per-CPU variable containing:

   - Some upper bits extracted from the APIC ID.  CPUs which have the
     same value in these bits share an L2 and have the same cluster_id.

     CPUs for which L2 cache information is unavailable will show 65535
     as the cluster_id.