[Crash-utility] Re: [PATCH] arm64: fix regression for the determination of VA_BITS and section_size_bits
Hi qiwu,
On Tue, Jul 30, 2024 at 8:42 PM <qiwu.chen(a)transsion.com> wrote:
Hi Tao,
> Could you explain why removing it can fix the issue? I cannot get a
> clue by reading the message. Also please put the root cause into the
> patch log as well.
>
The first change is a mistake fix which can be ignored, since the dump file contains a
wrong vmcoreinfo "NUMBER(TCR_EL1_T1SZ)=25\n"
which lead a wrong vabits_actual(27) got by arm64_set_va_bits_by_tcr(). The same issue as
Guanyou comments:
https://lists.crash-utility.osci.io/archives/list/devel@lists.crash-utili...
OK, if it is a mistake please remove the code change as well as the
related git commit message for v2.
>
> Looks like you have fixed 2 regressions, and I don't see the 2
> regressions are related to each other(correct me if I'm wrong), If so
> I would prefer you to send 2 individual patches, each dealing with
> only one regression. So we can track the patch history easier.
>
So there is only one regression left. I would follow your suggestion.
>
> Do you determine the android-12-GKI by checking
> CONFIG_ANDROID_KABI_RESERVE? If so could you also write it into the
> code comments, for people who don't have an android kernel background.
>
Checking CONFIG_ANDROID_KABI_RESERVE is a easiest way which introduced on android12-5.10
with the google change:
https://android.googlesource.com/kernel/common/+/3e9d5f02f0e124633edb3a63...
There is another way to determine the android-12-GKI by checking linux_banner:
crash> linux_banner
linux_banner = $1 = "Linux version 5.10.209-android12-9-g5cf27268b585-dirty
..."
Which way do you think is better?
Normally I would prefer not to check IKconfig if there are other
methods available. However IMHO the linux_banner doesn't give more
advancements than IKconfig. Does the string "android12-9" represent
android-12-GKI? How would you check if your version is higher than
android-12-GKI? E.g. to compare the version "Linux version
x.x.x-android13-10" higher than "Linux version 5.10.209-android12-9"?
I guess extra code needs to be introduced to parse the version string,
and it doesn't seem nice because the extra code can only serve the
android version parsing.
If you have a better solution to parse the android version string,
please show it in your v2. If not I would suggest keeping the IKconfig
check.
>
> I think machdep->section_size_bits should be initialized as 0 at the
> start of arm64_get_section_size_bits(), the exit block will assume its
> value to be 0 if error happens.
The initial value of machdep->section_size_bits is supposed to be 0 since the global
variable machdep is initialized as 0.
struct machdep_table machdep_table = { 0 };
struct machdep_table *machdep = &machdep_table;
OK, agreed, thanks for the explanation!
Thanks,
Tao Liu
Thanks
--
Crash-utility mailing list -- devel(a)lists.crash-utility.osci.io
To unsubscribe send an email to devel-leave(a)lists.crash-utility.osci.io
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki