Re: [PATCH] perf: RISC-V: fix IRQ detection on T-Head C908

From: Inochi Amaoto <inochiama@outlook.com>
To: Conor Dooley <conor@kernel.org>, Qingfang Deng <dqfext@gmail.com>
Cc: Inochi Amaoto <inochiama@outlook.com>,
	 Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	 Albert Ou <aou@eecs.berkeley.edu>,
	Atish Patra <atishp@atishpatra.org>,
	 Anup Patel <anup@brainfault.org>, Will Deacon <will@kernel.org>,
	 Mark Rutland <mark.rutland@arm.com>,
	Andrew Jones <ajones@ventanamicro.com>,
	 Conor Dooley <conor.dooley@microchip.com>,
	Heiko Stuebner <heiko@sntech.de>, Guo Ren <guoren@kernel.org>,
	 linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
	 linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] perf: RISC-V: fix IRQ detection on T-Head C908
Date: Wed, 13 Mar 2024 09:31:26 +0800	[thread overview]
Message-ID: <IA1PR20MB4953CE8999960BA71B46DE6CBB2A2@IA1PR20MB4953.namprd20.prod.outlook.com> (raw)
In-Reply-To: <20240312-evil-resource-66370b68b9b4@spud>

On Tue, Mar 12, 2024 at 02:07:31PM +0000, Conor Dooley wrote:
> On Mon, Mar 11, 2024 at 03:56:29PM +0800, Qingfang Deng wrote:
> > Hi Inochi,
> > 
> > On Mon, Mar 11, 2024 at 3:13 PM Inochi Amaoto <inochiama@outlook.com> wrote:
> > >
> > > On Mon, Mar 11, 2024 at 02:30:18PM +0800, Qingfang Deng wrote:
> > > > T-Head C908 has the same IRQ num and CSR as previous C9xx cores, but
> > > > reports non-zero marchid and mimpid. Remove the ID checks.
> > > >
> > >
> > > Hi, Qingfang,
> > >
> > > IIRC, the existed C908 SoC (such as K230) have an early version
> > > of C908 core. But C908 core itself may support Sscofpmf.
> > > So I do not think removing the ID checks is a good idea. Instead,
> > > I suggest adding CPUID of your SoC to this check.
> > 
> > As of Feb 2024, the latest C908 revision does not support Sscofpmf.
> > You may Google "C908R1S0" to see its user manual.
> > But I think you're right. Even though C908 does not have Sscofpmf,
> > T-Head may release new SoCs which do have Sscofpmf, and the check will
> > break. I will submit a new patch with your suggested changes.
> 
> If on an SoC where they have updated vector to 1.0 and implemented both
> Zicbom and Svpbmt instead of their custom stuff they did not implement
> Sscofpmf I think we can expect they won't move away from their custom
> implementation soon.
> I do agree that we should not remove the ID checks entirely, but I also
> do not want to be adding an ID for every SoC that needs this. I think we
> should be getting this information from DT going forward.
> The DT parsing is done prior to the application of boot time
> alternatives, so I think we could apply the "erratum" based on the DT.
> 
> I'm also pretty sure that we can also modify the existing code for the
> archid == impid == 0x0 case to set a pseudo isa extension so that the
> perf driver could do call riscv_isa_eextension_available() and not worry
> about the specfic conditions in which that is true. It'd be something
> like this patch:
> https://lore.kernel.org/linux-riscv/20240110073917.2398826-8-peterlin@andestech.com/
> Just without removing the archid == impid == 0x0 case from the errata
> code. If you're lost after reading that, I can probably throw together
> some untested code for it.
> 
> Thanks,
> Conor.

I agree to use something to replace the existing check, but using a pseudo
isa extension is not a good idea. There are two reasons: 

1. Pseudo isa is misleading. As it is not the real isa, setting this in isa
list may make userspace think errata a feather.
2. Using pseudo isa is more like an abuse of reserved isa bits, which means
kernel may need infinite bits to handle the errata. 

IMHO, it may be better to use a new DT property like "riscv,cpu-errata" or
"<vendor>,cpu-errata". It can achieve almost everything like using pseudo
isa. And the only cost I think is a small amount code to parse this.

Regards,
Inochi.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv