Re: [PATCH v2 1/2] powerpc: add `cur_cpu_spec` symbol to vmcoreinfo

[Michael Ellerman <mpe@ellerman.id.au>]

"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> Aditya Gupta <adityag@linux.ibm.com> writes:
>
>> Since below commit, address mapping for vmemmap has changed for Radix
>> MMU, where address mapping is stored in kernel page table itself,
>> instead of earlier used 'vmemmap_list'.
>>
>>     commit 368a0590d954 ("powerpc/book3s64/vmemmap: switch radix to use
>>     a different vmemmap handling function")
>>
>> Hence with upstream kernel, in case of Radix MMU, makedumpfile fails to do
>> address translation for vmemmap addresses, as it depended on vmemmap_list,
>> which can now be empty.
>>
>> While fixing the address translation in makedumpfile, it was identified
>> that currently makedumpfile cannot distinguish between Hash MMU and
>> Radix MMU, unless VMLINUX is passed with -x flag to makedumpfile.
>> And hence fails to assign offsets and shifts correctly (such as in L4 to
>> PGDIR offset calculation in makedumpfile).
>>
>> For getting the MMU, makedumpfile uses `cur_cpu_spec.mmu_features`.
>>
>> Add `cur_cpu_spec` symbol and offset of `mmu_features` in the
>> `cpu_spec` struct, to VMCOREINFO, so that makedumpfile can assign the
>> offsets correctly, without needing a VMLINUX.
>>
>> Fixes: 368a0590d954 ("powerpc/book3s64/vmemmap: switch radix to use a different vmemmap handling function")
>> Reported-by: Sachin Sant <sachinp@linux.ibm.com>
>> Tested-by: Sachin Sant <sachinp@linux.ibm.com>
>> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
>>
>> ---
>> Corresponding makedumpfile patches to fix address translation, in Radix
>> MMU case:
>>
>> Link: https://lore.kernel.org/kexec/B5F0F00E-F2B1-47D7-A143-5683D10DC29A@linux.ibm.com/T/#t
>> ---
>> ---
>>  arch/powerpc/kexec/core.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/powerpc/kexec/core.c b/arch/powerpc/kexec/core.c
>> index de64c7962991..369b8334a4f0 100644
>> --- a/arch/powerpc/kexec/core.c
>> +++ b/arch/powerpc/kexec/core.c
>> @@ -63,6 +63,8 @@ void arch_crash_save_vmcoreinfo(void)
>>  #ifndef CONFIG_NUMA
>>  	VMCOREINFO_SYMBOL(contig_page_data);
>>  #endif
>> +	VMCOREINFO_SYMBOL(cur_cpu_spec);
>> +	VMCOREINFO_OFFSET(cpu_spec, mmu_features);
>>  #if defined(CONFIG_PPC64) && defined(CONFIG_SPARSEMEM_VMEMMAP)
>>  	VMCOREINFO_SYMBOL(vmemmap_list);
>>  	VMCOREINFO_SYMBOL(mmu_vmemmap_psize);
>>
>
> That implies we now have to be careful when updating MMU_FTR_* #defines.
 
Yeah, that's a good point.

> It is not bad considering other hacks we do in crash to identify kernel
> changes tied to version number. But i am wondering if there another way
> to identify radix vs hash?

Ultimately crash just has to accept that the kernel will change over
time, and some times crash will have to adapt.

We can try not to change values unnecessarily, but given the knowledge
of kernel internals crash has there will be breakage from time to time.

The only other alternative I think is to create a well defined data
structure that is explicitly provided by the kernel for crash with
various information in a set format. With a committment that the data
structure will be maintained in a forward-compatible manner. Which
sounds like a bunch of work :)

cheers