Date: Tue, 6 Jun 2017 12:15:30 +0200
From: Peter Zijlstra <peterz@infradead.org>
To: Janakarajan Natarajan <Janakarajan.Natarajan@amd.com>
Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Borislav Petkov <bp@alien8.de>,
        Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Subject: Re: [PATCH 2/2] amd: uncore: Get correct number of cores sharing
 last level cache
Message-ID: <20170606101530.ehgizupxz4hy7ccv@hirez.programming.kicks-ass.net>
References: <cover.1496418338.git.Janakarajan.Natarajan@amd.com>
 <42ddf980f25ffd0ba585f3cf3a7ca9ade4fc7bee.1496418338.git.Janakarajan.Natarajan@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <42ddf980f25ffd0ba585f3cf3a7ca9ade4fc7bee.1496418338.git.Janakarajan.Natarajan@amd.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 912
Lines: 33

On Mon, Jun 05, 2017 at 11:13:19AM -0500, Janakarajan Natarajan wrote:
> +		/*
> +		 * Iterate over Cache Topology Definition leaves until no
> +		 * more cache descriptions are available
> +		 */
> +		while (1) {
> +			cpuid_count(0x8000001d, cache_level,
> +				    &eax, &ebx, &ecx, &edx);
> +			/*
> +			 * EAX[0:4] gives type of cache. 0 = No more caches
> +			 */
> +			if ((eax & 0x1f) == 0)
> +				break;
> +			cache_level++;
> +			prev_eax = eax;
> +		}
> +		nshared = ((prev_eax >> 14) & 0xfff) + 1;

Egads, could we pretty please write that in a less horrible fashion?

Maybe something like:

	for (cache_level = 0; cache_level < 3; cache_level++) {
		cpuid_count(0x8000001d, cache_level, &eax, &ebx, &ecx, &edx);

		if ((eax & 0x1f) == 0) /* EAX[0:4] gives cache type */
			break;

		prev_eax = eax;
	}

That way we'll not run off into the woods if CPUID goes funny (never
trust a BIOS/virt monkey).