[Crash-utility] [PATCH 01/16] Add MIPS64 framework code support

Youling Tang tangyouling at loongson.cn
Sat Mar 13 04:12:53 UTC 2021



On 03/12/2021 04:36 PM, HAGIO KAZUHITO(萩尾 一仁) wrote:
> -----Original Message-----
>> -----Original Message-----
>>> Hi Kazu,
>>>
>>> On 03/12/2021 12:21 PM, HAGIO KAZUHITO(萩尾 一仁) wrote:
>>>
>>>> -----Original Message-----
>>>>>>> --- a/diskdump.c
>>>>>>> +++ b/diskdump.c
>>>>>>> @@ -594,6 +594,9 @@ restart:
>>>>>>>     	else if (STRNEQ(header->utsname.machine, "mips") &&
>>>>>>>     	    machine_type_mismatch(file, "MIPS", NULL, 0))
>>>>>>>     		goto err;
>>>>>>> +	else if (STRNEQ(header->utsname.machine, "mips64") &&
>>>>>>> +	    machine_type_mismatch(file, "MIPS", NULL, 0))
>>>>>>> +		goto err;
>>>>>> Why do you make MACHINE_TYPE the same as the MIPS one?
>>>>>> With this, doesn't a MIPS64 crash match a MIPS vmcore?
>>>>> The value of the machine type e_machine in mips32 or mips64 is MIPS, which
>>>>> corresponds to EM_MIPS.
>>>>> The definition in gdb-7.6/include/elf/common.h:110 is as follows:
>>>>>        #define EM_MIPS        8        /* MIPS R3000 */
>>>>> But there is no related definition of EM_MIPS64 or other mips64, so both
>>>>> mips32 and mips64 should use EM_MIPS, and the corresponding e_machine is
>>>>> MIPS.
>>>>>
>>>>> If the MACHINE_TYPE of mips64 is defined as MIPS64, as follows:
>>>>>        define MACHINE_TYPE     "MIPS64"
>>>>>
>>>>> The following error will appear when running crash:
>>>>>        WARNING: machine type mismatch:
>>>>>                  crash utility: MIPS64
>>>>>                  vmcore: MIPS
>>>> Then, is there any problem with this?
>>>>     machine_type_mismatch(file, "MIPS64", NULL, 0))
>>>>
>>>> This can prevent a mips64 crash from trying to open a mips32 vmcore
>>>> and the reverse.
>>> Read the ELF header information of vmocre (whether it is mips32 or mips64),
>>> and the obtained machine type is MIPS, not MIPS64. In mips64, if you use
>>> machine_type_mismatch(file, "MIPS64", NULL, 0)), there will be a machine
>>> type
>>> mismatch. You can only use machine_type_mismatch(file, "MIPS", NULL, 0)),
>>> only then It can be successfully matched to the machine type.
>> Probably I misunderstand something, but then need to know it..
>>
>> The machine_type_mismatch() doesn't use the ELF's EM_MIPS value.
>> Why do we need to match crash's MACHINE_TYPE with ELF's EM_MIPS?
>>
>> int
>> machine_type(char *type)
>> {
>>          return STREQ(MACHINE_TYPE, type);
>> }
>>
>> int
>> machine_type_mismatch(char *file, char *e_machine, char *alt, ulong query)
>> {
>>          if (machine_type(e_machine) || machine_type(alt))
>>                  return FALSE;
>>
>>          if (query == KDUMP_LOCAL)  /* already printed by NETDUMP_LOCAL */
>>                  return TRUE;
>>
>>          error(WARNING, "machine type mismatch:\n");
>>
>>          fprintf(fp, "         crash utility: %s\n", MACHINE_TYPE);
>>          fprintf(fp, "         %s: %s%s%s\n\n", file, e_machine,
>>                  alt ? " or " : "", alt ? alt : "");
>>
>>          return TRUE;
>> }
>>
>> Without the following hunk, EM_MIPS has been used only for ELF32 in symbols.c.
>>
>> --- a/symbols.c
>> +++ b/symbols.c
>> @@ -3636,6 +3636,11 @@ is_kernel(char *file)
>>                                  goto bailout;
>>                          break;
>>
>> +               case EM_MIPS:
>> +                       if (machine_type_mismatch(file, "MIPS", NULL, 0))
>> +                               goto bailout;
>> +                       break;
>> +
>>                  default:
>>                          if (machine_type_mismatch(file, "(unknown)", NULL, 0))
>>                                  goto bailout;
>> @@ -3890,6 +3895,11 @@ is_shared_object(char *file)
>>                          if (machine_type("SPARC64"))
>>                                  return TRUE;
>>                          break;
>> +
>> +               case EM_MIPS:
>> +                       if (machine_type("MIPS"))
>> +                               return TRUE;
>> +                       break;
>>                  }
>>
>> So I thought that meybe we can also change these ELF64 cases to "MIPS64"
>> with defining MACHINE_TYPE as "MIPS64".
>>
>> This is wrong?  e.g. mips32 also can use an ELF64 kernel/module binary?
> If there is such a case, maybe we can also use these?:
>    machine_type_mismatch(file, "MIPS", "MIPS64", 0)
>    machine_type("MIPS") || machine_type("MIPS64")
>
> I'm thinking that as we define the values/functions for mips64 separately
> from mips32, it would be reasonable to make crash's MACHINE_TYPE different
> from mips32 as well, and we can remove some unusual ifdefs.
Based on the original patch, make modifications similar to the following:

