Re: [PATCH v5 09/16] tests/qtest: Update tests using PL011 UART

[Peter Maydell]

On Fri, 19 Jul 2024 at 19:11, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> We weren't enabling the PL011 TX UART before using it
> on the raspi and virt machines. Update the ASM code
> prefixing:
>
>   *UART_CTRL = UART_ENABLE | TX_ENABLE;
>
> to:
>
>   while (true) {
>       *UART_DATA = 'T';
>   }
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  tests/qtest/boot-serial-test.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
> index 3b92fa5d50..5cb309ccf0 100644
> --- a/tests/qtest/boot-serial-test.c
> +++ b/tests/qtest/boot-serial-test.c
> @@ -70,18 +70,23 @@ static const uint8_t kernel_plml605[] = {
>  };
>
>  static const uint8_t bios_raspi2[] = {
> -    0x08, 0x30, 0x9f, 0xe5,                 /* ldr   r3,[pc,#8]    Get base 
> */
> +    0x10, 0x30, 0x9f, 0xe5,                 /* ldr     r3,[pc,#8]    Get 
> base */

The instruction bytes have changed but the disassembly comment has not...

> +    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,                 /* strb    r2,[r3] */
> -    0xfb, 0xff, 0xff, 0xea,                 /* b       loop */
> +    0x00, 0x20, 0xc3, 0xe5,                 /* strb    r2,[r3]       loop: */

This placement of the "loop:" label is odd -- usually the label
goes before the instruction that a branch to the label will
start executing at.

> +    0xfd, 0xff, 0xff, 0xea,                 /* b       loop */

Here also the bytes changed but not the disassembly. Since
'b' is a relative branch, why does the offset change?

>      0x00, 0x10, 0x20, 0x3f,                 /* 0x3f201000 = UART0 base addr 
> */
> +    0x01, 0x01, 0x00, 0x00,                 /* 0x101      = CR UARTEN|TXE */
>  };
>
>  static const uint8_t kernel_aarch64[] = {
> -    0x81, 0x0a, 0x80, 0x52,                 /* mov     w1, #0x54 */
>      0x02, 0x20, 0xa1, 0xd2,                 /* mov     x2, #0x9000000 */
> +    0x21, 0x20, 0x80, 0x52,                 /* mov     w1, #0x101 */
> +    0x41, 0x60, 0x00, 0x79,                 /* strh    w1, [x2, #48] */
> +    0x81, 0x0a, 0x80, 0x52,                 /* mov     w1, #0x54 */
>      0x41, 0x00, 0x00, 0x39,                 /* strb    w1, [x2] */
> -    0xfd, 0xff, 0xff, 0x17,                 /* b       -12 (loop) */
> +    0xff, 0xff, 0xff, 0x17,                 /* b       -4 (loop) */

Another unexplained offset change here.

>  };
>
>  static const uint8_t kernel_nrf51[] = {
> --
> 2.41.0

thanks
-- PMM