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:
>> On Wed, Sep 20, 2023 at 05:45:36PM +0530, Aneesh Kumar K.V wrote:
>>> 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.
>>> 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?
>>>
>>
>> I could not find another way to get any other flag for RADIX vs HASH in
>> makedumpfile. And currently I don't know of any other way.
>>
>> Both makedumpfile and crash look for '0x40' flag set in
>> 'cur_cpu_spec.mmu_features', so only requirement for 'MMU_FTR_TYPE_RADIX' is to
>> be '0x40', or we will need to change the value accordingly in the tools.
>
> Instead of exporting cur_cpu_spec.mmu_feature, you could do
> coreinfo_mmu_features that does
>
> if (radix_enabled())
>    coreinfo_mmu_feature = VMCORE_INFO_RADIX_TRANSLATION;

On other arches they seem to use vmcoreinfo_append_str() for more things
than we do on powerpc.

eg. x86:

       vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n", pgtable_l5_enabled());

Could we do something like that instead:

       vmcoreinfo_append_str("RADIX_MMU=%d\n", early_radix_enabled());

cheers