2023-01-23 03:19:05

by Asahi Lina

[permalink] [raw]
Subject: [PATCH v2] soc: apple: rtkit: Add register dump decoding to crashlog

When the coprocessor crashes, it's useful to get a proper register dump
so we can find out what the firmware was doing. Add a decoder for this.

Originally this had ESR decoding by reusing the ARM64 arch header for
this, but that introduces some module linking and cross-arch compilation
issues, so let's leave that out for now.

Reviewed-by: Sven Peter <[email protected]>
Reviewed-by: Eric Curtin <[email protected]>
Signed-off-by: Asahi Lina <[email protected]>
---
drivers/soc/apple/rtkit-crashlog.c | 93 ++++++++++++++++++++++++++++++
1 file changed, 93 insertions(+)

diff --git a/drivers/soc/apple/rtkit-crashlog.c b/drivers/soc/apple/rtkit-crashlog.c
index 732deed64660..dfa74b32eda2 100644
--- a/drivers/soc/apple/rtkit-crashlog.c
+++ b/drivers/soc/apple/rtkit-crashlog.c
@@ -13,6 +13,17 @@
#define APPLE_RTKIT_CRASHLOG_VERSION FOURCC('C', 'v', 'e', 'r')
#define APPLE_RTKIT_CRASHLOG_MBOX FOURCC('C', 'm', 'b', 'x')
#define APPLE_RTKIT_CRASHLOG_TIME FOURCC('C', 't', 'i', 'm')
+#define APPLE_RTKIT_CRASHLOG_REGS FOURCC('C', 'r', 'g', '8')
+
+/* For COMPILE_TEST on non-ARM64 architectures */
+#ifndef PSR_MODE_EL0t
+#define PSR_MODE_EL0t 0x00000000
+#define PSR_MODE_EL1t 0x00000004
+#define PSR_MODE_EL1h 0x00000005
+#define PSR_MODE_EL2t 0x00000008
+#define PSR_MODE_EL2h 0x00000009
+#define PSR_MODE_MASK 0x0000000f
+#endif

struct apple_rtkit_crashlog_header {
u32 fourcc;
@@ -31,6 +42,24 @@ struct apple_rtkit_crashlog_mbox_entry {
};
static_assert(sizeof(struct apple_rtkit_crashlog_mbox_entry) == 0x18);

+struct apple_rtkit_crashlog_regs {
+ u32 unk_0;
+ u32 unk_4;
+ u64 regs[31];
+ u64 sp;
+ u64 pc;
+ u64 psr;
+ u64 cpacr;
+ u64 fpsr;
+ u64 fpcr;
+ u64 unk[64];
+ u64 far;
+ u64 unk_X;
+ u64 esr;
+ u64 unk_Z;
+};
+static_assert(sizeof(struct apple_rtkit_crashlog_regs) == 0x350);
+
static void apple_rtkit_crashlog_dump_str(struct apple_rtkit *rtk, u8 *bfr,
size_t size)
{
@@ -94,6 +123,66 @@ static void apple_rtkit_crashlog_dump_mailbox(struct apple_rtkit *rtk, u8 *bfr,
}
}

+static void apple_rtkit_crashlog_dump_regs(struct apple_rtkit *rtk, u8 *bfr,
+ size_t size)
+{
+ struct apple_rtkit_crashlog_regs regs;
+ const char *el;
+ int i;
+
+ if (size < sizeof(regs)) {
+ dev_warn(rtk->dev, "RTKit: Regs section too small: 0x%zx", size);
+ return;
+ }
+
+ memcpy(&regs, bfr, sizeof(regs));
+
+ switch (regs.psr & PSR_MODE_MASK) {
+ case PSR_MODE_EL0t:
+ el = "EL0t";
+ break;
+ case PSR_MODE_EL1t:
+ el = "EL1t";
+ break;
+ case PSR_MODE_EL1h:
+ el = "EL1h";
+ break;
+ case PSR_MODE_EL2t:
+ el = "EL2t";
+ break;
+ case PSR_MODE_EL2h:
+ el = "EL2h";
+ break;
+ default:
+ el = "unknown";
+ break;
+ }
+
+ dev_warn(rtk->dev, "RTKit: Exception dump:");
+ dev_warn(rtk->dev, " == Exception taken from %s ==", el);
+ dev_warn(rtk->dev, " PSR = 0x%llx", regs.psr);
+ dev_warn(rtk->dev, " PC = 0x%llx\n", regs.pc);
+ dev_warn(rtk->dev, " ESR = 0x%llx\n", regs.esr);
+ dev_warn(rtk->dev, " FAR = 0x%llx\n", regs.far);
+ dev_warn(rtk->dev, " SP = 0x%llx\n", regs.sp);
+ dev_warn(rtk->dev, "\n");
+
+ for (i = 0; i < 31; i += 4) {
+ if (i < 28)
+ dev_warn(rtk->dev,
+ " x%02d-x%02d = %016llx %016llx %016llx %016llx\n",
+ i, i + 3,
+ regs.regs[i], regs.regs[i + 1],
+ regs.regs[i + 2], regs.regs[i + 3]);
+ else
+ dev_warn(rtk->dev,
+ " x%02d-x%02d = %016llx %016llx %016llx\n", i, i + 3,
+ regs.regs[i], regs.regs[i + 1], regs.regs[i + 2]);
+ }
+
+ dev_warn(rtk->dev, "\n");
+}
+
void apple_rtkit_crashlog_dump(struct apple_rtkit *rtk, u8 *bfr, size_t size)
{
size_t offset;
@@ -140,6 +229,10 @@ void apple_rtkit_crashlog_dump(struct apple_rtkit *rtk, u8 *bfr, size_t size)
apple_rtkit_crashlog_dump_time(rtk, bfr + offset + 16,
section_size);
break;
+ case APPLE_RTKIT_CRASHLOG_REGS:
+ apple_rtkit_crashlog_dump_regs(rtk, bfr + offset + 16,
+ section_size);
+ break;
default:
dev_warn(rtk->dev,
"RTKit: Unknown crashlog section: %x",
--
2.35.1



2023-01-23 12:19:18

by Eric Curtin

[permalink] [raw]
Subject: Re: [PATCH v2] soc: apple: rtkit: Add register dump decoding to crashlog

On Mon, 23 Jan 2023 at 03:19, Asahi Lina <[email protected]> wrote:
>
> When the coprocessor crashes, it's useful to get a proper register dump
> so we can find out what the firmware was doing. Add a decoder for this.
>
> Originally this had ESR decoding by reusing the ARM64 arch header for
> this, but that introduces some module linking and cross-arch compilation
> issues, so let's leave that out for now.
>
> Reviewed-by: Sven Peter <[email protected]>
> Reviewed-by: Eric Curtin <[email protected]>
> Signed-off-by: Asahi Lina <[email protected]>
> ---

I would be thinking, rather that duplicating the PSR_MODE_* defines
and values, why not just ifdef the whole
"apple_rtkit_crashlog_dump_regs(" function and the "case
APPLE_RTKIT_CRASHLOG_REGS" part (and maybe even
apple_rtkit_crashlog_regs struct also). Is it worth compiling that
code on other CPU architectures when PSR seems to be an ARM specific
thing?

But this way also works with a little duplication so happy to give
this tag, the above change is optional, but it would be less code and
less duplication!

Reviewed-by: Eric Curtin <[email protected]>

> drivers/soc/apple/rtkit-crashlog.c | 93 ++++++++++++++++++++++++++++++
> 1 file changed, 93 insertions(+)
>
> diff --git a/drivers/soc/apple/rtkit-crashlog.c b/drivers/soc/apple/rtkit-crashlog.c
> index 732deed64660..dfa74b32eda2 100644
> --- a/drivers/soc/apple/rtkit-crashlog.c
> +++ b/drivers/soc/apple/rtkit-crashlog.c
> @@ -13,6 +13,17 @@
> #define APPLE_RTKIT_CRASHLOG_VERSION FOURCC('C', 'v', 'e', 'r')
> #define APPLE_RTKIT_CRASHLOG_MBOX FOURCC('C', 'm', 'b', 'x')
> #define APPLE_RTKIT_CRASHLOG_TIME FOURCC('C', 't', 'i', 'm')
> +#define APPLE_RTKIT_CRASHLOG_REGS FOURCC('C', 'r', 'g', '8')
> +
> +/* For COMPILE_TEST on non-ARM64 architectures */
> +#ifndef PSR_MODE_EL0t
> +#define PSR_MODE_EL0t 0x00000000
> +#define PSR_MODE_EL1t 0x00000004
> +#define PSR_MODE_EL1h 0x00000005
> +#define PSR_MODE_EL2t 0x00000008
> +#define PSR_MODE_EL2h 0x00000009
> +#define PSR_MODE_MASK 0x0000000f
> +#endif
>
> struct apple_rtkit_crashlog_header {
> u32 fourcc;
> @@ -31,6 +42,24 @@ struct apple_rtkit_crashlog_mbox_entry {
> };
> static_assert(sizeof(struct apple_rtkit_crashlog_mbox_entry) == 0x18);
>
> +struct apple_rtkit_crashlog_regs {
> + u32 unk_0;
> + u32 unk_4;
> + u64 regs[31];
> + u64 sp;
> + u64 pc;
> + u64 psr;
> + u64 cpacr;
> + u64 fpsr;
> + u64 fpcr;
> + u64 unk[64];
> + u64 far;
> + u64 unk_X;
> + u64 esr;
> + u64 unk_Z;
> +};
> +static_assert(sizeof(struct apple_rtkit_crashlog_regs) == 0x350);
> +
> static void apple_rtkit_crashlog_dump_str(struct apple_rtkit *rtk, u8 *bfr,
> size_t size)
> {
> @@ -94,6 +123,66 @@ static void apple_rtkit_crashlog_dump_mailbox(struct apple_rtkit *rtk, u8 *bfr,
> }
> }
>
> +static void apple_rtkit_crashlog_dump_regs(struct apple_rtkit *rtk, u8 *bfr,
> + size_t size)
> +{
> + struct apple_rtkit_crashlog_regs regs;
> + const char *el;
> + int i;
> +
> + if (size < sizeof(regs)) {
> + dev_warn(rtk->dev, "RTKit: Regs section too small: 0x%zx", size);
> + return;
> + }
> +
> + memcpy(&regs, bfr, sizeof(regs));
> +
> + switch (regs.psr & PSR_MODE_MASK) {
> + case PSR_MODE_EL0t:
> + el = "EL0t";
> + break;
> + case PSR_MODE_EL1t:
> + el = "EL1t";
> + break;
> + case PSR_MODE_EL1h:
> + el = "EL1h";
> + break;
> + case PSR_MODE_EL2t:
> + el = "EL2t";
> + break;
> + case PSR_MODE_EL2h:
> + el = "EL2h";
> + break;
> + default:
> + el = "unknown";
> + break;
> + }
> +
> + dev_warn(rtk->dev, "RTKit: Exception dump:");
> + dev_warn(rtk->dev, " == Exception taken from %s ==", el);
> + dev_warn(rtk->dev, " PSR = 0x%llx", regs.psr);
> + dev_warn(rtk->dev, " PC = 0x%llx\n", regs.pc);
> + dev_warn(rtk->dev, " ESR = 0x%llx\n", regs.esr);
> + dev_warn(rtk->dev, " FAR = 0x%llx\n", regs.far);
> + dev_warn(rtk->dev, " SP = 0x%llx\n", regs.sp);
> + dev_warn(rtk->dev, "\n");
> +
> + for (i = 0; i < 31; i += 4) {
> + if (i < 28)
> + dev_warn(rtk->dev,
> + " x%02d-x%02d = %016llx %016llx %016llx %016llx\n",
> + i, i + 3,
> + regs.regs[i], regs.regs[i + 1],
> + regs.regs[i + 2], regs.regs[i + 3]);
> + else
> + dev_warn(rtk->dev,
> + " x%02d-x%02d = %016llx %016llx %016llx\n", i, i + 3,
> + regs.regs[i], regs.regs[i + 1], regs.regs[i + 2]);
> + }
> +
> + dev_warn(rtk->dev, "\n");
> +}
> +
> void apple_rtkit_crashlog_dump(struct apple_rtkit *rtk, u8 *bfr, size_t size)
> {
> size_t offset;
> @@ -140,6 +229,10 @@ void apple_rtkit_crashlog_dump(struct apple_rtkit *rtk, u8 *bfr, size_t size)
> apple_rtkit_crashlog_dump_time(rtk, bfr + offset + 16,
> section_size);
> break;
> + case APPLE_RTKIT_CRASHLOG_REGS:
> + apple_rtkit_crashlog_dump_regs(rtk, bfr + offset + 16,
> + section_size);
> + break;
> default:
> dev_warn(rtk->dev,
> "RTKit: Unknown crashlog section: %x",
> --
> 2.35.1
>


2023-01-24 04:42:49

by Asahi Lina

[permalink] [raw]
Subject: Re: [PATCH v2] soc: apple: rtkit: Add register dump decoding to crashlog

On 23/01/2023 21.18, Eric Curtin wrote:
> On Mon, 23 Jan 2023 at 03:19, Asahi Lina <[email protected]> wrote:
>>
>> When the coprocessor crashes, it's useful to get a proper register dump
>> so we can find out what the firmware was doing. Add a decoder for this.
>>
>> Originally this had ESR decoding by reusing the ARM64 arch header for
>> this, but that introduces some module linking and cross-arch compilation
>> issues, so let's leave that out for now.
>>
>> Reviewed-by: Sven Peter <[email protected]>
>> Reviewed-by: Eric Curtin <[email protected]>
>> Signed-off-by: Asahi Lina <[email protected]>
>> ---
>
> I would be thinking, rather that duplicating the PSR_MODE_* defines
> and values, why not just ifdef the whole
> "apple_rtkit_crashlog_dump_regs(" function and the "case
> APPLE_RTKIT_CRASHLOG_REGS" part (and maybe even
> apple_rtkit_crashlog_regs struct also). Is it worth compiling that
> code on other CPU architectures when PSR seems to be an ARM specific
> thing?

It's mostly just for compile testing! The code is about the architecture
of the coprocessor, not the host CPU, so it still makes sense in
principle (though of course it's not very likely that SoCs with a
combination like that will ever exist). It helped the kernel test robot
find a bad format specifier in this code during a compile test run for a
32-bit arch, so I think it makes sense to keep it.

~~ Lina

2023-01-31 11:45:46

by Hector Martin

[permalink] [raw]
Subject: Re: [PATCH v2] soc: apple: rtkit: Add register dump decoding to crashlog

On 23/01/2023 12.17, Asahi Lina wrote:
> When the coprocessor crashes, it's useful to get a proper register dump
> so we can find out what the firmware was doing. Add a decoder for this.
>
> Originally this had ESR decoding by reusing the ARM64 arch header for
> this, but that introduces some module linking and cross-arch compilation
> issues, so let's leave that out for now.
>
> Reviewed-by: Sven Peter <[email protected]>
> Reviewed-by: Eric Curtin <[email protected]>
> Signed-off-by: Asahi Lina <[email protected]>
> ---
> drivers/soc/apple/rtkit-crashlog.c | 93 ++++++++++++++++++++++++++++++
> 1 file changed, 93 insertions(+)
>

Thanks, applied to asahi-soc/soc!

- Hector