Re: Problem with atomic accesses in pstore on some ARM CPUs

[Guenter Roeck]

On Fri, Aug 19, 2016 at 2:35 AM, Russell King - ARM Linux
<linux@xxxxxxxxxxxxxxx> wrote:
> On Tue, Aug 16, 2016 at 10:35:52AM -0700, Colin Cross wrote:
>> persistent_ram uses atomic ops in uncached memory to store the start
>> and end positions in the ringbuffer so that the state of the
>> ringbuffer will be valid if the kernel crashes at any time.  This was
>> inherited from Android's ram_console implementation, and worked
>> through armv7.
>
> That statement is actually inaccurate.  It may have worked on _some_
> ARMv7 implementations, but it's not architecturally compliant.
>
> The exclusive access instructions are not portable to anything but
> "normal memory":
>
>    It is IMPLEMENTATION DEFINED whether LDREX and STREX operations can
>    be performed to a memory region with the Device or Strongly-ordered
>    memory attribute. Unless the implementation documentation explicitly
>    states that LDREX and STREX operations to a memory region with the
>    Device or Strongly-ordered attribute are permitted, the effect of
>    such operations is UNPREDICTABLE.
>
> pgprot_noncached() gives strongly-ordered memory, and so is unsuitable
> to place semaphores in for all ARMv7 implementations.
>
> Also:
>
> +       if (memtype)
> +               va = ioremap(start, size);
>
> this returns *device* memory, which is also unsuitable for all ARMv7
> implementations.
>
> It seems that pstore is playing in areas of the architecture which are
> implementation defined, so it's no surprise that folk are seeing
> different behaviours with different implementations.
>
> The code isn't architecturally wrong, it just isn't portable to all ARMv7
> implementations.
>
> So, saying that it works on some ARMv7 implementations is irrelevent.
>
> Note that LDREX and STREX are used for all operations that require
> atomicity - iow, atomics and locks.
>
> pgprot_writecombine() and ioremap_wc() will return memory which is
> suitable for these exclusive accesses - it maps to the architectures'
> "normal memory, uncacheable".
>

Unfortunately, pgprot_writecombine() doesn't work in my case (for
rk3288 and rk3399). It was also reported not to work on Tegra Logan by
Nvidia. As mentioned earlier, PAGE_KERNEL works, at least for rk3288
and rk3399.

Guenter

> So, I suspect the OP should not be using mem_type=1 or using the
> "unbuffered" DT attribute, but should leave it was the default
> (mem_type=0).
>
>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.