[PATCH] ARM: mm: clear SCTLR.HA instead of setting it for LPAE

[PATCH] ARM: mm: clear SCTLR.HA instead of setting it for LPAE

[Will Deacon]

SCTLR.HA (hardware access flag) is deprecated, not actually implemented
by any CPUs and would probably break Linux if it was (since we don't use
atomics when accessing page table entries). Furthermore, it can confuse
cr_alignment checks where the whole value of SCTLR is compared against
the value sitting in the hardware, since the bit is actually RAZ/WI and
will not match the saved cr_alignment value.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/mm/proc-v7-3level.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
index b64e67c7f176..d3daed0ae0ad 100644
--- a/arch/arm/mm/proc-v7-3level.S
+++ b/arch/arm/mm/proc-v7-3level.S
@@ -157,9 +157,9 @@ ENDPROC(cpu_v7_set_pte_ext)
 	 *  TFR   EV X F   IHD LR    S
 	 * .EEE ..EE PUI. .TAT 4RVI ZWRS BLDP WCAM
 	 * rxxx rrxx xxx0 0101 xxxx xxxx x111 xxxx < forced
-	 *   11    0 110    1  0011 1100 .111 1101 < we want
+	 *   11    0 110    0  0011 1100 .111 1101 < we want
 	 */
 	.align	2
 	.type	v7_crval, #object
 v7_crval:
-	crval	clear=0x0120c302, mmuset=0x30c23c7d, ucset=0x00c01c7c
+	crval	clear=0x0122c302, mmuset=0x30c03c7d, ucset=0x00c01c7c
-- 
2.1.0

[PATCH] ARM: mm: clear SCTLR.HA instead of setting it for LPAE

[Catalin Marinas]

On Mon, Sep 22, 2014 at 04:02:17PM +0100, Will Deacon wrote:
> SCTLR.HA (hardware access flag) is deprecated, not actually implemented
> by any CPUs

That I agree.

> and would probably break Linux if it was (since we don't use
> atomics when accessing page table entries).

But not here. The ARMv7 ARM states:

  Any implementation of hardware management of the Access flag must
  ensure that any software changes to the translation table are not
  overwritten. The architecture does not require software that changes
  translation table entries to use interlocked operations. The hardware
  management mechanisms for the Access flag must prevent any loss of
  data written to translation table entries that might occur when, for
  example, a write by another processor occurs between the read and
  write phases of a translation table walk that updates the Access flag.

So basically you should not be required to change the OS for atomic
accesses to the page table entries. There is indeed a chance that the OS
would inadvertently clear the AF bit but that's only affecting the
statistics a bit.

> Furthermore, it can confuse cr_alignment checks where the whole value
> of SCTLR is compared against the value sitting in the hardware, since
> the bit is actually RAZ/WI and will not match the saved cr_alignment
> value.

Is this the right fix for cr_alignment? What if we get other bits in the
future which are RAZ/WI on ARMv7 and RW on 32-bit ARMv8?

-- 
Catalin

[PATCH] ARM: mm: clear SCTLR.HA instead of setting it for LPAE

[Will Deacon]

On Tue, Sep 23, 2014 at 05:16:29PM +0100, Catalin Marinas wrote:
> On Mon, Sep 22, 2014 at 04:02:17PM +0100, Will Deacon wrote:
> > SCTLR.HA (hardware access flag) is deprecated, not actually implemented
> > by any CPUs
> 
> That I agree.
> 
> > and would probably break Linux if it was (since we don't use
> > atomics when accessing page table entries).
> 
> But not here. The ARMv7 ARM states:
> 
>   Any implementation of hardware management of the Access flag must
>   ensure that any software changes to the translation table are not
>   overwritten. The architecture does not require software that changes
>   translation table entries to use interlocked operations. The hardware
>   management mechanisms for the Access flag must prevent any loss of
>   data written to translation table entries that might occur when, for
>   example, a write by another processor occurs between the read and
>   write phases of a translation table walk that updates the Access flag.
> 
> So basically you should not be required to change the OS for atomic
> accesses to the page table entries. There is indeed a chance that the OS
> would inadvertently clear the AF bit but that's only affecting the
> statistics a bit.

Well, that explains why nobody managed to implement this in hardware! I'll
remove that part from the commit message.

> > Furthermore, it can confuse cr_alignment checks where the whole value
> > of SCTLR is compared against the value sitting in the hardware, since
> > the bit is actually RAZ/WI and will not match the saved cr_alignment
> > value.
> 
> Is this the right fix for cr_alignment? What if we get other bits in the
> future which are RAZ/WI on ARMv7 and RW on 32-bit ARMv8?

In that case, we'd need to adjust our masks to avoid setting RAZ/WI bits.
This will only be a problem where we were trying to use a feature that was
removed, so it's a useful exercise to go through anyway (an alternative is
to read back the value and see what stuck, but that feels fragile).

Will