2024-05-02 10:11:32

by Jiaxun Yang

[permalink] [raw]
Subject: [PATCH v3 0/9] MIPS: Unify low-level debugging functionalities

Hi all,

This is a attempt to bring all low-level debugging print functions
together and provide a arm-like low-level debugging interface and
a further capability to debug early exceptions.

This patch elimiate platform specific early_printk, zboot printing
functions and cps-vec-ns16550 by newly introduced debug_ll.

Hope you'll find them handy :-)

Happy hacking!

Thanks

Signed-off-by: Jiaxun Yang <[email protected]>
---
Changes in v3:
- Collect review tags
- Fix an indentation
- Link to v2: https://lore.kernel.org/r/[email protected]

---
Jiaxun Yang (9):
MIPS: asm: Move strings to .rodata.str section
MIPS: debug: Implement low-level debugging functions
MIPS: debug: Hook up DEBUG_LL with early printk
MIPS: debug: Provide an early exception vector for low-level debugging
MIPS: debug_ll: Add Kconfig symbols for some 8250 uarts
MIPS: debug_ll: Implement support for Alchemy uarts
MIPS: debug_ll: Implement support for AR933X uarts
MIPS: zboot: Convert to use debug_ll facilities
MIPS: CPS: Convert to use debug_ll facilities

arch/mips/Kconfig | 12 +-
arch/mips/Kconfig.debug | 240 +++++++++++++++++++++++--------
arch/mips/boot/compressed/Makefile | 9 +-
arch/mips/boot/compressed/dbg.c | 39 -----
arch/mips/boot/compressed/debug-vec.S | 3 +
arch/mips/boot/compressed/debug.S | 3 +
arch/mips/boot/compressed/decompress.h | 8 +-
arch/mips/boot/compressed/head.S | 6 +
arch/mips/boot/compressed/uart-16550.c | 49 -------
arch/mips/boot/compressed/uart-alchemy.c | 9 --
arch/mips/boot/compressed/uart-ath79.c | 2 -
arch/mips/boot/compressed/uart-prom.c | 9 --
arch/mips/include/asm/asm.h | 2 +-
arch/mips/include/debug/8250.S | 60 ++++++++
arch/mips/include/debug/alchemy.S | 46 ++++++
arch/mips/include/debug/ar933x.S | 41 ++++++
arch/mips/include/debug/uhi.S | 48 +++++++
arch/mips/kernel/Makefile | 4 +-
arch/mips/kernel/cps-vec.S | 16 +--
arch/mips/kernel/debug-vec.S | 194 +++++++++++++++++++++++++
arch/mips/kernel/debug.S | 130 +++++++++++++++++
arch/mips/kernel/early_printk.c | 19 +++
arch/mips/kernel/head.S | 4 +
23 files changed, 756 insertions(+), 197 deletions(-)
---
base-commit: 084c8e315db34b59d38d06e684b1a0dd07d30287
change-id: 20240326-mips_debug_ll-ce72fee1b6a2

Best regards,
--
Jiaxun Yang <[email protected]>



2024-05-02 10:17:09

by Jiaxun Yang

[permalink] [raw]
Subject: [PATCH v3 8/9] MIPS: zboot: Convert to use debug_ll facilities

Since now debug_ll facilities can cover all platforms supported
by zboot debug print, and it provides extra capability on debugging
exceptions, switch zboot to use those facilities.

Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Signed-off-by: Jiaxun Yang <[email protected]>
---
arch/mips/Kconfig | 12 ++------
arch/mips/Kconfig.debug | 46 +++++++++++++-----------------
arch/mips/boot/compressed/Makefile | 9 ++----
arch/mips/boot/compressed/dbg.c | 39 -------------------------
arch/mips/boot/compressed/debug-vec.S | 3 ++
arch/mips/boot/compressed/debug.S | 3 ++
arch/mips/boot/compressed/decompress.h | 8 +++---
arch/mips/boot/compressed/head.S | 6 ++++
arch/mips/boot/compressed/uart-16550.c | 49 --------------------------------
arch/mips/boot/compressed/uart-alchemy.c | 9 ------
arch/mips/boot/compressed/uart-ath79.c | 2 --
arch/mips/boot/compressed/uart-prom.c | 9 ------
12 files changed, 39 insertions(+), 156 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 06ef440d16ce..6729bf1d158d 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -240,7 +240,7 @@ config ATH79
select SYS_SUPPORTS_32BIT_KERNEL
select SYS_SUPPORTS_BIG_ENDIAN
select SYS_SUPPORTS_MIPS16
- select SYS_SUPPORTS_ZBOOT_UART_PROM
+ select SYS_SUPPORTS_ZBOOT
select USE_OF
select USB_EHCI_ROOT_HUB_TT if USB_EHCI_HCD_PLATFORM
help
@@ -422,7 +422,7 @@ config MACH_INGENIC_SOC
select MIPS_GENERIC
select MACH_INGENIC
select MACH_GENERIC_CORE
- select SYS_SUPPORTS_ZBOOT_UART16550
+ select SYS_SUPPORTS_ZBOOT
select CPU_SUPPORTS_CPUFREQ
select MIPS_EXTERNAL_TIMER

@@ -1792,14 +1792,6 @@ config SYS_SUPPORTS_ZBOOT
select HAVE_KERNEL_XZ
select HAVE_KERNEL_ZSTD

-config SYS_SUPPORTS_ZBOOT_UART16550
- bool
- select SYS_SUPPORTS_ZBOOT
-
-config SYS_SUPPORTS_ZBOOT_UART_PROM
- bool
- select SYS_SUPPORTS_ZBOOT
-
config CPU_LOONGSON2EF
bool
select CPU_SUPPORTS_32BIT_KERNEL
diff --git a/arch/mips/Kconfig.debug b/arch/mips/Kconfig.debug
index a6687c503c34..0ce6d24d05b3 100644
--- a/arch/mips/Kconfig.debug
+++ b/arch/mips/Kconfig.debug
@@ -79,33 +79,6 @@ config SB1XXX_CORELIS
Select compile flags that produce code that can be processed by the
Corelis mksym utility and UDB Emulator.

-config DEBUG_ZBOOT
- bool "Enable compressed kernel support debugging"
- depends on DEBUG_KERNEL && SYS_SUPPORTS_ZBOOT
- default n
- help
- If you want to add compressed kernel support to a new board, and the
- board supports uart16550 compatible serial port, please select
- SYS_SUPPORTS_ZBOOT_UART16550 for your board and enable this option to
- debug it.
-
- If your board doesn't support uart16550 compatible serial port, you
- can try to select SYS_SUPPORTS_ZBOOT and use the other methods to
- debug it. for example, add a new serial port support just as
- arch/mips/boot/compressed/uart-16550.c does.
-
- After the compressed kernel support works, please disable this option
- to reduce the kernel image size and speed up the booting procedure a
- little.
-
-config ZBOOT_INGENIC_UART
- int "UART to use for compressed kernel debugging"
- depends on DEBUG_ZBOOT && MACH_INGENIC_SOC
- default 0
- range 0 4
- help
- Specify the UART that should be used for compressed kernel debugging.
-
config SPINLOCK_TEST
bool "Enable spinlock timing tests in debugfs"
depends on DEBUG_FS
@@ -328,3 +301,22 @@ config DEBUG_UART_8250_WIDTH
int "Register width for the 8250 debug UART"
depends on DEBUG_LL_UART_8250 || DEBUG_UART_8250
default 1
+
+config DEBUG_ZBOOT
+ bool "Enable compressed kernel debugging via DEBUG_LL output"
+ depends on DEBUG_LL && SYS_SUPPORTS_ZBOOT
+ help
+ Say Y here if you want to enable debugging of a compressed kernel
+ via the DEBUG_LL output. This is useful if you are debugging
+ decompressor issues.
+
+ If unsure, say N.
+
+config DEBUG_ZBOOT_EXCEPT
+ bool "Enable compressed kernel debugging of exceptions"
+ depends on DEBUG_ZBOOT
+ help
+ Say Y here if you want to enable debugging of exceptions happen
+ during decompression of a compressed kernel via the DEBUG_LL output.
+
+ If unsure, say N.
diff --git a/arch/mips/boot/compressed/Makefile b/arch/mips/boot/compressed/Makefile
index 6cc28173bee8..78c65db8dd04 100644
--- a/arch/mips/boot/compressed/Makefile
+++ b/arch/mips/boot/compressed/Makefile
@@ -43,13 +43,8 @@ KCSAN_SANITIZE := n
# decompressor objects (linked with vmlinuz)
vmlinuzobjs-y := $(obj)/head.o $(obj)/decompress.o $(obj)/string.o $(obj)/bswapsi.o

-ifdef CONFIG_DEBUG_ZBOOT
-vmlinuzobjs-$(CONFIG_DEBUG_ZBOOT) += $(obj)/dbg.o
-vmlinuzobjs-$(CONFIG_SYS_SUPPORTS_ZBOOT_UART16550) += $(obj)/uart-16550.o
-vmlinuzobjs-$(CONFIG_SYS_SUPPORTS_ZBOOT_UART_PROM) += $(obj)/uart-prom.o
-vmlinuzobjs-$(CONFIG_MIPS_ALCHEMY) += $(obj)/uart-alchemy.o
-vmlinuzobjs-$(CONFIG_ATH79) += $(obj)/uart-ath79.o
-endif
+vmlinuzobjs-$(CONFIG_DEBUG_ZBOOT) += $(obj)/debug.o
+vmlinuzobjs-$(CONFIG_DEBUG_ZBOOT_EXCEPT) += $(obj)/debug-vec.o

vmlinuzobjs-$(CONFIG_KERNEL_XZ) += $(obj)/ashldi3.o

diff --git a/arch/mips/boot/compressed/dbg.c b/arch/mips/boot/compressed/dbg.c
deleted file mode 100644
index 95405292accd..000000000000
--- a/arch/mips/boot/compressed/dbg.c
+++ /dev/null
@@ -1,39 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * MIPS-specific debug support for pre-boot environment
- *
- * NOTE: putc() is board specific, if your board have a 16550 compatible uart,
- * please select SYS_SUPPORTS_ZBOOT_UART16550 for your machine. otherwise, you
- * need to implement your own putc().
- */
-#include <linux/compiler.h>
-#include <linux/types.h>
-
-#include "decompress.h"
-
-void __weak putc(char c)
-{
-}
-
-void puts(const char *s)
-{
- char c;
- while ((c = *s++) != '\0') {
- putc(c);
- if (c == '\n')
- putc('\r');
- }
-}
-
-void puthex(unsigned long long val)
-{
-
- unsigned char buf[10];
- int i;
- for (i = 7; i >= 0; i--) {
- buf[i] = "0123456789ABCDEF"[val & 0x0F];
- val >>= 4;
- }
- buf[8] = '\0';
- puts(buf);
-}
diff --git a/arch/mips/boot/compressed/debug-vec.S b/arch/mips/boot/compressed/debug-vec.S
new file mode 100644
index 000000000000..e7bedb183da8
--- /dev/null
+++ b/arch/mips/boot/compressed/debug-vec.S
@@ -0,0 +1,3 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include "../../kernel/debug-vec.S"
diff --git a/arch/mips/boot/compressed/debug.S b/arch/mips/boot/compressed/debug.S
new file mode 100644
index 000000000000..0cf3da958f7e
--- /dev/null
+++ b/arch/mips/boot/compressed/debug.S
@@ -0,0 +1,3 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include "../../kernel/debug.S"
diff --git a/arch/mips/boot/compressed/decompress.h b/arch/mips/boot/compressed/decompress.h
index 073b64593b3d..e375d50c71a0 100644
--- a/arch/mips/boot/compressed/decompress.h
+++ b/arch/mips/boot/compressed/decompress.h
@@ -7,11 +7,11 @@ extern unsigned char __image_begin[], __image_end[];

/* debug interfaces */
#ifdef CONFIG_DEBUG_ZBOOT
-extern void putc(char c);
-extern void puts(const char *s);
-extern void puthex(unsigned long long val);
+extern void printascii(const char *s);
+extern void printhexl(unsigned long val);
+#define puts(s) printascii(s)
+#define puthex(val) printhexl(val)
#else
-#define putc(s) do {} while (0)
#define puts(s) do {} while (0)
#define puthex(val) do {} while (0)
#endif
diff --git a/arch/mips/boot/compressed/head.S b/arch/mips/boot/compressed/head.S
index d237a834b85e..b05e9591d3af 100644
--- a/arch/mips/boot/compressed/head.S
+++ b/arch/mips/boot/compressed/head.S
@@ -22,6 +22,12 @@
move s2, a2
move s3, a3

+#ifdef CONFIG_DEBUG_ZBOOT_EXCEP
+ /* Set up the exception vector */
+ PTR_LA t9, setup_debug_ll_exception
+ jalr t9
+#endif
+
/* Clear BSS */
PTR_LA a0, _edata
PTR_LA a2, _end
diff --git a/arch/mips/boot/compressed/uart-16550.c b/arch/mips/boot/compressed/uart-16550.c
deleted file mode 100644
index db618e72a0c4..000000000000
--- a/arch/mips/boot/compressed/uart-16550.c
+++ /dev/null
@@ -1,49 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * 16550 compatible uart based serial debug support for zboot
- */
-
-#include <linux/types.h>
-#include <linux/serial_reg.h>
-
-#include <asm/addrspace.h>
-
-#include "decompress.h"
-
-#if defined(CONFIG_MACH_LOONGSON64) || defined(CONFIG_MIPS_MALTA)
-#define UART_BASE 0x1fd003f8
-#define PORT(offset) (CKSEG1ADDR(UART_BASE) + (offset))
-#endif
-
-#ifdef CONFIG_MACH_INGENIC
-#define INGENIC_UART_BASE_ADDR (0x10030000 + 0x1000 * CONFIG_ZBOOT_INGENIC_UART)
-#define PORT(offset) (CKSEG1ADDR(INGENIC_UART_BASE_ADDR) + (4 * offset))
-#endif
-
-#ifndef IOTYPE
-#define IOTYPE char
-#endif
-
-#ifndef PORT
-#error please define the serial port address for your own machine
-#endif
-
-static inline unsigned int serial_in(int offset)
-{
- return *((volatile IOTYPE *)PORT(offset)) & 0xFF;
-}
-
-static inline void serial_out(int offset, int value)
-{
- *((volatile IOTYPE *)PORT(offset)) = value & 0xFF;
-}
-
-void putc(char c)
-{
- int timeout = 1000000;
-
- while (((serial_in(UART_LSR) & UART_LSR_THRE) == 0) && (timeout-- > 0))
- ;
-
- serial_out(UART_TX, c);
-}
diff --git a/arch/mips/boot/compressed/uart-alchemy.c b/arch/mips/boot/compressed/uart-alchemy.c
deleted file mode 100644
index 003967c084b3..000000000000
--- a/arch/mips/boot/compressed/uart-alchemy.c
+++ /dev/null
@@ -1,9 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-#include <asm/mach-au1x00/au1000.h>
-
-#include "decompress.h"
-
-void putc(char c)
-{
- alchemy_uart_putchar(AU1000_UART0_PHYS_ADDR, c);
-}
diff --git a/arch/mips/boot/compressed/uart-ath79.c b/arch/mips/boot/compressed/uart-ath79.c
deleted file mode 100644
index d686820921be..000000000000
--- a/arch/mips/boot/compressed/uart-ath79.c
+++ /dev/null
@@ -1,2 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-#include "../../ath79/early_printk.c"
diff --git a/arch/mips/boot/compressed/uart-prom.c b/arch/mips/boot/compressed/uart-prom.c
deleted file mode 100644
index 5fa3b9945333..000000000000
--- a/arch/mips/boot/compressed/uart-prom.c
+++ /dev/null
@@ -1,9 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-#include <asm/setup.h>
-
-#include "decompress.h"
-
-void putc(char c)
-{
- prom_putchar(c);
-}

--
2.34.1


2024-05-15 21:28:48

by Jiaxun Yang

[permalink] [raw]
Subject: Re: [PATCH v3 0/9] MIPS: Unify low-level debugging functionalities



在2024年5月2日五月 上午10:59,Jiaxun Yang写道:
> Hi all,
>
> This is a attempt to bring all low-level debugging print functions
> together and provide a arm-like low-level debugging interface and
> a further capability to debug early exceptions.
>
> This patch elimiate platform specific early_printk, zboot printing
> functions and cps-vec-ns16550 by newly introduced debug_ll.
>
> Hope you'll find them handy :-)
>
> Happy hacking!
>
> Thanks
>
> Signed-off-by: Jiaxun Yang <[email protected]>

A gentle ping.

Our reviewing capacity is quite low recently, hope everything is fine
with Thomas.

Thanks
[...]
- Jiaxun

2024-05-22 07:17:02

by Jiaxun Yang

[permalink] [raw]
Subject: Re: [PATCH v3 0/9] MIPS: Unify low-level debugging functionalities



在2024年5月15日五月 下午10:28,Jiaxun Yang写道:
[...]
>
> A gentle ping.
>
> Our reviewing capacity is quite low recently, hope everything is fine
> with Thomas.

Another gentle-ish ping after 6.10 merge window.

This series has been floating here for so long, if I missed the merge
window, I think I deserve a notice.

Thanks
>
> Thanks
> [...]
> - Jiaxun

--
- Jiaxun

2024-05-22 08:04:06

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH v3 0/9] MIPS: Unify low-level debugging functionalities

On Wed, May 22, 2024 at 08:15:22AM +0100, Jiaxun Yang wrote:
>
>
> 在2024年5月15日五月 下午10:28,Jiaxun Yang写道:
> [...]
> >
> > A gentle ping.
> >
> > Our reviewing capacity is quite low recently, hope everything is fine
> > with Thomas.
>
> Another gentle-ish ping after 6.10 merge window.
>
> This series has been floating here for so long, if I missed the merge
> window, I think I deserve a notice.

hmmm, I thought I was clear enough on version 1 of this series.

I don't want an additional printk like debug interface, There is
prom_putchar() and early printk console, which always got me past
any boot issue.

Thomas.

--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]

2024-05-22 08:29:30

by Jiaxun Yang

[permalink] [raw]
Subject: Re: [PATCH v3 0/9] MIPS: Unify low-level debugging functionalities



在2024年5月22日五月 上午9:03,Thomas Bogendoerfer写道:
[...]
>
> hmmm, I thought I was clear enough on version 1 of this series.
>
> I don't want an additional printk like debug interface, There is
> prom_putchar() and early printk console, which always got me past
> any boot issue.

So it's not an additional printk like debug interface, it actually
merged 3 existing debug interfaces, the first being zboot's assembly
print routines, the second being CPS's assembly print routines, the
third being some platform specific early printk. I think they are
all essential for debugging early faults, for zboot that's the only
way to print something at decompressing stage, for CPS as other cores
are booting in non-coherent state we can't safely use any kernel
functions, for early_printk that can help us *reduce* the amount
of early printk code by just adding UART base to config.

The only thing being added is the ability to debug very early exception,
even that is partially ported from existing CPS assembly debugging routines.

Please let me know your thoughts.

Thanks
>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 23 ]

--
- Jiaxun

2024-05-22 15:08:45

by Jiaxun Yang

[permalink] [raw]
Subject: Re: [PATCH v3 0/9] MIPS: Unify low-level debugging functionalities



在2024年5月22日五月 上午9:28,Jiaxun Yang写道:
> 在2024年5月22日五月 上午9:03,Thomas Bogendoerfer写道:
> [...]
>>
>> hmmm, I thought I was clear enough on version 1 of this series.
>>
>> I don't want an additional printk like debug interface, There is
>> prom_putchar() and early printk console, which always got me past
>> any boot issue.
>
> So it's not an additional printk like debug interface, it actually
> merged 3 existing debug interfaces, the first being zboot's assembly
> print routines, the second being CPS's assembly print routines, the
> third being some platform specific early printk. I think they are
> all essential for debugging early faults, for zboot that's the only
> way to print something at decompressing stage, for CPS as other cores
> are booting in non-coherent state we can't safely use any kernel
> functions, for early_printk that can help us *reduce* the amount
> of early printk code by just adding UART base to config.
>
> The only thing being added is the ability to debug very early exception,
> even that is partially ported from existing CPS assembly debugging routines.
>
> Please let me know your thoughts.

That being said, have you noticed that prom_putchar and early_printk is
a non-extant on generic mach, ingenic, ralink etc? That's because we
really don't want to introduce any platform specific UART code for
early debugging on new platforms. With DEBUG_LL introduced by Arm it's
only a Kconfig option to do the trick.

I've got review tags in PATCH v2, that means not only me feeling that
this series is reasonable.

