Messages in this thread | | | Date | Wed, 13 Mar 2024 09:31:26 +0800 | From | Inochi Amaoto <> | Subject | Re: [PATCH] perf: RISC-V: fix IRQ detection on T-Head C908 |
| |
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.
| |