linux-kernel - Re: [RFC PATCH 1/3] pstore-ram: use write-combine mappings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <51656992.60203@gmail.com>
Date:	Wed, 10 Apr 2013 08:30:58 -0500
From:	Rob Herring <robherring2@...il.com>
To:	Colin Cross <ccross@...roid.com>
CC:	lkml <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Rob Herring <rob.herring@...xeda.com>,
	Anton Vorontsov <cbouatmailru@...il.com>,
	Kees Cook <keescook@...omium.org>,
	Tony Luck <tony.luck@...el.com>
Subject: Re: [RFC PATCH 1/3] pstore-ram: use write-combine mappings

On 04/09/2013 10:53 PM, Colin Cross wrote:
> On Tue, Apr 9, 2013 at 8:08 PM, Rob Herring <robherring2@...il.com> wrote:
>> From: Rob Herring <rob.herring@...xeda.com>
>>
>> Atomic operations are undefined behavior on ARM for device or strongly
>> ordered memory types. So use write-combine variants for mappings. This
>> corresponds to normal, non-cacheable memory on ARM. For many other
>> architectures, this change should not change the mapping type.
> 
> This is going to make ramconsole less reliable.  A debugging printk
> followed by a __raw_writel that causes an immediate hard crash is
> likely to lose the last updates, including the most useful message, in
> the write buffers.

It would have to be a write that hangs the bus. In my experience with
AXI, the bus doesn't actually hang until you hit max outstanding
transactions.

I think exclusive stores will limit the buffering, but that is probably
not architecturally guaranteed.

I could put a wb() in at the end of persistent_ram_write.

> Also, isn't this patch unnecessary after patch 3 in this set?

It is still needed in the main memory case to be architecturally correct
to avoid multiple mappings of different memory types and exclusive
accesses to device memory. At least on an A9, it doesn't really seem to
matter. I could remove this for the ioremap case.

Rob

>> Signed-off-by: Rob Herring <rob.herring@...xeda.com>
>> Cc: Anton Vorontsov <cbouatmailru@...il.com>
>> Cc: Colin Cross <ccross@...roid.com>
>> Cc: Kees Cook <keescook@...omium.org>
>> Cc: Tony Luck <tony.luck@...el.com>
>> Cc: linux-kernel@...r.kernel.org
>> ---
>>  fs/pstore/ram_core.c |    4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
>> index 0306303..e126d9f 100644
>> --- a/fs/pstore/ram_core.c
>> +++ b/fs/pstore/ram_core.c
>> @@ -337,7 +337,7 @@ static void *persistent_ram_vmap(phys_addr_t start, size_t size)
>>         page_start = start - offset_in_page(start);
>>         page_count = DIV_ROUND_UP(size + offset_in_page(start), PAGE_SIZE);
>>
>> -       prot = pgprot_noncached(PAGE_KERNEL);
>> +       prot = pgprot_writecombine(PAGE_KERNEL);
> Is this necessary?  Won't pgprot_noncached already be normal memory?
> 
>>         pages = kmalloc(sizeof(struct page *) * page_count, GFP_KERNEL);
>>         if (!pages) {
>> @@ -364,7 +364,7 @@ static void *persistent_ram_iomap(phys_addr_t start, size_t size)
>>                 return NULL;
>>         }
>>
>> -       return ioremap(start, size);
>> +       return ioremap_wc(start, size);
> 
> ioremap_wc corresponds to MT_DEVICE_WC, which is still device memory,
> so I don't see how this helps solve the problem in the commit message.
> 
>>  }
>>
>>  static int persistent_ram_buffer_map(phys_addr_t start, phys_addr_t size,
>> --
>> 1.7.10.4
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/