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

[Qi Zheng]

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.

Thanks,
Qi

> 
> Thanks,
> Qi
> 
>>
>> Thanks,
>> Kazu
>>
>>> +
>>> +            ptr = sp->value + kt->__per_cpu_offset[i];
>>> +            if (ptr && readmem(ptr, KVADDR, &ptr, sizeof(uint32_t),
>>> +                "xen_vcpu_id", FAULT_ON_ERROR))
>>> +                return TRUE;
>>> +            return FALSE;
>>> +        }
>>> +    }
>>>
>>>       return FALSE;
>>>   }
>>> -- 
>>> 2.11.0
> 

-- 
Thanks,
Qi