Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Feedback-ID: i0ad843c9:Fastmail
Message-ID: <a0a95390-ac9e-4d82-9727-d4880e5e48ae@sholland.org>
Date:   Wed, 22 Nov 2023 19:43:09 -0600
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [RFC PATCH v2 07/10] perf: RISC-V: Move T-Head PMU to CPU feature
 alternative framework
Content-Language: en-US
To:     Conor Dooley <conor@kernel.org>,
        Yu-Chien Peter Lin <peterlin@andestech.com>
Cc:     mark.rutland@arm.com, heiko@sntech.de, atishp@atishpatra.org,
        conor.dooley@microchip.com, guoren@kernel.org, jszhang@kernel.org,
        linux-riscv@lists.infradead.org, will@kernel.org,
        anup@brainfault.org, dminus@andestech.com, dylan@andestech.com,
        ajones@ventanamicro.com, aou@eecs.berkeley.edu,
        prabhakar.mahadev-lad.rj@bp.renesas.com, locus84@andestech.com,
        tim609@andestech.com, paul.walmsley@sifive.com,
        linux-arm-kernel@lists.infradead.org, rdunlap@infradead.org,
        linux-kernel@vger.kernel.org, evan@rivosinc.com,
        palmer@dabbelt.com, Jernej Skrabec <jernej.skrabec@gmail.com>
References: <20231019140119.3659651-1-peterlin@andestech.com>
 <20231019-predator-quartet-e56f43d5aa8d@spud> <ZTJAYqk_DnrR9-Sc@APC323>
 <20231020-snippet-diffusive-1a6052d52aae@spud> <ZTTmtVnZrioRWpJx@APC323>
 <20231023-impulse-quickness-4c7076e6eb58@spud>
From:   Samuel Holland <samuel@sholland.org>
In-Reply-To: <20231023-impulse-quickness-4c7076e6eb58@spud>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 10/23/23 03:26, Conor Dooley wrote:
> On Sun, Oct 22, 2023 at 05:09:09PM +0800, Yu-Chien Peter Lin wrote:
>> On Fri, Oct 20, 2023 at 10:05:20AM +0100, Conor Dooley wrote:
>>> On Fri, Oct 20, 2023 at 04:54:58PM +0800, Yu-Chien Peter Lin wrote:
>>>> On Thu, Oct 19, 2023 at 05:13:00PM +0100, Conor Dooley wrote:
>>>>> On Thu, Oct 19, 2023 at 10:01:19PM +0800, Yu Chien Peter Lin wrote:
>>>>>
>>>>> $subject: perf: RISC-V: Move T-Head PMU to CPU feature alternative framework
>>>>>
>>>>> IMO, this should be "RISC-V, perf:" or just "RISC-V" as the changes
>>>>> being made to the arch code are far more meaningful than those
>>>>> elsewhere.
>>>>
>>>> OK will update the subject to "RISC-V:"
>>>>
>>>>>> The custom PMU extension was developed to support perf event sampling
>>>>>> prior to the ratification of Sscofpmf. Instead of utilizing the standard
>>>>>> bits and CSR of Sscofpmf, a set of custom CSRs is added. So we may
>>>>>> consider it as a CPU feature rather than an erratum.
>>>>>>
>>>>>> T-Head cores need to append "xtheadpmu" to the riscv,isa-extensions
>>>>>> for each cpu node in device tree, and enable CONFIG_THEAD_CUSTOM_PMU
>>>>>> for proper functioning as of this commit.
>>>>>
>>>>> And in doing so, you regress break perf for existing DTs :(
>>>>> You didn't add the property to existing DTS in-kernel either, so if this
>>>>> series was applied, perf would just entirely stop working, no?
>>>>
>>>> Only `perf record/top` stop working I think.
>>>>
>>>> There are too many users out there, and don't have the boards to
>>>> test, so leave those DTS unchanged, it would be great if T-Head
>>>> community could help to check/update their DTS.
>>>
>>> So, there are too many users to add xtheadpmu to the devicetrees, but
>>> not too many users to make changes that will cause a regression?
>>> I'm not following the logic here, sorry.
>>
>> humm, I'll try. I assume that the sun20i-d1s.dtsi is all I need
>> to update for T-Head PMU.
> 
> I think you can actually add it to all users of T-Head CPUs currently in
> mainline since all those cpus report the 0 mimpid and 0 marchid that is
> being used as the detection method in the current code.
> 
> That said, changing the in-kernel devicetrees doesn't solve the
> regression problem. Not every dts lives in the linux codebase, for
> example, and just because they don't, doesn't mean we can just not
> care about them!
> 
> As a result, I don't think that we can just do a conversion here from
> one method to another like this, since it's likely to break things for
> people. Certainly interested in hearing from those that support the
> T-Head IP based SoCs about whether they'd be okay with something like
> this.

PMU support is not required to boot, and it didn't really work correctly anyway
until OpenSBI commit c9a296d0edc9 ("platform: generic: allwinner: fix OF process
for T-HEAD c9xx pmu"), which is still not in any released OpenSBI version.

So I am fine with requiring a devicetree update for continued PMU support.

Regards,
Samuel