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

[Qi Zheng]

On 12/16/21 3:01 PM, HAGIO KAZUHITO(萩尾 一仁) wrote:
> -----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.

I sent the patch v2. But I missed marking v2 on the subject title,
do I need to resend it?

Thanks,
Qi

> 
> Thanks,
> Kazu
> 
> 

-- 
Thanks,
Qi