On Fri, 21 Jun 2024, Jiaxun Yang wrote: > > I think this ought not to be done in two places independently and the > > pieces in <asm/mach-*/cpu-feature-overrides.h> need to be removed, likely > > in the same change even, *however* not without double-checking whether > > there is not a case among them where a platform actually has LL/SC support > > disabled despite the CPU used there having architectural support for the > > feature. Otherwise we may end up with a case where a platform has LL/SC > > support disabled via its <asm/mach-*/cpu-feature-overrides.h> setting and > > yet we enable ARCH_SUPPORTS_ATOMIC_RMW or ARCH_HAVE_NMI_SAFE_CMPXCHG for > > it via Kconfig. > > IMO it's necessary for platforms who know what are they doing such as ATH25, > which we took care in this series. > > I'll add a build time assertion to ensure when CONFIG_CPU_HAS_LLSC is selected > kernel_uses_llsc is statically 1, so any incorrect overrides can be spotted > at build time. That might do in the interim as a sanity check, however ultimately the sole reason these <asm/mach-*/cpu-feature-overrides.h> exist (and the `cpu_has_llsc' setting there) is so that a dynamic check at run time is avoided where the result is known from elsewhere beforehand anyway, and your change effectively supersedes the overrides, and therefore they need to be removed. > It's better to clean up platform's overrides at some point, I'll leave > it to a future patch. I think it will best be done now while we are at it and the change is in scope. It should be possible to automatically run over the available defconfigs and see if there is any change to `vmlinux' produced between the current state upstream and the state where this patch has been applied and the `cpu_has_llsc' setting removed from <asm/mach-*/cpu-feature-overrides.h> both at a time for each defconfig. There should be none. It'll be tougher once your changes to add ARCH_SUPPORTS_ATOMIC_RMW or ARCH_HAVE_NMI_SAFE_CMPXCHG have been applied. Maciej
