* [PATCH v6 1/4] tests/qtest/boot-serial-test: Improve ASM comments of PL011 tests
2025-01-02 10:58 [PATCH v6 0/4] tests/qtest: Update tests using PL011 UART Philippe Mathieu-Daudé
@ 2025-01-02 10:58 ` Philippe Mathieu-Daudé
2025-01-03 14:20 ` Fabiano Rosas
2025-01-02 10:58 ` [PATCH v6 2/4] tests/qtest/boot-serial-test: Reduce for() loop in " Philippe Mathieu-Daudé
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-01-02 10:58 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Alex Bennée, Paolo Bonzini, Laurent Vivier,
Peter Maydell, qemu-arm, Philippe Mathieu-Daudé
Re-indent ASM comments adding the 'loop:' label.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
tests/qtest/boot-serial-test.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
index 3b92fa5d506..a71d2857807 100644
--- a/tests/qtest/boot-serial-test.c
+++ b/tests/qtest/boot-serial-test.c
@@ -70,18 +70,18 @@ static const uint8_t kernel_plml605[] = {
};
static const uint8_t bios_raspi2[] = {
- 0x08, 0x30, 0x9f, 0xe5, /* ldr r3,[pc,#8] Get base */
- 0x54, 0x20, 0xa0, 0xe3, /* mov r2,#'T' */
- 0x00, 0x20, 0xc3, 0xe5, /* strb r2,[r3] */
- 0xfb, 0xff, 0xff, 0xea, /* b loop */
- 0x00, 0x10, 0x20, 0x3f, /* 0x3f201000 = UART0 base addr */
+ 0x08, 0x30, 0x9f, 0xe5, /* loop: ldr r3, [pc, #8] Get &UART0 */
+ 0x54, 0x20, 0xa0, 0xe3, /* mov r2, #'T' */
+ 0x00, 0x20, 0xc3, 0xe5, /* strb r2, [r3] *TXDAT = 'T' */
+ 0xfb, 0xff, 0xff, 0xea, /* b -12 (loop) */
+ 0x00, 0x10, 0x20, 0x3f, /* UART0: 0x3f201000 */
};
static const uint8_t kernel_aarch64[] = {
- 0x81, 0x0a, 0x80, 0x52, /* mov w1, #0x54 */
- 0x02, 0x20, 0xa1, 0xd2, /* mov x2, #0x9000000 */
- 0x41, 0x00, 0x00, 0x39, /* strb w1, [x2] */
- 0xfd, 0xff, 0xff, 0x17, /* b -12 (loop) */
+ 0x81, 0x0a, 0x80, 0x52, /* loop: mov w1, #'T' */
+ 0x02, 0x20, 0xa1, 0xd2, /* mov x2, #0x9000000 Load UART0 */
+ 0x41, 0x00, 0x00, 0x39, /* strb w1, [x2] *TXDAT = 'T' */
+ 0xfd, 0xff, 0xff, 0x17, /* b -12 (loop) */
};
static const uint8_t kernel_nrf51[] = {
--
2.47.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v6 2/4] tests/qtest/boot-serial-test: Reduce for() loop in PL011 tests
2025-01-02 10:58 [PATCH v6 0/4] tests/qtest: Update tests using PL011 UART Philippe Mathieu-Daudé
2025-01-02 10:58 ` [PATCH v6 1/4] tests/qtest/boot-serial-test: Improve ASM comments of PL011 tests Philippe Mathieu-Daudé
@ 2025-01-02 10:58 ` Philippe Mathieu-Daudé
2025-01-03 14:23 ` Fabiano Rosas
2025-01-02 10:58 ` [PATCH v6 3/4] tests/qtest/boot-serial-test: Reorder pair of instructions in PL011 test Philippe Mathieu-Daudé
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-01-02 10:58 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Alex Bennée, Paolo Bonzini, Laurent Vivier,
Peter Maydell, qemu-arm, Philippe Mathieu-Daudé
Since registers are not modified, we don't need
to refill their values. Directly jump to the previous
store instruction to keep filling the TXDAT register.
The equivalent C code remains:
while (true) {
*UART_DATA = 'T';
}
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
tests/qtest/boot-serial-test.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
index a71d2857807..553575ca75c 100644
--- a/tests/qtest/boot-serial-test.c
+++ b/tests/qtest/boot-serial-test.c
@@ -70,18 +70,18 @@ static const uint8_t kernel_plml605[] = {
};
static const uint8_t bios_raspi2[] = {
- 0x08, 0x30, 0x9f, 0xe5, /* loop: ldr r3, [pc, #8] Get &UART0 */
+ 0x08, 0x30, 0x9f, 0xe5, /* ldr r3, [pc, #8] Get &UART0 */
0x54, 0x20, 0xa0, 0xe3, /* mov r2, #'T' */
- 0x00, 0x20, 0xc3, 0xe5, /* strb r2, [r3] *TXDAT = 'T' */
- 0xfb, 0xff, 0xff, 0xea, /* b -12 (loop) */
+ 0x00, 0x20, 0xc3, 0xe5, /* loop: strb r2, [r3] *TXDAT = 'T' */
+ 0xff, 0xff, 0xff, 0xea, /* b -4 (loop) */
0x00, 0x10, 0x20, 0x3f, /* UART0: 0x3f201000 */
};
static const uint8_t kernel_aarch64[] = {
- 0x81, 0x0a, 0x80, 0x52, /* loop: mov w1, #'T' */
+ 0x81, 0x0a, 0x80, 0x52, /* mov w1, #'T' */
0x02, 0x20, 0xa1, 0xd2, /* mov x2, #0x9000000 Load UART0 */
- 0x41, 0x00, 0x00, 0x39, /* strb w1, [x2] *TXDAT = 'T' */
- 0xfd, 0xff, 0xff, 0x17, /* b -12 (loop) */
+ 0x41, 0x00, 0x00, 0x39, /* loop: strb w1, [x2] *TXDAT = 'T' */
+ 0xff, 0xff, 0xff, 0x17, /* b -4 (loop) */
};
static const uint8_t kernel_nrf51[] = {
--
2.47.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v6 3/4] tests/qtest/boot-serial-test: Reorder pair of instructions in PL011 test
2025-01-02 10:58 [PATCH v6 0/4] tests/qtest: Update tests using PL011 UART Philippe Mathieu-Daudé
2025-01-02 10:58 ` [PATCH v6 1/4] tests/qtest/boot-serial-test: Improve ASM comments of PL011 tests Philippe Mathieu-Daudé
2025-01-02 10:58 ` [PATCH v6 2/4] tests/qtest/boot-serial-test: Reduce for() loop in " Philippe Mathieu-Daudé
@ 2025-01-02 10:58 ` Philippe Mathieu-Daudé
2025-01-03 14:24 ` Fabiano Rosas
2025-01-02 10:58 ` [PATCH v6 4/4] tests/qtest/boot-serial-test: Initialize PL011 Control register Philippe Mathieu-Daudé
2025-01-02 15:02 ` [PATCH v6 0/4] tests/qtest: Update tests using PL011 UART Richard Henderson
4 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-01-02 10:58 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Alex Bennée, Paolo Bonzini, Laurent Vivier,
Peter Maydell, qemu-arm, Philippe Mathieu-Daudé
In the next commit we are going to use a different value
for the $w1 register, maintaining the same $x2 value. In
order to keep the next commit trivial to review, set $x2
before $w1.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
tests/qtest/boot-serial-test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
index 553575ca75c..bcfa504826c 100644
--- a/tests/qtest/boot-serial-test.c
+++ b/tests/qtest/boot-serial-test.c
@@ -78,8 +78,8 @@ static const uint8_t bios_raspi2[] = {
};
static const uint8_t kernel_aarch64[] = {
- 0x81, 0x0a, 0x80, 0x52, /* mov w1, #'T' */
0x02, 0x20, 0xa1, 0xd2, /* mov x2, #0x9000000 Load UART0 */
+ 0x81, 0x0a, 0x80, 0x52, /* mov w1, #'T' */
0x41, 0x00, 0x00, 0x39, /* loop: strb w1, [x2] *TXDAT = 'T' */
0xff, 0xff, 0xff, 0x17, /* b -4 (loop) */
};
--
2.47.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v6 4/4] tests/qtest/boot-serial-test: Initialize PL011 Control register
2025-01-02 10:58 [PATCH v6 0/4] tests/qtest: Update tests using PL011 UART Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2025-01-02 10:58 ` [PATCH v6 3/4] tests/qtest/boot-serial-test: Reorder pair of instructions in PL011 test Philippe Mathieu-Daudé
@ 2025-01-02 10:58 ` Philippe Mathieu-Daudé
2025-01-03 15:00 ` Fabiano Rosas
2025-01-02 15:02 ` [PATCH v6 0/4] tests/qtest: Update tests using PL011 UART Richard Henderson
4 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-01-02 10:58 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Alex Bennée, Paolo Bonzini, Laurent Vivier,
Peter Maydell, qemu-arm, Philippe Mathieu-Daudé
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 */
+ 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) */
--
2.47.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v6 4/4] tests/qtest/boot-serial-test: Initialize PL011 Control register
2025-01-02 10:58 ` [PATCH v6 4/4] tests/qtest/boot-serial-test: Initialize PL011 Control register Philippe Mathieu-Daudé
@ 2025-01-03 15:00 ` Fabiano Rosas
2025-01-03 15:08 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 11+ messages in thread
From: Fabiano Rosas @ 2025-01-03 15:00 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Alex Bennée, Paolo Bonzini, Laurent Vivier, Peter Maydell,
qemu-arm, Philippe Mathieu-Daudé
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?
> + 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) */
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v6 4/4] tests/qtest/boot-serial-test: Initialize PL011 Control register
2025-01-03 15:00 ` Fabiano Rosas
@ 2025-01-03 15:08 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-01-03 15:08 UTC (permalink / raw)
To: Fabiano Rosas, qemu-devel
Cc: Alex Bennée, Paolo Bonzini, Laurent Vivier, Peter Maydell,
qemu-arm
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) */
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v6 0/4] tests/qtest: Update tests using PL011 UART
2025-01-02 10:58 [PATCH v6 0/4] tests/qtest: Update tests using PL011 UART Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2025-01-02 10:58 ` [PATCH v6 4/4] tests/qtest/boot-serial-test: Initialize PL011 Control register Philippe Mathieu-Daudé
@ 2025-01-02 15:02 ` Richard Henderson
4 siblings, 0 replies; 11+ messages in thread
From: Richard Henderson @ 2025-01-02 15:02 UTC (permalink / raw)
To: qemu-devel
On 1/2/25 02:58, Philippe Mathieu-Daudé wrote:
> This series split a patch [*] which was previously posted
> too compact in multiple trivial steps. The last patch
> initializes the PL011 Control register, enabling the
> UART and its transmit channel, before using it to send
> the 'T' character in the boot-serial-test.
>
> [*] https://lore.kernel.org/qemu-devel/20240719181041.49545-10-philmd@linaro.org/
>
> Supersedes: <20240719181041.49545-10-philmd@linaro.org>
>
> Philippe Mathieu-Daudé (4):
> tests/qtest/boot-serial-test: Improve ASM comments of PL011 tests
> tests/qtest/boot-serial-test: Reduce for() loop in PL011 tests
> tests/qtest/boot-serial-test: Reorder pair of instructions in PL011
> test
> tests/qtest/boot-serial-test: Initialize PL011 Control register
>
> tests/qtest/boot-serial-test.c | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 11+ messages in thread