Re: [PATCH v6 4/4] tests/qtest/boot-serial-test: Initialize PL011 Control register

[Philippe Mathieu-Daudé]

On 3/1/25 16:00, Fabiano Rosas wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>
> > The tests using the PL011 UART of the virt and raspi machines
> > weren't properly enabling the UART and its transmitter previous
> > to sending characters. Follow the PL011 manual initialization
> > recommendation by setting the proper bits of the control register.
> >
> > Update the ASM code prefixing:
> >
> >    *UART_CTRL = UART_ENABLE | TX_ENABLE;
> >
> > to:
> >
> >    while (true) {
> >        *UART_DATA = 'T';
> >    }
> >
> > Note, since commit 51b61dd4d56 ("hw/char/pl011: Warn when using
> > disabled transmitter") incomplete PL011 initialization can be
> > logged using the '-d guest_errors' command line option.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   tests/qtest/boot-serial-test.c | 7 ++++++-
> >   1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
> > index bcfa504826c..6ea7e62e076 100644
> > --- a/tests/qtest/boot-serial-test.c
> > +++ b/tests/qtest/boot-serial-test.c
> > @@ -70,15 +70,20 @@ static const uint8_t kernel_plml605[] = {
> >   };
> >   
> >   static const uint8_t bios_raspi2[] = {
> > -    0x08, 0x30, 0x9f, 0xe5,                 /*        ldr     r3, [pc, #8]   Get 
> > &UART0 */
> > +    0x10, 0x30, 0x9f, 0xe5,                 /*        ldr     r3, [pc, #8]   Get 
> > &UART0 */
>
> The comment needs updating, no?

Oops indeed, good catch :)

   "ldr     r3, [pc, #16]"

> > +    0x10, 0x20, 0x9f, 0xe5,                 /*        ldr     r2, [pc, #8]   Get 
> > &CR */
> > +    0xb0, 0x23, 0xc3, 0xe1,                 /*        strh    r2, [r3, #48]  
> > Set CR */
> >       0x54, 0x20, 0xa0, 0xe3,                 /*        mov     r2, #'T' */
> >       0x00, 0x20, 0xc3, 0xe5,                 /* loop:  strb    r2, [r3]       
> > *TXDAT = 'T' */
> >       0xff, 0xff, 0xff, 0xea,                 /*        b       -4             
> > (loop) */
> >       0x00, 0x10, 0x20, 0x3f,                 /* UART0: 0x3f201000 */
> > +    0x01, 0x01, 0x00, 0x00,                 /* CR:    0x101 = UARTEN|TXE */
> >   };
> >   
> >   static const uint8_t kernel_aarch64[] = {
> >       0x02, 0x20, 0xa1, 0xd2,                 /*        mov    x2, #0x9000000  
> > Load UART0 */
> > +    0x21, 0x20, 0x80, 0x52,                 /*        mov    w1, 0x101       
> > CR = UARTEN|TXE */
> > +    0x41, 0x60, 0x00, 0x79,                 /*        strh   w1, [x2, #48]   
> > Set CR */
> >       0x81, 0x0a, 0x80, 0x52,                 /*        mov    w1, #'T' */
> >       0x41, 0x00, 0x00, 0x39,                 /* loop:  strb   w1, [x2]        
> > *TXDAT = 'T' */
> >       0xff, 0xff, 0xff, 0x17,                 /*        b      -4              
> > (loop) */