Hi qiwu,
On Sun, Jul 14, 2024 at 5:29 PM <qiwu.chen(a)transsion.com> wrote:
1. The commit f02c8e87 will cause a regression issue for the determination of VA_BITS
on Linux 4.19 and earlier kernels, the crash session fails during initialization with
the error message due to get a wrong vabits_actual:
vmcoreinfo : vabits_actual: 27
crash: invalid kernel virtual address: ffffffa890a41318 type:
"kernel_config_data"
WARNING: cannot read kernel_config_data
crash: invalid kernel virtual address: ffffffa89106db50 type: "possible"
WARNING: cannot read cpu_possible_map
crash: invalid kernel virtual address: ffffffa89106db48 type: "present"
WARNING: cannot read cpu_present_map
crash: invalid kernel virtual address: ffffffa89106db40 type: "online"
WARNING: cannot read cpu_online_map
crash: invalid kernel virtual address: ffffffa89106db58 type: "active"
WARNING: cannot read cpu_active_map
crash: invalid kernel virtual address: ffffffa89143cb80 type: "shadow_timekeeper
xtime_sec"
crash: invalid kernel virtual address: ffffffa89107b76c type: "init_uts_ns"
WARNING: invalid linux_banner pointer: ffffffa890a30018
crash: vmlinux and SYS_COREDUMP do not match!
Fix it by remove arm64_set_va_bits_by_tcr() if vabits_actual is missing.
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.
2. The commit 568c6f04 will cause a regression issue for the determination of
section_size_bits
on Linux 5.12 and earlier kernels. The section_size_bits compatible with linux upstream
and
android GKI changes should be:
Before android-12-GKI or Linux 5.12:
SECTION_SIZE_BITS = 30
After android-12-gki:
SECTION_SIZE_BITS = 27 when defined 4K_PAGES or 16K_PAGES.
SECTION_SIZE_BITS = 29 when defined 64K_PAGES.
Fixes: f02c8e87 ("arm64: use TCR_EL1_T1SZ to get the correct info if vabits_actual
is missing")
Fixes: 568c6f04 ("arm64: section_size_bits compatible with macro definitions")
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.
Signed-off-by: qiwu.chen <qiwu.chen(a)transsion.com>
---
arm64.c | 37 ++++++++++++++++++-------------------
1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/arm64.c b/arm64.c
index b3040d7..176c465 100644
--- a/arm64.c
+++ b/arm64.c
@@ -1613,8 +1613,15 @@ arm64_get_section_size_bits(void)
{
int ret;
char *string;
+ bool is_ikconfig_avail;
- if (THIS_KERNEL_VERSION >= LINUX(5,12,0)) {
+ if (arm64_get_vmcoreinfo(&machdep->section_size_bits,
"NUMBER(SECTION_SIZE_BITS)", NUM_DEC))
+ goto exit;
+
+ is_ikconfig_avail = kt->ikconfig_flags & IKCONFIG_AVAIL ? TRUE : FALSE;
+ /* The commit reduce section size for arm64 sparsemem is introduced on
linux-v5.12 and android-12-GKI */
+ if (THIS_KERNEL_VERSION >= LINUX(5,12,0) || (is_ikconfig_avail &&
+ get_kernel_config("CONFIG_ANDROID_KABI_RESERVE", NULL) ==
IKCONFIG_Y)) {
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.
if (machdep->pagesize == 65536)
machdep->section_size_bits = _SECTION_SIZE_BITS_5_12_64K;
else
@@ -1622,24 +1629,18 @@ arm64_get_section_size_bits(void)
} else
machdep->section_size_bits = _SECTION_SIZE_BITS;
- if (arm64_get_vmcoreinfo(&machdep->section_size_bits,
"NUMBER(SECTION_SIZE_BITS)", NUM_DEC)) {
- /* nothing */
- } else if (kt->ikconfig_flags & IKCONFIG_AVAIL) {
- if ((ret = get_kernel_config("CONFIG_MEMORY_HOTPLUG", NULL)) ==
IKCONFIG_Y) {
- if ((ret =
get_kernel_config("CONFIG_HOTPLUG_SIZE_BITS", &string)) == IKCONFIG_STR)
- machdep->section_size_bits = atol(string);
- }
-
- /* arm64: reduce section size for sparsemem */
- if ((ret = get_kernel_config("CONFIG_ARM64_4K_PAGES", NULL)) ==
IKCONFIG_Y
- || (ret = get_kernel_config("CONFIG_ARM64_16K_PAGES",
NULL)) == IKCONFIG_Y)
- machdep->section_size_bits = _SECTION_SIZE_BITS_5_12;
- else if ((ret = get_kernel_config("CONFIG_ARM64_64K_PAGES",
NULL)) == IKCONFIG_Y)
- machdep->section_size_bits = _SECTION_SIZE_BITS_5_12_64K;
+ /* section_size_bits for arm64 vendor special case */
+ if (is_ikconfig_avail &&
get_kernel_config("CONFIG_MEMORY_HOTPLUG", NULL) == IKCONFIG_Y) {
+ if (get_kernel_config("CONFIG_HOTPLUG_SIZE_BITS", &string)
== IKCONFIG_STR)
+ machdep->section_size_bits = atol(string);
}
- if (CRASHDEBUG(1))
- fprintf(fp, "SECTION_SIZE_BITS: %ld\n",
machdep->section_size_bits);
+exit:
+ if (machdep->section_size_bits) {
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.
+ if (CRASHDEBUG(1))
+ fprintf(fp, "SECTION_SIZE_BITS: %ld\n",
machdep->section_size_bits);
+ } else
+ error(FATAL, "cannot determine SECTION_SIZE_BITS\n");
}
/*
@@ -4733,8 +4734,6 @@ arm64_calc_VA_BITS(void)
*/
machdep->flags |= FLIPPED_VM;
return;
- } else if (arm64_set_va_bits_by_tcr()) {
- return;
} else if (machdep->machspec->VA_BITS_ACTUAL) {
machdep->machspec->VA_BITS =
machdep->machspec->VA_BITS_ACTUAL;
machdep->machspec->VA_START =
_VA_START(machdep->machspec->VA_BITS_ACTUAL);
--
2.25.1
--
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