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

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(a)redhat.com >> https://listman.redhat.com/mailman/listinfo/crash-utility