diff --git a/defs.h b/defs.h
index dcc58b8..e1aa0fa 100644
--- a/defs.h
+++ b/defs.h
@@ -6376,7 +6376,7 @@ void s390x_dump_machdep_table(ulong);
  #endif

  #ifdef __mips__ /* MIPS & MIPS64 */
-#define MACHINE_TYPE   "MIPS"
+#define MACHINE_TYPE   "MIPS64"
  #endif

  /*
diff --git a/diskdump.c b/diskdump.c
index b4712ad..9e0fd89 100644
--- a/diskdump.c
+++ b/diskdump.c
@@ -595,7 +595,7 @@ restart:
             machine_type_mismatch(file, "MIPS", NULL, 0))
                 goto err;
         else if (STRNEQ(header->utsname.machine, "mips64") &&
-           machine_type_mismatch(file, "MIPS", NULL, 0))
+           machine_type_mismatch(file, "MIPS", "MIPS64", 0))
                 goto err;
         else if (STRNEQ(header->utsname.machine, "s390x") &&
             machine_type_mismatch(file, "S390X", NULL, 0))
@@ -754,7 +754,7 @@ restart:

         if (machine_type("ARM"))
                 dd->machine_type = EM_ARM;
-       else if (machine_type("MIPS"))
+       else if (machine_type("MIPS") || machine_type("MIPS64"))
                 dd->machine_type = EM_MIPS;
         else if (machine_type("X86"))
                 dd->machine_type = EM_386;
diff --git a/netdump.c b/netdump.c
index 2ca39e2..ef2f241 100644
--- a/netdump.c
+++ b/netdump.c
@@ -290,7 +290,7 @@ is_netdump(char *file, ulong source_query)
                         break;

                 case EM_MIPS:
-                       if (machine_type_mismatch(file, "MIPS", NULL,
+                       if (machine_type_mismatch(file, "MIPS", "MIPS64",
                             source_query))
                                 goto bailout;
                         break;
diff --git a/symbols.c b/symbols.c
index b30b92c..97b3bdc 100644
--- a/symbols.c
+++ b/symbols.c
@@ -3634,7 +3634,7 @@ is_kernel(char *file)
                         break;

                 case EM_MIPS:
-                       if (machine_type_mismatch(file, "MIPS", NULL, 0))
+                       if (machine_type_mismatch(file, "MIPS", 
"MIPS64", 0))
                                 goto bailout;
                         break;

@@ -3894,7 +3894,7 @@ is_shared_object(char *file)
                         break;

                 case EM_MIPS:
-                       if (machine_type("MIPS"))
+                       if (machine_type("MIPS") || machine_type("MIPS64"))
                                 return TRUE;
                         break;
                 }

After this modification, it can run successfully.

I personally think this scheme is feasible.
The MACHINE_TYPE of mips64 is defined as MIPS64, but EM_MIPS is still used
in the case, but the processing part is as above. This can be distinguished
from mips32 and use a different MACHINE_TYPE.

Do you think this modification is feasible and look forward to your 
suggestions?
> Thanks,
> Kazu
Thanks,
Youling
>
>>>>> # readelf -h vmcore
>>>>> ...
>>>>>      Type:                              CORE (Core file)
>>>>>      Machine:                           MIPS R3000
>>>>> ...
>>>>>
>>>>> Therefore, the MACHINE_TYPE of mips32 and mips64 both define MIPS.
>>>>>
>>>>> Thanks,
>>>>> Youling
>>
>> --
>> Crash-utility mailing list
>> Crash-utility at redhat.com
>> https://listman.redhat.com/mailman/listinfo/crash-utility




More information about the Crash-utility mailing list