arm64 / riscv doesn't need that because they are well standardized
and it's almost guaranteed that kernel can boot into earlycon without
much drama. For MIPS that's not the case, there are too many things that
may go wrong, from zboot decompressor to cpu-probe and memblock. We really
lacks a way to debug things early, we need something that is available
at 1st instruction at kernel entry. Furthermore, many MIPS processors
don't come with JTAG or alike debugging support, that makes debugging
even harder, there is no way to debug an early exception if your firmware
doesn't handle it. That's all the motivation behind the series.

Besides, I think our communication needs to be improved. At PATCH v1
you made your point in reply, that's fair. So I also replied twice for
clarification. I heard nothing back, so I assume you want to see how
would it develop to address your concern. Then I posted PATCH v2 and
v3 to further improve the series, after that I pinged twice on PATCH v3.

That's in a 6-month timeframe with multiple transactions, you need to
inform us your intention, even if it's a NAK or you don't want to engage
on this topic further.

Quoting the maintainer handbook [1]: "If the review process or validation
for a particular change will take longer than the expected review timeline
for the subsystem, maintainer should reply to the submission indicating
that the work is being done, and when to expect full results." Radio silence
won't help anything, it's wasting time for both of us. Please, give a
shout if it's possible.

I can see some other series being slipped away this way, like I6500
multi-cluster patch, which is sent even earlier and respined many times
over. I can recall Mobileye series had a hard time on getting your
attention, luckily we went through it.

Quoting the maintainer handbook [1]: "Nonetheless when the work does
arrive (in form of patches which need review, user bug reports etc.)
it has to be acted upon promptly.". I understand Linux/MIPS is not
your day job, also you need to take breaks or go on holidays. Sometimes
you may burn out from your maintainer duties. That's fine, we are all
human beings. I'm not expecting a 1-week SLA or something, but 6 months
or longer to expect an action is appalling to me.

I'd strongly recommend you to look for a secondary maintainer, as mentioned
in maintainer handbook [1]: "Modern best practices dictate that there should
be at least two maintainers for any piece of code, no matter how trivial".
I understand you reject the idea once when Paul handed maintainership to
you, but there are clear evidences to show that something needs to be done.
You might need a hand on handling stuff promptly and understanding some
modern MIPS stuff.

I have many, many tiny improvements to MIPS kernel locally. Furthermore,
I do bring-up for both new and ancient MIPS systems. I never got a chance
to send them out because I want you prioritise on those fundamental series.

Apologise for potential aggressive tone in this email. I just can't clam
down when I think back about your reply, and I think we really need to
talk about it.

Thanks

[1]: https://docs.kernel.org/maintainer/feature-and-driver-maintainers.html
--
- Jiaxun

2024-05-26 12:44:35

by Maciej W. Rozycki

[permalink] [raw]
Subject: Re: [PATCH v3 0/9] MIPS: Unify low-level debugging functionalities

On Wed, 22 May 2024, Jiaxun Yang wrote:

> That being said, have you noticed that prom_putchar and early_printk is
> a non-extant on generic mach, ingenic, ralink etc? That's because we
> really don't want to introduce any platform specific UART code for
> early debugging on new platforms. With DEBUG_LL introduced by Arm it's
> only a Kconfig option to do the trick.

IMHO that is however the logical thing to do. And then you need no magic
options to fiddle with and say a distribution kernel will dump whatever it
has to say if something wrong has happened early on.

IOW just wire `prom_putchar' as required, using C code preferably. NB
YAMON does have a `print' entry point for console output, so for the Malta
platform you can trivially use just that, no need for messy ad hoc 8250
code.

As to intercepting exceptions, it depends. Again YAMON does handle that
and dumps the register state, so with the Malta you get the information
required. For less capable ones it might make sense, but it ISTM like a
candidate for an independent change, and then again I fail to see why the
handler has to be written in the assembly language rather than C.

Maciej

2024-05-26 15:03:05

by Jiaxun Yang

[permalink] [raw]
Subject: Re: [PATCH v3 0/9] MIPS: Unify low-level debugging functionalities



