12:44 p.m.
Hi Vinayak,
Thanks for the patch.
Some queries below:
On Thu, 25 Mar 2021 at 20:08, Vinayak Menon <vinayakm.list(a)gmail.com> wrote:
Raw ramdumps without vmcoreinfo does not work currently
with pointer authentication or memory tagging enabled.
The arm capability array can be queried but that creates
a dependency on the bits identifying the capabilities.
Add a machdep option instead to ignore the tags when any
feature using the tag bits is enabled.
Hmm.. I am still trying to understand why we need to ignore the tags.
Can you please explain the issue at hand and also share the steps you
used to reproduce it.
I have an arm64 hardware with memory tagging support available, I can
try to reproduce it at my end.
I suspect we need to enable reading capabilities instead. Do you
foresee any issues with the same?
Signed-off-by: Vinayak Menon <vinayakm.list(a)gmail.com>
---
arm64.c | 26 +++++++++++++++++++-------
crash.8 | 1 +
defs.h | 1 +
help.c | 1 +
4 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/arm64.c b/arm64.c
index 37aed07..3ed6795 100644
--- a/arm64.c
+++ b/arm64.c
@@ -745,7 +745,8 @@ arm64_parse_machdep_arg_l(char *argstring, char *param, ulong
*value)
char *p;
len = strlen(param);
- if (!STRNEQ(argstring, param) || (argstring[len] != '='))
+ if (!STRNEQ(argstring, param) || (!STRNEQ(argstring, "tag_ignore")
&&
+ (argstring[len] != '=')))
return FALSE;
if ((LASTCHAR(argstring) == 'm') ||
@@ -763,6 +764,8 @@ arm64_parse_machdep_arg_l(char *argstring, char *param, ulong
*value)
*value = dtol(p, flags, &err);
} else if (STRNEQ(argstring, "vabits_actual")) {
*value = dtol(p, flags, &err);
+ } else if (STRNEQ(argstring, "tag_ignore")) {
+ *value = 1;
} else if (megabytes) {
*value = dtol(p, flags, &err);
if (!err)
@@ -797,12 +800,6 @@ arm64_parse_cmdline_args(void)
if (!machdep->cmdline_args[index])
break;
- if (!strstr(machdep->cmdline_args[index], "=")) {
- error(WARNING, "ignoring --machdep option: %x\n",
- machdep->cmdline_args[index]);
- continue;
- }
-
strcpy(buf, machdep->cmdline_args[index]);
for (p = buf; *p; p++) {
@@ -838,6 +835,11 @@ arm64_parse_cmdline_args(void)
"setting vabits_actual to: %ld\n\n",
machdep->machspec->VA_BITS_ACTUAL);
continue;
+ } else if (arm64_parse_machdep_arg_l(arglist[i],
"tag_ignore",
+ &machdep->machspec->tag_ignore)) {
+ error(NOTE,
+ "setting tag_ignore\n\n");
+ continue;
}
error(WARNING, "ignoring --machdep option: %s\n",
@@ -4122,6 +4124,9 @@ arm64_swp_offset(ulong pte)
return pte;
}
+#define __GENMASK_ULL(h, l) \
+ (((~0ULL) - (1ULL << (l)) + 1) & \
+ (~0ULL >> (64 - 1 - (h))))
Please add it to defs.h instead of a .c file. Also since this is
picked up from Linux kernel, please add a comment about this being
borrowed from the Linux kernel (something like: Borrowed from /*
include/linux/bits.h */). Also add the relevant Linux comment which is
very useful:
/*
* Create a contiguous bitmask starting at bit position @l and ending at
* position @h. For example
* GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
*/
static void arm64_calc_KERNELPACMASK(void)
{
ulong value;
@@ -4133,6 +4138,13 @@ static void arm64_calc_KERNELPACMASK(void)
machdep->machspec->CONFIG_ARM64_KERNELPACMASK = value;
if (CRASHDEBUG(1))
fprintf(fp, "CONFIG_ARM64_KERNELPACMASK: %lx\n",
value);
+ } else if (machdep->machspec->VA_BITS_ACTUAL &&
+ machdep->machspec->tag_ignore) {
+ machdep->machspec->CONFIG_ARM64_KERNELPACMASK =
+ __GENMASK_ULL(63, machdep->machspec->VA_BITS_ACTUAL);
+ if (CRASHDEBUG(1))
+ fprintf(fp, "CONFIG_ARM64_KERNELPACMASK: %lx\n",
+ machdep->machspec->CONFIG_ARM64_KERNELPACMASK);
We can make this fprintf common for the if and else-if legs, right?
Just take it out and calculate value in both the cases and print it.
Also why do we set machdep->machspec->CONFIG_ARM64_KERNELPACMASK =
__GENMASK_ULL(63, machdep->machspec->VA_BITS_ACTUAL)
here, can you add some comments to this le, since the kernel passed a
0 value for KERNELPACMASK in vmcoreinfo.
Thanks,
Bhupesh
}
}
diff --git a/crash.8 b/crash.8
index 5020ce1..91f01fc 100644
--- a/crash.8
+++ b/crash.8
@@ -289,6 +289,7 @@ ARM64:
kimage_voffset=<kimage_voffset-value>
max_physmem_bits=<value>
vabits_actual=<value>
+ tag_ignore
X86:
page_offset=<CONFIG_PAGE_OFFSET-value>
.fi
diff --git a/defs.h b/defs.h
index 35b983a..cd3bcf9 100644
--- a/defs.h
+++ b/defs.h
@@ -3331,6 +3331,7 @@ struct machine_specific {
ulong VA_START;
ulong CONFIG_ARM64_KERNELPACMASK;
ulong physvirt_offset;
+ ulong tag_ignore;
};
struct arm64_stackframe {
diff --git a/help.c b/help.c
index 531f50a..d66125b 100644
--- a/help.c
+++ b/help.c
@@ -182,6 +182,7 @@ char *program_usage_info[] = {
" kimage_voffset=<kimage_voffset-value>",
" max_physmem_bits=<value>",
" vabits_actual=<value>",
+ " tag_ignore",
" X86:",
" page_offset=<CONFIG_PAGE_OFFSET-value>",
"",
--
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://listman.redhat.com/mailman/listinfo/crash-utility