We log a GUEST_ERROR message "PL011 data written to disabled UART" if
the guest writes data to the TX FIFO when it has not set the enable
bit in the UART.  The idea is to note that the guest has done
something dubious but let it work anyway.  However, since we print
this message for every output character, it floods the logs when
running a guest that does this.

Keep a note of whether we've printed the log message or not, so we
only output it once.  If the guest actively disables the UART, we
re-arm the log message.

Notably, the Linux kernel does not bother to enable the UART if it is
used for earlycon, relying on the firmware having already done that.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
The log messages here were getting in my way trying to debug something
else, but I didn't want to just delete them completely.

 hw/char/pl011.c         | 24 ++++++++++++++++++++++--
 include/hw/char/pl011.h |  1 +
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/hw/char/pl011.c b/hw/char/pl011.c
index b2ca68e3e8..cb12c3e224 100644
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -227,10 +227,25 @@ static void pl011_loopback_tx(PL011State *s, uint32_t value)
 static void pl011_write_txdata(PL011State *s, uint8_t data)
 {
     if (!(s->cr & CR_UARTEN)) {
-        qemu_log_mask(LOG_GUEST_ERROR,
-                      "PL011 data written to disabled UART\n");
+        /*
+         * Only log this message once, not every time the guest outputs:
+         * otherwise we would flood the logs with this message, making
+         * harder to debug guests. (Some very popular guests like Linux
+         * don't actively enable the UART.)
+         */
+        if (!s->logged_disabled_uart) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "PL011 data written to disabled UART\n");
+            s->logged_disabled_uart = true;
+        }
     }
     if (!(s->cr & CR_TXE)) {
+        /*
+         * We don't bother with the only-log-once machinery for this check
+         * because TXE is enabled by default from PL011 reset, so there
+         * isn't likely to be existing in-the-wild guest code that trips
+         * over this one.
+         */
         qemu_log_mask(LOG_GUEST_ERROR,
                       "PL011 data written to disabled TX UART\n");
     }
@@ -457,6 +472,10 @@ static void pl011_write(void *opaque, hwaddr offset,
         break;
     case 12: /* UARTCR */
         /* ??? Need to implement the enable bit.  */
+        if ((s->cr ^ value) & CR_UARTEN) {
+            /* Re-arm the log warning when the guest toggles UARTEN */
+            s->logged_disabled_uart = false;
+        }
         s->cr = value;
         pl011_loopback_mdmctrl(s);
         break;
@@ -665,6 +684,7 @@ static void pl011_reset(DeviceState *dev)
     s->ifl = 0x12;
     s->cr = 0x300;
     s->flags = 0;
+    s->logged_disabled_uart = false;
     pl011_reset_rx_fifo(s);
     pl011_reset_tx_fifo(s);
 }
diff --git a/include/hw/char/pl011.h b/include/hw/char/pl011.h
index ff735b5234..5695787650 100644
--- a/include/hw/char/pl011.h
+++ b/include/hw/char/pl011.h
@@ -51,6 +51,7 @@ struct PL011State {
     qemu_irq irq[6];
     Clock *clk;
     bool migrate_clk;
+    bool logged_disabled_uart;
     const unsigned char *id;
     /*
      * Since some users embed this struct directly, we must
-- 
2.43.0