Re: [PATCH] x86/mm/init: respect memblock reserved regions when destroying mappings

[Jeremy Fitzhardinge <jeremy@goop.org>]

On 01/31/2011 07:18 AM, Stefano Stabellini wrote:
> x86/mm/init: respect memblock reserved regions when destroying mappings
>
> In init_memory_mapping we are destroying all the mappings between
> _brk_end and _end, no matter if some memory areas in that range have
> been reserved using memblock_x86_reserve_range.

What's reserving things in that range?  How do they get addresses
there?  I would have thought they should all be brk-allocated?

> Besides if _end is not pmd aligned we might destroy the
> mappings for valid memory between _end and the following pmd.

Yes, that could be messy.

    J

> In order to avoid this problem, before clearing any pmds we check if the
> corresponding memory area has been reserved and we only destroy the
> mapping if it hasn't.
>
> We found this problem because under Xen we have a valid mapping at _end,
> and if _end is not pmd aligned the current code destroys the initial
> part of it.
>
> In practice this fix does not have any impact on native.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>
> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
> index 947f42a..66637bd 100644
> --- a/arch/x86/mm/init.c
> +++ b/arch/x86/mm/init.c
> @@ -283,6 +283,8 @@ unsigned long __init_refok init_memory_mapping(unsigned long start,
>  	if (!after_bootmem && !start) {
>  		pud_t *pud;
>  		pmd_t *pmd;
> +		unsigned long addr;
> +		u64 size, memblock_addr;
>  
>  		mmu_cr4_features = read_cr4();
>  
> @@ -291,11 +293,18 @@ unsigned long __init_refok init_memory_mapping(unsigned long start,
>  		 * located on different 2M pages. cleanup_highmap(), however,
>  		 * can only consider _end when it runs, so destroy any
>  		 * mappings beyond _brk_end here.
> +		 * Respect memblock reserved regions.
>  		 */
>  		pud = pud_offset(pgd_offset_k(_brk_end), _brk_end);
>  		pmd = pmd_offset(pud, _brk_end - 1);
> -		while (++pmd <= pmd_offset(pud, (unsigned long)_end - 1))
> -			pmd_clear(pmd);
> +		addr = (_brk_end + PMD_SIZE - 1) & PMD_MASK;
> +		while (++pmd <= pmd_offset(pud, (unsigned long)_end - 1)) {
> +			memblock_addr = memblock_x86_find_in_range_size(__pa(addr),
> +					&size, PMD_SIZE);
> +			if (memblock_addr == (u64) __pa(addr) && size >= PMD_SIZE)
> +				pmd_clear(pmd);
> +			addr += PMD_SIZE;
> +		}
>  	}
>  #endif
>  	__flush_tlb_all();