powerpc/crashdump : fix page frame number check in copy_oldmem_page

In copy_oldmem_page, the current check using max_pfn and min_low_pfn to
decide if the page is backed or not, is not valid when the memory layout is
not continuous.

This happens when running as a QEMU/KVM guest, where RTAS is mapped higher
in the memory. In that case max_pfn points to the end of RTAS, and a hole
between the end of the kdump kernel and RTAS is not backed by PTEs. As a
consequence, the kdump kernel is crashing in copy_oldmem_page when accessing
in a direct way the pages in that hole.

This fix relies on the memblock's service memblock_is_region_memory to
check if the read page is part or not of the directly accessible memory.

Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/crash_dump.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

On Mon, 2014-02-24 at 17:30 +0100, Laurent Dufour wrote:
> In copy_oldmem_page, the current check using max_pfn and min_low_pfn to
> decide if the page is backed or not, is not valid when the memory layout is
> not continuous.
> 
> This happens when running as a QEMU/KVM guest, where RTAS is mapped higher
> in the memory. In that case max_pfn points to the end of RTAS, and a hole
> between the end of the kdump kernel and RTAS is not backed by PTEs. As a
> consequence, the kdump kernel is crashing in copy_oldmem_page when accessing
> in a direct way the pages in that hole.
> 
> This fix relies on the memblock's service memblock_is_region_memory to
> check if the read page is part or not of the directly accessible memory.

Hi Laurent,

This looks good to me, assuming you've tested it on a PowerVM system as well as
under KVM.

cheers

On 25/02/2014 02:47, Michael Ellerman wrote:
> On Mon, 2014-02-24 at 17:30 +0100, Laurent Dufour wrote:
>> In copy_oldmem_page, the current check using max_pfn and min_low_pfn to
>> decide if the page is backed or not, is not valid when the memory layout is
>> not continuous.
>>
>> This happens when running as a QEMU/KVM guest, where RTAS is mapped higher
>> in the memory. In that case max_pfn points to the end of RTAS, and a hole
>> between the end of the kdump kernel and RTAS is not backed by PTEs. As a
>> consequence, the kdump kernel is crashing in copy_oldmem_page when accessing
>> in a direct way the pages in that hole.
>>
>> This fix relies on the memblock's service memblock_is_region_memory to
>> check if the read page is part or not of the directly accessible memory.
> 
> Hi Laurent,
> 
> This looks good to me, assuming you've tested it on a PowerVM system as well as
> under KVM.

Hi Michael,

Yes I tested it on PowerVM (BE), KVM (BE) and Qemu TCG (BE).

Cheers,
Laurent.

On 2014-02-24 17:30:55 Mon, Laurent Dufour wrote:
> In copy_oldmem_page, the current check using max_pfn and min_low_pfn to
> decide if the page is backed or not, is not valid when the memory layout is
> not continuous.
> 
> This happens when running as a QEMU/KVM guest, where RTAS is mapped higher
> in the memory. In that case max_pfn points to the end of RTAS, and a hole
> between the end of the kdump kernel and RTAS is not backed by PTEs. As a
> consequence, the kdump kernel is crashing in copy_oldmem_page when accessing
> in a direct way the pages in that hole.
> 
> This fix relies on the memblock's service memblock_is_region_memory to
> check if the read page is part or not of the directly accessible memory.
> 
> Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>

Tested on PowerNV (BE), where without this patch we see cp and 
makedumpfile fails with "Bad address" while reading /proc/vmcore.
With this patch makedumpfile and cp succeeds.

Tested-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>

> ---
>  arch/powerpc/kernel/crash_dump.c |    8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/crash_dump.c b/arch/powerpc/kernel/crash_dump.c
> index 11c1d06..7a13f37 100644
> --- a/arch/powerpc/kernel/crash_dump.c
> +++ b/arch/powerpc/kernel/crash_dump.c
> @@ -98,17 +98,19 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf,
>  			size_t csize, unsigned long offset, int userbuf)
>  {
>  	void  *vaddr;
> +	phys_addr_t paddr;
>  
>  	if (!csize)
>  		return 0;
>  
>  	csize = min_t(size_t, csize, PAGE_SIZE);
> +	paddr = pfn << PAGE_SHIFT;
>  
> -	if ((min_low_pfn < pfn) && (pfn < max_pfn)) {
> -		vaddr = __va(pfn << PAGE_SHIFT);
> +	if (memblock_is_region_memory(paddr, csize)) {
> +		vaddr = __va(paddr);
>  		csize = copy_oldmem_vaddr(vaddr, buf, csize, offset, userbuf);
>  	} else {
> -		vaddr = __ioremap(pfn << PAGE_SHIFT, PAGE_SIZE, 0);
> +		vaddr = __ioremap(paddr, PAGE_SIZE, 0);
>  		csize = copy_oldmem_vaddr(vaddr, buf, csize, offset, userbuf);
>  		iounmap(vaddr);
>  	}
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev