All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] soc: apple: rtkit: Add register dump decoding to crashlog
@ 2023-01-23  3:17 ` Asahi Lina
  0 siblings, 0 replies; 9+ messages in thread
From: Asahi Lina @ 2023-01-23  3:17 UTC (permalink / raw)
  To: Hector Martin, Sven Peter
  Cc: Alyssa Rosenzweig, asahi, linux-arm-kernel, linux-kernel,
	Asahi Lina, Eric Curtin

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 <sven@svenpeter.dev>
Reviewed-by: Eric Curtin <ecurtin@redhat.com>
Signed-off-by: Asahi Lina <lina@asahilina.net>
---
 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH v2] soc: apple: rtkit: Add register dump decoding to crashlog
@ 2023-01-23  3:17 ` Asahi Lina
  0 siblings, 0 replies; 9+ messages in thread
From: Asahi Lina @ 2023-01-23  3:17 UTC (permalink / raw)
  To: Hector Martin, Sven Peter
  Cc: Alyssa Rosenzweig, asahi, linux-arm-kernel, linux-kernel,
	Asahi Lina, Eric Curtin

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 <sven@svenpeter.dev>
Reviewed-by: Eric Curtin <ecurtin@redhat.com>
Signed-off-by: Asahi Lina <lina@asahilina.net>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH v2] soc: apple: rtkit: Add register dump decoding to crashlog
  2023-01-23  3:17 ` Asahi Lina
@ 2023-01-23 12:18   ` Eric Curtin
  -1 siblings, 0 replies; 9+ messages in thread
From: Eric Curtin @ 2023-01-23 12:18 UTC (permalink / raw)
  To: Asahi Lina
  Cc: Hector Martin, Sven Peter, Alyssa Rosenzweig, asahi,
	linux-arm-kernel, linux-kernel

On Mon, 23 Jan 2023 at 03:19, Asahi Lina <lina@asahilina.net> 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 <sven@svenpeter.dev>
> Reviewed-by: Eric Curtin <ecurtin@redhat.com>
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> ---

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 <ecurtin@redhat.com>

>  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
>


^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH v2] soc: apple: rtkit: Add register dump decoding to crashlog
@ 2023-01-23 12:18   ` Eric Curtin
  0 siblings, 0 replies; 9+ messages in thread
From: Eric Curtin @ 2023-01-23 12:18 UTC (permalink / raw)
  To: Asahi Lina
  Cc: Hector Martin, Sven Peter, Alyssa Rosenzweig, asahi,
	linux-arm-kernel, linux-kernel

On Mon, 23 Jan 2023 at 03:19, Asahi Lina <lina@asahilina.net> 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 <sven@svenpeter.dev>
> Reviewed-by: Eric Curtin <ecurtin@redhat.com>
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> ---

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 <ecurtin@redhat.com>

>  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
>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH v2] soc: apple: rtkit: Add register dump decoding to crashlog
  2023-01-23 12:18   ` Eric Curtin
@ 2023-01-24  4:42     ` Asahi Lina
  -1 siblings, 0 replies; 9+ messages in thread
From: Asahi Lina @ 2023-01-24  4:42 UTC (permalink / raw)
  To: Eric Curtin
  Cc: Hector Martin, Sven Peter, Alyssa Rosenzweig, asahi,
	linux-arm-kernel, linux-kernel

On 23/01/2023 21.18, Eric Curtin wrote:
> On Mon, 23 Jan 2023 at 03:19, Asahi Lina <lina@asahilina.net> 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 <sven@svenpeter.dev>
>> Reviewed-by: Eric Curtin <ecurtin@redhat.com>
>> Signed-off-by: Asahi Lina <lina@asahilina.net>
>> ---
> 
> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH v2] soc: apple: rtkit: Add register dump decoding to crashlog
@ 2023-01-24  4:42     ` Asahi Lina
  0 siblings, 0 replies; 9+ messages in thread
From: Asahi Lina @ 2023-01-24  4:42 UTC (permalink / raw)
  To: Eric Curtin
  Cc: Hector Martin, Sven Peter, Alyssa Rosenzweig, asahi,
	linux-arm-kernel, linux-kernel

On 23/01/2023 21.18, Eric Curtin wrote:
> On Mon, 23 Jan 2023 at 03:19, Asahi Lina <lina@asahilina.net> 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 <sven@svenpeter.dev>
>> Reviewed-by: Eric Curtin <ecurtin@redhat.com>
>> Signed-off-by: Asahi Lina <lina@asahilina.net>
>> ---
> 
> 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH v2] soc: apple: rtkit: Add register dump decoding to crashlog
  2023-01-23  3:17 ` Asahi Lina
  (?)
  (?)
@ 2023-01-24 22:41 ` kernel test robot
  -1 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2023-01-24 22:41 UTC (permalink / raw)
  To: Asahi Lina; +Cc: llvm, oe-kbuild-all

Hi Asahi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on soc/for-next]
[also build test WARNING on linus/master v6.2-rc5 next-20230124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Asahi-Lina/soc-apple-rtkit-Add-register-dump-decoding-to-crashlog/20230123-112025
base:   https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link:    https://lore.kernel.org/r/20230123031728.22515-1-lina%40asahilina.net
patch subject: [PATCH v2] soc: apple: rtkit: Add register dump decoding to crashlog
config: powerpc-buildonly-randconfig-r006-20230123 (https://download.01.org/0day-ci/archive/20230125/202301250612.UZ8rgt6i-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 4196ca3278f78c6e19246e54ab0ecb364e37d66a)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install powerpc cross compiling tool for clang build
        # apt-get install binutils-powerpc-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/e505dcb428ededed4847191eaa3504d3dae39bea
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Asahi-Lina/soc-apple-rtkit-Add-register-dump-decoding-to-crashlog/20230123-112025
        git checkout e505dcb428ededed4847191eaa3504d3dae39bea
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/soc/apple/rtkit-crashlog.c:186:6: warning: stack frame size (1040) exceeds limit (1024) in 'apple_rtkit_crashlog_dump' [-Wframe-larger-than]
   void apple_rtkit_crashlog_dump(struct apple_rtkit *rtk, u8 *bfr, size_t size)
        ^
   164/1040 (15.77%) spills, 876/1040 (84.23%) variables
   1 warning generated.


vim +/apple_rtkit_crashlog_dump +186 drivers/soc/apple/rtkit-crashlog.c

e505dcb428eded Asahi Lina 2023-01-23  185  
9bd1d9a0d8bb1a Sven Peter 2022-05-01 @186  void apple_rtkit_crashlog_dump(struct apple_rtkit *rtk, u8 *bfr, size_t size)

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH v2] soc: apple: rtkit: Add register dump decoding to crashlog
  2023-01-23  3:17 ` Asahi Lina
@ 2023-01-31 11:45   ` Hector Martin
  -1 siblings, 0 replies; 9+ messages in thread
From: Hector Martin @ 2023-01-31 11:45 UTC (permalink / raw)
  To: Asahi Lina, Sven Peter
  Cc: Alyssa Rosenzweig, asahi, linux-arm-kernel, linux-kernel, Eric Curtin

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 <sven@svenpeter.dev>
> Reviewed-by: Eric Curtin <ecurtin@redhat.com>
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> ---
>  drivers/soc/apple/rtkit-crashlog.c | 93 ++++++++++++++++++++++++++++++
>  1 file changed, 93 insertions(+)
> 

Thanks, applied to asahi-soc/soc!

- Hector

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH v2] soc: apple: rtkit: Add register dump decoding to crashlog
@ 2023-01-31 11:45   ` Hector Martin
  0 siblings, 0 replies; 9+ messages in thread
From: Hector Martin @ 2023-01-31 11:45 UTC (permalink / raw)
  To: Asahi Lina, Sven Peter
  Cc: Alyssa Rosenzweig, asahi, linux-arm-kernel, linux-kernel, Eric Curtin

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 <sven@svenpeter.dev>
> Reviewed-by: Eric Curtin <ecurtin@redhat.com>
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> ---
>  drivers/soc/apple/rtkit-crashlog.c | 93 ++++++++++++++++++++++++++++++
>  1 file changed, 93 insertions(+)
> 

Thanks, applied to asahi-soc/soc!

- Hector

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-01-31 11:46 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-23  3:17 [PATCH v2] soc: apple: rtkit: Add register dump decoding to crashlog Asahi Lina
2023-01-23  3:17 ` Asahi Lina
2023-01-23 12:18 ` Eric Curtin
2023-01-23 12:18   ` Eric Curtin
2023-01-24  4:42   ` Asahi Lina
2023-01-24  4:42     ` Asahi Lina
2023-01-24 22:41 ` kernel test robot
2023-01-31 11:45 ` Hector Martin
2023-01-31 11:45   ` Hector Martin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.