linux-kernel - Re: [PATCH] mtd: bcm47xxsflash: use ioremap

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: <alpine.DEB.2.00.1602261411591.15885@tp.orcam.me.uk>
Date:	Fri, 26 Feb 2016 14:18:39 +0000
From:	"Maciej W. Rozycki" <macro@...tec.com>
To:	Rafał Miłecki <zajec5@...il.com>
CC:	Brian Norris <computersforpeace@...il.com>,
	"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
	Javier Martinez Canillas <javier@....samsung.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Fengguang Wu <fengguang.wu@...el.com>,
	Michael Ellerman <mpe@...erman.id.au>,
	Luis de Bethencourt <luisbg@....samsung.com>,
	Jeremy Kerr <jk@...abs.org>,
	Neelesh Gupta <neelegup@...ux.vnet.ibm.com>,
	David Woodhouse <dwmw2@...radead.org>,
	Cyril Bur <cyrilbur@...il.com>,
	Ralf Baechle <ralf@...ux-mips.org>
Subject: Re: [PATCH] mtd: bcm47xxsflash: use ioremap_cachable() instead of
 KSEG0ADDR()

On Fri, 26 Feb 2016, Rafał Miłecki wrote:

> >> KSEG0ADDR was translating 0x1c000000 into 0x9c000000. With
> >> ioremap_cachable we use MIPS's __ioremap (and remap_area_pages). This
> >> results in different address (e.g. 0xc0080000) but it still should be
> >> cached as expected and it was successfully tested with BCM47186B0.
> >
> >  This is due to this piece:
> >
> >         /*
> >          * Map uncached objects in the low 512mb of address space using KSEG1,
> >          * otherwise map using page tables.
> >          */
> >         if (IS_LOW512(phys_addr) && IS_LOW512(last_addr) &&
> >             flags == _CACHE_UNCACHED)
> >                 return (void __iomem *) CKSEG1ADDR(phys_addr);
> >
> > special-casing uncached mapping only (replicated in 2 places).  I think
> > there will really be no harm from returning a KSEG0 mapping for calls
> > requesting a caching mode equal to `_page_cachable_default', which --
> > depending on the cache architecture -- will have been either hardwired or
> > prearranged via Config.K0.  I think there's really no need to put pressure
> > on the TLB, which may be small, in cases where a fixed mapping will do.
> 
> No, it isn't hitting condition you pointed. We call ioremap_cachable
> which uses _page_cachable_default as a flag. This flag
> (_page_cachable_default) isn't equal to the _CACHE_UNCACHED.

 That's exactly what I wrote: code I quoted is "special-casing uncached 
mapping only" -- which as you have correctly observed does not apply after 
your change anymore.  Which is why previously you got an address in the 
unmapped KSEG1 segment and now you get an address in the mapped KSEG2 
rather than the unmapped KSEG0 segment.

> Moreover code you pointed uses CKSEG1ADDR which would result in
> setting bit KSEG1 (0xa0000000). As I pointed in the commit message
> address it ORed with KSEG2 (0xc0000000).

 It's not merely ORed, it's actually mapped via the TLB.

 I hope this makes things clear.

  Maciej