2016-03-29 07:40:44

by Jonas Rabenstein

[permalink] [raw]
Subject: [PATCH] arm: remove tautologic #ifdefs in proc-v7-3level.S

The file arch/arm/mm/proc-v7-3level.S is only used by the #include
directive in arch/arm/mm/proc-v7.S:23. This #include is conditional and
depends on CONFIG_ARM_LPAE (otherwise proc-v7-2level.S is used).
CONFIG_ARM_LPAE has a dependency on CONFIG_MMU defined in Kconfig.
Consequently, checks for CONFIG_MMU in proc-v7-3level.S are superfluous.

Signed-off-by: Jonas Rabenstein <[email protected]>
---
I detected the issue with chimaera, a tool I currently develop for my bachelor
thesis extending the undertaker tool suite (https://undertaker.cs.fau.de).

arch/arm/mm/proc-v7-3level.S | 4 ----
1 file changed, 4 deletions(-)

diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
index 5e5720e..6903f34 100644
--- a/arch/arm/mm/proc-v7-3level.S
+++ b/arch/arm/mm/proc-v7-3level.S
@@ -55,13 +55,11 @@
* the new TTB).
*/
ENTRY(cpu_v7_switch_mm)
-#ifdef CONFIG_MMU
mmid r2, r2
asid r2, r2
orr rpgdh, rpgdh, r2, lsl #(48 - 32) @ upper 32-bits of pgd
mcrr p15, 0, rpgdl, rpgdh, c2 @ set TTB 0
isb
-#endif
ret lr
ENDPROC(cpu_v7_switch_mm)

@@ -81,7 +79,6 @@ ENDPROC(cpu_v7_switch_mm)
* - pte - PTE value to store (64-bit in r2 and r3)
*/
ENTRY(cpu_v7_set_pte_ext)
-#ifdef CONFIG_MMU
tst rl, #L_PTE_VALID
beq 1f
tst rh, #1 << (57 - 32) @ L_PTE_NONE
@@ -97,7 +94,6 @@ ENTRY(cpu_v7_set_pte_ext)
1: strd r2, r3, [r0]
ALT_SMP(W(nop))
ALT_UP (mcr p15, 0, r0, c7, c10, 1) @ flush_pte
-#endif
ret lr
ENDPROC(cpu_v7_set_pte_ext)

--
2.7.3


2016-03-29 08:06:33

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] arm: remove tautologic #ifdefs in proc-v7-3level.S

On Tuesday 29 March 2016 09:37:51 Jonas Rabenstein wrote:
> The file arch/arm/mm/proc-v7-3level.S is only used by the #include
> directive in arch/arm/mm/proc-v7.S:23. This #include is conditional and
> depends on CONFIG_ARM_LPAE (otherwise proc-v7-2level.S is used).
> CONFIG_ARM_LPAE has a dependency on CONFIG_MMU defined in Kconfig.
> Consequently, checks for CONFIG_MMU in proc-v7-3level.S are superfluous.
>
> Signed-off-by: Jonas Rabenstein <[email protected]>
> ---
> I detected the issue with chimaera, a tool I currently develop for my bachelor
> thesis extending the undertaker tool suite (https://undertaker.cs.fau.de).

Nice catch!

Reviewed-by: Arnd Bergmann <[email protected]>

I guess you should submit the same thing for the other file as well,
either in the same patch or as a series.

You can also add

Fixes: 8d2cd3a38fd6 ("ARM: LPAE: Factor out classic-MMU specific code into proc-v7-2level.S")

to mark the commit that led to the situation.

Arnd

2016-03-29 08:33:00

by Jonas Rabenstein

[permalink] [raw]
Subject: Re: [PATCH] arm: remove tautologic #ifdefs in proc-v7-3level.S

On Tue, Mar 29, 2016 at 10:05:58AM +0200, Arnd Bergmann wrote:
> On Tuesday 29 March 2016 09:37:51 Jonas Rabenstein wrote:
> > The file arch/arm/mm/proc-v7-3level.S is only used by the #include
> > directive in arch/arm/mm/proc-v7.S:23. This #include is conditional and
> > depends on CONFIG_ARM_LPAE (otherwise proc-v7-2level.S is used).
> > CONFIG_ARM_LPAE has a dependency on CONFIG_MMU defined in Kconfig.
> > Consequently, checks for CONFIG_MMU in proc-v7-3level.S are superfluous.
> >
> > Signed-off-by: Jonas Rabenstein <[email protected]>
> > ---
> > I detected the issue with chimaera, a tool I currently develop for my bachelor
> > thesis extending the undertaker tool suite (https://undertaker.cs.fau.de).
>
> Nice catch!
>
> Reviewed-by: Arnd Bergmann <[email protected]>
>
> I guess you should submit the same thing for the other file as well,
> either in the same patch or as a series.
I do not get, what you mean with 'the other file'? For the
proc-v7-2level.S this precondition does not hold, as proc-v7-2level.S is
included if !CONFIG_ARM_LPAE. Consequently, no evidence about the MMU
state is available in their.

> You can also add
>
> Fixes: 8d2cd3a38fd6 ("ARM: LPAE: Factor out classic-MMU specific code into proc-v7-2level.S")
Shouldn't it be:
Fixes: 1b6ba46b7efa ("ARM: LPAE: MMU setup for the 3-level page table
format")?

Thanks for your help and advice,
Jonas

2016-03-29 08:49:12

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] arm: remove tautologic #ifdefs in proc-v7-3level.S

On Tuesday 29 March 2016 10:29:11 Jonas Rabenstein wrote:
> On Tue, Mar 29, 2016 at 10:05:58AM +0200, Arnd Bergmann wrote:
> > On Tuesday 29 March 2016 09:37:51 Jonas Rabenstein wrote:
> > > The file arch/arm/mm/proc-v7-3level.S is only used by the #include
> > > directive in arch/arm/mm/proc-v7.S:23. This #include is conditional and
> > > depends on CONFIG_ARM_LPAE (otherwise proc-v7-2level.S is used).
> > > CONFIG_ARM_LPAE has a dependency on CONFIG_MMU defined in Kconfig.
> > > Consequently, checks for CONFIG_MMU in proc-v7-3level.S are superfluous.
> > >
> > > Signed-off-by: Jonas Rabenstein <[email protected]>
> > > ---
> > > I detected the issue with chimaera, a tool I currently develop for my bachelor
> > > thesis extending the undertaker tool suite (https://undertaker.cs.fau.de).
> >
> > Nice catch!
> >
> > Reviewed-by: Arnd Bergmann <[email protected]>
> >
> > I guess you should submit the same thing for the other file as well,
> > either in the same patch or as a series.
> I do not get, what you mean with 'the other file'? For the
> proc-v7-2level.S this precondition does not hold, as proc-v7-2level.S is
> included if !CONFIG_ARM_LPAE. Consequently, no evidence about the MMU
> state is available in their.

My mistake. I misread this as the both files being used only for
MMU-enabled kernels, which would make sense because they deal with
page tables that are not being used without MMU, but they are in fact
being compiled anyway.

> > You can also add
> >
> > Fixes: 8d2cd3a38fd6 ("ARM: LPAE: Factor out classic-MMU specific code into proc-v7-2level.S")
> Shouldn't it be:
> Fixes: 1b6ba46b7efa ("ARM: LPAE: MMU setup for the 3-level page table
> format")?

Nevermind then. My line was incorrect, and the other one is a bit
redundant as it is the one that adds the file in the first place.

Arnd