arm64 ioremap question

arm64 ioremap question

[Mark Salter]

ioremap() has a test to prevent RAM from being remapped:

	/*
	 * Don't allow RAM to be mapped.
	 */
	if (WARN_ON(pfn_valid(__phys_to_pfn(phys_addr))))
		return NULL;


Is this really necessary even for reserved pages which are not being
used for anything else?

--Mark

arm64 ioremap question

[Russell King - ARM Linux]

On Tue, Oct 15, 2013 at 11:20:04AM -0400, Mark Salter wrote:
> ioremap() has a test to prevent RAM from being remapped:
> 
> 	/*
> 	 * Don't allow RAM to be mapped.
> 	 */
> 	if (WARN_ON(pfn_valid(__phys_to_pfn(phys_addr))))
> 		return NULL;
> 
> 
> Is this really necessary even for reserved pages which are not being
> used for anything else?

It helps to stop the abuse of using ioremap() on normal memory, which has
happened soo much in Aarch32 for years.  Unfortuantely, bad habbits die
hard.

"Reserved pages not being used for anything else" are still mapped, and
still subject to speculative fetches.

arm64 ioremap question

[Catalin Marinas]

On Tue, Oct 15, 2013 at 04:52:16PM +0100, Russell King - ARM Linux wrote:
> On Tue, Oct 15, 2013 at 11:20:04AM -0400, Mark Salter wrote:
> > ioremap() has a test to prevent RAM from being remapped:
> > 
> > 	/*
> > 	 * Don't allow RAM to be mapped.
> > 	 */
> > 	if (WARN_ON(pfn_valid(__phys_to_pfn(phys_addr))))
> > 		return NULL;
> > 
> > 
> > Is this really necessary even for reserved pages which are not being
> > used for anything else?
> 
> It helps to stop the abuse of using ioremap() on normal memory, which has
> happened soo much in Aarch32 for years.  Unfortuantely, bad habbits die
> hard.
> 
> "Reserved pages not being used for anything else" are still mapped, and
> still subject to speculative fetches.

Indeed. The architecture does not guarantee the semantics of the
ioremap-returned memory (e.g. Device) as long as there are other aliases
with different memory attributes.

-- 
Catalin

arm64 ioremap question

[Lucas Stach]

Am Dienstag, den 15.10.2013, 16:52 +0100 schrieb Russell King - ARM
Linux:
> On Tue, Oct 15, 2013 at 11:20:04AM -0400, Mark Salter wrote:
> > ioremap() has a test to prevent RAM from being remapped:
> > 
> > 	/*
> > 	 * Don't allow RAM to be mapped.
> > 	 */
> > 	if (WARN_ON(pfn_valid(__phys_to_pfn(phys_addr))))
> > 		return NULL;
> > 
> > 
> > Is this really necessary even for reserved pages which are not being
> > used for anything else?
> 
> It helps to stop the abuse of using ioremap() on normal memory, which has
> happened soo much in Aarch32 for years.  Unfortuantely, bad habbits die
> hard.
> 
> "Reserved pages not being used for anything else" are still mapped, and
> still subject to speculative fetches.

Only slightly related to the issue here, but I'm really wondering if the
speculative fill can happen on real implementations.
I'm well aware that there is nothing in the arch preventing a
speculative fill on a cached alias, but at least for the A9
implementation the ARM infocenter claims that the prefetcher will not
cross a 4KB boundary [1].
So as long as we are talking about full pages, having a conflicting
mapping should not cause any issues as long as you don't touch the
memory region explicitly through the cached mapping.

I know that we generally aim to avoid basing any kernel infrastructure
on such vague guarantees, but I'm really wondering if my analysis is
correct or is there anything I'm overlooking here?

Regards,
Lucas

[1]
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0388i/CBBIAAAA.html
-- 
Pengutronix e.K.                           | Lucas Stach                 |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

arm64 ioremap question

[Russell King - ARM Linux]

On Tue, Oct 15, 2013 at 06:14:12PM +0200, Lucas Stach wrote:
> Am Dienstag, den 15.10.2013, 16:52 +0100 schrieb Russell King - ARM
> Linux:
> > On Tue, Oct 15, 2013 at 11:20:04AM -0400, Mark Salter wrote:
> > > ioremap() has a test to prevent RAM from being remapped:
> > > 
> > > 	/*
> > > 	 * Don't allow RAM to be mapped.
> > > 	 */
> > > 	if (WARN_ON(pfn_valid(__phys_to_pfn(phys_addr))))
> > > 		return NULL;
> > > 
> > > 
> > > Is this really necessary even for reserved pages which are not being
> > > used for anything else?
> > 
> > It helps to stop the abuse of using ioremap() on normal memory, which has
> > happened soo much in Aarch32 for years.  Unfortuantely, bad habbits die
> > hard.
> > 
> > "Reserved pages not being used for anything else" are still mapped, and
> > still subject to speculative fetches.
> 
> Only slightly related to the issue here, but I'm really wondering if the
> speculative fill can happen on real implementations.
> I'm well aware that there is nothing in the arch preventing a
> speculative fill on a cached alias, but at least for the A9
> implementation the ARM infocenter claims that the prefetcher will not
> cross a 4KB boundary [1].
> So as long as we are talking about full pages, having a conflicting
> mapping should not cause any issues as long as you don't touch the
> memory region explicitly through the cached mapping.
> 
> I know that we generally aim to avoid basing any kernel infrastructure
> on such vague guarantees, but I'm really wondering if my analysis is
> correct or is there anything I'm overlooking here?

And I know where this is leading so frankly I'm not answering.  I don't
want to see any more of people commenting out code like this in their
vendor kernels because they don't care.  Sorry if I'm being unhelpful
but I have experience of what happens here.