Re: [RFC PATCH 2/4] cpufreq: acpi-cpufreq: Add processor to the ignore PSD override list

From: Punit Agrawal <punitagrawal@gmail.com>
To: Borislav Petkov <bp@alien8.de>
Cc: Wei Huang <whuang2@amd.com>,
	rjw@rjwysocki.net, wei.huang2@amd.com,
	linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
	x86@kernel.org
Subject: Re: [RFC PATCH 2/4] cpufreq: acpi-cpufreq: Add processor to the ignore PSD override list
Date: Thu, 17 Dec 2020 22:27:14 +0900	[thread overview]
Message-ID: <87ft44tvf1.fsf@stealth> (raw)
In-Reply-To: <20201214142540.GB25916@zn.tnic> (Borislav Petkov's message of "Mon, 14 Dec 2020 15:25:40 +0100")

Borislav Petkov <bp@alien8.de> writes:

> On Mon, Dec 14, 2020 at 10:27:09PM +0900, Punit Agrawal wrote:
>> IIUC, this suggests that Linux booting on anything prior to Zen3 is down
>> to pure luck - I hope that wasn't the case.
>
> WTF does this have to do with linux booting?!

I guess I misunderstood the comment from your previous mail. Pasting
back for context (emphasis mine) -

    VS the clear statement from AMD that from zen3 onwards, all BIOS
    will be tested. *I hope they boot Linux at least before they ship.*

>> At the moment acpi thermals is bust on this and other affected AMD
>> system I have access to. That'll need fixing before any sensible
>> measurements can be run.
>
> Nope, still not answering my questions.
>
>> Tbh, I didn't quite expect the patch to the PSD exclusion list to be
>> so controversial
>
> It won't be if you explain properly what your patch is fixing. That is,
> if it fixes anything.

I stared at the driver code (and the ACPI tables for the platform) to
see if I could provide a better explanation.

That's when I realised that as the platform advertises hardware
frequency co-ordination, even without the override it still skips
setting the policy cpus -

    /*
     * Will let policy->cpus know about dependency only when software
     * coordination is required.
     */
    if (policy->shared_type == CPUFREQ_SHARED_TYPE_ALL ||
        policy->shared_type == CPUFREQ_SHARED_TYPE_ANY) {
            cpumask_copy(policy->cpus, perf->shared_cpu_map);
    }

This ends up treating each CPU as an independent frequency domain
anyways. So even ignoring the override for the CPU, doesn't change
anything other than dropping the message from boot log -

    overriding BIOS provided _PSD data

As such, there's no point in merging the patch as it is.

Apologies for the noise. I should've checked more thoroughly before
sending the patches. 

[ Aside: If Zen3 is using hardware co-ordination it'll probably face the
issue described above as well. ]