Re: [PATCH 02/13] openrisc: Cleanup emergency print handling

From: Stafford Horne
Date: Sun May 15 2022 - 17:36:21 EST

On Sun, May 15, 2022 at 11:03:47AM -0500, Samuel Holland wrote:
> Hi Stafford,
>
> On 5/15/22 7:41 AM, Stafford Horne wrote:
> > The emergency print support only works for 8250 compatible serial ports.
> > Now that OpenRISC platforms may be configured with different serial port
> > hardware we don't want emergency print to try to print to non-existent
> > hardware which will cause lockups.
> >
> > This patch contains several fixes to get emergency print working again:
> >
> > - Update symbol loading to not assume the location of symbols
> > - Split the putc print operation out to its own function to allow
> > for different future implementations.
> > - Update _emergency_print_nr and _emergency_print to use the putc
> > function.
> > - Guard serial 8250 specific sequences by CONFIG_SERIAL_8250
> > - Update string line feed from lf,cr to cr,lf.
> >
> > Signed-off-by: Stafford Horne <shorne@xxxxxxxxx>
> > ---
> > arch/openrisc/kernel/head.S | 148 +++++++++++++++++++++---------------
> > 1 file changed, 85 insertions(+), 63 deletions(-)
> >
> > diff --git a/arch/openrisc/kernel/head.S b/arch/openrisc/kernel/head.S
> > index 15f1b38dfe03..b1f3a65c271c 100644
> > --- a/arch/openrisc/kernel/head.S
> > +++ b/arch/openrisc/kernel/head.S
> > @@ -297,19 +297,23 @@
> > /* temporary store r3, r9 into r1, r10 */ ;\
> > l.addi r1,r3,0x0 ;\
> > l.addi r10,r9,0x0 ;\
> > - /* the string referenced by r3 must be low enough */ ;\
> > + LOAD_SYMBOL_2_GPR(r9,_string_unhandled_exception) ;\
> > + tophys (r3,r9) ;\
> > l.jal _emergency_print ;\
> > - l.ori r3,r0,lo(_string_unhandled_exception) ;\
> > + l.nop ;\
> > l.mfspr r3,r0,SPR_NPC ;\
> > l.jal _emergency_print_nr ;\
> > - l.andi r3,r3,0x1f00 ;\
> > - /* the string referenced by r3 must be low enough */ ;\
> > + l.andi r3,r3,0x1f00 ;\
> > + LOAD_SYMBOL_2_GPR(r9,_string_epc_prefix) ;\
> > + tophys (r3,r9) ;\
> > l.jal _emergency_print ;\
> > - l.ori r3,r0,lo(_string_epc_prefix) ;\
> > + l.nop ;\
> > l.jal _emergency_print_nr ;\
> > - l.mfspr r3,r0,SPR_EPCR_BASE ;\
> > + l.mfspr r3,r0,SPR_EPCR_BASE ;\
> > + LOAD_SYMBOL_2_GPR(r9,_string_nl) ;\
> > + tophys (r3,r9) ;\
> > l.jal _emergency_print ;\
> > - l.ori r3,r0,lo(_string_nl) ;\
> > + l.nop ;\
> > /* end of printing */ ;\
> > l.addi r3,r1,0x0 ;\
> > l.addi r9,r10,0x0 ;\
> > @@ -1530,65 +1534,99 @@ trampoline_out:
> > l.jr r9
> > l.nop
> >
> > -
> > /*
> > - * DSCR: prints a string referenced by r3.
> > + * DESC: Prints ASCII character stored in r7
> > *
> > - * PRMS: r3 - address of the first character of null
> > - * terminated string to be printed
> > + * PRMS: r7 - a 32-bit value with an ASCII character in the first byte
> > + * position.
> > *
> > - * PREQ: UART at UART_BASE_ADD has to be initialized
> > + * PREQ: The UART at UART_BASE_ADD has to be initialized
> > *
> > - * POST: caller should be aware that r3, r9 are changed
> > + * POST: internally used but restores:
> > + * r4 - to store UART_BASE_ADD
> > + * r5 - for loading OFF_TXFULL / THRE,TEMT
> > + * r6 - for storing bitmask (SERIAL_8250)
> > */
> > -ENTRY(_emergency_print)
> > +ENTRY(_emergency_putc)
> > EMERGENCY_PRINT_STORE_GPR4
> > EMERGENCY_PRINT_STORE_GPR5
> > EMERGENCY_PRINT_STORE_GPR6
> > - EMERGENCY_PRINT_STORE_GPR7
> > -2:
> > - l.lbz r7,0(r3)
> > - l.sfeq r7,r0
> > - l.bf 9f
> > - l.nop
> >
> > -// putc:
> > l.movhi r4,hi(UART_BASE_ADD)
> > + l.ori r4,r4,lo(UART_BASE_ADD)
> >
> > +#elif defined(CONFIG_SERIAL_8250)
>
> This needs to use #if in this patch (and #elif in the next patch).

