Re: ioremap_page_range: remapping of physical RAM ranges

[John Hubbard <jhubbard@nvidia.com>]

On 01/25/2017 03:15 PM, Ahmed Samy wrote:
> On Wed, Jan 25, 2017 at 02:27:27PM -0800, John Hubbard wrote:
>>
>> Hi A. Samy,
>>
>> I'm sorry this caught you by surprise, let's try get your use case covered.
>>
>> My thinking on this was: the exported ioremap* family of functions was
>> clearly intended to provide just what the name says: mapping of IO (non-RAM)
>> memory. If normal RAM is to be re-mapped, then it should not be done
>> "casually" in a driver, as a (possibly unintended) side effect of a function
>> that implies otherwise. Either it should be done within the core mm code, or
>> perhaps a new, better-named wrapper could be provided, for cases such as
>> yours.
> Hi John,
>
> I agree.  I assume whoever exported it was also doing it for the same
> purpose as mine[?]
>>
>> After a very quick peek at your github code, it seems that your mm_remap()
>> routine already has some code in common with __ioremap_caller(), so I'm
>> thinking that we could basically promote your mm_remap to the in-tree kernel
>> and EXPORT it, and maybe factor out the common parts (or not--it's small,
>> after all). Thoughts? If you like it, I'll put something together here.
> That'd be a good solution, it's actually sometimes useful to remap physical
> ram in general, specifically for memory imaging tools, etc.
>
> How about also exporting walk_system_ram_range()?  It seems to be defined
> conditionally, so I am not sure if that would be a good idea.

That routine has an interesting history. At first glance, I think it used to be 
exported. And now it is not. And it's ifdef'd out only for powerpc. I'll look into 
the history and intentions of that some more...

> 	[ See also mm_cache_ram_ranges() in mm.c in github – it's also a hacky
> 	  way to get RAM ranges.  ]

Yes, I see.

>
> How about something like:
>
> 	/* vm_flags incase locking is required, in my case, I need it for VMX
> 	 * root where there is no interrupts.  */
> 	void *remap_ram_range(unsigned long phys, unsigned long size,
> 			      unsigned long vm_flags)
> 	{
> 		struct vm_struct *area;
> 		unsigned long psize;
> 		unsigned long vaddr;
>
> 		psize = (size >> PAGE_SHIFT) + (size & (PAGE_SIZE - 1)) != 0;
> 		area = get_vm_area_caller(size, VM_IOREMAP | vm_flags,
> 					  __builtin_return_address(0));
> 		if (!area)
> 			return NULL;
>
> 		area->phys_addr = phys & ~(PAGE_SIZE - 1);
> 		vaddr = (unsigned long)area->addr;
> 		if (remap_page_range(vaddr, vaddr + size, phys, size))

That's ioremap_page_range, I assume (rather than remap_page_range)?

Overall, the remap_ram_range approach looks reasonable to me so far. I'll look into 
the details tomorrow.

I'm sure that most people on this list already know this, but...could you say a few 
more words about how remapping system ram is used, why it's a good thing and not a 
bad thing? :)

thanks
john h

> 			goto err_remap;
>
> 		return (void *)vaddr + phys & (PAGE_SIZE - 1);
> err_remap:
> 		free_vm_area(area);
> 		return NULL;
> 	}
>
> Of course you can add protection, etc.
>>
>> thanks
>> john h
>>
> Thanks,
> 	asamy
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>