Message ID | 20211028141938.3530-1-lukas.bulwahn@gmail.com |
---|---|
Headers | show |
Series | Kconfig symbol clean-up on ./arch/arm{64} | expand |
On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > Commit 89d4f98ae90d ("ARM: remove zte zx platform") removes the config > DEBUG_ZTE_ZX. Hence, since then, the "ifdef CONFIG_DEBUG_ZTE_ZX" in > ./arch/arm/include/debug/pl01x.S is dead code. > > Fortunately, ./scripts/checkkconfigsymbols.py detects this and warns: > > DEBUG_ZTE_ZX > Referencing files: arch/arm/include/debug/pl01x.S > > So, remove the obsolete ifdef CONFIG_DEBUG_ZTE_ZX. > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> Reviewed-by: Arnd Bergmann <arnd@arndb.de> I see another copy of these constants in drivers/tty/serial/amba-pl011.c, which we should probably clean up as well. Arnd
On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > Commit d2b310b0234c ("ARM: debug: Use generic 8250 debug_ll for omap2 and > omap3/4/5 common uarts") adds address definitions of DEBUG_UART_PHYS for > OMAP2, OMAP3, OMAP4 and OMAP5 in ./arch/arm/Kconfig.debug. > > These definitions depend on DEBUG_OMAP{2,3,4,5}UART{1,2}; however, only > DEBUG_OMAP2UART{1,2} are defined in ./arch/arm/Kconfig.debug, and > DEBUG_OMAP{3,4,5}UART{1,2} are not defined. Hence, the script > ./scripts/checkkconfigsymbols.py warns here on non-existing symbols. > Simply reuse the config DEBUG_OMAP2UART{1,2}; there is no need to define > separate config symbols for OMAP{3,4,5}. So, just delete the dead > references to DEBUG_OMAP{3,4,5}UART{1,2}. > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > Commit c0c89fafa289 ("ARM: Remove mach-msm and associated ARM architecture > code") removes the definition of the config ARCH_MSM. Since then, the > reference to ARCH_MSM in the dependencies of UNCOMPRESS_INCLUDE in > Kconfig.debug is dead. > > Fortunately, ./scripts/checkkconfigsymbols.py warns: > > ARCH_MSM > Referencing files: arch/arm/Kconfig.debug > > Drop the dependency on this removed config. > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> > --- > arch/arm/Kconfig.debug | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug > index 83484564b1d9..87aa6e92ee6e 100644 > --- a/arch/arm/Kconfig.debug > +++ b/arch/arm/Kconfig.debug > @@ -1886,7 +1886,7 @@ config DEBUG_UNCOMPRESS > > config UNCOMPRESS_INCLUDE > string > - default "debug/uncompress.h" if ARCH_MULTIPLATFORM || ARCH_MSM || \ > + default "debug/uncompress.h" if ARCH_MULTIPLATFORM || \ > PLAT_SAMSUNG || ARM_SINGLE_ARMV7M The PLAT_SAMSUNG reference is also misplaced here, I think you just want ARCH_S3C24XX instead, since the other samsung ones already require ARCH_MULTIPLATFORM. Arnd
On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > Commit 6da5238fa384 ("ARM: 8993/1: remove it8152 PCI controller driver") > removes the config PCI_HOST_ITE8152, but left a dangling obsolete ifndef > in ./arch/arm/kernel/bios32.c. > > Hence, ./scripts/checkkconfigsymbols.py warns: > > PCI_HOST_ITE8152 > Referencing files: arch/arm/kernel/bios32.c > > Remove this obsolete ifndef. > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> Reviewed-by: Arnd Bergmann <arnd@arndb.de> I wonder if we should just remove this function and use the (non-empty) default version instead. 96c5590058d7 ("PCI: Pull PCI 'latency timer' setup up into the core") introduced that generic version, and I suspect the arm version was left out by mistake, but it's not clear from that patch. Arnd
On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > Commit b7fb14d3ac63 ("ide: remove the legacy ide driver") removes the > definition of the config BLK_DEV_PALMCHIP_BK3710. > > So, remove the obsolete references in the mach-davinci architecture. > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > Commit 73d04ca5f4ac ("ARM: ixp4xx: Delete Intel reference design > boardfiles") removes the definition of the configs MACH_IXDP465 and > MACH_KIXRP435, but misses to remove the configs CPU_IXP43X and CPU_IXP46X > that depend on those removed configs, and hence are dead now. > > Fortunately, ./scripts/checkkconfigsymbols.py warns: > > MACH_IXDP465 > Referencing files: arch/arm/mach-ixp4xx/Kconfig > > MACH_KIXRP435 > Referencing files: arch/arm/mach-ixp4xx/Kconfig > > Remove the dead configs CPU_IXP43X and CPU_IXP46X. > > A further quick grep for the name of those two symbols did not show any > use of the two config symbols; so, there are no further clean-up activities > beyond this config removal needed. > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > Since commit 4b563a066611 ("ARM: imx: Remove imx21 support"), the config > DEBUG_IMX21_IMX27_UART is really only debug support for IMX27. > > So, rename this option to DEBUG_IMX27_UART and adjust dependencies in > Kconfig and rename the definitions to IMX27 as further clean-up. > > This issue was discovered with ./scripts/checkkconfigsymbols.py, which > reported that DEBUG_IMX21_IMX27_UART depends on the non-existing config > SOC_IMX21. > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > The patch series "Add basic support for Socionext Milbeaut M10V SoC" (see > Link) introduced the config ARCH_MILBEAUT_M10V "Milbeaut SC2000/M10V > platform" in ./arch/arm/mach-milbeaut/ and intended to introduce timer, > clock, pinctrl and serial controller drivers. > > However, during patch submission in March 2019, the introduction of the > milbeaut pinctrl driver was dropped from v2 to v3 of the patch series. > Since then, there was no further patch series to add this pinctrl driver > later on. > > Hence, selecting PINCTRL_MILBEAUT in config is simply dangling and > referring to a non-existing config symbols. > Fortunately, ./scripts/checkkconfigsymbols.py warns: > > PINCTRL_MILBEAUT > Referencing files: arch/arm/mach-milbeaut/Kconfig > > Remove this select of the non-existing PINCTRL_MILBEAUT for now. > > Link: https://lore.kernel.org/linux-arm-kernel/1551243056-10521-1-git-send-email-sugaya.taichi@socionext.com/ > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> I would take that as an indication that there is no interest in supporting this platform upstream any more, the version we merged probably never worked without the rest of the drivers. I've added the original authors of the other drivers to Cc. Should we remove all of this? Arnd
On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > Commit 85b6fcadcf66 ("clocksource/drivers/ux500: Drop Ux500 custom > SCHED_CLOCK") removes a sched_clock workaround and its corresponding > config CLKSRC_NOMADIK_MTU_SCHED_CLOCK. Since then, selecting > CLKSRC_NOMADIK_MTU_SCHED_CLOCK in ./arch/arm/mach-nomadik/Kconfig has no > effect and ./scripts/checkkconfigsymbols.py warns: > > CLKSRC_NOMADIK_MTU_SCHED_CLOCK > Referencing files: arch/arm/mach-nomadik/Kconfig > > Simply drop selecting the obsolete CLKSRC_NOMADIK_MTU_SCHED_CLOCK. > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > There is no and never was a Kconfig for ARM_ERRATA_794072 in the kernel > tree. So, there is no need to select ARM_ERRATA_794072 in > ./arch/arm/mach-npcm/Kconfig. > > Simply drop selecting the non-existing ARM_ERRATA_794072. > > This issue was discovered with ./scripts/checkkconfigsymbols.py. > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> > --- Could this be a typo? Maybe we need to enable a different errata workaround here, or maybe that code is actually needed and has to get sent. Arnd
On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > Commit fbc125afdc50 ("ARM: socfpga: Turn on ARM errata for L2 cache") adds > some selects in ./arch/arm/mach-socfpga/Kconfig, with one select being > conditionally selected on the non-existing Kconfig symbol PL310. > > Hence, ./scripts/checkkconfigsymbols.py warns: > > PL310 > Referencing files: arch/arm/mach-socfpga/Kconfig > > Assuming that this errata should actually be selected for > ARCH_INTEL_SOCFPGA, simply select this config unconditionally. > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> I think it has to be select PL310_ERRATA_753970 if CACHE_L2X0 otherwise you get a Kconfig warning when building a kernel without the L2X0 driver. Arnd
On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > Commit 5615f69bc209 ("ARM: 9016/2: Initialize the mapping of KASan shadow > memory") adds some nested ifdef's in ./arch/arm/mm/pgd.c, and follows the > good practice to annotate the endif's with a comment to indicate the > corresponding ifdef condition. > > One comment annotation refers to CONFIG_LPAE, whereas the config is > actually called CONFIG_ARM_LPAE. That imprecision in a comment is probably > tolerable for all human readers. > > However, the script ./scripts/checkkconfigsymbols.py, which checks the > kernel tree for references to non-existing Kconfig symbols, identifies and > reports that the reference to CONFIG_LPAE is invalid. > > The script ./scripts/checkkconfigsymbols.py has been quite useful to > identify a number of bugs with Kconfig symbols and deserves to be executed > and checked regularly. > > So, repair the comment to reduce the reports from this script and simplify > to use this script, as new issues are easier to spot when the list of > reports is shorter. > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
On Thu, Oct 28, 2021 at 04:46:38PM +0200, Arnd Bergmann wrote: > On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > > > Commit 6da5238fa384 ("ARM: 8993/1: remove it8152 PCI controller driver") > > removes the config PCI_HOST_ITE8152, but left a dangling obsolete ifndef > > in ./arch/arm/kernel/bios32.c. > > > > Hence, ./scripts/checkkconfigsymbols.py warns: > > > > PCI_HOST_ITE8152 > > Referencing files: arch/arm/kernel/bios32.c > > > > Remove this obsolete ifndef. > > > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> > > Reviewed-by: Arnd Bergmann <arnd@arndb.de> > > I wonder if we should just remove this function and use the > (non-empty) default version instead. > > 96c5590058d7 ("PCI: Pull PCI 'latency timer' setup up into the core") > introduced that generic version, and I suspect the arm version > was left out by mistake, but it's not clear from that patch. That was because PCI_HOST_ITE8152 needed something different from the "do nothing" default (setting the PCI latency timer to default to 64 as the new generic code did.)
On Thu, Oct 28, 2021 at 5:35 PM Russell King (Oracle) <linux@armlinux.org.uk> wrote: > > On Thu, Oct 28, 2021 at 04:46:38PM +0200, Arnd Bergmann wrote: > > On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > > > > > Commit 6da5238fa384 ("ARM: 8993/1: remove it8152 PCI controller driver") > > > removes the config PCI_HOST_ITE8152, but left a dangling obsolete ifndef > > > in ./arch/arm/kernel/bios32.c. > > > > > > Hence, ./scripts/checkkconfigsymbols.py warns: > > > > > > PCI_HOST_ITE8152 > > > Referencing files: arch/arm/kernel/bios32.c > > > > > > Remove this obsolete ifndef. > > > > > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> > > > > Reviewed-by: Arnd Bergmann <arnd@arndb.de> > > > > I wonder if we should just remove this function and use the > > (non-empty) default version instead. > > > > 96c5590058d7 ("PCI: Pull PCI 'latency timer' setup up into the core") > > introduced that generic version, and I suspect the arm version > > was left out by mistake, but it's not clear from that patch. > > That was because PCI_HOST_ITE8152 needed something different from the > "do nothing" default (setting the PCI latency timer to default to 64 > as the new generic code did.) > So, can we just drop the empty pcibios_set_master() function in bios32.c and the pci handling will now (after the removal of PCI_HOST_ITE8152) just do The Right Thing(TM)? If you can confirm that, I will send an updated patch here. Thanks, Lukas
On Fri, Oct 29, 2021 at 7:34 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > On Thu, Oct 28, 2021 at 4:38 PM Arnd Bergmann <arnd@arndb.de> wrote: > > > > Arnd, did you have something like this---see below---for serial > amba-pl011 in mind? > > Then, I would adjust the patch to remove all the zte_zx serial > left-over in one commit. > Yes, looks good to me. Arnd
On Thu, 28 Oct 2021 at 14:57, Arnd Bergmann <arnd@arndb.de> wrote: > > On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > > > There is no and never was a Kconfig for ARM_ERRATA_794072 in the kernel > > tree. So, there is no need to select ARM_ERRATA_794072 in > > ./arch/arm/mach-npcm/Kconfig. > > > > Simply drop selecting the non-existing ARM_ERRATA_794072. > > > > This issue was discovered with ./scripts/checkkconfigsymbols.py. > > > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> > > --- > > Could this be a typo? Maybe we need to enable a different errata workaround > here, or maybe that code is actually needed and has to get sent. Doing some searching, u-boot had a workaround for something called ARM_ERRATA_794072. https://github.com/u-boot/u-boot/commit/f71cbfe3ca5d2ad20159871700e8e248c8818ba8 Lore has the review history for that patch: https://lore.kernel.org/all/6be32e0b5b454ed7b609317266a8e798@BLUPR03MB358.namprd03.prod.outlook.com/ It looks like it's the same workaround as ARM_ERRATA_742230, which the kernel does implement. It would be good to hear from the Nuvoton people, or an Arm person. Cheers, Joel
On 2021/10/28 23:55, Arnd Bergmann wrote: > On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: >> >> The patch series "Add basic support for Socionext Milbeaut M10V SoC" (see >> Link) introduced the config ARCH_MILBEAUT_M10V "Milbeaut SC2000/M10V >> platform" in ./arch/arm/mach-milbeaut/ and intended to introduce timer, >> clock, pinctrl and serial controller drivers. >> >> However, during patch submission in March 2019, the introduction of the >> milbeaut pinctrl driver was dropped from v2 to v3 of the patch series. >> Since then, there was no further patch series to add this pinctrl driver >> later on. >> >> Hence, selecting PINCTRL_MILBEAUT in config is simply dangling and >> referring to a non-existing config symbols. >> Fortunately, ./scripts/checkkconfigsymbols.py warns: >> >> PINCTRL_MILBEAUT >> Referencing files: arch/arm/mach-milbeaut/Kconfig >> >> Remove this select of the non-existing PINCTRL_MILBEAUT for now. >> >> Link: https://lore.kernel.org/linux-arm-kernel/1551243056-10521-1-git-send-email-sugaya.taichi@socionext.com/ >> >> Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> > > I would take that as an indication that there is no interest in supporting this > platform upstream any more, the version we merged probably never worked > without the rest of the drivers. > > I've added the original authors of the other drivers to Cc. Should we remove > all of this? > > Arnd > It is okay to drop PINCTRL_MILBEAUT. I will add it again when working at the pinctrl driver. But don`t remove the milbeaut platform. Actually we haven't been doing maintenance recently, but we have plans to add drivers in the future. Thanks, Taichi Sugaya
On Fri, Oct 29, 2021 at 8:36 AM Joel Stanley <joel@jms.id.au> wrote: > > On Thu, 28 Oct 2021 at 14:57, Arnd Bergmann <arnd@arndb.de> wrote: > > > > On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > > > > > There is no and never was a Kconfig for ARM_ERRATA_794072 in the kernel > > > tree. So, there is no need to select ARM_ERRATA_794072 in > > > ./arch/arm/mach-npcm/Kconfig. > > > > > > Simply drop selecting the non-existing ARM_ERRATA_794072. > > > > > > This issue was discovered with ./scripts/checkkconfigsymbols.py. > > > > > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> > > > --- > > > > Could this be a typo? Maybe we need to enable a different errata workaround > > here, or maybe that code is actually needed and has to get sent. > > Doing some searching, u-boot had a workaround for something called > ARM_ERRATA_794072. > > https://github.com/u-boot/u-boot/commit/f71cbfe3ca5d2ad20159871700e8e248c8818ba8 > > Lore has the review history for that patch: > > https://lore.kernel.org/all/6be32e0b5b454ed7b609317266a8e798@BLUPR03MB358.namprd03.prod.outlook.com/ > > It looks like it's the same workaround as ARM_ERRATA_742230, which the > kernel does implement. > > It would be good to hear from the Nuvoton people, or an Arm person. > I will happily update the patch to select ARM_ERRATA_742230 instead of the dead non-existing ARM_ERRATA_794072. In contrast to the current patch that basically only cleans up "dead config" and has no effective functional change, the new patch would change the behaviour. I cannot test this patch (beyond some basic compile test) on the hardware; so, we certainly need someone to have that hardware, knows how to test it or confirm otherwise that we should select the ARM_ERRATA_742230 fix for this hardware. The current patch should be subsumed by the new patch; the submission of the new patch is deferred until that person shows up. Let's see. Lukas
On Thu, Oct 28, 2021 at 4:42 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > > > Commit c0c89fafa289 ("ARM: Remove mach-msm and associated ARM architecture > > code") removes the definition of the config ARCH_MSM. Since then, the > > reference to ARCH_MSM in the dependencies of UNCOMPRESS_INCLUDE in > > Kconfig.debug is dead. > > > > Fortunately, ./scripts/checkkconfigsymbols.py warns: > > > > ARCH_MSM > > Referencing files: arch/arm/Kconfig.debug > > > > Drop the dependency on this removed config. > > > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> > > --- > > arch/arm/Kconfig.debug | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug > > index 83484564b1d9..87aa6e92ee6e 100644 > > --- a/arch/arm/Kconfig.debug > > +++ b/arch/arm/Kconfig.debug > > @@ -1886,7 +1886,7 @@ config DEBUG_UNCOMPRESS > > > > config UNCOMPRESS_INCLUDE > > string > > - default "debug/uncompress.h" if ARCH_MULTIPLATFORM || ARCH_MSM || \ > > + default "debug/uncompress.h" if ARCH_MULTIPLATFORM || \ > > PLAT_SAMSUNG || ARM_SINGLE_ARMV7M > > The PLAT_SAMSUNG reference is also misplaced here, I think you just want > ARCH_S3C24XX instead, since the other samsung ones already require > ARCH_MULTIPLATFORM. > Agree. I can clean up (or better stated: optimize) the dependencies further in such a way. But that config dependency optimization goes beyond just removing dead symbols and deserves to be its own patch. Patch will follow later this week. Lukas
On Tue, Nov 2, 2021 at 8:31 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > On Fri, Oct 29, 2021 at 8:36 AM Joel Stanley <joel@jms.id.au> wrote: > > On Thu, 28 Oct 2021 at 14:57, Arnd Bergmann <arnd@arndb.de> wrote: > > > On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > https://lore.kernel.org/all/6be32e0b5b454ed7b609317266a8e798@BLUPR03MB358.namprd03.prod.outlook.com/ > > > > It looks like it's the same workaround as ARM_ERRATA_742230, which the > > kernel does implement. > > > > It would be good to hear from the Nuvoton people, or an Arm person. > > I will happily update the patch to select ARM_ERRATA_742230 instead of > the dead non-existing ARM_ERRATA_794072. > > In contrast to the current patch that basically only cleans up "dead > config" and has no effective functional change, the new patch would > change the behaviour. I cannot test this patch (beyond some basic > compile test) on the hardware; so, we certainly need someone to have > that hardware, knows how to test it or confirm otherwise that we > should select the ARM_ERRATA_742230 fix for this hardware. > > The current patch should be subsumed by the new patch; the submission > of the new patch is deferred until that person shows up. Let's see. I'd prefer to leave the broken Kconfig symbol in place as a reminder until it gets fixed properly then. Arnd
Hi, At Nuvoton we implied this WA in the past, not because we encountered a specific problem but since the errata says so and we saw this in other patches like: https://patchwork.kernel.org/project/linux-arm-kernel/patch/1396298072-13254-2-git-send-email-nitin.garg@freescale.com/ But we didn't upstream the arch/arm/mm/proc-v7.S From the errata document. "794072 A short loop including a DMB instruction might cause a denial of service on another processor which executes a CP15 broadcast operation Status Affects: Fault Type: Fault Status: Cortex-A9 MPCore. Programmer Category B Present in: All r1, r2, r3 and r4 revisions Open Description A processor which continuously executes a short loop containing a DMB instruction might prevent a CP15 operation broadcast by another processor making further progress, causing a denial of service. Configurations affected This erratum affects all Cortex-A9 MPCore processors with two or more processors. Conditions The erratum requires the following conditions: - Two or more processors are working in SMP mode (ACTLR.SMP=1) - One of the processors continuously executes a short loop containing at least one DMB instruction. - Another processor executes a CP15 maintenance operation that is broadcast. This requires that this processor has enabled the broadcasting of CP15 operations (ACTLR.FW=1) For the erratum to occur, the short loop containing the DMB instruction must meet all of the following additional conditions: - No more than 10 instructions other than the DMB are executed between each DMB - No non-conditional Load or Store, or conditional Load or Store which pass the condition code check, are executed between each DMB When all the conditions for the erratum are met, the short loop creates a continuous stream of DMB instructions. This might cause a denial of service, by preventing the processor executing the short loop from executing the received broadcast CP15 operation. As a result, the processor that originally executed the broadcast CP15 operation is stalled until the execution of the loop is interrupted. Note that because the process issuing the CP15 broadcast operation cannot complete operation, it cannot enter any debug-mode, and cannot take any interrupt. If the processor executing the short loop also cannot be interrupted, for example if it has disabled its interrupts, or if no interrupts are routed to this processor, this erratum might cause a system livelock. Implications The erratum might create performance issues, or in the worst case it might cause a system livelock if the processor executing the DMB is in an infinite loop that cannot be interrupted. Workaround This erratum can be worked round by setting bit[4] of the undocumented Diagnostic Control Register to 1. This register is encoded as CP15 c15 0 c0 1. This bit can be written in Secure state only, with the following Read/Modify/Write code sequence: MRC p15,0,rt,c15,c0,1 ORR rt,rt,#0x10 MCR p15,0,rt,c15,c0,1 When it is set, this bit causes the DMB instruction to be decoded and executed like a DSB. Using this software workaround is not expected to have any impact on the overall performance of the processor on a typical code base. Other workarounds are also available for this erratum, to either prevent or interrupt the continuous stream of DMB instructions that causes the deadlock. For example: - Inserting a non-conditional Load or Store instruction in the loop between each DMB - Inserting additional instructions in the loop, such as NOPs, to avoid the processor seeing back to back DMB instructions. - Making the processor executing the short loop take regular interrupts." Avi On Tue, Nov 2, 2021 at 9:31 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > On Fri, Oct 29, 2021 at 8:36 AM Joel Stanley <joel@jms.id.au> wrote: > > > > On Thu, 28 Oct 2021 at 14:57, Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > > > > > > > There is no and never was a Kconfig for ARM_ERRATA_794072 in the kernel > > > > tree. So, there is no need to select ARM_ERRATA_794072 in > > > > ./arch/arm/mach-npcm/Kconfig. > > > > > > > > Simply drop selecting the non-existing ARM_ERRATA_794072. > > > > > > > > This issue was discovered with ./scripts/checkkconfigsymbols.py. > > > > > > > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> > > > > --- > > > > > > Could this be a typo? Maybe we need to enable a different errata workaround > > > here, or maybe that code is actually needed and has to get sent. > > > > Doing some searching, u-boot had a workaround for something called > > ARM_ERRATA_794072. > > > > https://github.com/u-boot/u-boot/commit/f71cbfe3ca5d2ad20159871700e8e248c8818ba8 > > > > Lore has the review history for that patch: > > > > https://lore.kernel.org/all/6be32e0b5b454ed7b609317266a8e798@BLUPR03MB358.namprd03.prod.outlook.com/ > > > > It looks like it's the same workaround as ARM_ERRATA_742230, which the > > kernel does implement. > > > > It would be good to hear from the Nuvoton people, or an Arm person. > > > > I will happily update the patch to select ARM_ERRATA_742230 instead of > the dead non-existing ARM_ERRATA_794072. > > In contrast to the current patch that basically only cleans up "dead > config" and has no effective functional change, the new patch would > change the behaviour. I cannot test this patch (beyond some basic > compile test) on the hardware; so, we certainly need someone to have > that hardware, knows how to test it or confirm otherwise that we > should select the ARM_ERRATA_742230 fix for this hardware. > > The current patch should be subsumed by the new patch; the submission > of the new patch is deferred until that person shows up. Let's see. > > Lukas
On Tue, Nov 2, 2021 at 10:22 AM Avi Fishman <avifishman70@gmail.com> wrote: > > Hi, > > At Nuvoton we implied this WA in the past, not because we encountered > a specific problem but since the errata says so and we saw this in > other patches like: > https://patchwork.kernel.org/project/linux-arm-kernel/patch/1396298072-13254-2-git-send-email-nitin.garg@freescale.com/ > But we didn't upstream the arch/arm/mm/proc-v7.S > > From the errata document. > "794072 A short loop including a DMB instruction might cause a denial > of service on > another processor which executes a CP15 broadcast operation > Status > Affects: > Fault Type: > Fault Status: Cortex-A9 MPCore. > Programmer Category B > Present in: All r1, r2, r3 and r4 revisions Open > Description > A processor which continuously executes a short loop containing a DMB > instruction might prevent a CP15 > operation broadcast by another processor making further progress, > causing a denial of service. > Configurations affected > This erratum affects all Cortex-A9 MPCore processors with two or more > processors. > Conditions > The erratum requires the following conditions: > - Two or more processors are working in SMP mode (ACTLR.SMP=1) > - One of the processors continuously executes a short loop containing > at least one DMB instruction. > - Another processor executes a CP15 maintenance operation that is > broadcast. This requires that this > processor has enabled the broadcasting of CP15 operations (ACTLR.FW=1) > For the erratum to occur, the short loop containing the DMB > instruction must meet all of the following additional > conditions: > - No more than 10 instructions other than the DMB are executed between each DMB > - No non-conditional Load or Store, or conditional Load or Store which > pass the condition code check, are > executed between each DMB > When all the conditions for the erratum are met, the short loop > creates a continuous stream of DMB instructions. > This might cause a denial of service, by preventing the processor > executing the short loop from executing the > received broadcast CP15 operation. As a result, the processor that > originally executed the broadcast CP15 > operation is stalled until the execution of the loop is interrupted. > Note that because the process issuing the CP15 broadcast operation > cannot complete operation, it cannot enter > any debug-mode, and cannot take any interrupt. If the processor > executing the short loop also cannot be > interrupted, for example if it has disabled its interrupts, or if no > interrupts are routed to this processor, this > erratum might cause a system livelock. > Implications > The erratum might create performance issues, or in the worst case it > might cause a system livelock if the > processor executing the DMB is in an infinite loop that cannot be interrupted. > Workaround > This erratum can be worked round by setting bit[4] of the undocumented > Diagnostic Control Register to 1. This > register is encoded as CP15 c15 0 c0 1. > This bit can be written in Secure state only, with the following > Read/Modify/Write code sequence: > MRC p15,0,rt,c15,c0,1 > ORR rt,rt,#0x10 > MCR p15,0,rt,c15,c0,1 > When it is set, this bit causes the DMB instruction to be decoded and > executed like a DSB. > Using this software workaround is not expected to have any impact on > the overall performance of the processor > on a typical code base. > Other workarounds are also available for this erratum, to either > prevent or interrupt the continuous stream of > DMB instructions that causes the deadlock. For example: > - Inserting a non-conditional Load or Store instruction in the loop > between each DMB > - Inserting additional instructions in the loop, such as NOPs, to > avoid the processor seeing back to back > DMB instructions. > - Making the processor executing the short loop take regular interrupts." > > Avi > > On Tue, Nov 2, 2021 at 9:31 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > > > On Fri, Oct 29, 2021 at 8:36 AM Joel Stanley <joel@jms.id.au> wrote: > > > > > > On Thu, 28 Oct 2021 at 14:57, Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > > > On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > > > > > > > > > There is no and never was a Kconfig for ARM_ERRATA_794072 in the kernel > > > > > tree. So, there is no need to select ARM_ERRATA_794072 in > > > > > ./arch/arm/mach-npcm/Kconfig. > > > > > > > > > > Simply drop selecting the non-existing ARM_ERRATA_794072. > > > > > > > > > > This issue was discovered with ./scripts/checkkconfigsymbols.py. > > > > > > > > > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> > > > > > --- > > > > > > > > Could this be a typo? Maybe we need to enable a different errata workaround > > > > here, or maybe that code is actually needed and has to get sent. > > > > > > Doing some searching, u-boot had a workaround for something called > > > ARM_ERRATA_794072. > > > > > > https://github.com/u-boot/u-boot/commit/f71cbfe3ca5d2ad20159871700e8e248c8818ba8 > > > > > > Lore has the review history for that patch: > > > > > > https://lore.kernel.org/all/6be32e0b5b454ed7b609317266a8e798@BLUPR03MB358.namprd03.prod.outlook.com/ > > > > > > It looks like it's the same workaround as ARM_ERRATA_742230, which the > > > kernel does implement. > > > > > > It would be good to hear from the Nuvoton people, or an Arm person. > > > > > > > I will happily update the patch to select ARM_ERRATA_742230 instead of > > the dead non-existing ARM_ERRATA_794072. > > > > In contrast to the current patch that basically only cleans up "dead > > config" and has no effective functional change, the new patch would > > change the behaviour. I cannot test this patch (beyond some basic > > compile test) on the hardware; so, we certainly need someone to have > > that hardware, knows how to test it or confirm otherwise that we > > should select the ARM_ERRATA_742230 fix for this hardware. Note that ARM_ERRATA_742230 is applied in code up-to CORTEX-A9 r2p2 but while ARM_ERRATA_794072 exist also in CORTEX-A9 r4p1 https://github.com/torvalds/linux/blob/322a3b843d7f475b857646ed8f95b40431d3ecd0/arch/arm/mm/proc-v7.S#L347 > > > > The current patch should be subsumed by the new patch; the submission > > of the new patch is deferred until that person shows up. Let's see. > > > > Lukas > > > > -- > Regards, > Avi
On Tue, Nov 2, 2021 at 9:11 AM Arnd Bergmann <arnd@arndb.de> wrote: > > On Tue, Nov 2, 2021 at 8:31 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > On Fri, Oct 29, 2021 at 8:36 AM Joel Stanley <joel@jms.id.au> wrote: > > > On Thu, 28 Oct 2021 at 14:57, Arnd Bergmann <arnd@arndb.de> wrote: > > > > On Thu, Oct 28, 2021 at 4:19 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > > https://lore.kernel.org/all/6be32e0b5b454ed7b609317266a8e798@BLUPR03MB358.namprd03.prod.outlook.com/ > > > > > > It looks like it's the same workaround as ARM_ERRATA_742230, which the > > > kernel does implement. > > > > > > It would be good to hear from the Nuvoton people, or an Arm person. > > > > I will happily update the patch to select ARM_ERRATA_742230 instead of > > the dead non-existing ARM_ERRATA_794072. > > > > In contrast to the current patch that basically only cleans up "dead > > config" and has no effective functional change, the new patch would > > change the behaviour. I cannot test this patch (beyond some basic > > compile test) on the hardware; so, we certainly need someone to have > > that hardware, knows how to test it or confirm otherwise that we > > should select the ARM_ERRATA_742230 fix for this hardware. > > > > The current patch should be subsumed by the new patch; the submission > > of the new patch is deferred until that person shows up. Let's see. > > I'd prefer to leave the broken Kconfig symbol in place as a reminder until it > gets fixed properly then. > Agree, this patch here should not be integrated. I rather expect that Avi or others at Nuvoton will provide a proper patch to act appropriately for the ARM_ERRATA_794072, or after proper analysis can determine that there is no change in the kernel required. Lukas
On Thu, Oct 28, 2021 at 4:20 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > Commit 5615f69bc209 ("ARM: 9016/2: Initialize the mapping of KASan shadow > memory") adds some nested ifdef's in ./arch/arm/mm/pgd.c, and follows the > good practice to annotate the endif's with a comment to indicate the > corresponding ifdef condition. > > One comment annotation refers to CONFIG_LPAE, whereas the config is > actually called CONFIG_ARM_LPAE. That imprecision in a comment is probably > tolerable for all human readers. > > However, the script ./scripts/checkkconfigsymbols.py, which checks the > kernel tree for references to non-existing Kconfig symbols, identifies and > reports that the reference to CONFIG_LPAE is invalid. > > The script ./scripts/checkkconfigsymbols.py has been quite useful to > identify a number of bugs with Kconfig symbols and deserves to be executed > and checked regularly. > > So, repair the comment to reduce the reports from this script and simplify > to use this script, as new issues are easier to spot when the list of > reports is shorter. > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Please put this patch into Russell's patch tracker! Yours, Linus Walleij
On Thu, Oct 28, 2021 at 4:20 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > Commit 73d04ca5f4ac ("ARM: ixp4xx: Delete Intel reference design > boardfiles") removes the definition of the configs MACH_IXDP465 and > MACH_KIXRP435, but misses to remove the configs CPU_IXP43X and CPU_IXP46X > that depend on those removed configs, and hence are dead now. > > Fortunately, ./scripts/checkkconfigsymbols.py warns: > > MACH_IXDP465 > Referencing files: arch/arm/mach-ixp4xx/Kconfig > > MACH_KIXRP435 > Referencing files: arch/arm/mach-ixp4xx/Kconfig > > Remove the dead configs CPU_IXP43X and CPU_IXP46X. > > A further quick grep for the name of those two symbols did not show any > use of the two config symbols; so, there are no further clean-up activities > beyond this config removal needed. > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> Patch applied to my IXP4xx tree! Yours, Linus Walleij
On Thu, Oct 28, 2021 at 4:20 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > Commit 85b6fcadcf66 ("clocksource/drivers/ux500: Drop Ux500 custom > SCHED_CLOCK") removes a sched_clock workaround and its corresponding > config CLKSRC_NOMADIK_MTU_SCHED_CLOCK. Since then, selecting > CLKSRC_NOMADIK_MTU_SCHED_CLOCK in ./arch/arm/mach-nomadik/Kconfig has no > effect and ./scripts/checkkconfigsymbols.py warns: > > CLKSRC_NOMADIK_MTU_SCHED_CLOCK > Referencing files: arch/arm/mach-nomadik/Kconfig > > Simply drop selecting the obsolete CLKSRC_NOMADIK_MTU_SCHED_CLOCK. > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> Patch applied to my Nomadik tree! Yours, Linus Walleij
On Thu, Oct 28, 2021 at 04:19:33PM +0200, Lukas Bulwahn wrote: > Since commit 4b563a066611 ("ARM: imx: Remove imx21 support"), the config > DEBUG_IMX21_IMX27_UART is really only debug support for IMX27. > > So, rename this option to DEBUG_IMX27_UART and adjust dependencies in > Kconfig and rename the definitions to IMX27 as further clean-up. > > This issue was discovered with ./scripts/checkkconfigsymbols.py, which > reported that DEBUG_IMX21_IMX27_UART depends on the non-existing config > SOC_IMX21. > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> Applied, thanks!