Thnak you. right, I split these patches up at the last moment
and I messed up these ifdefs. They work together but not separated.

I will fix this, do some testing and send a v2 tomorrow.

Patches posted here:

https://github.com/stffrdhrn/linux/commits/or1k-5.19-cleanups

-Stafford

> > + /* Check UART LSR THRE (hold) bit */
> > l.addi r6,r0,0x20
> > 1: l.lbz r5,5(r4)
> > l.andi r5,r5,0x20
> > l.sfeq r5,r6
> > l.bnf 1b
> > - l.nop
> > + l.nop
> >
> > + /* Write character */
> > l.sb 0(r4),r7
> >
> > + /* Check UART LSR THRE|TEMT (hold, empty) bits */
> > l.addi r6,r0,0x60
> > 1: l.lbz r5,5(r4)
> > l.andi r5,r5,0x60
> > l.sfeq r5,r6
> > l.bnf 1b
> > - l.nop
> > + l.nop
> > +#endif
> > + EMERGENCY_PRINT_LOAD_GPR6
> > + EMERGENCY_PRINT_LOAD_GPR5
> > + EMERGENCY_PRINT_LOAD_GPR4
> > + l.jr r9
> > + l.nop
> > +
> > +/*
> > + * DSCR: prints a string referenced by r3.
> > + *
> > + * PRMS: r3 - address of the first character of null
> > + * terminated string to be printed
> > + *
> > + * PREQ: UART at UART_BASE_ADD has to be initialized
> > + *
> > + * POST: caller should be aware that r3, r9 are changed
> > + */
> > +ENTRY(_emergency_print)
> > + EMERGENCY_PRINT_STORE_GPR7
> > + EMERGENCY_PRINT_STORE_GPR9
> > +
> > + /* Load character to r7, check for null terminator */
> > +2: l.lbz r7,0(r3)
> > + l.sfeqi r7,0x0
> > + l.bf 9f
> > + l.nop
> > +
> > + l.jal _emergency_putc
> > + l.nop
> >
> > /* next character */
> > l.j 2b
> > - l.addi r3,r3,0x1
> > + l.addi r3,r3,0x1
> >
> > 9:
> > + EMERGENCY_PRINT_LOAD_GPR9
> > EMERGENCY_PRINT_LOAD_GPR7
> > - EMERGENCY_PRINT_LOAD_GPR6
> > - EMERGENCY_PRINT_LOAD_GPR5
> > - EMERGENCY_PRINT_LOAD_GPR4
> > l.jr r9
> > - l.nop
> > + l.nop
> >
> > +/*
> > + * DSCR: prints a number in r3 in hex.
> > + *
> > + * PRMS: r3 - a 32-bit unsigned integer
> > + *
> > + * PREQ: UART at UART_BASE_ADD has to be initialized
> > + *
> > + * POST: caller should be aware that r3, r9 are changed
> > + */
> > ENTRY(_emergency_print_nr)
> > - EMERGENCY_PRINT_STORE_GPR4
> > - EMERGENCY_PRINT_STORE_GPR5
> > - EMERGENCY_PRINT_STORE_GPR6
> > EMERGENCY_PRINT_STORE_GPR7
> > EMERGENCY_PRINT_STORE_GPR8
> > + EMERGENCY_PRINT_STORE_GPR9
> >
> > l.addi r8,r0,32 // shift register
> >
> > @@ -1600,58 +1638,39 @@ ENTRY(_emergency_print_nr)
> > /* don't skip the last zero if number == 0x0 */
> > l.sfeqi r8,0x4
> > l.bf 2f
> > - l.nop
> > + l.nop
> >
> > l.sfeq r7,r0
> > l.bf 1b
> > - l.nop
> > + l.nop
> >
> > 2:
> > l.srl r7,r3,r8
> >
> > l.andi r7,r7,0xf
> > l.sflts r8,r0
> > - l.bf 9f
> > + l.bf 9f
> >
> > + /* Numbers greater than 9 translate to a-f */
> > l.sfgtui r7,0x9
> > l.bnf 8f
> > - l.nop
> > + l.nop
> > l.addi r7,r7,0x27
> >
> > -8:
> > - l.addi r7,r7,0x30
> > -// putc:
> > - l.movhi r4,hi(UART_BASE_ADD)
> > -
> > - l.addi r6,r0,0x20
> > -1: l.lbz r5,5(r4)
> > - l.andi r5,r5,0x20
> > - l.sfeq r5,r6
> > - l.bnf 1b
> > - l.nop
> > -
> > - l.sb 0(r4),r7
> > -
> > - l.addi r6,r0,0x60
> > -1: l.lbz r5,5(r4)
> > - l.andi r5,r5,0x60
> > - l.sfeq r5,r6
> > - l.bnf 1b
> > - l.nop
> > + /* Convert to ascii and output character */
> > +8: l.jal _emergency_putc
> > + l.addi r7,r7,0x30
> >
> > /* next character */
> > l.j 2b
> > l.addi r8,r8,-0x4
> >
> > 9:
> > + EMERGENCY_PRINT_LOAD_GPR9
> > EMERGENCY_PRINT_LOAD_GPR8
> > EMERGENCY_PRINT_LOAD_GPR7
> > - EMERGENCY_PRINT_LOAD_GPR6
> > - EMERGENCY_PRINT_LOAD_GPR5
> > - EMERGENCY_PRINT_LOAD_GPR4
> > l.jr r9
> > - l.nop
> > -
> > + l.nop
> >
> > /*
> > * This should be used for debugging only.
> > @@ -1676,7 +1695,9 @@ ENTRY(_emergency_print_nr)
> >
> > ENTRY(_early_uart_init)
> > l.movhi r3,hi(UART_BASE_ADD)
> > + l.ori r3,r3,lo(UART_BASE_ADD)
> >
> > +#if defined(CONFIG_SERIAL_8250)
> > l.addi r4,r0,0x7
> > l.sb 0x2(r3),r4
> >
> > @@ -1694,9 +1715,10 @@ ENTRY(_early_uart_init)
> > l.addi r4,r0,((UART_DIVISOR) & 0x000000ff)
> > l.sb UART_DLL(r3),r4
> > l.sb 0x3(r3),r5
> > +#endif
> >
> > l.jr r9
> > - l.nop
> > + l.nop
> >
> > .align 0x1000
> > .global _secondary_evbar
> > @@ -1711,13 +1733,13 @@ _secondary_evbar:
> >
> > .section .rodata
> > _string_unhandled_exception:
> > - .string "\n\rRunarunaround: Unhandled exception 0x\0"
> > + .string "\r\nRunarunaround: Unhandled exception 0x\0"
> >
> > _string_epc_prefix:
> > .string ": EPC=0x\0"
> >
> > _string_nl:
> > - .string "\n\r\0"
> > + .string "\r\n\0"
> >
> >
> > /* ========================================[ page aligned structures ]=== */
> >
>