在2024年5月26日五月 下午1:44,Maciej W. Rozycki写道:
> On Wed, 22 May 2024, Jiaxun Yang wrote:
>
>> That being said, have you noticed that prom_putchar and early_printk is
>> a non-extant on generic mach, ingenic, ralink etc? That's because we
>> really don't want to introduce any platform specific UART code for
>> early debugging on new platforms. With DEBUG_LL introduced by Arm it's
>> only a Kconfig option to do the trick.

Hi Maciej,

Thanks for your thoughts, my two cents below.
>
> IMHO that is however the logical thing to do. And then you need no magic
> options to fiddle with and say a distribution kernel will dump whatever it
> has to say if something wrong has happened early on.

This is a strict debug only options, we are not expecting any distribution
kernel to enable it. It has made itself explicit that no production device
should enable it.

Setting UART address for debug console by developers know what are they
doing is a proven approach among multiple places in multiple projects.

For kernel we have general earlycon cmdline option that would take MMIO
base address, Arm's DEBUG_LL had taken a similar approach and U-Boot have
CONFIG_DEBUG_UART_BASE, even our old zboot debug print code is taking such
approach.

It takes a balance between platform dependent code addition and bring-up
debugging capability. I fail to see why does it suddenly become an undesired
thing here.

>
> IOW just wire `prom_putchar' as required, using C code preferably. NB
> YAMON does have a `print' entry point for console output, so for the Malta
> platform you can trivially use just that, no need for messy ad hoc 8250
> code.

Sadly for the majority of modern MIPS devices are dominated by U-Boot or
vendor's simple loader. In most cases, runtime APIs are not provided by
default. Even if it's enabled, there are still a couple of reasons preventing
it to be utilized properly. U-Boot relies on global pointer stored in K0 to
save global runtime data, kernel will clobber it very early and makes U-Boot
non-functional. On devices with limited memory, it's easy to get U-Boot memory
being clobbered by kernel and render U-Boot's runtime useless anyway.

That's why many new-ish platforms such as lantiq brings it's own UART
implementations for prom_putchar.

However, for generic platform implementing prom_putchar means we need to
introduce platform dependent code, which we had to pay all the price to
avoid. We have many in-tree and out-of tree generic platform users who
don't need to add any single line of code to bring up their platform,
thanks to DeviceTree, but they still need something to help with debugging
bring up process when devicetree went wrong or early panics.

>
> As to intercepting exceptions, it depends. Again YAMON does handle that
> and dumps the register state, so with the Malta you get the information
> required. For less capable ones it might make sense, but it ISTM like a
> candidate for an independent change, and then again I fail to see why the
> handler has to be written in the assembly language rather than C.

Again for U-Boot debug exception dumping is optional and I know many devices
not shipping with that enabled. Even if it's enabled, it will stop to work
after U-Boot's memory/global pointer being clobbered or ebase being overridden
by kernel. Not to mention that Bootloader's exception dumping won't work
with CPS secondary cores.

Paul wrote cps-vec.S and cps-vec-ns16550.S in pure assembly for reasons.
Stack pointer is not initialized at second core & we really want to reduce
code footprint on secondary core to minimize side effects before coherence
is enabled.

When the infra is here, expand it to generic early exception is just tens
of lines. I fail to see the reason to bring in hundreds lines of C code for
the same functionality.

>
> Maciej

Although it's a pure technical discussion, I still want to expand that while
I appreciate what you have done to the MIPS, sometimes I feel like we are not
on the same page because you guys are away from frontliner for so long and
missed many contexts. MIPS is still evolving, although I never appeared here
with my corp email, I'm one of those behind the scene. I draft new architecture
specs, write AVP and internal simulators, do RTL coding for future core products,
helping customers design SoC products, design software architecture and bringing
them up. I kept FOSS as my hobby and I tried my best to keep upstream in sync with
modern practices.

I love MIPS heritages, I own an SGI Indy and Algorithmics P-4032, I made some
fixes on MAME emulator for indy to keep kernel running on it. I'm still frequently
fascinated by those brilliant old designs. But I think we still need to make progress.
While maintaining compatibility with all those old things, we need to adopt common
practices that have been proven by other architectures and make our own innovations.
We need to make the development process agile, so no developer is turned away. We
need to adopt modern booting protocols like EFI and ensure generic kernel is really
generic and not being diverted because of different loading address....

I don't know if you would agree with my in both technical details and ideology,
but I think it's the time to make my intention clear.

Thanks
--
- Jiaxun