arm64: Kconfig.platforms: fix warning unmet direct dependencies

Message ID 20190115191839.13823-1-anders.roxell@linaro.org
State Accepted
Commit a29c78234942fcfba2c5c305adc85b64332f9a95
Headers show
Series
  • arm64: Kconfig.platforms: fix warning unmet direct dependencies
Related show

Commit Message

Anders Roxell Jan. 15, 2019, 7:18 p.m.
When ARCH_MXC get enabled, ARM64_ERRATUM_845719 will be selected and
this warning will happen when COMPAT isn't set.

WARNING: unmet direct dependencies detected for ARM64_ERRATUM_845719
  Depends on [n]: COMPAT [=n]
  Selected by [y]:
  - ARCH_MXC [=y]

Rework to add 'if COMPAT' before ARM64_ERRATUM_845719 gets selected,
since ARM64_ERRATUM_845719 depends on COMPAT.

Signed-off-by: Anders Roxell <anders.roxell@linaro.org>

---
 arch/arm64/Kconfig.platforms | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.19.2

Comments

Catalin Marinas Jan. 25, 2019, 2:32 p.m. | #1
On Tue, Jan 15, 2019 at 08:18:39PM +0100, Anders Roxell wrote:
> When ARCH_MXC get enabled, ARM64_ERRATUM_845719 will be selected and

> this warning will happen when COMPAT isn't set.

> 

> WARNING: unmet direct dependencies detected for ARM64_ERRATUM_845719

>   Depends on [n]: COMPAT [=n]

>   Selected by [y]:

>   - ARCH_MXC [=y]

> 

> Rework to add 'if COMPAT' before ARM64_ERRATUM_845719 gets selected,

> since ARM64_ERRATUM_845719 depends on COMPAT.

> 

> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>

> ---

>  arch/arm64/Kconfig.platforms | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms

> index 251ecf34cb02..d4faca775d9c 100644

> --- a/arch/arm64/Kconfig.platforms

> +++ b/arch/arm64/Kconfig.platforms

> @@ -145,7 +145,7 @@ config ARCH_MVEBU

>  config ARCH_MXC

>  	bool "ARMv8 based NXP i.MX SoC family"

>  	select ARM64_ERRATUM_843419

> -	select ARM64_ERRATUM_845719

> +	select ARM64_ERRATUM_845719 if COMPAT

>  	help

>  	  This enables support for the ARMv8 based SoCs in the

>  	  NXP i.MX family.


