diff mbox series

[4/9] hw/char/pl011: Really use RX FIFO depth

Message ID 20250219210841.94797-5-philmd@linaro.org (mailing list archive)
State New
Headers show
Series hw/char: Improve RX FIFO depth uses | expand

Commit Message

Philippe Mathieu-Daudé Feb. 19, 2025, 9:08 p.m. UTC 
While we model a 16-elements RX FIFO since the PL011 model was
introduced in commit cdbdb648b7c ("ARM Versatile Platform Baseboard
emulation"), we only read 1 char at a time!

Have the IOCanReadHandler handler return how many elements are
available, and use that in the IOReadHandler handler.

Example of FIFO better used by enabling the pl011 tracing events
and running the tests/functional/test_aarch64_virt.py tests:

  pl011_can_receive LCR 0x70, RX FIFO used 0/16, can_receive 16 chars
  pl011_receive recv 5 chars
  pl011_fifo_rx_put RX FIFO push char [0x72] 1/16 depth used
  pl011_irq_state irq state 1
  pl011_fifo_rx_put RX FIFO push char [0x6f] 2/16 depth used
  pl011_fifo_rx_put RX FIFO push char [0x6f] 3/16 depth used
  pl011_fifo_rx_put RX FIFO push char [0x74] 4/16 depth used
  pl011_fifo_rx_put RX FIFO push char [0x0d] 5/16 depth used
  pl011_can_receive LCR 0x70, RX FIFO used 5/16, can_receive 11 chars
  pl011_can_receive LCR 0x70, RX FIFO used 5/16, can_receive 11 chars
  pl011_write addr 0x038 value 0x00000050 reg IMSC
  pl011_irq_state irq state 1
  pl011_can_receive LCR 0x70, RX FIFO used 5/16, can_receive 11 chars
  pl011_read addr 0x03c value 0x00000030 reg RIS
  pl011_write addr 0x044 value 0x00000000 reg ICR
  pl011_irq_state irq state 1
  pl011_read addr 0x018 value 0x00000080 reg FR
  pl011_read_fifo RX FIFO read, used 4/16
  pl011_irq_state irq state 1
  pl011_read addr 0x000 value 0x00000072 reg DR
  pl011_can_receive LCR 0x70, RX FIFO used 4/16, can_receive 12 chars
  pl011_read addr 0x018 value 0x00000080 reg FR
  pl011_read_fifo RX FIFO read, used 3/16
  pl011_irq_state irq state 1
  pl011_read addr 0x000 value 0x0000006f reg DR
  pl011_can_receive LCR 0x70, RX FIFO used 3/16, can_receive 13 chars
  pl011_read addr 0x018 value 0x00000080 reg FR
  pl011_read_fifo RX FIFO read, used 2/16
  pl011_irq_state irq state 1
  pl011_read addr 0x000 value 0x0000006f reg DR
  pl011_can_receive LCR 0x70, RX FIFO used 2/16, can_receive 14 chars
  pl011_read addr 0x018 value 0x00000080 reg FR
  pl011_read_fifo RX FIFO read, used 1/16
  pl011_irq_state irq state 1
  pl011_read addr 0x000 value 0x00000074 reg DR
  pl011_can_receive LCR 0x70, RX FIFO used 1/16, can_receive 15 chars
  pl011_read addr 0x018 value 0x00000080 reg FR
  pl011_read_fifo RX FIFO read, used 0/16
  pl011_irq_state irq state 0
  pl011_read addr 0x000 value 0x0000000d reg DR
  pl011_can_receive LCR 0x70, RX FIFO used 0/16, can_receive 16 chars
  pl011_read addr 0x018 value 0x00000090 reg FR
  pl011_read addr 0x03c value 0x00000020 reg RIS
  pl011_write addr 0x038 value 0x00000050 reg IMSC
  pl011_irq_state irq state 0
  pl011_can_receive LCR 0x70, RX FIFO used 0/16, can_receive 16 chars
  pl011_can_receive LCR 0x70, RX FIFO used 0/16, can_receive 16 chars
  pl011_read addr 0x018 value 0x00000090 reg FR
  pl011_write addr 0x000 value 0x00000072 reg DR

Inspired-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/char/pl011.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Luc Michel Feb. 20, 2025, 8:17 a.m. UTC | #1 
On 22:08 Wed 19 Feb     , Philippe Mathieu-Daudé wrote:
> While we model a 16-elements RX FIFO since the PL011 model was
> introduced in commit cdbdb648b7c ("ARM Versatile Platform Baseboard
> emulation"), we only read 1 char at a time!
> 
> Have the IOCanReadHandler handler return how many elements are
> available, and use that in the IOReadHandler handler.
> 
> Example of FIFO better used by enabling the pl011 tracing events
> and running the tests/functional/test_aarch64_virt.py tests:
> 
>   pl011_can_receive LCR 0x70, RX FIFO used 0/16, can_receive 16 chars
>   pl011_receive recv 5 chars
>   pl011_fifo_rx_put RX FIFO push char [0x72] 1/16 depth used
>   pl011_irq_state irq state 1
>   pl011_fifo_rx_put RX FIFO push char [0x6f] 2/16 depth used
>   pl011_fifo_rx_put RX FIFO push char [0x6f] 3/16 depth used
>   pl011_fifo_rx_put RX FIFO push char [0x74] 4/16 depth used
>   pl011_fifo_rx_put RX FIFO push char [0x0d] 5/16 depth used
>   pl011_can_receive LCR 0x70, RX FIFO used 5/16, can_receive 11 chars
>   pl011_can_receive LCR 0x70, RX FIFO used 5/16, can_receive 11 chars
>   pl011_write addr 0x038 value 0x00000050 reg IMSC
>   pl011_irq_state irq state 1
>   pl011_can_receive LCR 0x70, RX FIFO used 5/16, can_receive 11 chars
>   pl011_read addr 0x03c value 0x00000030 reg RIS
>   pl011_write addr 0x044 value 0x00000000 reg ICR
>   pl011_irq_state irq state 1
>   pl011_read addr 0x018 value 0x00000080 reg FR
>   pl011_read_fifo RX FIFO read, used 4/16
>   pl011_irq_state irq state 1
>   pl011_read addr 0x000 value 0x00000072 reg DR
>   pl011_can_receive LCR 0x70, RX FIFO used 4/16, can_receive 12 chars
>   pl011_read addr 0x018 value 0x00000080 reg FR
>   pl011_read_fifo RX FIFO read, used 3/16
>   pl011_irq_state irq state 1
>   pl011_read addr 0x000 value 0x0000006f reg DR
>   pl011_can_receive LCR 0x70, RX FIFO used 3/16, can_receive 13 chars
>   pl011_read addr 0x018 value 0x00000080 reg FR
>   pl011_read_fifo RX FIFO read, used 2/16
>   pl011_irq_state irq state 1
>   pl011_read addr 0x000 value 0x0000006f reg DR
>   pl011_can_receive LCR 0x70, RX FIFO used 2/16, can_receive 14 chars
>   pl011_read addr 0x018 value 0x00000080 reg FR
>   pl011_read_fifo RX FIFO read, used 1/16
>   pl011_irq_state irq state 1
>   pl011_read addr 0x000 value 0x00000074 reg DR
>   pl011_can_receive LCR 0x70, RX FIFO used 1/16, can_receive 15 chars
>   pl011_read addr 0x018 value 0x00000080 reg FR
>   pl011_read_fifo RX FIFO read, used 0/16
>   pl011_irq_state irq state 0
>   pl011_read addr 0x000 value 0x0000000d reg DR
>   pl011_can_receive LCR 0x70, RX FIFO used 0/16, can_receive 16 chars
>   pl011_read addr 0x018 value 0x00000090 reg FR
>   pl011_read addr 0x03c value 0x00000020 reg RIS
>   pl011_write addr 0x038 value 0x00000050 reg IMSC
>   pl011_irq_state irq state 0
>   pl011_can_receive LCR 0x70, RX FIFO used 0/16, can_receive 16 chars
>   pl011_can_receive LCR 0x70, RX FIFO used 0/16, can_receive 16 chars
>   pl011_read addr 0x018 value 0x00000090 reg FR
>   pl011_write addr 0x000 value 0x00000072 reg DR
> 
> Inspired-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Luc Michel <luc.michel@amd.com>

> ---
>  hw/char/pl011.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/char/pl011.c b/hw/char/pl011.c
> index 148a7d0dc60..57d293d1e3a 100644
> --- a/hw/char/pl011.c
> +++ b/hw/char/pl011.c
> @@ -489,7 +489,6 @@ static int pl011_can_receive(void *opaque)
>      PL011State *s = (PL011State *)opaque;
>      unsigned fifo_depth = pl011_get_fifo_depth(s);
>      unsigned fifo_available = fifo_depth - s->read_count;
> -    int r = fifo_available ? 1 : 0;
> 
>      if (!(s->cr & CR_UARTEN)) {
>          qemu_log_mask(LOG_GUEST_ERROR, "PL011 reading data on disabled UART\n");
> @@ -498,7 +497,8 @@ static int pl011_can_receive(void *opaque)
>          qemu_log_mask(LOG_GUEST_ERROR, "PL011 reading data on disabled TX UART\n");
>      }
>      trace_pl011_can_receive(s->lcr, s->read_count, fifo_depth, fifo_available);
> -    return r;
> +
> +    return fifo_available;
>  }
> 
>  static void pl011_receive(void *opaque, const uint8_t *buf, int size)
> @@ -513,7 +513,9 @@ static void pl011_receive(void *opaque, const uint8_t *buf, int size)
>          return;
>      }
> 
> -    pl011_fifo_rx_put(opaque, *buf);
> +    for (int i = 0; i < size; i++) {
> +        pl011_fifo_rx_put(opaque, buf[i]);
> +    }
>  }
> 
>  static void pl011_event(void *opaque, QEMUChrEvent event)
> --
> 2.47.1
> 

