Thank you for the fix, Shijie.
On Wed, Aug 24, 2022 at 2:26 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx> wrote:
On 2022/08/22 18:29, Huang Shijie wrote:
> After kernel patch:
> "0d9b1ffefabe arm64: mm: make vabits_actual a build time constant if possible"
> the "vabits_actual" is not compiled to kernel symbols when "VA_BITS > 48"
> is false.
>
> So the crash will not find the "vabits_actual" symbol, and it will
> fail in the end.
>
Can you help add the actual failure information to the patch log? Kazu.
WARNING: VA_BITS: calculated: 46 vmcoreinfo: 48
crash: invalid kernel virtual address: ffff88177ffff000 type: "pud page"
This change looks good and the test is ok. So: Ack.
Thanks.
Lianbo
> This patch introduces the arm64_set_va_bits_by_tcr(), and if crash cannot
> find "vabits_actual" symbol, it will use the TCR_EL1_T1SZ register to get
> the correct VA_BITS_ACTUAL/VA_BITS/VA_START.
>
> Tested this patch with:
> 1.) the live mode with /proc/kcore
> 2.) the kdump file with /proc/vmcore.
>
> Signed-off-by: Huang Shijie <shijie@xxxxxxxxxxxxxxxxxxxxxx>
> ---
> arm64.c | 50 +++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 33 insertions(+), 17 deletions(-)
>
> diff --git a/arm64.c b/arm64.c
> index b6b7aa1..3a613f9 100644
> --- a/arm64.c
> +++ b/arm64.c
> @@ -4586,6 +4586,35 @@ arm64_IS_VMALLOC_ADDR(ulong vaddr)
> (vaddr >= ms->modules_vaddr && vaddr <= ms->modules_end));
> }
>
> +/* Return TRUE if we succeed, return FALSE on failure. */
> +static int arm64_set_va_bits_by_tcr()
$ make -j $(nproc) warn
...
arm64.c:4590:12: warning: function declaration isn't a prototype [-Wstrict-prototypes]
static int arm64_set_va_bits_by_tcr()
^~~~~~~~~~~~~~~~~~~~~~~~
and please follow the style of the other functions as far as possible:
-static int arm64_set_va_bits_by_tcr()
+static int
+arm64_set_va_bits_by_tcr(void)
(hmm, some functions in arm64.c already don't follow it, though...)
But this can be fixed when merging, you don't need to respin only for this.
Otherwise, looks good to me. Thank you for the early fix.
Acked-by: Kazuhito Hagio <k-hagio-ab@xxxxxxx>
Thanks,
Kazu
> +{
> + ulong value;
> + char *string;
> +
> + if ((string = pc->read_vmcoreinfo("NUMBER(TCR_EL1_T1SZ)")) ||
> + (string = pc->read_vmcoreinfo("NUMBER(tcr_el1_t1sz)"))) {
> + /* See ARMv8 ARM for the description of
> + * TCR_EL1.T1SZ and how it can be used
> + * to calculate the vabits_actual
> + * supported by underlying kernel.
> + *
> + * Basically:
> + * vabits_actual = 64 - T1SZ;
> + */
> + value = 64 - strtoll(string, NULL, 0);
> + if (CRASHDEBUG(1))
> + fprintf(fp, "vmcoreinfo : vabits_actual: %ld\n", value);
> + free(string);
> + machdep->machspec->VA_BITS_ACTUAL = value;
> + machdep->machspec->VA_BITS = value;
> + machdep->machspec->VA_START = _VA_START(machdep->machspec->VA_BITS_ACTUAL);
> + return TRUE;
> + }
> +
> + return FALSE;
> +}
> +
> static void
> arm64_calc_VA_BITS(void)
> {
> @@ -4616,23 +4645,8 @@ arm64_calc_VA_BITS(void)
> } else if (ACTIVE())
> error(FATAL, "cannot determine VA_BITS_ACTUAL: please use /proc/kcore\n");
> else {
> - if ((string = pc->read_vmcoreinfo("NUMBER(TCR_EL1_T1SZ)")) ||
> - (string = pc->read_vmcoreinfo("NUMBER(tcr_el1_t1sz)"))) {
> - /* See ARMv8 ARM for the description of
> - * TCR_EL1.T1SZ and how it can be used
> - * to calculate the vabits_actual
> - * supported by underlying kernel.
> - *
> - * Basically:
> - * vabits_actual = 64 - T1SZ;
> - */
> - value = 64 - strtoll(string, NULL, 0);
> - if (CRASHDEBUG(1))
> - fprintf(fp, "vmcoreinfo : vabits_actual: %ld\n", value);
> - free(string);
> - machdep->machspec->VA_BITS_ACTUAL = value;
> - machdep->machspec->VA_BITS = value;
> - machdep->machspec->VA_START = _VA_START(machdep->machspec->VA_BITS_ACTUAL);
> + if (arm64_set_va_bits_by_tcr()) {
> + /* nothing */
> } 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);
> @@ -4654,6 +4668,8 @@ arm64_calc_VA_BITS(void)
> */
> machdep->flags |= FLIPPED_VM;
> return;
> + } else if (arm64_set_va_bits_by_tcr()) {
> + return;
> }
>
> if (!(sp = symbol_search("swapper_pg_dir")) &&
-- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/crash-utility Contribution Guidelines: https://github.com/crash-utility/crash/wiki
