diff mbox series

hw/char/bcm2835_aux: Fix incorrect interrupt ID when RX disabled

Message ID 20250328123725.94176-1-yeechen0207@gmail.com (mailing list archive)
State New
Headers show
Series hw/char/bcm2835_aux: Fix incorrect interrupt ID when RX disabled | expand

Commit Message

Chung-Yi Chen March 28, 2025, 12:37 p.m. UTC
This patch fixes a misconfiguration issue in the read implementation of
the AUX_MU_IIR_REG register. This issue can lead to a transmit interrupt
being incorrectly interpreted as a receive interrupt when the receive
interrupt is disabled and the receive FIFO holds valid bytes.

The AUX_MU_IIR_REG register (interrupt ID bits [2:1]) indicates the
status of mini UART interrupts:

    - 00: No interrupts
    - 01: Transmit FIFO is empty
    - 10: Receive FIFO is not empty
    - 11: <Not possible>

When the transmit interrupt is enabled and the receive interrupt is
disabled, the original code incorrectly sets the interrupt ID bits.
Specifically:

    1. Transmit FIFO empty, receive FIFO empty
        - Expected 0b01, returned 0b01 (correct)
    2. Transmit FIFO empty, receive FIFO not empty
        - Expected 0b01, returned 0b10 (incorrect)

In the second case, the code sets the interrupt ID to 0b10 (receive FIFO
is not empty) even if the receive interrupt is disabled.

To fix this, the patch adds additional condition for setting the
interrupt ID bits to also check if the receive interrupt is enabled.

Reference: BCM2835 ARM Peripherals, page 13. Available on
https://datasheets.raspberrypi.com/bcm2835/bcm2835-peripherals.pdf

Signed-off-by: Chung-Yi Chen <yeechen0207@gmail.com>
---
 hw/char/bcm2835_aux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Philippe Mathieu-Daudé March 30, 2025, 8:55 a.m. UTC | #1
On 28/3/25 13:37, Chung-Yi Chen wrote:
> This patch fixes a misconfiguration issue in the read implementation of
> the AUX_MU_IIR_REG register. This issue can lead to a transmit interrupt
> being incorrectly interpreted as a receive interrupt when the receive
> interrupt is disabled and the receive FIFO holds valid bytes.
> 
> The AUX_MU_IIR_REG register (interrupt ID bits [2:1]) indicates the
> status of mini UART interrupts:
> 
>      - 00: No interrupts
>      - 01: Transmit FIFO is empty
>      - 10: Receive FIFO is not empty
>      - 11: <Not possible>
> 
> When the transmit interrupt is enabled and the receive interrupt is
> disabled, the original code incorrectly sets the interrupt ID bits.
> Specifically:
> 
>      1. Transmit FIFO empty, receive FIFO empty
>          - Expected 0b01, returned 0b01 (correct)
>      2. Transmit FIFO empty, receive FIFO not empty
>          - Expected 0b01, returned 0b10 (incorrect)
> 
> In the second case, the code sets the interrupt ID to 0b10 (receive FIFO
> is not empty) even if the receive interrupt is disabled.
> 
> To fix this, the patch adds additional condition for setting the
> interrupt ID bits to also check if the receive interrupt is enabled.
> 
> Reference: BCM2835 ARM Peripherals, page 13. Available on
> https://datasheets.raspberrypi.com/bcm2835/bcm2835-peripherals.pdf
> 
> Signed-off-by: Chung-Yi Chen <yeechen0207@gmail.com>
> ---
>   hw/char/bcm2835_aux.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Fixes: 97398d900ca ("bcm2835_aux: add emulation of BCM2835 AUX (aka 
UART1) block")
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Philippe Mathieu-Daudé March 31, 2025, 11:20 a.m. UTC | #2
On 28/3/25 13:37, Chung-Yi Chen wrote:
> This patch fixes a misconfiguration issue in the read implementation of
> the AUX_MU_IIR_REG register. This issue can lead to a transmit interrupt
> being incorrectly interpreted as a receive interrupt when the receive
> interrupt is disabled and the receive FIFO holds valid bytes.
> 
> The AUX_MU_IIR_REG register (interrupt ID bits [2:1]) indicates the
> status of mini UART interrupts:
> 
>      - 00: No interrupts
>      - 01: Transmit FIFO is empty
>      - 10: Receive FIFO is not empty
>      - 11: <Not possible>
> 
> When the transmit interrupt is enabled and the receive interrupt is
> disabled, the original code incorrectly sets the interrupt ID bits.
> Specifically:
> 
>      1. Transmit FIFO empty, receive FIFO empty
>          - Expected 0b01, returned 0b01 (correct)
>      2. Transmit FIFO empty, receive FIFO not empty
>          - Expected 0b01, returned 0b10 (incorrect)
> 
> In the second case, the code sets the interrupt ID to 0b10 (receive FIFO
> is not empty) even if the receive interrupt is disabled.
> 
> To fix this, the patch adds additional condition for setting the
> interrupt ID bits to also check if the receive interrupt is enabled.
> 
> Reference: BCM2835 ARM Peripherals, page 13. Available on
> https://datasheets.raspberrypi.com/bcm2835/bcm2835-peripherals.pdf
> 
> Signed-off-by: Chung-Yi Chen <yeechen0207@gmail.com>
> ---
>   hw/char/bcm2835_aux.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Queued to hw-misc, thanks!
diff mbox series

Patch

diff --git a/hw/char/bcm2835_aux.c b/hw/char/bcm2835_aux.c
index c6e7eccf7d..9b073fc330 100644
--- a/hw/char/bcm2835_aux.c
+++ b/hw/char/bcm2835_aux.c
@@ -98,7 +98,7 @@  static uint64_t bcm2835_aux_read(void *opaque, hwaddr offset, unsigned size)
          * interrupts are active, besides that this cannot occur. At
          * present, we choose to prioritise the rx interrupt, since
          * the tx fifo is always empty. */
-        if (s->read_count != 0) {
+        if ((s->iir & RX_INT) && s->read_count != 0) {
             res |= 0x4;
         } else {
             res |= 0x2;