--
diff mbox series

Patch

diff --git a/hw/char/pl011.c b/hw/char/pl011.c
index 148a7d0dc60..57d293d1e3a 100644
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -489,7 +489,6 @@  static int pl011_can_receive(void *opaque)
     PL011State *s = (PL011State *)opaque;
     unsigned fifo_depth = pl011_get_fifo_depth(s);
     unsigned fifo_available = fifo_depth - s->read_count;
-    int r = fifo_available ? 1 : 0;
 
     if (!(s->cr & CR_UARTEN)) {
         qemu_log_mask(LOG_GUEST_ERROR, "PL011 reading data on disabled UART\n");
@@ -498,7 +497,8 @@  static int pl011_can_receive(void *opaque)
         qemu_log_mask(LOG_GUEST_ERROR, "PL011 reading data on disabled TX UART\n");
     }
     trace_pl011_can_receive(s->lcr, s->read_count, fifo_depth, fifo_available);
-    return r;
+
+    return fifo_available;
 }
 
 static void pl011_receive(void *opaque, const uint8_t *buf, int size)
@@ -513,7 +513,9 @@  static void pl011_receive(void *opaque, const uint8_t *buf, int size)
         return;
     }
 
-    pl011_fifo_rx_put(opaque, *buf);
+    for (int i = 0; i < size; i++) {
+        pl011_fifo_rx_put(opaque, buf[i]);
+    }
 }
 
 static void pl011_event(void *opaque, QEMUChrEvent event)