[Crash-utility] [PATCH] arm64: store phy_offset and memstart_addr separately

[piliu]

On 3/23/21 3:04 PM, lijiang wrote:
> Hi, Pingfan
> Thanks for the fix.
> 在 2021年03月18日 09:36, Pingfan Liu 写道:
>> With kernel commit 7bc1a0f9e176 ("arm64: mm: use single quantity to
>> represent the PA to VA translation"), memstart_addr can be negative,
>> which makes it different from real phy_offset.
>>
>> In crash utility, PTOV() needs memstart_addr, while getting PFN offset
>> in a dumpfile, phy_offset is required. So storing them separately for
>> different purpose.
>>
>> Signed-off-by: Pingfan Liu <piliu at redhat.com>
>> Cc: Lianbo Jiang <lijiang at redhat.com>
>> To: crash-utility at redhat.com
>> ---
>>   arm64.c | 22 +++++++++++++++++++---
>>   defs.h  |  1 +
>>   2 files changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/arm64.c b/arm64.c
>> index 37aed07..e560d07 100644
>> --- a/arm64.c
>> +++ b/arm64.c
>> @@ -687,6 +687,7 @@ arm64_dump_machdep_table(ulong arg)
>>   		fprintf(fp, "        kimage_voffset: %016lx\n", ms->kimage_voffset);
>>   	}
>>   	fprintf(fp, "           phys_offset: %lx\n", ms->phys_offset);
>> +	fprintf(fp, "         memstart_addr: %lx\n", ms->memstart_addr);
>>   	fprintf(fp, "__exception_text_start: %lx\n", ms->__exception_text_start);
>>   	fprintf(fp, "  __exception_text_end: %lx\n", ms->__exception_text_end);
>>   	fprintf(fp, " __irqentry_text_start: %lx\n", ms->__irqentry_text_start);
>> @@ -987,7 +988,7 @@ arm64_calc_physvirt_offset(void)
>>   	ulong physvirt_offset;
>>   	struct syment *sp;
>>   
>> -	ms->physvirt_offset = ms->phys_offset - ms->page_offset;
>> +	ms->physvirt_offset = ms->memstart_addr - ms->page_offset;
>>   
>>   	if ((sp = kernel_symbol_search("physvirt_offset")) &&
>>   			machdep->machspec->kimage_voffset) {
>> @@ -1028,7 +1029,11 @@ arm64_calc_phys_offset(void)
>>   		    ms->kimage_voffset && (sp = kernel_symbol_search("memstart_addr"))) {
>>   			if (pc->flags & PROC_KCORE) {
>>   				if ((string = pc->read_vmcoreinfo("NUMBER(PHYS_OFFSET)"))) {
>> -					ms->phys_offset = htol(string, QUIET, NULL);
>> +					ms->memstart_addr = htol(string, QUIET, NULL);
>> +					if (ms->memstart_addr < 0)
>> +						ms->phys_offset = ms->memstart_addr + 0xffff000000000000 - 0xfff0000000000000;
> 
> Is that possible to explicitly use the following ways instead of the magic number? For example:
> 
> ((UL(1) << (52))) - ((UL(1) << (48))) or ((UL(1) << (VA_BITS))) something?
> 
Yes. I will update with V2.

Thanks,
Pingfan

> Thanks.
> Lianbo
> 
>> +					else
>> +						ms->phys_offset = ms->memstart_addr;
>>   					free(string);
>>   					return;
>>   				}
>> @@ -1080,7 +1085,18 @@ arm64_calc_phys_offset(void)
>>   	} else if (DISKDUMP_DUMPFILE() && diskdump_phys_base(&phys_offset)) {
>>   		ms->phys_offset = phys_offset;
>>   	} else if (KDUMP_DUMPFILE() && arm64_kdump_phys_base(&phys_offset)) {
>> -		ms->phys_offset = phys_offset;
>> +		/*
>> +		 * When running a 52bits kernel on 48bits hardware. Kernel plays a trick:
>> +		 * if (IS_ENABLED(CONFIG_ARM64_VA_BITS_52) && (vabits_actual != 52))
>> +                 *       memstart_addr -= _PAGE_OFFSET(48) - _PAGE_OFFSET(52);
>> +		 *
>> +		 * In crash, this should be detected to get a real physical start address.
>> +		 */
>> +		ms->memstart_addr = phys_offset;
>> +		if ((long)phys_offset < 0)
>> +			ms->phys_offset = phys_offset + 0xffff000000000000 - 0xfff0000000000000;
>> +		else
>> +			ms->phys_offset = phys_offset;
>>   	} else {
>>   		error(WARNING,
>>   			"phys_offset cannot be determined from the dumpfile.\n");
>> diff --git a/defs.h b/defs.h
>> index 35b983a..64f2bcb 100644
>> --- a/defs.h
>> +++ b/defs.h
>> @@ -3290,6 +3290,7 @@ struct machine_specific {
>>   	ulong modules_vaddr;
>>   	ulong modules_end;
>>   	ulong phys_offset;
>> +	long memstart_addr;
>>   	ulong __exception_text_start;
>>   	ulong __exception_text_end;
>>   	struct arm64_pt_regs *panic_task_regs;
>>