[Crash-utility] [PATCH] Fix pvops Xen detection for arm machine

[HAGIO KAZUHITO(萩尾 一仁)]

-----Original Message-----
> On 12/16/21 2:36 PM, Qi Zheng wrote:
> >
> >
> > On 12/15/21 3:42 PM, HAGIO KAZUHITO(萩尾 一仁) wrote:
> >> Hi Qi, Xen folks,
> >>
> >> -----Original Message-----
> >>> Since the xen_start_info on the arm/arm64 platform is static defined:
> >>>
> >>> ./arm/xen/enlighten.c:40:static struct start_info _xen_start_info;
> >>> ./arm/xen/enlighten.c:41:struct start_info *xen_start_info =
> >>> &_xen_start_info;
> >>> ./arm/xen/enlighten.c:42:EXPORT_SYMBOL(xen_start_info);
> >>>
> >>> The is_pvops_xen() in commit 4badc6229c69f5cd9da7eb7bdf400a53ec6db01a
> >>> ("Fix pvops Xen detection for kernels >= v4.20") always return TRUE.
> >>> Then the following error will be reported because p2m_mid_missing
> >>> and xen_p2m_addr are not defined:
> >>>
> >>>     crash: cannot resolve "p2m_top"
> >>>
> >>> Fix it by using xen_vcpu_id instead of xen_start_info to detect Xen
> >>> dumps.
> >>>
> >>> Signed-off-by: Qi Zheng <zhengqi.arch at bytedance.com>
> >>> ---
> >>>   kernel.c | 21 ++++++++++++++++-----
> >>>   1 file changed, 16 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/kernel.c b/kernel.c
> >>> index f4598ea..b216fbe 100644
> >>> --- a/kernel.c
> >>> +++ b/kernel.c
> >>> @@ -10745,6 +10745,9 @@ is_pvops_xen(void)
> >>>   {
> >>>       ulong addr;
> >>>       char *sym;
> >>> +    struct syment * sp;
> >>> +    ulong ptr;
> >>> +    int i;
> >>>
> >>>       if (!PVOPS())
> >>>           return FALSE;
> >>> @@ -10757,11 +10760,19 @@ is_pvops_xen(void)
> >>>            STREQ(sym, "paravirt_patch_default")))
> >>>           return TRUE;
> >>>
> >>> -    if (symbol_exists("xen_start_info") &&
> >>> -        readmem(symbol_value("xen_start_info"), KVADDR, &addr,
> >>> -        sizeof(void *), "xen_start_info", RETURN_ON_ERROR) &&
> >>> -        addr != 0)
> >>> -        return TRUE;
> >>> +    sp = per_cpu_symbol_search("xen_vcpu_id");
> >>> +    if (sp) {
> >>> +        for (i = 0; i < NR_CPUS; i++) {
> >>> +            if (!kt->__per_cpu_offset[i])
> >>> +                continue;
> >>
> >> Thanks for the patch.
> >>
> >> But seems that kt->__per_cpu_offset is initialized after the
> >> is_pvops_xen()
> >> call.  So this block always return FALSE?
> >>
> >> Is there any other variable that we can use?  or another solution..
> >
> > Or use xen_vcpu_info for arm/arm64 machine and xen_start_info for x86
> > machine? Both of these variables are assigned during the initialization
> > path.
> 
> Like this:
> 
> diff --git a/kernel.c b/kernel.c
> index f4598ea..4b4d789 100644
> --- a/kernel.c
> +++ b/kernel.c
> @@ -10757,11 +10757,18 @@ is_pvops_xen(void)
>   	     STREQ(sym, "paravirt_patch_default")))
>   		return TRUE;
> 
> -	if (symbol_exists("xen_start_info") &&
> -	    readmem(symbol_value("xen_start_info"), KVADDR, &addr,
> -	    sizeof(void *), "xen_start_info", RETURN_ON_ERROR) &&
> -	    addr != 0)
> +	if (machine_type("ARM") || machine_type("ARM64")) {
> +		if (symbol_exists("xen_vcpu_info") &&
> +		    readmem(symbol_value("xen_vcpu_info"), KVADDR, &addr,
> +		    sizeof(void *), "xen_vcpu_info", RETURN_ON_ERROR) &&
> +		    addr != 0)
> +			return TRUE;
> +	} else if (symbol_exists("xen_start_info") &&
> +		   readmem(symbol_value("xen_start_info"), KVADDR, &addr,
> +		   sizeof(void *), "xen_start_info", RETURN_ON_ERROR) &&
> +		   addr != 0) {
>   		return TRUE;
> +	}
> 
>   	return FALSE;
>   }
> 
> If it make sense, I can send the next version.

Nice, looks good to me.

Thanks,
Kazu