Actually, do we need to select the errata workarounds explicitly? That
seems to be the only case where we do it (commit 930507c18304, "arm64:
add basic Kconfig symbols for i.MX8"). They are default y, so we
shouldn't need to force them on.

-- 
Catalin
Lucas Stach Jan. 25, 2019, 3:57 p.m. | #2
Am Freitag, den 25.01.2019, 14:32 +0000 schrieb Catalin Marinas:
> On Tue, Jan 15, 2019 at 08:18:39PM +0100, Anders Roxell wrote:

> > When ARCH_MXC get enabled, ARM64_ERRATUM_845719 will be selected and

> > this warning will happen when COMPAT isn't set.

> > 

> > WARNING: unmet direct dependencies detected for ARM64_ERRATUM_845719

> >   Depends on [n]: COMPAT [=n]

> >   Selected by [y]:

> >   - ARCH_MXC [=y]

> > 

> > Rework to add 'if COMPAT' before ARM64_ERRATUM_845719 gets selected,

> > since ARM64_ERRATUM_845719 depends on COMPAT.

> > 

> > > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org>

> > ---

> >  arch/arm64/Kconfig.platforms | 2 +-

> >  1 file changed, 1 insertion(+), 1 deletion(-)

> > 

> > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms

> > index 251ecf34cb02..d4faca775d9c 100644

> > --- a/arch/arm64/Kconfig.platforms

> > +++ b/arch/arm64/Kconfig.platforms

> > @@ -145,7 +145,7 @@ config ARCH_MVEBU

> >  config ARCH_MXC

> > > >  	bool "ARMv8 based NXP i.MX SoC family"

> > > >  	select ARM64_ERRATUM_843419

> > > > -	select ARM64_ERRATUM_845719

> > > > +	select ARM64_ERRATUM_845719 if COMPAT

> > > >  	help

> > > >  	  This enables support for the ARMv8 based SoCs in the

> >  	  NXP i.MX family.

> 

> Actually, do we need to select the errata workarounds explicitly? That

> seems to be the only case where we do it (commit 930507c18304, "arm64:

> add basic Kconfig symbols for i.MX8"). They are default y, so we

> shouldn't need to force them on.


This is based on past experience. We've had a lot of cases were people
did not enable the necessary CPU errata workaround, which then usually
lead to very hard to debug system failures. It is on our list of things
to look out for now, but I would feel much better if there is just no
chance for a user to misconfigure the kernel in this way.

Regards,
Lucas
Anders Roxell Jan. 30, 2019, 8:43 a.m. | #3
On 2019-01-25 16:57, Lucas Stach wrote:
> Am Freitag, den 25.01.2019, 14:32 +0000 schrieb Catalin Marinas:

> > On Tue, Jan 15, 2019 at 08:18:39PM +0100, Anders Roxell wrote:

> > > When ARCH_MXC get enabled, ARM64_ERRATUM_845719 will be selected and

> > > this warning will happen when COMPAT isn't set.

> > > 

> > > WARNING: unmet direct dependencies detected for ARM64_ERRATUM_845719

> > >   Depends on [n]: COMPAT [=n]

> > >   Selected by [y]:

> > >   - ARCH_MXC [=y]

> > > 

> > > Rework to add 'if COMPAT' before ARM64_ERRATUM_845719 gets selected,

> > > since ARM64_ERRATUM_845719 depends on COMPAT.

> > > 

> > > > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org>

> > > ---

> > >  arch/arm64/Kconfig.platforms | 2 +-

> > >  1 file changed, 1 insertion(+), 1 deletion(-)

> > > 

> > > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms

> > > index 251ecf34cb02..d4faca775d9c 100644

> > > --- a/arch/arm64/Kconfig.platforms

> > > +++ b/arch/arm64/Kconfig.platforms

> > > @@ -145,7 +145,7 @@ config ARCH_MVEBU

> > >  config ARCH_MXC

> > > > >  	bool "ARMv8 based NXP i.MX SoC family"

> > > > >  	select ARM64_ERRATUM_843419

> > > > > -	select ARM64_ERRATUM_845719

> > > > > +	select ARM64_ERRATUM_845719 if COMPAT

> > > > >  	help

> > > > >  	  This enables support for the ARMv8 based SoCs in the

> > >  	  NXP i.MX family.

> > 

> > Actually, do we need to select the errata workarounds explicitly? That

> > seems to be the only case where we do it (commit 930507c18304, "arm64:

> > add basic Kconfig symbols for i.MX8"). They are default y, so we

> > shouldn't need to force them on.

> 

> This is based on past experience. We've had a lot of cases were people

> did not enable the necessary CPU errata workaround, which then usually

> lead to very hard to debug system failures. It is on our list of things

> to look out for now, but I would feel much better if there is just no

> chance for a user to misconfigure the kernel in this way.

> 


Would a better approach be to remove the 'depends on COMPAT' in the
Kconfig and move it into the src files instead see below.

Cheers,
Anders


diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index d9ab8ff3f7e5..04b11806e2d7 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -430,7 +430,6 @@ config ARM64_ERRATUM_834220
 
 config ARM64_ERRATUM_845719
 	bool "Cortex-A53: 845719: a load might read incorrect data"
-	depends on COMPAT
 	default y
 	help
 	  This option adds an alternative code sequence to work around ARM
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 9950bb0cbd52..43b14a5fc8c1 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -641,7 +641,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 		MIDR_FIXED(0x4, BIT(8)),
 	},
 #endif
-#ifdef CONFIG_ARM64_ERRATUM_845719
+#if defined(CONFIG_ARM64_ERRATUM_845719) && defined(CONFIG_COMPAT)
 	{
 	/* Cortex-A53 r0p[01234] */
 		.desc = "ARM erratum 845719",
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 0ec0c46b2c0c..e2be37ee5615 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -311,7 +311,7 @@ alternative_else_nop_endif
 	tst	x22, #PSR_MODE32_BIT		// native task?
 	b.eq	3f
 
-#ifdef CONFIG_ARM64_ERRATUM_845719
+#if defined(CONFIG_ARM64_ERRATUM_845719) && defined(CONFIG_COMPAT)
 alternative_if ARM64_WORKAROUND_845719
 #ifdef CONFIG_PID_IN_CONTEXTIDR
 	mrs	x29, contextidr_el1
Will Deacon Jan. 30, 2019, 6:21 p.m. | #4
On Wed, Jan 30, 2019 at 09:43:21AM +0100, Anders Roxell wrote:
> On 2019-01-25 16:57, Lucas Stach wrote:

> > Am Freitag, den 25.01.2019, 14:32 +0000 schrieb Catalin Marinas:

> > > On Tue, Jan 15, 2019 at 08:18:39PM +0100, Anders Roxell wrote:

> > > > When ARCH_MXC get enabled, ARM64_ERRATUM_845719 will be selected and

> > > > this warning will happen when COMPAT isn't set.

> > > > 

> > > > WARNING: unmet direct dependencies detected for ARM64_ERRATUM_845719

> > > >   Depends on [n]: COMPAT [=n]

> > > >   Selected by [y]:

> > > >   - ARCH_MXC [=y]

> > > > 

> > > > Rework to add 'if COMPAT' before ARM64_ERRATUM_845719 gets selected,

> > > > since ARM64_ERRATUM_845719 depends on COMPAT.

> > > > 

> > > > > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org>

> > > > ---

> > > >  arch/arm64/Kconfig.platforms | 2 +-

> > > >  1 file changed, 1 insertion(+), 1 deletion(-)

> > > > 

> > > > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms

> > > > index 251ecf34cb02..d4faca775d9c 100644

> > > > --- a/arch/arm64/Kconfig.platforms

> > > > +++ b/arch/arm64/Kconfig.platforms

> > > > @@ -145,7 +145,7 @@ config ARCH_MVEBU

> > > >  config ARCH_MXC

> > > > > >  	bool "ARMv8 based NXP i.MX SoC family"

> > > > > >  	select ARM64_ERRATUM_843419

> > > > > > -	select ARM64_ERRATUM_845719

> > > > > > +	select ARM64_ERRATUM_845719 if COMPAT

> > > > > >  	help

> > > > > >  	  This enables support for the ARMv8 based SoCs in the

> > > >  	  NXP i.MX family.

> > > 

> > > Actually, do we need to select the errata workarounds explicitly? That

> > > seems to be the only case where we do it (commit 930507c18304, "arm64:

> > > add basic Kconfig symbols for i.MX8"). They are default y, so we

> > > shouldn't need to force them on.

> > 

> > This is based on past experience. We've had a lot of cases were people

> > did not enable the necessary CPU errata workaround, which then usually

> > lead to very hard to debug system failures. It is on our list of things

> > to look out for now, but I would feel much better if there is just no

> > chance for a user to misconfigure the kernel in this way.

> > 

> 

> Would a better approach be to remove the 'depends on COMPAT' in the

> Kconfig and move it into the src files instead see below.


I'd prefer to keep the dependency here, because the erratum is not
applicable for native (64-bit) userspace.

I think your original patch is fine, so:

Acked-by: Will Deacon <will.deacon@arm.com>


Will

Patch

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 251ecf34cb02..d4faca775d9c 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -145,7 +145,7 @@  config ARCH_MVEBU
 config ARCH_MXC
 	bool "ARMv8 based NXP i.MX SoC family"
 	select ARM64_ERRATUM_843419
-	select ARM64_ERRATUM_845719
+	select ARM64_ERRATUM_845719 if COMPAT
 	help
 	  This enables support for the ARMv8 based SoCs in the
 	  NXP i.MX family.